Fix: Prevent broken traces when HttpClient content headers contain tracing headers. (#1843) (#1888)

Author: nrcventura

src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/HttpClient/SendAsync.cs

@@ -135,6 +135,9 @@ private static void TryAttachHeadersToRequest(IAgent agent, HttpRequestMessage h
         {
             var setHeaders = new Action<HttpRequestMessage, string, string>((carrier, key, value) =>
             {
+                // Content headers should not contain the expected header keys, but sometimes they do,
+                // and their presence can cause problems downstream by having 2 values for the same key.
+                carrier.Content?.Headers?.Remove(key);
                 // "Add" will throw if value exists, so we must remove it first
                 carrier.Headers?.Remove(key);
                 carrier.Headers?.Add(key, value);

tests/Agent/IntegrationTests/Applications/AspNetCoreDistTracingApplication/Controllers/SecondCallController.cs

@@ -18,7 +18,17 @@ public async Task<string> CallNext(string nextUrl)
             {
                 using (var httpClient = new HttpClient())
                 {
-                    var result = await httpClient.GetStringAsync(nextUrl);
+                    var requestMessage = new HttpRequestMessage(HttpMethod.Get, nextUrl);
+                    // Purposely include bad traceparent headers to try to break the distributed trace
+                    requestMessage.Headers.TryAddWithoutValidation("traceparent", "bad-traceparent-requestheader");
+                    requestMessage.Content = new StringContent(string.Empty);
+                    // This is not a valid content header, but there are some cases where this has happened
+                    requestMessage.Content.Headers.TryAddWithoutValidation("traceparent", "bad-traceparent-contentheader");
+
+                    //var result = await httpClient.GetStringAsync(nextUrl);
+                    var response = await httpClient.SendAsync(requestMessage);
+                    var result = await response.Content.ReadAsStringAsync();
+
                     return result;
                 }
             }

tests/Agent/IntegrationTests/IntegrationTests/DistributedTracing/W3CInstrumentationTests/HttpClientW3CTestsNetCore.cs

@@ -64,10 +64,15 @@ public void Test()
             var receiverAppTxEvents = _fixture.SecondCallApplication.AgentLog.GetTransactionEvents().FirstOrDefault();
             Assert.NotNull(receiverAppTxEvents);
 
+            var lastCallAppTxEvents = _fixture.RemoteApplication.AgentLog.GetTransactionEvents().FirstOrDefault();
+            Assert.NotNull(lastCallAppTxEvents);
+
             var senderAppSpanEvents = _fixture.FirstCallApplication.AgentLog.GetSpanEvents();
             var receiverAppSpanEvents = _fixture.SecondCallApplication.AgentLog.GetSpanEvents();
+            var lastCallAppSpanEvents = _fixture.RemoteApplication.AgentLog.GetSpanEvents();
 
             Assert.Equal(senderAppTxEvent.IntrinsicAttributes["guid"], receiverAppTxEvents.IntrinsicAttributes["parentId"]);
+            Assert.Equal(receiverAppTxEvents.IntrinsicAttributes["guid"], lastCallAppTxEvents.IntrinsicAttributes["parentId"]);
 
             foreach (var span in senderAppSpanEvents)
             {
@@ -79,10 +84,18 @@ public void Test()
                 Assert.Equal(TestTraceId, span.IntrinsicAttributes["traceId"]);
             }
 
+            foreach (var span in lastCallAppSpanEvents)
+            {
+                Assert.Equal(TestTraceId, span.IntrinsicAttributes["traceId"]);
+            }
+
             var senderRootSpanEvent = senderAppSpanEvents.Where(@event => @event?.IntrinsicAttributes?["name"]?.ToString() == "WebTransaction/MVC/FirstCall/CallNext/{nextUrl}").FirstOrDefault();
             var externalSpanEvent = senderAppSpanEvents.Where(@event => @event?.IntrinsicAttributes?["name"]?.ToString() == "External/localhost/Stream/GET").FirstOrDefault();
 
             var receiverRootSpanEvent = receiverAppSpanEvents.Where(@event => @event?.IntrinsicAttributes?["name"]?.ToString() == "WebTransaction/MVC/SecondCall/CallNext/{nextUrl}").FirstOrDefault();
+            var receiverExternalSpanEvent = receiverAppSpanEvents.Where(@event => @event?.IntrinsicAttributes?["name"]?.ToString() == "External/localhost/Stream/GET").FirstOrDefault();
+
+            var lastRootSpanEvent = lastCallAppSpanEvents.Where(@event => @event?.IntrinsicAttributes?["name"]?.ToString() == "WebTransaction/MVC/LastCall/CallEnd").FirstOrDefault();
 
             Assert.NotNull(senderRootSpanEvent);
             Assert.Equal(TestTracingVendors, senderRootSpanEvent.IntrinsicAttributes["tracingVendors"]);
@@ -97,6 +110,13 @@ public void Test()
             Assert.Equal(externalSpanEvent.IntrinsicAttributes["guid"], receiverRootSpanEvent.IntrinsicAttributes["trustedParentId"]);
             Assert.True(AttributeComparer.IsEqualTo(senderAppTxEvent.IntrinsicAttributes["priority"], receiverRootSpanEvent.IntrinsicAttributes["priority"]));
 
+            Assert.NotNull(lastRootSpanEvent);
+            Assert.Equal(TestTracingVendors, lastRootSpanEvent.IntrinsicAttributes["tracingVendors"]);
+            Assert.Equal(receiverExternalSpanEvent.IntrinsicAttributes["guid"], lastRootSpanEvent.IntrinsicAttributes["parentId"]);
+            Assert.Equal(receiverExternalSpanEvent.IntrinsicAttributes["guid"], lastCallAppTxEvents.IntrinsicAttributes["parentSpanId"]);
+            Assert.Equal(receiverExternalSpanEvent.IntrinsicAttributes["guid"], lastRootSpanEvent.IntrinsicAttributes["trustedParentId"]);
+            Assert.True(AttributeComparer.IsEqualTo(receiverAppTxEvents.IntrinsicAttributes["priority"], lastRootSpanEvent.IntrinsicAttributes["priority"]));
+
             var senderExpectedMetrics = new List<Assertions.ExpectedMetric>
             {
                 new Assertions.ExpectedMetric { metricName = @"Supportability/DistributedTrace/CreatePayload/Success", callCount = 1 },