Skip to content

Commit

Permalink
Improved Exception Replay frame matching algorithm + capturing except…
Browse files Browse the repository at this point in the history
…ions that have duplicated frames due to await in finally blocks + added missing attributes to snapshot (exceptionHash, exceptionId, frameIndex)
  • Loading branch information
GreenMatan committed Jan 15, 2025
1 parent e28a1dc commit a3a217d
Show file tree
Hide file tree
Showing 21 changed files with 1,096 additions and 211 deletions.
24 changes: 12 additions & 12 deletions Datadog.Trace.sln
Original file line number Diff line number Diff line change
Expand Up @@ -612,14 +612,14 @@ Global
Release|Any CPU = Release|Any CPU
EndGlobalSection
GlobalSection(ProjectConfigurationPlatforms) = postSolution
{91B6272F-5780-4C94-8071-DBBA7B4F67F3}.Debug|Any CPU.ActiveCfg = Debug|x64
{91B6272F-5780-4C94-8071-DBBA7B4F67F3}.Debug|Any CPU.Build.0 = Debug|x64
{91B6272F-5780-4C94-8071-DBBA7B4F67F3}.Release|Any CPU.ActiveCfg = Release|x64
{91B6272F-5780-4C94-8071-DBBA7B4F67F3}.Release|Any CPU.Build.0 = Release|x64
{C0C8D381-D6B9-4C76-9428-F40F2FA93A9A}.Debug|Any CPU.ActiveCfg = Debug|x64
{C0C8D381-D6B9-4C76-9428-F40F2FA93A9A}.Debug|Any CPU.Build.0 = Debug|x64
{C0C8D381-D6B9-4C76-9428-F40F2FA93A9A}.Release|Any CPU.ActiveCfg = Release|x64
{C0C8D381-D6B9-4C76-9428-F40F2FA93A9A}.Release|Any CPU.Build.0 = Release|x64
{91B6272F-5780-4C94-8071-DBBA7B4F67F3}.Debug|Any CPU.ActiveCfg = Debug|Win32
{91B6272F-5780-4C94-8071-DBBA7B4F67F3}.Debug|Any CPU.Build.0 = Debug|Win32
{91B6272F-5780-4C94-8071-DBBA7B4F67F3}.Release|Any CPU.ActiveCfg = Release|Win32
{91B6272F-5780-4C94-8071-DBBA7B4F67F3}.Release|Any CPU.Build.0 = Release|Win32
{C0C8D381-D6B9-4C76-9428-F40F2FA93A9A}.Debug|Any CPU.ActiveCfg = Debug|Win32
{C0C8D381-D6B9-4C76-9428-F40F2FA93A9A}.Debug|Any CPU.Build.0 = Debug|Win32
{C0C8D381-D6B9-4C76-9428-F40F2FA93A9A}.Release|Any CPU.ActiveCfg = Release|Win32
{C0C8D381-D6B9-4C76-9428-F40F2FA93A9A}.Release|Any CPU.Build.0 = Release|Win32
{5DFDF781-F24C-45B1-82EF-9125875A80A4}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{5DFDF781-F24C-45B1-82EF-9125875A80A4}.Debug|Any CPU.Build.0 = Debug|Any CPU
{5DFDF781-F24C-45B1-82EF-9125875A80A4}.Release|Any CPU.ActiveCfg = Release|Any CPU
Expand Down Expand Up @@ -648,10 +648,10 @@ Global
{3C6DD42E-9214-4747-92BA-78DE29AACE59}.Debug|Any CPU.Build.0 = Debug|Any CPU
{3C6DD42E-9214-4747-92BA-78DE29AACE59}.Release|Any CPU.ActiveCfg = Release|Any CPU
{3C6DD42E-9214-4747-92BA-78DE29AACE59}.Release|Any CPU.Build.0 = Release|Any CPU
{5728056A-51AA-4FF5-AD0C-E86E44E36102}.Debug|Any CPU.ActiveCfg = Debug|x64
{5728056A-51AA-4FF5-AD0C-E86E44E36102}.Debug|Any CPU.Build.0 = Debug|x64
{5728056A-51AA-4FF5-AD0C-E86E44E36102}.Release|Any CPU.ActiveCfg = Release|x64
{5728056A-51AA-4FF5-AD0C-E86E44E36102}.Release|Any CPU.Build.0 = Release|x64
{5728056A-51AA-4FF5-AD0C-E86E44E36102}.Debug|Any CPU.ActiveCfg = Debug|Win32
{5728056A-51AA-4FF5-AD0C-E86E44E36102}.Debug|Any CPU.Build.0 = Debug|Win32
{5728056A-51AA-4FF5-AD0C-E86E44E36102}.Release|Any CPU.ActiveCfg = Release|Win32
{5728056A-51AA-4FF5-AD0C-E86E44E36102}.Release|Any CPU.Build.0 = Release|Win32
{FDB5C8D0-018D-4FF9-9680-C6A5078F819B}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{FDB5C8D0-018D-4FF9-9680-C6A5078F819B}.Debug|Any CPU.Build.0 = Debug|Any CPU
{FDB5C8D0-018D-4FF9-9680-C6A5078F819B}.Release|Any CPU.ActiveCfg = Release|Any CPU
Expand Down
1 change: 0 additions & 1 deletion exploration-tests/protobuf
Submodule protobuf deleted from 7c40b2
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ internal static ExceptionCase Instrument(ExceptionIdentifier exceptionId, string
{
Log.Information("Instrumenting {ExceptionId}", exceptionId);

var parsedFramesFromExceptionToString = ExceptionNormalizer.Instance.ParseFrames(exceptionToString).ToArray();
var stackTrace = exceptionId.StackTrace.Where(frame => parsedFramesFromExceptionToString.Any(f => f.Contains(frame.Method.Name))).ToArray();
var parsedFramesFromExceptionToString = StackTraceProcessor.ParseFrames(exceptionToString).ToArray();
var stackTrace = exceptionId.StackTrace.Where(frame => parsedFramesFromExceptionToString.Any(f => MethodMatcher.IsMethodMatch(f, frame.Method))).ToArray();
var participatingUserMethods = GetMethodsToRejit(stackTrace);

var uniqueMethods = participatingUserMethods
Expand Down Expand Up @@ -84,53 +84,11 @@ bool ShouldInstrumentFrameAtIndex(int i)
}
}

