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 committed Aug 5, 2024
1 parent 315f00b commit dc427de
Show file tree
Hide file tree
Showing 11 changed files with 343 additions and 49 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ namespace Datadog.Trace.Debugger.ExceptionAutoInstrumentation
{
internal class ExceptionDebuggingSettings
{
public const int DefaultMaxFramesToCapture = 3;
public const int DefaultMaxFramesToCapture = 4;
public const int DefaultRateLimitSeconds = 60 * 60; // 1 hour
public const int DefaultMaxExceptionAnalysisLimit = 100;

Expand Down
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);
}
}
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
9 changes: 7 additions & 2 deletions tracer/src/Datadog.Trace/Debugger/LiveDebugger.cs
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,8 @@
using Datadog.Trace.DogStatsd;
using Datadog.Trace.Logging;
using Datadog.Trace.RemoteConfigurationManagement;
using Datadog.Trace.Util;
using Datadog.Trace.Vendors.dnlib.DotNet;
using Datadog.Trace.Vendors.StatsdClient;
using ProbeInfo = Datadog.Trace.Debugger.Expressions.ProbeInfo;

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

case ProbeLocationType.Method:
{
var signature = SignatureParser.SafeParse(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 Down
120 changes: 120 additions & 0 deletions tracer/src/Datadog.Trace/Debugger/SignatureParser.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
// <copyright file="SignatureParser.cs" company="Datadog">
// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License.
// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
// </copyright>

using System;
using System.Collections.Generic;
using Datadog.Trace.Logging;
using Datadog.Trace.VendoredMicrosoftCode.System.Buffers;

#nullable enable
namespace Datadog.Trace.Debugger
{
internal static class SignatureParser
{
private static readonly IDatadogLogger Log = DatadogLogging.GetLoggerFor(typeof(SignatureParser));

internal static string[]? SafeParse(string? signature)
{
try
{
return Parse(signature);
}
catch (Exception e)
{
Log.Error(e, "Error parsing the signature {Signature}. Fall back to empty signature", signature);
return null;
}
}

internal static string[]? Parse(string? signature)
{
if (string.IsNullOrWhiteSpace(signature) || signature == "()")
{
return null;
}

signature = signature!.Replace(" ", string.Empty);

if (signature.StartsWith("(") && signature.EndsWith(")"))
{
signature = signature.Substring(1, signature.Length - 2);
}

// Fast path if we don't deal with generics
if (!signature.Contains("<") && !signature.Contains(">"))
{
return signature.Split(',');
}

return ParseGenericTypes(signature);
}

private static string[]? ParseGenericTypes(string signature)
{
var result = new List<string>();
var bufferIndex = 0;
var genericDepth = 0;
var buffer = ArrayPool<char>.Shared.Rent(signature.Length);

try
{
for (var i = 0; i < signature.Length; i++)
{
var c = signature[i];

if (c == ',' && genericDepth == 0)
{
if (bufferIndex > 0)
{
result.Add(new string(buffer, 0, bufferIndex));
bufferIndex = 0;
}
}
else if (i < signature.Length - 1 && signature[i] == '<' && signature[i + 1] == '>')
{
buffer[bufferIndex++] = '<';
buffer[bufferIndex++] = '>';
i++; // Skip the next '>'
}
else if (c == '<')
{
genericDepth++;
buffer[bufferIndex++] = '[';
}
else if (c == '>')
{
if (genericDepth == 0)
{
return null; // Unmatched closing generic bracket
}

genericDepth--;
buffer[bufferIndex++] = ']';
}
else
{
buffer[bufferIndex++] = c;
}
}

if (genericDepth != 0)
{
return null; // Unmatched opening generic bracket
}

if (bufferIndex > 0)
{
result.Add(new string(buffer, 0, bufferIndex));
}

return result.Count > 0 ? result.ToArray() : null;
}
finally
{
ArrayPool<char>.Shared.Return(buffer, true);
}
}
}
}
43 changes: 43 additions & 0 deletions tracer/src/Datadog.Tracer.Native/debugger_rejit_preprocessor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,49 @@ const bool DebuggerRejitPreprocessor::SupportsSelectiveEnablement()
return false;
}

bool DebuggerRejitPreprocessor::CheckExactSignatureMatch(ComPtr<IMetaDataImport2>& metadataImport,
const FunctionInfo& functionInfo, const MethodReference& targetMethod)
{
const auto numOfArgs = functionInfo.method_signature.NumberOfArguments();
const auto isStatic = !(functionInfo.method_signature.CallingConvention() & IMAGE_CEE_CS_CALLCONV_HASTHIS);
const auto thisArg = isStatic ? 0 : 1;
const auto numOfArgsTargetMethod = targetMethod.signature_types.size();

// Compare if the current mdMethodDef contains the same number of arguments as the
// instrumentation target
if (numOfArgs != numOfArgsTargetMethod - thisArg)
{
Logger::Info(" * Skipping ", functionInfo.type.name, ".", functionInfo.name,
": the methoddef doesn't have the right number of arguments (", numOfArgs, " arguments).");
return false;
}

// Compare each mdMethodDef argument type to the instrumentation target
bool argumentsMismatch = false;
const auto& methodArguments = functionInfo.method_signature.GetMethodArguments();

Logger::Debug(" * Comparing signature for method: ", functionInfo.type.name, ".", functionInfo.name);
for (unsigned int i = 0; i < numOfArgs && (i + thisArg) < numOfArgsTargetMethod; i++)
{
const auto argumentTypeName = methodArguments[i].GetTypeTokName(metadataImport);
const auto integrationArgumentTypeName = targetMethod.signature_types[i + thisArg];
Logger::Debug(" -> ", argumentTypeName, " = ", integrationArgumentTypeName);
if (argumentTypeName != integrationArgumentTypeName && integrationArgumentTypeName != WStr("_"))
{
argumentsMismatch = true;
break;
}
}
if (argumentsMismatch)
{
Logger::Info(" * Skipping ", targetMethod.method_name,
": the methoddef doesn't have the right type of arguments.");
return false;
}

return true;
}


const std::unique_ptr<RejitHandlerModuleMethod>
DebuggerRejitPreprocessor::CreateMethod(const mdMethodDef methodDef, RejitHandlerModule* module,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ class DebuggerRejitPreprocessor : public RejitPreprocessor<std::shared_ptr<Metho
const bool GetIsExactSignatureMatch(const std::shared_ptr<MethodProbeDefinition>& definition) final;
const bool GetIsEnabled(const std::shared_ptr<MethodProbeDefinition>& definition) final;
const bool SupportsSelectiveEnablement() final;

bool CheckExactSignatureMatch(ComPtr<IMetaDataImport2>& metadataImport, const FunctionInfo& functionInfo,
const MethodReference& targetMethod) override;

const std::unique_ptr<RejitHandlerModuleMethod>
CreateMethod(mdMethodDef methodDef, RejitHandlerModule* module, const FunctionInfo& functionInfo,
const std::shared_ptr<MethodProbeDefinition>& methodProbe) final;
Expand Down
Loading

0 comments on commit dc427de

Please sign in to comment.