Skip to content

Commit

Permalink
Fixed issues where we failed to instrument methods received by SymDb
Browse files Browse the repository at this point in the history
  • Loading branch information
@GreenMatan
GreenMatan committed Jul 30, 2024
1 parent c6dd499 commit 788cc9a
Show file tree
Hide file tree
Showing 4 changed files with 133 additions and 6 deletions.
7 changes: 6 additions & 1 deletion .../Datadog.Trace/Debugger/ExceptionAutoInstrumentation/ExceptionReplayDiagnosticTagNames.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ namespace Datadog.Trace.Debugger.ExceptionAutoInstrumentation
/// </summary>
internal static class ExceptionReplayDiagnosticTagNames
{
public const string None = nameof(None);
public const string Eligible = nameof(Eligible);
public const string EmptyShadowStack = nameof(EmptyShadowStack);
public const string ExceptionTrackManagerNotInitialized = nameof(ExceptionTrackManagerNotInitialized);
Expand All @@ -27,5 +26,11 @@ internal static class ExceptionReplayDiagnosticTagNames
public const string CachedDoneExceptionCase = nameof(CachedDoneExceptionCase);
public const string InvalidatedExceptionCase = nameof(InvalidatedExceptionCase);
public const string CircuitBreakerIsOpen = nameof(CircuitBreakerIsOpen);
public const string NonCachedDoneExceptionCase = nameof(NonCachedDoneExceptionCase);
public const string NotSupportedExceptionType = nameof(NotSupportedExceptionType);
public const string NoFramesToInstrument = nameof(NoFramesToInstrument);
public const string EmptyCallStackTreeWhileCollecting = nameof(EmptyCallStackTreeWhileCollecting);
public const string InvalidatedCase = nameof(InvalidatedCase);
public const string NewCase = nameof(NewCase);
}
}
31 changes: 31 additions & 0 deletions tracer/src/Datadog.Trace/Debugger/ExceptionAutoInstrumentation/ExceptionTrackManager.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,11 +139,21 @@ private static void ProcessException(Exception exception, ErrorOriginKind errorO

if (allParticipatingFramesFlattened.Length == 0)
{
if (rootSpan != null)
{
SetDiagnosticTag(rootSpan, ExceptionReplayDiagnosticTagNames.NoFramesToInstrument);
}

return;
}

if (!ShouldReportException(exception, allParticipatingFramesFlattened))
{
if (rootSpan != null)
{
SetDiagnosticTag(rootSpan, ExceptionReplayDiagnosticTagNames.NotSupportedExceptionType);
}

Log.Information(exception, "Skipping the processing of an exception by Exception Debugging.");
return;
}
Expand All @@ -163,6 +173,10 @@ private static void ProcessException(Exception exception, ErrorOriginKind errorO

if (trackedExceptionCase.IsDone)
{
if (rootSpan != null)
{
SetDiagnosticTag(rootSpan, ExceptionReplayDiagnosticTagNames.NonCachedDoneExceptionCase);
}
}
else if (trackedExceptionCase.IsInvalidated)
{
Expand Down Expand Up @@ -254,6 +268,18 @@ private static void ProcessException(Exception exception, ErrorOriginKind errorO
Log.Information("Invalidating the exception case of the empty stack tree since none of the methods were instrumented, for exception: {Name}, Message: {Message}, StackTrace: {StackTrace}", exception.GetType().Name, exception.Message, exception.StackTrace);
trackedExceptionCase.InvalidateCase();
_invalidatedCases.Add(exception.ToString());

if (rootSpan != null)
{
SetDiagnosticTag(rootSpan, ExceptionReplayDiagnosticTagNames.InvalidatedCase);
}
}
else
{
if (rootSpan != null)
{
SetDiagnosticTag(rootSpan, ExceptionReplayDiagnosticTagNames.EmptyCallStackTreeWhileCollecting);
}
}

return;
Expand Down Expand Up @@ -361,6 +387,11 @@ private static void ProcessException(Exception exception, ErrorOriginKind errorO
{
Log.Information("New exception case occurred, initiating data collection for exception: {Name}, Message: {Message}, StackTrace: {StackTrace}", exception.GetType().Name, exception.Message, exception.StackTrace);
trackedExceptionCase.Instrument();

if (rootSpan != null)
{
SetDiagnosticTag(rootSpan, ExceptionReplayDiagnosticTagNames.NewCase);
}
}
}

Expand Down
91 changes: 89 additions & 2 deletions tracer/src/Datadog.Trace/Debugger/LiveDebugger.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading;
using System.Threading.Tasks;
using Datadog.Trace.Agent.DiscoveryService;
Expand All @@ -25,6 +26,7 @@
using Datadog.Trace.DogStatsd;
using Datadog.Trace.Logging;
using Datadog.Trace.RemoteConfigurationManagement;
using Datadog.Trace.Util;
using Datadog.Trace.Vendors.StatsdClient;
using ProbeInfo = Datadog.Trace.Debugger.Expressions.ProbeInfo;

Expand Down Expand Up @@ -245,15 +247,17 @@ internal void UpdateAddedProbeInstrumentations(IReadOnlyList<ProbeDefinition> ad

case ProbeLocationType.Method:
{
var signature = SafeParseSignature(probe.Where.Signature);

fetchProbeStatus.Add(new FetchProbeStatus(probe.Id, probe.Version ?? 0));
if (probe is SpanProbe)
{
var spanDefinition = new NativeSpanProbeDefinition(probe.Id, probe.Where.TypeName, probe.Where.MethodName, probe.Where.Signature?.Split(separator: ','));
var spanDefinition = new NativeSpanProbeDefinition(probe.Id, probe.Where.TypeName, probe.Where.MethodName, signature);
spanProbes.Add(spanDefinition);
}
else
{
var nativeDefinition = new NativeMethodProbeDefinition(probe.Id, probe.Where.TypeName, probe.Where.MethodName, probe.Where.Signature?.Split(separator: ','));
var nativeDefinition = new NativeMethodProbeDefinition(probe.Id, probe.Where.TypeName, probe.Where.MethodName, signature);
methodProbes.Add(nativeDefinition);
ProbeExpressionsProcessor.Instance.AddProbeProcessor(probe);
SetRateLimit(probe);
Expand All @@ -280,6 +284,89 @@ internal void UpdateAddedProbeInstrumentations(IReadOnlyList<ProbeDefinition> ad
}
}

private static string[] SafeParseSignature(string signature)
{
if (string.IsNullOrEmpty(signature))
{
return null;
}

try
{
return ParseSignature(signature);
}
catch (Exception e)
{
Log.Error(e, "Error parsing the signature {Signature}. Fall back to empty signature", signature);
return null;
}
}

private static string[] ParseSignature(string signature)
{
if (signature.StartsWith("(") && signature.EndsWith(")"))
{
signature = signature.Substring(1, signature.Length - 2);
}

var types = new List<string>();
var initialCapacity = 32;
var currentType = StringBuilderCache.Acquire(initialCapacity);
var stack = new Stack<char>();

try
{
foreach (char c in signature)
{
if (c == ' ')
{
continue;
}

if (c == ',' && stack.Count == 0)
{
types.Add(StringBuilderCache.GetStringAndRelease(currentType));
currentType = StringBuilderCache.Acquire(initialCapacity);
}
else
{
if (c == '<')
{
stack.Push(c);
currentType.Append('['); // Replace < with [
}
else if (c == '>')
{
if (stack.Count > 0 && stack.Peek() == '<')
{
stack.Pop();
currentType.Append(']'); // Replace > with ]
}
else
{
throw new InvalidOperationException("Mismatched angle brackets in the signature.");
}
}
else
{
currentType.Append(c);
}
}
}

if (currentType.Length > 0)
{
types.Add(StringBuilderCache.GetStringAndRelease(currentType));
}
}
finally
{
StringBuilderCache.Release(currentType);
}

return types.ToArray();
}

private static void SetRateLimit(ProbeDefinition probe)
{
if (probe is not LogProbe logProbe)
Expand Down
10 changes: 7 additions & 3 deletions tracer/src/Datadog.Tracer.Native/rejit_preprocessor.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -356,9 +356,13 @@ void RejitPreprocessor<RejitRequestDefinition>::ProcessTypeDefForRejit(
auto is_exact_signature_match = GetIsExactSignatureMatch(definition);
if (is_exact_signature_match)
{
const auto isStatic = !(functionInfo.method_signature.CallingConvention() & IMAGE_CEE_CS_CALLCONV_HASTHIS);
const auto thisArg = isStatic ? 0 : 1;
const auto numOfArgsTargetMethod = target_method.signature_types.size();

// Compare if the current mdMethodDef contains the same number of arguments as the
// instrumentation target
if (numOfArgs != target_method.signature_types.size() - 1)
if (numOfArgs != numOfArgsTargetMethod - thisArg)
{
Logger::Info(" * Skipping ", caller.type.name, ".", caller.name,
": the methoddef doesn't have the right number of arguments (", numOfArgs, " arguments).");
Expand All @@ -370,10 +374,10 @@ void RejitPreprocessor<RejitRequestDefinition>::ProcessTypeDefForRejit(
const auto& methodArguments = functionInfo.method_signature.GetMethodArguments();

Logger::Debug(" * Comparing signature for method: ", caller.type.name, ".", caller.name);
for (unsigned int i = 0; i < numOfArgs; i++)
for (unsigned int i = 0; i < numOfArgs && i < numOfArgsTargetMethod + thisArg; i++)
{
const auto argumentTypeName = methodArguments[i].GetTypeTokName(metadataImport);
const auto integrationArgumentTypeName = target_method.signature_types[i + 1];
const auto integrationArgumentTypeName = target_method.signature_types[i + thisArg];
Logger::Debug(" -> ", argumentTypeName, " = ", integrationArgumentTypeName);
if (argumentTypeName != integrationArgumentTypeName && integrationArgumentTypeName != WStr("_"))
{
Expand Down

0 comments on commit 788cc9a

Please sign in to comment.