[Http] Fix propagation issues

src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md

@@ -2,7 +2,10 @@
 
 ## Unreleased
 
-* *Breaking change** The `Enrich` callback option has been removed. For better
+* Added back `netstandard2.0` target.
+  ([#3787](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3787))
+
+* **Breaking change**: The `Enrich` callback option has been removed. For better
   usability, it has been replaced by three separate options: In case of
   `HttpClient` the new options are `EnrichWithHttpRequestMessage`,
   `EnrichWithHttpResponseMessage` and `EnrichWithException` and in case of
@@ -15,11 +18,12 @@
   `HttpClient` and `HttpWebRequest`,`HttpWebResponse` and `Exception` in case of
   `HttpWebRequest`. The separate callbacks make it clear what event triggers
   them and there is no longer the need to cast the argument to the expected
-type.
-([#3792](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3792))
+  type.
+  ([#3792](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3792))
 
-* Added back `netstandard2.0` target.
-([#3787](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3787))
+* Fixed an issue which prevented custom propagators from being called on .NET 7+
+  runtimes for non-sampled outgoing `HttpClient` spans.
+  ([#3828](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3828))
 
 ## 1.0.0-rc9.8
 

src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs

@@ -100,11 +100,7 @@ public void OnStartActivity(Activity activity, object payload)
             // By this time, samplers have already run and
             // activity.IsAllDataRequested populated accordingly.
 
-            // For .NET7.0 or higher versions, activity is created using activity source
-            // However, the framework will fallback to creating activity if the sampler's decision is to drop and there is a active diagnostic listener.
-            // To prevent processing such activities we first check the source name to confirm if it was created using
-            // activity source or not.
-            if (Sdk.SuppressInstrumentation || (IsNet7OrGreater && string.IsNullOrEmpty(activity.Source.Name)))
+            if (Sdk.SuppressInstrumentation)
             {
                 return;
             }
@@ -122,6 +118,15 @@ public void OnStartActivity(Activity activity, object payload)
                 textMapPropagator.Inject(new PropagationContext(activity.Context, Baggage.Current), request, HttpRequestMessageContextPropagation.HeaderValueSetter);
             }
 
+            // For .NET7.0 or higher versions, activity is created using activity source.
+            // However the framework will fallback to creating activity if the sampler's decision is to drop and there is a active diagnostic listener.
+            // To prevent processing such activities we first check the source name to confirm if it was created using
+            // activity source or not.
+            if (IsNet7OrGreater && string.IsNullOrEmpty(activity.Source.Name))
+            {
+                activity.IsAllDataRequested = false;
+            }
+
             // enrich Activity from payload only if sampling decision
             // is favorable.
             if (activity.IsAllDataRequested)
@@ -171,15 +176,6 @@ public void OnStartActivity(Activity activity, object payload)
 
         public void OnStopActivity(Activity activity, object payload)
         {
-            // For .NET7.0 or higher versions, activity is created using activity source
-            // However, the framework will fallback to creating activity if the sampler's decision is to drop and there is a active diagnostic listener.
-            // To prevent processing such activities we first check the source name to confirm if it was created using
-            // activity source or not.
-            if (IsNet7OrGreater && string.IsNullOrEmpty(activity.Source.Name))
-            {
-                return;
-            }
-
             if (activity.IsAllDataRequested)
             {
                 // https://github.com/dotnet/runtime/blob/master/src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHandler.cs
@@ -229,15 +225,6 @@ public void OnStopActivity(Activity activity, object payload)
 
         public void OnException(Activity activity, object payload)
         {
-            // For .NET7.0 or higher versions, activity is created using activity source
-            // However, the framework will fallback to creating activity if the sampler's decision is to drop and there is a active diagnostic listener.
-            // To prevent processing such activities we first check the source name to confirm if it was created using
-            // activity source or not.
-            if (IsNet7OrGreater && string.IsNullOrEmpty(activity.Source.Name))
-            {
-                return;
-            }
-
             if (activity.IsAllDataRequested)
             {
                 if (!this.stopExceptionFetcher.TryFetch(payload, out Exception exc) || exc == null)

test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs

@@ -584,6 +584,81 @@ public async Task HttpClientInstrumentationDoesNotReportExceptionEventOnErrorRes
             Assert.Empty(exportedItems[0].Events);
         }
 
+        [Theory]
+        [InlineData(true, true)]
+        [InlineData(true, false)]
+        [InlineData(false, true)]
+        [InlineData(false, false)]
+        public async Task CustomPropagatorCalled(bool sample, bool createParentActivity)
+        {
+            ActivityContext parentContext = default;
+            ActivityContext contextFromPropagator = default;
+
+            var propagator = new Mock<TextMapPropagator>();
+            propagator.Setup(m => m.Inject(It.IsAny<PropagationContext>(), It.IsAny<HttpRequestMessage>(), It.IsAny<Action<HttpRequestMessage, string, string>>()))
+                .Callback<PropagationContext, HttpRequestMessage, Action<HttpRequestMessage, string, string>>((context, carrier, setter) =>
+                {
+                    contextFromPropagator = context.ActivityContext;
+
+                    setter(carrier, "custom_traceparent", $"00/{contextFromPropagator.TraceId}/{contextFromPropagator.SpanId}/01");
+                    setter(carrier, "custom_tracestate", contextFromPropagator.TraceState);
+                });
+
+            var previousDefaultTextMapPropagator = Propagators.DefaultTextMapPropagator;
+            Sdk.SetDefaultTextMapPropagator(propagator.Object);
+
+            var exportedItems = new List<Activity>();
+
+            using (var traceprovider = Sdk.CreateTracerProviderBuilder()
+               .AddHttpClientInstrumentation()
+               .AddInMemoryExporter(exportedItems)
+               .SetSampler(sample ? new ParentBasedSampler(new AlwaysOnSampler()) : new AlwaysOffSampler())
+               .Build())
+            {
+                Activity parent = null;
+                if (createParentActivity)
+                {
+                    parent = new Activity("parent")
+                        .SetIdFormat(ActivityIdFormat.W3C)
+                        .Start();
+
+                    parent.TraceStateString = "k1=v1,k2=v2";
+                    parent.ActivityTraceFlags = ActivityTraceFlags.Recorded;
+
+                    parentContext = parent.Context;
+                }
+
+                var request = new HttpRequestMessage
+                {
+                    RequestUri = new Uri(this.url),
+                    Method = new HttpMethod("GET"),
+                };
+
+                using var c = new HttpClient();
+                await c.SendAsync(request);
+
+                parent?.Stop();
+            }
+
+            if (!sample)
+            {
+                Assert.Empty(exportedItems);
+            }
+            else
+            {
+                Assert.Single(exportedItems);
+            }
+
+            // Make sure custom propagator was called.
+            Assert.True(contextFromPropagator != default);
+            if (sample)
+            {
+                Assert.Equal(contextFromPropagator, exportedItems[0].Context);
+            }
+
+            Sdk.SetDefaultTextMapPropagator(previousDefaultTextMapPropagator);
+        }
+
         public void Dispose()
         {
             this.serverLifeTime?.Dispose();