Fix OpenTracing shim under legacy AspNetCore activities (#3506)

open-telemetry · Aug 3, 2022 · 3929727 · 3929727
1 parent 2821898
commit 3929727
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 20 deletions.
diff --git a/src/OpenTelemetry.Shims.OpenTracing/CHANGELOG.md b/src/OpenTelemetry.Shims.OpenTracing/CHANGELOG.md
@@ -2,6 +2,10 @@
 
 ## Unreleased
 
+* Fix: Handling of OpenTracing spans when used in conjunction
+  with legacy "Microsoft.AspNetCore.Hosting.HttpRequestIn" activities.
+  ([#3509](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3506))
+
 ## 1.0.0-rc9.4
 
 Released 2022-Jun-03

diff --git a/src/OpenTelemetry.Shims.OpenTracing/SpanBuilderShim.cs b/src/OpenTelemetry.Shims.OpenTracing/SpanBuilderShim.cs
@@ -16,7 +16,6 @@
 
 using System;
 using System.Collections.Generic;
-using System.Diagnostics;
 using OpenTelemetry.Internal;
 using OpenTelemetry.Trace;
 using OpenTracing;
@@ -50,14 +49,6 @@ internal sealed class SpanBuilderShim : ISpanBuilder
         /// </summary>
         private readonly List<KeyValuePair<string, object>> attributes = new();
 
-        /// <summary>
-        /// The set of operation names for System.Diagnostics.Activity based automatic instrumentations that indicate a root span.
-        /// </summary>
-        private readonly IList<string> rootOperationNamesForActivityBasedAutoInstrumentations = new List<string>
-        {
-            "Microsoft.AspNetCore.Hosting.HttpRequestIn",
-        };
-
         /// <summary>
         /// The parent as an TelemetrySpan, if any.
         /// </summary>
@@ -79,15 +70,14 @@ internal sealed class SpanBuilderShim : ISpanBuilder
 
         private bool error;
 
-        public SpanBuilderShim(Tracer tracer, string spanName, IList<string> rootOperationNamesForActivityBasedAutoInstrumentations = null)
+        public SpanBuilderShim(Tracer tracer, string spanName)
         {
             Guard.ThrowIfNull(tracer);
             Guard.ThrowIfNull(spanName);
 
             this.tracer = tracer;
             this.spanName = spanName;
             this.ScopeManager = new ScopeManagerShim(this.tracer);
-            this.rootOperationNamesForActivityBasedAutoInstrumentations = rootOperationNamesForActivityBasedAutoInstrumentations ?? this.rootOperationNamesForActivityBasedAutoInstrumentations;
         }
 
         private IScopeManager ScopeManager { get; }
@@ -172,13 +162,6 @@ public ISpan Start()
             {
                 span = this.tracer.StartSpan(this.spanName, this.spanKind, this.parentSpanContext, default, this.links, this.explicitStartTime ?? default);
             }
-            else if (this.parentSpan == null && !this.parentSpanContext.IsValid && Activity.Current != null && Activity.Current.IdFormat == ActivityIdFormat.W3C)
-            {
-                if (this.rootOperationNamesForActivityBasedAutoInstrumentations.Contains(Activity.Current.OperationName))
-                {
-                    span = Tracer.CurrentSpan;
-                }
-            }
 
             if (span == null)
             {

diff --git a/test/OpenTelemetry.Shims.OpenTracing.Tests/SpanBuilderShimTests.cs b/test/OpenTelemetry.Shims.OpenTracing.Tests/SpanBuilderShimTests.cs
@@ -127,13 +127,13 @@ public void Start_ActivityOperationRootSpanChecks()
 
             // matching root operation name
             var tracer = TracerProvider.Default.GetTracer(TracerName);
-            var shim = new SpanBuilderShim(tracer, "foo", new List<string> { "foo" });
+            var shim = new SpanBuilderShim(tracer, "foo");
             var spanShim1 = (SpanShim)shim.Start();
 
             Assert.Equal("foo", spanShim1.Span.Activity.OperationName);
 
             // mis-matched root operation name
-            shim = new SpanBuilderShim(tracer, "foo", new List<string> { "bar" });
+            shim = new SpanBuilderShim(tracer, "foo");
             var spanShim2 = (SpanShim)shim.Start();
 
             Assert.Equal("foo", spanShim2.Span.Activity.OperationName);
@@ -330,5 +330,29 @@ public void Start()
             Assert.NotNull(span);
             Assert.Equal("foo", span.Span.Activity.OperationName);
         }
+
+        [Fact]
+        public void Start_UnderAspNetCoreInstrumentation()
+        {
+            // Simulate a span from AspNetCore instrumentation as parent.
+            using var source = new ActivitySource("Microsoft.AspNetCore.Hosting.HttpRequestIn");
+            using var parentSpan = source.StartActivity("OTelParent");
+            Assert.NotNull(parentSpan);
+
+            // Start the OpenTracing span.
+            var tracer = TracerProvider.Default.GetTracer(TracerName);
+            var builderShim = new SpanBuilderShim(tracer, "foo");
+            var spanShim = builderShim.StartActive().Span as SpanShim;
+            Assert.NotNull(spanShim);
+
+            var telemetrySpan = spanShim.Span;
+            Assert.Same(telemetrySpan.Activity, Activity.Current);
+            Assert.Same(parentSpan, telemetrySpan.Activity.Parent);
+
+            // Dispose the spanShim.Span and ensure correct state for Activity.Current
+            spanShim.Span.Dispose();
+
+            Assert.Same(parentSpan, Activity.Current);
+        }
     }
 }