[Instrumentation.AspNetCore] Fix issue causing Activity is not export…

…ed when used with custom propagator/sampler. (#4637)
open-telemetry · Jul 10, 2023 · b23b146 · b23b146
1 parent 66a6062
commit b23b146
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 5 deletions.
diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md
@@ -7,6 +7,10 @@
   which attributes are emitted by setting the environment variable
   `OTEL_SEMCONV_STABILITY_OPT_IN`.
 
+* Fixed an issue affecting NET 7.0+. If custom propagation is being used
+  and tags are added to an Activity during sampling then that Activity would be dropped.
+  ([#4637](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4637))
+
 ## 1.5.0-beta.1
 
 Released 2023-Jun-05

diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs
@@ -323,7 +323,12 @@ public void OnStopActivity(Activity activity, object payload)
                 }
             }
 
+#if NET7_0_OR_GREATER
+            var tagValue = activity.GetTagValue("IsCreatedByInstrumentation");
+            if (ReferenceEquals(tagValue, bool.TrueString))
+#else
             if (activity.TryCheckFirstTag("IsCreatedByInstrumentation", out var tagValue) && ReferenceEquals(tagValue, bool.TrueString))
+#endif
             {
                 // If instrumentation started a new Activity, it must
                 // be stopped here.

diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs
@@ -194,8 +194,10 @@ public async Task SuccessfulTemplateControllerCallUsesParentContext()
             ValidateAspNetCoreActivity(activity, "/api/values/2");
         }
 
-        [Fact]
-        public async Task CustomPropagator()
+        [Theory]
+        [InlineData(true)]
+        [InlineData(false)]
+        public async Task CustomPropagator(bool addSampler)
         {
             try
             {
@@ -219,7 +221,18 @@ public async Task CustomPropagator()
                             builder.ConfigureTestServices(services =>
                             {
                                 Sdk.SetDefaultTextMapPropagator(propagator.Object);
-                                this.tracerProvider = Sdk.CreateTracerProviderBuilder().AddAspNetCoreInstrumentation().AddInMemoryExporter(exportedItems).Build();
+                                var tracerProviderBuilder = Sdk.CreateTracerProviderBuilder();
+
+                                if (addSampler)
+                                {
+                                    tracerProviderBuilder
+                                        .SetSampler(new TestSampler(SamplingDecision.RecordAndSample, new Dictionary<string, object> { { "SomeTag", "SomeKey" }, }));
+                                }
+
+                                this.tracerProvider = tracerProviderBuilder
+                                                        .AddAspNetCoreInstrumentation()
+                                                        .AddInMemoryExporter(exportedItems)
+                                                        .Build();
                             });
                             builder.ConfigureLogging(loggingBuilder => loggingBuilder.ClearProviders());
                         }))
@@ -1134,15 +1147,17 @@ public override void Inject<T>(PropagationContext context, T carrier, Action<T,
         private class TestSampler : Sampler
         {
             private readonly SamplingDecision samplingDecision;
+            private readonly IEnumerable<KeyValuePair<string, object>> attributes;
 
-            public TestSampler(SamplingDecision samplingDecision)
+            public TestSampler(SamplingDecision samplingDecision, IEnumerable<KeyValuePair<string, object>> attributes = null)
             {
                 this.samplingDecision = samplingDecision;
+                this.attributes = attributes;
             }
 
             public override SamplingResult ShouldSample(in SamplingParameters samplingParameters)
             {
-                return new SamplingResult(this.samplingDecision);
+                return new SamplingResult(this.samplingDecision, this.attributes);
             }
         }