[DSM] - Fixes for IbmMq instrumentation (#5271)

* Use byte properties instead of strings * Fixed nullability files * Added some debug info * Fixed lint issues * Added a bit more logs * Using slow byte->sbyte conversion * Added noop headers adapter * Fixed nullability files * Added more logs * Cleaned up debug logs * Removed symlink * Update tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/IbmMq/IbmMqHeadersAdapter.cs Removed debug code Co-authored-by: Andrew Lock <andrew.lock@datadoghq.com> * Update tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/IbmMq/IbmMqHeadersAdapter.cs Using Unsafe.As instead of BlockCopy Co-authored-by: Andrew Lock <andrew.lock@datadoghq.com> * Update tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/IbmMq/IbmMqHeadersAdapter.cs Use Unsafe.As instead of BlockCopy Co-authored-by: Andrew Lock <andrew.lock@datadoghq.com> * Addressed some of the comments * Removed context propagation options --------- Co-authored-by: Andrew Lock <andrew.lock@datadoghq.com>
DataDog · Mar 11, 2024 · 0d511f9 · 0d511f9
1 parent 5684a72
commit 0d511f9
Show file tree

Hide file tree

Showing 6 changed files with 80 additions and 25 deletions.
diff --git a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/IbmMq/GetIntegration.cs b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/IbmMq/GetIntegration.cs
@@ -22,7 +22,7 @@ namespace Datadog.Trace.ClrProfiler.AutoInstrumentation.IbmMq
         TypeName = IbmMqConstants.MqDestinationTypeName,
         MethodName = "Get",
         ReturnTypeName = ClrNames.Void,
-        ParameterTypeNames = new[] { IbmMqConstants.MqMessageTypeName, IbmMqConstants.MqMessageGetOptionsTypeName, ClrNames.Int32 },
+        ParameterTypeNames = [IbmMqConstants.MqMessageTypeName, IbmMqConstants.MqMessageGetOptionsTypeName, ClrNames.Int32],
         MinimumVersion = "9.0.0",
         MaximumVersion = "9.*.*",
         IntegrationName = IbmMqConstants.IntegrationName)]
@@ -68,7 +68,7 @@ internal static CallTargetReturn OnMethodEnd<TTarget>(TTarget instance, Exceptio
                             pathwayContext);
                         // we need to inject new context, since message objects can theoretically be reused
                         // hence we need to make sure the parent hash changes properly
-                        dataStreams.InjectPathwayContextAsBase64String(scope.Span.Context.PathwayContext, new IbmMqHeadersAdapter(msg));
+                        dataStreams.InjectPathwayContextAsBase64String(scope.Span.Context.PathwayContext, IbmMqHelper.GetHeadersAdapter(msg));
                     }
 
                     scope.DisposeWithException(exception);

diff --git a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/IbmMq/IMqMessage.cs b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/IbmMq/IMqMessage.cs
@@ -14,14 +14,9 @@ internal interface IMqMessage
 {
     public int MessageLength { get;  }
 
-    [DuckField(Name = "properties")]
-    public Hashtable Properties { get; }
+    public void SetBytesProperty(string name, sbyte[] value);
 
-    public void SetStringProperty(string name, string value);
-
-    public string GetStringProperty(string name);
-
-    public void WriteBytes(string s);
+    public sbyte[] GetBytesProperty(string name);
 
     public void DeleteProperty(string name);
 }
diff --git a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/IbmMq/IbmMqHeadersAdapter.cs b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/IbmMq/IbmMqHeadersAdapter.cs
@@ -6,14 +6,16 @@
 #nullable enable
 
 using System.Collections.Generic;
+using System.Text;
 using Datadog.Trace.Headers;
 using Datadog.Trace.Util;
+using Datadog.Trace.VendoredMicrosoftCode.System.Runtime.CompilerServices.Unsafe;
 
 namespace Datadog.Trace.ClrProfiler.AutoInstrumentation.IbmMq;
 
 internal readonly struct IbmMqHeadersAdapter : IHeadersCollection
 {
-    private static readonly string[] EmptyValue = { };
+    private static readonly string[] EmptyValue = [];
     private readonly IMqMessage _message;
 
     public IbmMqHeadersAdapter(IMqMessage message)
@@ -43,7 +45,11 @@ public IEnumerable<string> GetValues(string name)
         {
             // there's no way to check if the value exists,
             // and reading non-existent value causes an exception
-            return new[] { _message.GetStringProperty(NormalizeName(name)) };
+            var normName = NormalizeName(name);
+            var buf = _message.GetBytesProperty(normName);
+            var val = StringFromSignedBytes(buf);
+
+            return new[] { val };
         }
         catch
         {
@@ -55,14 +61,13 @@ public void Set(string name, string value)
     {
         var normalizedName = NormalizeName(name);
         RemoveNormalized(normalizedName);
-        _message.SetStringProperty(normalizedName, value);
+        var val = StringToUnsignedBytes(value);
+        _message.SetBytesProperty(normalizedName, val);
     }
 
     public void Add(string name, string value)
     {
-        var normalizedName = NormalizeName(name);
-        RemoveNormalized(normalizedName);
-        _message.SetStringProperty(normalizedName, value);
+        Set(name, value);
     }
 
     private void RemoveNormalized(string normalizedName)
@@ -77,6 +82,19 @@ private void RemoveNormalized(string normalizedName)
         }
     }
 
