From 0d511f99bb339ddfdd113f2e5eb7be3121aee785 Mon Sep 17 00:00:00 2001 From: Igor Kravchenko <21974069+kr-igor@users.noreply.github.com> Date: Mon, 11 Mar 2024 09:35:23 -0500 Subject: [PATCH] [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 * Update tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/IbmMq/IbmMqHeadersAdapter.cs Using Unsafe.As instead of BlockCopy Co-authored-by: Andrew Lock * Update tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/IbmMq/IbmMqHeadersAdapter.cs Use Unsafe.As instead of BlockCopy Co-authored-by: Andrew Lock * Addressed some of the comments * Removed context propagation options --------- Co-authored-by: Andrew Lock --- .../IbmMq/GetIntegration.cs | 4 +-- .../AutoInstrumentation/IbmMq/IMqMessage.cs | 9 ++--- .../IbmMq/IbmMqHeadersAdapter.cs | 30 ++++++++++++---- .../IbmMq/IbmMqHeadersAdapterNoop.cs | 36 +++++++++++++++++++ .../AutoInstrumentation/IbmMq/IbmMqHelper.cs | 20 +++++++---- .../IbmMq/PutIntegration.cs | 6 ++-- 6 files changed, 80 insertions(+), 25 deletions(-) create mode 100644 tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/IbmMq/IbmMqHeadersAdapterNoop.cs diff --git a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/IbmMq/GetIntegration.cs b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/IbmMq/GetIntegration.cs index 7499f2268491..f80f10f44f44 100644 --- 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 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 index 3ade0b859344..0b2f79b6ad9e 100644 --- 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 index b04939da95cf..3a9b8c5371d5 100644 --- 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 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(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(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 new file mode 100644 index 000000000000..ca30fe118ead --- /dev/null +++ b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/IbmMq/IbmMqHeadersAdapterNoop.cs @@ -0,0 +1,36 @@ +// +// 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. +// + +#nullable enable + +using System.Collections.Generic; +using Datadog.Trace.Headers; + +namespace Datadog.Trace.ClrProfiler.AutoInstrumentation.IbmMq; + +/// +/// Noop adapter used to disable context propagation +/// +internal readonly struct IbmMqHeadersAdapterNoop : IHeadersCollection +{ + private static readonly string[] EmptyValue = []; + + public IEnumerable 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 index bfda7d10a461..7eff556854e3 100644 --- a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/IbmMq/IbmMqHelper.cs +++ b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/IbmMq/IbmMqHelper.cs @@ -6,11 +6,9 @@ #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; @@ -18,6 +16,16 @@ 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 index 6000416d903e..34d10d21d466 100644 --- 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(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);