[Dynamic Instrumentation] Improved instrumentation matching of symbols received through SymDb

tracer/src/Datadog.Trace/Debugger/ExceptionAutoInstrumentation/ExceptionDebuggingSettings.cs

@@ -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;
 

tracer/src/Datadog.Trace/Debugger/ExceptionAutoInstrumentation/ExceptionReplayDiagnosticTagNames.cs

@@ -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);
@@ -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);
     }
 }

tracer/src/Datadog.Trace/Debugger/ExceptionAutoInstrumentation/ExceptionTrackManager.cs

@@ -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;
             }
@@ -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)
             {
@@ -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;
@@ -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);
+                }
             }
         }
 

tracer/src/Datadog.Trace/Debugger/LiveDebugger.cs

@@ -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;
@@ -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;
 
@@ -246,15 +249,17 @@ internal void UpdateAddedProbeInstrumentations(IReadOnlyList<ProbeDefinition> ad
 
                         case ProbeLocationType.Method:
                         {
+                            SignatureParser.TryParse(probe.Where.Signature, out var 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);

tracer/src/Datadog.Trace/Debugger/SignatureParser.cs

@@ -0,0 +1,123 @@
+// <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 System.Diagnostics.CodeAnalysis;
+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 bool TryParse(string? signature, [NotNullWhen(true)] out string[]? signatures)
+        {
+            try
+            {
+                signatures = Parse(signature);
+            }
+            catch (Exception e)
+            {
+                Log.Error(e, "Error parsing the signature {Signature}. Fall back to empty signature", signature);
+                signatures = null;
+            }
+
+            return !string.IsNullOrEmpty(signature);
+        }
+
+        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);
+            }
+        }
+    }
+}

tracer/src/Datadog.Tracer.Native/debugger_rejit_preprocessor.cpp

@@ -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,

tracer/src/Datadog.Tracer.Native/debugger_rejit_preprocessor.h

@@ -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;