+    private string StringFromSignedBytes(sbyte[] buf)
+    {
+        // since the text is ASCII signed and unsigned bytes are the same (0-127)
+        return Encoding.ASCII.GetString(Unsafe.As<byte[]>(buf));
+    }
+
+    private sbyte[] StringToUnsignedBytes(string str)
+    {
+        var buf = Encoding.ASCII.GetBytes(str);
+        // since the text is ASCII signed and unsigned bytes are the same.
+        return Unsafe.As<sbyte[]>(buf);
+    }
+
     public void Remove(string name)
     {
         RemoveNormalized(NormalizeName(name));

diff --git a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/IbmMq/IbmMqHeadersAdapterNoop.cs b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/IbmMq/IbmMqHeadersAdapterNoop.cs
@@ -0,0 +1,36 @@
+// <copyright file="IbmMqHeadersAdapterNoop.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>
+
+#nullable enable
+
+using System.Collections.Generic;
+using Datadog.Trace.Headers;
+
+namespace Datadog.Trace.ClrProfiler.AutoInstrumentation.IbmMq;
+
+/// <summary>
+/// Noop adapter used to disable context propagation
+/// </summary>
+internal readonly struct IbmMqHeadersAdapterNoop : IHeadersCollection
+{
+    private static readonly string[] EmptyValue = [];
+
+    public IEnumerable<string> GetValues(string name)
+    {
+        return EmptyValue;
+    }
+
+    public void Set(string name, string value)
+    {
+    }
+
+    public void Add(string name, string value)
+    {
+    }
+
+    public void Remove(string name)
+    {
+    }
+}
diff --git a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/IbmMq/IbmMqHelper.cs b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/IbmMq/IbmMqHelper.cs
@@ -6,18 +6,26 @@
 #nullable enable
 
 using System;
-using Datadog.Trace.ClrProfiler.CallTarget;
 using Datadog.Trace.Configuration;
-using Datadog.Trace.DataStreamsMonitoring;
+using Datadog.Trace.Headers;
 using Datadog.Trace.Propagators;
-using Datadog.Trace.Tagging;
 using Datadog.Trace.Vendors.Serilog;
 
 namespace Datadog.Trace.ClrProfiler.AutoInstrumentation.IbmMq;
 
 internal static class IbmMqHelper
 {
     private const string MessagingType = "ibmmq";
+    private static readonly IbmMqHeadersAdapterNoop NoopAdapter = new();
+
+    internal static IHeadersCollection GetHeadersAdapter(IMqMessage message)
+    {
+        // we temporary switch to noop adapter, since
+        // multiple customers reported issues with context propagation.
+        // The goal is to allow context injection only when we have a way of configuring
+        // this on per-instrumentation basis.
+        return NoopAdapter;
+    }
 
     internal static Scope? CreateProducerScope(Tracer tracer, IMqQueue queue, IMqMessage message)
     {
@@ -49,8 +57,7 @@ internal static class IbmMqHelper
             span.ResourceName = resourceName;
             span.SetTag(Tags.SpanKind, SpanKinds.Producer);
 
-            var adapter = new IbmMqHeadersAdapter(message);
-            SpanContextPropagator.Instance.Inject(span.Context, adapter);
+            SpanContextPropagator.Instance.Inject(span.Context, GetHeadersAdapter(message));
         }
         catch (Exception ex)
         {
@@ -87,10 +94,9 @@ internal static class IbmMqHelper
 
             SpanContext? propagatedContext = null;
 
-            var adapter = new IbmMqHeadersAdapter(message);
             try
             {
-                propagatedContext = SpanContextPropagator.Instance.Extract(adapter);
+                propagatedContext = SpanContextPropagator.Instance.Extract(GetHeadersAdapter(message));
             }
             catch (Exception ex)
             {

diff --git a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/IbmMq/PutIntegration.cs b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/IbmMq/PutIntegration.cs
@@ -21,7 +21,7 @@ namespace Datadog.Trace.ClrProfiler.AutoInstrumentation.IbmMq
         TypeName = IbmMqConstants.MqDestinationTypeName,
         MethodName = "Put",
         ReturnTypeName = ClrNames.Void,
-        ParameterTypeNames = new[] { IbmMqConstants.MqMessageTypeName, IbmMqConstants.MqMessagePutOptionsTypeName },
+        ParameterTypeNames = [IbmMqConstants.MqMessageTypeName, IbmMqConstants.MqMessagePutOptionsTypeName],
         MinimumVersion = "9.0.0",
         MaximumVersion = "9.*.*",
         IntegrationName = IbmMqConstants.IntegrationName)]
@@ -42,11 +42,11 @@ internal static CallTargetState OnMethodBegin<TTarget, TMessage, TOptions>(TTarg
             if (scope is not null)
             {
                 var dataStreams = Tracer.Instance.TracerManager.DataStreamsManager;
-                if (dataStreams.IsEnabled && ((IDuckType)instance).Instance != null && ((IDuckType)msg).Instance != null)
+                if (dataStreams.IsEnabled && (instance).Instance != null && (msg).Instance != null)
                 {
                     var edgeTags = new[] { "direction:out", $"topic:{instance.Name}", $"type:{IbmMqConstants.QueueType}" };
                     scope.Span.SetDataStreamsCheckpoint(dataStreams, CheckpointKind.Produce, edgeTags, msg.MessageLength, 0);
-                    dataStreams.InjectPathwayContextAsBase64String(scope.Span.Context.PathwayContext, new IbmMqHeadersAdapter(msg));
+                    dataStreams.InjectPathwayContextAsBase64String(scope.Span.Context.PathwayContext, IbmMqHelper.GetHeadersAdapter(msg));
                 }
 
                 return new CallTargetState(scope);