fix: allow the agent to accept multiple versions of legacy NR distributed tracing headers (#1489)

* Initial implementation and unit tests

* Remove unreachable and obsolete code and tests associated with AcceptDistributedTracePayload

* More cleanup

* Forgot one cleanup item

Author: nr-ahemsath

src/Agent/NewRelic/Agent/Core/Agent.cs

@@ -225,24 +225,6 @@ bool ShouldRunExplain()
 
         #region inbound CAT request, outbound CAT response
 
-        public bool TryGetDistributedTracePayloadFromHeaders<T>(IEnumerable<KeyValuePair<string, T>> headers, out T payload) where T : class
-        {
-            payload = null;
-            if (headers != null)
-            {
-                foreach (var header in headers)
-                {
-                    if (header.Key.Equals(Constants.DistributedTracePayloadKey, StringComparison.OrdinalIgnoreCase))
-                    {
-                        payload = header.Value;
-                        return true;
-                    }
-                }
-            }
-
-            return false;
-        }
-
         internal void TryProcessCatRequestData<T>(IInternalTransaction transaction, T carrier, Func<T, string, IEnumerable<string>> getter)
         {
             try

src/Agent/NewRelic/Agent/Core/DistributedTracing/DistributedTracePayloadHandler.cs

@@ -28,8 +28,6 @@ public interface IDistributedTracePayloadHandler
 
         DistributedTracePayload TryDecodeInboundSerializedDistributedTracePayload(string serializedPayload);
 
-        ITracingState AcceptDistributedTracePayload(string serializedPayload, TransportType transportType, DateTime transactionStartTime);
-
         ITracingState AcceptDistributedTraceHeaders<T>(T carrier, Func<T, string, IEnumerable<string>> getter, TransportType transportType, DateTime transactionStartTime);
 
         void InsertDistributedTraceHeaders<T>(IInternalTransaction transaction, T carrier, Action<T, string, string> setter);
@@ -68,11 +66,11 @@ public void InsertDistributedTraceHeaders<T>(IInternalTransaction transaction, T
                 var timestamp = DateTime.UtcNow;
                 if (!_configurationService.Configuration.ExcludeNewrelicHeader)
                 {
-                    //set "Newrelic" header
+                    //set "newrelic" header
                     var distributedTracePayload = GetOutboundHeader(DistributedTraceHeaderType.NewRelic, transaction, timestamp);
                     if (!string.IsNullOrWhiteSpace(distributedTracePayload))
                     {
-                        setter(carrier, Constants.DistributedTracePayloadKey, distributedTracePayload);
+                        setter(carrier, Constants.DistributedTracePayloadKeyAllLower, distributedTracePayload);
                     }
                 }
 
@@ -249,25 +247,6 @@ private IDistributedTracePayload TryGetOutboundDistributedTraceApiModelInternal(
 
         #region Incoming/Accept
 
-        public ITracingState AcceptDistributedTracePayload(string serializedPayload, TransportType transportType, DateTime transactionStartTime)
-        {
-            var tracingState = TracingState.AcceptDistributedTracePayload(serializedPayload, transportType, _configurationService.Configuration.TrustedAccountKey, transactionStartTime);
-
-            if (tracingState.IngestErrors != null)
-            {
-                ReportIncomingErrors(tracingState.IngestErrors);
-            }
-            else
-            {
-                if (_configurationService.Configuration.PayloadSuccessMetricsEnabled)
-                {
-                    _agentHealthReporter.ReportSupportabilityDistributedTraceAcceptPayloadSuccess();
-                }
-            }
-
-            return tracingState;
-        }
-
         public ITracingState AcceptDistributedTraceHeaders<T>(T carrier, Func<T, string, IEnumerable<string>> getter, TransportType transportType, DateTime transactionStartTime)
         {
             if (getter == null)

src/Agent/NewRelic/Agent/Core/DistributedTracing/TracingState.cs

@@ -205,25 +205,6 @@ public float? Priority
         private DistributedTracePayload _newRelicPayload;
         private W3CTraceContext _traceContext;
 
-        public static ITracingState AcceptDistributedTracePayload(string encodedPayload, TransportType transportType, string agentTrustKey, DateTime transactionStartTime)
-        {
-            var tracingState = new TracingState();
-            var errors = new List<IngestErrorType>();
-            tracingState._newRelicPayload = DistributedTracePayload.TryDecodeAndDeserializeDistributedTracePayload(encodedPayload, agentTrustKey, errors);
-            tracingState.NewRelicPayloadWasAccepted = tracingState._newRelicPayload != null ? true : false;
-            tracingState._transactionStartTime = tracingState._newRelicPayload != null ? transactionStartTime : default;
-
-            if (errors.Any())
-            {
-                tracingState.IngestErrors = errors;
-            }
-
-            tracingState.HasDataForAttributes = tracingState.NewRelicPayloadWasAccepted;
-            tracingState.TransportType = transportType;
-
-            return tracingState;
-        }
-
         public static ITracingState AcceptDistributedTraceHeaders<T>(T carrier, Func<T, string, IEnumerable<string>> getter, TransportType transportType, string agentTrustKey, DateTime transactionStartTime)
         {
             var tracingState = new TracingState();
@@ -242,8 +223,19 @@ public static ITracingState AcceptDistributedTraceHeaders<T>(T carrier, Func<T,
             // if traceparent was present (regardless if valid), ignore newrelic header
             if (!tracingState._traceContext.TraceparentPresent)
             {
-                var newRelicHeaderList = getter(carrier, Constants.DistributedTracePayloadKey);
-                if (newRelicHeaderList?.Count() > 0) // the Newrelic header key was present
+                // Search for the following header keys in this order: "newrelic", "NEWRELIC", "Newrelic"
+                // If the getter function makes a case-insensitive search it will find any of the three
+                // variants on the first call.
+                var newRelicHeaderList = getter(carrier, Constants.DistributedTracePayloadKeyAllLower);
+                if (newRelicHeaderList?.Any() == false)
+                {
+                    newRelicHeaderList = getter(carrier, Constants.DistributedTracePayloadKeyAllUpper);
+                }
+                if (newRelicHeaderList?.Any() == false)
+                {
+                    newRelicHeaderList = getter(carrier, Constants.DistributedTracePayloadKeySingleUpper);
+                }
+                if (newRelicHeaderList?.Any() == true) // a NR header key was present
                 {
                     tracingState._newRelicPayload = DistributedTracePayload.TryDecodeAndDeserializeDistributedTracePayload(newRelicHeaderList.FirstOrDefault(), agentTrustKey, errors);
                     tracingState.NewRelicPayloadWasAccepted = tracingState._newRelicPayload != null ? true : false;

src/Agent/NewRelic/Agent/Core/NewRelic.Agent.Core.Metric/ApiSupportabilityMetricCounters.cs

@@ -11,35 +11,32 @@ namespace NewRelic.Agent.Core.Metric
 {
     public enum ApiMethod
     {
-        CreateDistributedTracePayload = 0,
-        AcceptDistributedTracePayload = 1,
-        CurrentTransaction = 2,
-        AddCustomParameter = 3,     //To Be replaced by AddCustomAttribute
-        DisableBrowserMonitoring = 4,
-        GetBrowserTimingHeader = 5,
-        IgnoreApdex = 6,
-        IgnoreTransaction = 7,
-        IncrementCounter = 8,
-        NoticeError = 9,
-        RecordCustomEvent = 10,
-        RecordMetric = 11,
-        RecordResponseTimeMetric = 12,
-        SetApplicationName = 13,
-        SetTransactionName = 14,
-        SetUserParameters = 15,
-        GetBrowserTimingFooter = 16,
-        StartAgent = 17,
-        SetTransactionUri = 18,
-        TraceMetadata = 19,
-        GetLinkingMetadata = 20,
-        TransactionAddCustomAttribute = 21,
-        TransactionGetCurrentSpan = 22,
-        SpanAddCustomAttribute = 23,
-        InsertDistributedTraceHeaders = 24,
-        AcceptDistributedTraceHeaders = 25,
-        SpanSetName = 26,
-        SetErrorGroupCallback = 27,
-        SetUserId = 28
+        CurrentTransaction = 0,
+        DisableBrowserMonitoring = 1,
+        GetBrowserTimingHeader = 2,
+        IgnoreApdex = 3,
+        IgnoreTransaction = 4,
+        IncrementCounter = 5,
+        NoticeError = 6,
+        RecordCustomEvent = 7,
+        RecordMetric = 8,
+        RecordResponseTimeMetric = 9,
+        SetApplicationName = 10,
+        SetTransactionName = 11,
+        SetUserParameters = 12,
+        GetBrowserTimingFooter = 13,
+        StartAgent = 14,
+        SetTransactionUri = 15,
+        TraceMetadata = 16,
+        GetLinkingMetadata = 17,
+        TransactionAddCustomAttribute = 18,
+        TransactionGetCurrentSpan = 19,
+        SpanAddCustomAttribute = 20,
+        InsertDistributedTraceHeaders = 21,
+        AcceptDistributedTraceHeaders = 22,
+        SpanSetName = 23,
+        SetErrorGroupCallback = 24,
+        SetUserId = 25
     }
 
     public interface IApiSupportabilityMetricCounters : IOutOfBandMetricSource

src/Agent/NewRelic/Agent/Core/Transactions/NoOpTransaction.cs

@@ -107,11 +107,6 @@ public IEnumerable<KeyValuePair<string, string>> GetRequestMetadata()
             return Enumerable.Empty<KeyValuePair<string, string>>();
         }
 
-        public void AcceptDistributedTracePayload(string payload, TransportType transportType)
-        {
-            Log.Debug("Tried to accept distributed trace payload, but there was no transaction");
-        }
-
         public IDistributedTracePayload CreateDistributedTracePayload()
         {
             Log.Debug("Tried to create distributed trace payload, but there was no transaction");

src/Agent/NewRelic/Agent/Core/Transactions/Transaction.cs

@@ -559,7 +559,7 @@ public IEnumerable<KeyValuePair<string, string>> GetRequestMetadata()
                 {
                     return headers;
                 }
-                return headers.Concat(new[] { new KeyValuePair<string, string>(Constants.DistributedTracePayloadKey, payload.HttpSafe()) });
+                return headers.Concat(new[] { new KeyValuePair<string, string>(Constants.DistributedTracePayloadKeyAllLower, payload.HttpSafe()) });
             }
 
             // CAT
@@ -607,28 +607,6 @@ private void UpdatePathHash(TransactionMetricName transactionMetricName)
             TransactionMetadata.SetCrossApplicationPathHash(pathHash);
         }
 
-        public void AcceptDistributedTracePayload(string payload, TransportType transportType)
-        {
-            if (!_configuration.DistributedTracingEnabled)
-            {
-                return;
-            }
-            if (TransactionMetadata.HasOutgoingTraceHeaders)
-            {
-                Agent._agentHealthReporter.ReportSupportabilityDistributedTraceAcceptPayloadIgnoredCreateBeforeAccept();
-                return;
-            }
-            if (TracingState != null)
-            {
-                Agent._agentHealthReporter.ReportSupportabilityDistributedTraceAcceptPayloadIgnoredMultiple();
-                return;
-            }
-
-            var isUnknownTransportType = transportType < TransportType.Unknown || transportType > TransportType.Other;
-
-            TracingState = Agent._distributedTracePayloadHandler.AcceptDistributedTracePayload(payload, isUnknownTransportType ? TransportType.Unknown : transportType, StartTime);
-        }
-
         public void AcceptDistributedTraceHeaders<T>(T carrier, Func<T, string, IEnumerable<string>> getter, TransportType transportType)
         {
             if (_configuration.DistributedTracingEnabled)

src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Api/IAgent.cs

@@ -92,8 +92,6 @@ public interface IAgent : IAgentExperimental
         /// <param name="requestPath">The path of the request</param>
         Stream TryGetStreamInjector(Stream stream, Encoding encoding, string contentType, string requestPath);
 
-        bool TryGetDistributedTracePayloadFromHeaders<T>(IEnumerable<KeyValuePair<string, T>> headers, out T payload) where T : class;
-
         /// <summary>
         /// Returns the Trace Metadata of the currently executing transaction.
         /// </summary>

src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Api/ITransaction.cs

@@ -112,12 +112,6 @@ public interface ITransaction
         /// <returns>Collection of key value pairs representing the response metadata</returns>
         IEnumerable<KeyValuePair<string, string>> GetResponseMetadata();
 
-        /// <summary>
-        /// Processes incoming distributed trace request.
-        /// </summary>
-        /// <param name="payload">Serialized incoming distributed trace payload.  May or may not be Base64 Encoded.</param>
-        void AcceptDistributedTracePayload(string payload, TransportType transportType);
-
         /// <summary>
         /// Returns the distributed trace payload model that is attached to the outbound request.
         /// </summary>

src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Providers/Wrapper/Constants.cs

@@ -2,7 +2,6 @@
 // SPDX-License-Identifier: Apache-2.0
 
 using System.Collections.Concurrent;
-using System.Linq;
 using System.Text;
 
 namespace NewRelic.Agent.Extensions.Providers.Wrapper
@@ -14,8 +13,11 @@ public static class Constants
     {
         /// <summary>
         /// This is the key-part that the agent recognizes when trying to find a DistributedTracePayload, typically passed as a KeyValuePair in the header of a request.
+        /// Per the agent specs, the agent should accept the following variants of the header key name: newrelic, NEWRELIC, Newrelic
         /// </summary>
-        public const string DistributedTracePayloadKey = "newrelic";
+        public const string DistributedTracePayloadKeyAllLower = "newrelic";
+        public const string DistributedTracePayloadKeyAllUpper = "NEWRELIC";
+        public const string DistributedTracePayloadKeySingleUpper = "Newrelic";
 
         public const string TraceParentHeaderKey = "traceparent";
 

src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/RabbitMqHelper.cs

@@ -51,19 +51,6 @@ public static string ResolveDestinationName(MessageBrokerDestinationType destina
                 : queueNameOrRoutingKey;
         }
 
-        public static bool TryGetPayloadFromHeaders(Dictionary<string, object> messageHeaders, IAgent agent,
-            out string serializedPayload)
-        {
-            if (agent.TryGetDistributedTracePayloadFromHeaders(messageHeaders, out var payload))
-            {
-                serializedPayload = Encoding.UTF8.GetString((byte[])(payload));
-                return true;
-            }
-
-            serializedPayload = null;
-            return false;
-        }
-
         public static ISegment CreateSegmentForPublishWrappers(InstrumentedMethodCall instrumentedMethodCall, ITransaction transaction, int basicPropertiesIndex)
         {
             // ATTENTION: We have validated that the use of dynamic here is appropriate based on the visibility of the data we're working with.

tests/Agent/IntegrationTests/Applications/DistributedTracingApiApplication/Program.cs

@@ -68,36 +68,13 @@ static void Main(string[] args)
             }
             else
             {
-                var dtPayload = CallCreateDTPayload();
-                CallAcceptDTPayload(dtPayload);
+                throw new NotImplementedException();
             }
 
             // wait for the test harness to tell us to shut down
             eventWaitHandle.WaitOne(TimeSpan.FromMinutes(5));
         }
 
-        [Transaction]
-        [MethodImpl(MethodImplOptions.NoInlining)]
-        private static IDistributedTracePayload CallCreateDTPayload()
-        {
-            var currentTransaction = _agent.CurrentTransaction;
-            // As long as this deprecated API is still shipping, we still need to test it
-#pragma warning disable CS0618 // Type or member is obsolete
-            return currentTransaction.CreateDistributedTracePayload();
-#pragma warning restore CS0618 // Type or member is obsolete
-        }
-
-        [Transaction]
-        [MethodImpl(MethodImplOptions.NoInlining)]
-        private static void CallAcceptDTPayload(IDistributedTracePayload payload)
-        {
-            var currentTransaction = _agent.CurrentTransaction;
-            // As long as this deprecated API is still shipping, we still need to test it
-#pragma warning disable CS0618 // Type or member is obsolete
-            currentTransaction.AcceptDistributedTracePayload(payload.HttpSafe());
-#pragma warning restore CS0618 // Type or member is obsolete
-        }
-
         [Transaction]
         [MethodImpl(MethodImplOptions.NoInlining)]
         private static void CallInsertDTHeaders()

tests/Agent/UnitTests/CompositeTests/CompositeTests.csproj

@@ -43,9 +43,6 @@
     <Service Include="{82a7f48d-3b50-4b1e-b82e-3ada8210c358}" />
   </ItemGroup>
   <ItemGroup>
-    <None Update="CrossAgentTests\DistributedTracing\distributed_tracing.json">
-      <CopyToOutputDirectory>Always</CopyToOutputDirectory>
-    </None>
     <None Update="CrossAgentTests\DistributedTracing\trace_context.json">
       <CopyToOutputDirectory>Always</CopyToOutputDirectory>
     </None>