private static bool ContainsExceptionDispatchInfoThrow(MethodBase method)
{
var methodBody = method.GetMethodBody();
if (methodBody == null)
{
return false;
}

byte[] ilBytes = methodBody.GetILAsByteArray();

if (ilBytes == null)
{
return false;
}

for (int i = 0; i < ilBytes.Length; i++)
{
if (ilBytes[i] == (byte)OpCodes.Call.Value || ilBytes[i] == (byte)OpCodes.Callvirt.Value)
{
// The next 4 bytes after a call instruction contain the metadata token
if (i + 4 < ilBytes.Length)
{
int metadataToken = BitConverter.ToInt32(ilBytes, i + 1);
try
{
MethodInfo calledMethod = (MethodInfo)method.Module.ResolveMethod(metadataToken);
if (calledMethod.DeclaringType == typeof(System.Runtime.ExceptionServices.ExceptionDispatchInfo) &&
calledMethod.Name == "Throw")
{
return true;
}
}
catch (ArgumentException)
{
// If we can't resolve the method, just continue
continue;
}
}
}
}

return false;
}

private static List<MethodUniqueIdentifier> GetMethodsToRejit(ParticipatingFrame[] allFrames)
{
var methodsToRejit = new List<MethodUniqueIdentifier>();
MethodUniqueIdentifier? lastMethod = null;
var wasLastMisleading = false;

foreach (var frame in allFrames)
{
Expand All @@ -149,11 +107,24 @@ private static List<MethodUniqueIdentifier> GetMethodsToRejit(ParticipatingFrame
continue;
}

methodsToRejit.Add(frame.MethodIdentifier);
var currentMethod = frame.MethodIdentifier;
var isCurrentMisleading = currentMethod.IsMisleadMethod();

// Add the method if either:
// 1. It's not misleading (we keep all non-misleading methods)
// 2. It's misleading but different from the last misleading method we saw
// 3. It's the first misleading method after non-misleading methods
if (!isCurrentMisleading || currentMethod != lastMethod || !wasLastMisleading)
{
methodsToRejit.Add(currentMethod);
}

lastMethod = currentMethod;
wasLastMisleading = isCurrentMisleading;
}
catch (Exception ex)
{
Log.Error(ex, "Failed to instrument frame the frame: {FrameToRejit}", frame);
Log.Error(ex, "Failed to instrument the frame: {FrameToRejit}", frame);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ internal class ExceptionDebuggingProcessor : IProbeProcessor
private static readonly IDatadogLogger Log = DatadogLogging.GetLoggerFor(typeof(ExceptionDebuggingProcessor));
private readonly object _lock = new();
private readonly int _maxFramesToCapture;
private readonly bool _isMisleadingMethod;
private ExceptionProbeProcessor[] _processors;

internal ExceptionDebuggingProcessor(string probeId, MethodUniqueIdentifier method)
Expand All @@ -27,6 +28,7 @@ internal ExceptionDebuggingProcessor(string probeId, MethodUniqueIdentifier meth
ProbeId = probeId;
Method = method;
_maxFramesToCapture = ExceptionDebugging.Settings.MaximumFramesToCapture;
_isMisleadingMethod = method.IsMisleadMethod();
}

public string ProbeId { get; }
Expand Down Expand Up @@ -68,7 +70,17 @@ public bool Process<TCapture>(ref CaptureInfo<TCapture> info, IDebuggerSnapshotC
case MethodState.EntryStart:
case MethodState.EntryAsync:
shadowStack = ShadowStackHolder.EnsureShadowStackEnabled();
snapshotCreator.EnterHash = shadowStack.CurrentStackFrameNode?.EnterSequenceHash ?? Fnv1aHash.FnvOffsetBias;
var currentFrame = shadowStack.CurrentStackFrameNode;
snapshotCreator.EnterHash = Fnv1aHash.Combine(info.Method.MetadataToken, shadowStack.CurrentStackFrameNode?.EnterSequenceHash ?? Fnv1aHash.FnvOffsetBias);

if (currentFrame?.Method == info.Method && _isMisleadingMethod)
{
// Methods marked as `misleading` are methods we tolerate being in the shadow stack multiple times.
// We flatten those methods due to `ExceptionDispatchInfo.Capture(X).Throw()` API causing frames to appear twice
// while in reality they were involved only once.
snapshotCreator.TrackedStackFrameNode = shadowStack.EnterFake(info.Method);
return true;
}

var shouldProcess = false;
foreach (var processor in snapshotCreator.Processors)
Expand Down Expand Up @@ -113,7 +125,15 @@ public bool Process<TCapture>(ref CaptureInfo<TCapture> info, IDebuggerSnapshotC

var exception = info.Value as Exception;
snapshotCreator.TrackedStackFrameNode.LeavingException = exception;
snapshotCreator.LeaveHash = shadowStack.CurrentStackFrameNode?.LeaveSequenceHash ?? Fnv1aHash.FnvOffsetBias;
snapshotCreator.LeaveHash = shadowStack.CurrentStackFrameNode!.LeaveSequenceHash;

if (snapshotCreator.TrackedStackFrameNode is FakeTrackedStackFrameNode)
{
shadowStack.Leave(snapshotCreator.TrackedStackFrameNode, exception);
snapshotCreator.TrackedStackFrameNode.CapturingStrategy = SnapshotCapturingStrategy.FullSnapshot;
snapshotCreator.TrackedStackFrameNode.AddScopeMember(info.Name, info.Type, info.Value, info.MemberKind);
return true;
}

var leavingExceptionType = info.Value.GetType();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,54 +104,83 @@ protected virtual int HashLine(VendoredMicrosoftCode.System.ReadOnlySpan<char> l
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal IEnumerable<string> ParseFrames(string exceptionString)
internal List<string> ParseFrames(string exceptionString)
{
if (string.IsNullOrEmpty(exceptionString))
{
throw new ArgumentException(@"Exception string cannot be null or empty", nameof(exceptionString));
}

var exceptionSpan = VendoredMicrosoftCode.System.MemoryExtensions.AsSpan(exceptionString);
var inSpan = VendoredMicrosoftCode.System.MemoryExtensions.AsSpan(" in ");
var atSpan = VendoredMicrosoftCode.System.MemoryExtensions.AsSpan("at ");
var lambdaSpan = VendoredMicrosoftCode.System.MemoryExtensions.AsSpan("lambda_");
var datadogSpan = VendoredMicrosoftCode.System.MemoryExtensions.AsSpan("at Datadog.");
var results = new List<string>();
var currentSpan = VendoredMicrosoftCode.System.MemoryExtensions.AsSpan(exceptionString);

while (!exceptionSpan.IsEmpty)
while (!currentSpan.IsEmpty)
{
var lineEndIndex = exceptionSpan.IndexOfAny('\r', '\n');
var lineEndIndex = currentSpan.IndexOfAny('\r', '\n');
VendoredMicrosoftCode.System.ReadOnlySpan<char> line;

if (lineEndIndex >= 0)
{
line = exceptionSpan.Slice(0, lineEndIndex);
exceptionSpan = exceptionSpan.Slice(lineEndIndex + 1);
if (!exceptionSpan.IsEmpty && exceptionSpan[0] == '\n')
line = currentSpan.Slice(0, lineEndIndex);
currentSpan = currentSpan.Slice(lineEndIndex + 1);
if (!currentSpan.IsEmpty && currentSpan[0] == '\n')
{
exceptionSpan = exceptionSpan.Slice(1);
currentSpan = currentSpan.Slice(1);
}
}
else
{
line = exceptionSpan;
exceptionSpan = default;
line = currentSpan;
currentSpan = default;
}

// Is frame line (starts with `in `).
if (VendoredMicrosoftCode.System.MemoryExtensions.StartsWith(line.TrimStart(), atSpan, StringComparison.Ordinal))
{
var index = VendoredMicrosoftCode.System.MemoryExtensions.IndexOf(line, inSpan, StringComparison.Ordinal);
line = index > 0 ? line.Slice(0, index) : line;
ProcessLine(line, results);
}

if (VendoredMicrosoftCode.System.MemoryExtensions.Contains(line, lambdaSpan, StringComparison.Ordinal) ||
VendoredMicrosoftCode.System.MemoryExtensions.Contains(line, datadogSpan, StringComparison.Ordinal))
{
continue;
}
return results;
}

yield return line.ToString();
}
[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static void ProcessLine(VendoredMicrosoftCode.System.ReadOnlySpan<char> line, List<string> results)
{
line = line.TrimStart();
if (line.IsEmpty)
{
return;
}

// Check if it's a stack frame line (starts with "at ")
if (!VendoredMicrosoftCode.System.MemoryExtensions.StartsWith(line, VendoredMicrosoftCode.System.MemoryExtensions.AsSpan("at "), StringComparison.Ordinal))
{
return;
}

// Skip the "at " prefix
line = line.Slice(3);

// Skip lambda and Datadog frames early
if (ContainsAny(line, "lambda_", "at Datadog."))
{
return;
}

// Find the " in " marker and truncate if found
var inIndex = VendoredMicrosoftCode.System.MemoryExtensions.IndexOf(line, VendoredMicrosoftCode.System.MemoryExtensions.AsSpan(" in "), StringComparison.Ordinal);

if (inIndex > 0)
{
line = line.Slice(0, inIndex);
}

// Only create a string when we're sure we want to keep this frame
results.Add(line.ToString());
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static bool ContainsAny(VendoredMicrosoftCode.System.ReadOnlySpan<char> source, string first, string second)
{
return VendoredMicrosoftCode.System.MemoryExtensions.Contains(source, VendoredMicrosoftCode.System.MemoryExtensions.AsSpan(first), StringComparison.Ordinal) ||
VendoredMicrosoftCode.System.MemoryExtensions.Contains(source, VendoredMicrosoftCode.System.MemoryExtensions.AsSpan(second), StringComparison.Ordinal);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -116,19 +116,17 @@ private bool EnsureLeaveHashComputed()
Array.Reverse(installedProbes);
}

if (!installedProbes.Any())
{
return Fnv1aHash.FnvOffsetBias;
}

var hash = Fnv1aHash.FnvOffsetBias;

foreach (var probe in installedProbes)
if (installedProbes.Any())
{
hash = Fnv1aHash.Combine(probe.Method.MethodToken, hash);
foreach (var probe in installedProbes)
{
hash = Fnv1aHash.Combine(probe.Method.Method.MetadataToken, hash);
}
}

return hash;
return Fnv1aHash.Combine(ExceptionDebuggingProcessor.Method.Method.MetadataToken, hash);
}

internal void InvalidateEnterLeave()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,36 +17,32 @@ namespace Datadog.Trace.Debugger.ExceptionAutoInstrumentation
{
internal class ExceptionReplaySnapshotCreator : DebuggerSnapshotCreator
{
private readonly string _exceptionHash;
private readonly string _exceptionCaptureId;
private readonly int _frameIndex;

public ExceptionReplaySnapshotCreator(bool isFullSnapshot, ProbeLocation location, bool hasCondition, string[] tags, CaptureLimitInfo limitInfo, string exceptionHash, string exceptionCaptureId, int frameIndex)
public ExceptionReplaySnapshotCreator(bool isFullSnapshot, ProbeLocation location, bool hasCondition, string[] tags, CaptureLimitInfo limitInfo)
: base(isFullSnapshot, location, hasCondition, tags, limitInfo)
{
_exceptionHash = exceptionHash;
_exceptionCaptureId = exceptionCaptureId;
_frameIndex = frameIndex;
}

public ExceptionReplaySnapshotCreator(bool isFullSnapshot, ProbeLocation location, bool hasCondition, string[] tags, MethodScopeMembers methodScopeMembers, CaptureLimitInfo limitInfo, string exceptionHash, string exceptionCaptureId, int frameIndex)
public ExceptionReplaySnapshotCreator(bool isFullSnapshot, ProbeLocation location, bool hasCondition, string[] tags, MethodScopeMembers methodScopeMembers, CaptureLimitInfo limitInfo)
: base(isFullSnapshot, location, hasCondition, tags, methodScopeMembers, limitInfo)
{
_exceptionHash = exceptionHash;
_exceptionCaptureId = exceptionCaptureId;
_frameIndex = frameIndex;
}

internal static string ExceptionHash { get; } = Guid.NewGuid().ToString();

internal static string ExceptionCaptureId { get; } = Guid.NewGuid().ToString();

internal static string FrameIndex { get; } = Guid.NewGuid().ToString();

internal override DebuggerSnapshotCreator EndSnapshot()
{
JsonWriter.WritePropertyName("exception_hash");
JsonWriter.WriteValue(_exceptionHash);
JsonWriter.WritePropertyName("exceptionHash");
JsonWriter.WriteValue(ExceptionHash);

JsonWriter.WritePropertyName("exception_capture_id");
JsonWriter.WriteValue(_exceptionCaptureId);
JsonWriter.WritePropertyName("exceptionId");
JsonWriter.WriteValue(ExceptionCaptureId);

JsonWriter.WritePropertyName("frame_index");
JsonWriter.WriteValue(_frameIndex);
JsonWriter.WritePropertyName("frameIndex");
JsonWriter.WriteValue(FrameIndex);

return base.EndSnapshot();
}
Expand Down
Loading

0 comments on commit a3a217d

Please sign in to comment.