From 39297276754cc3ed73d635345c6870ed52b4b9f2 Mon Sep 17 00:00:00 2001
From: Paulo Janotti <pjanotti@splunk.com>
Date: Tue, 2 Aug 2022 17:38:10 -0700
Subject: [PATCH] Fix OpenTracing shim under legacy AspNetCore activities
 (#3506)

---
 .../CHANGELOG.md                              |  4 +++
 .../SpanBuilderShim.cs                        | 19 +------------
 .../SpanBuilderShimTests.cs                   | 28 +++++++++++++++++--
 3 files changed, 31 insertions(+), 20 deletions(-)

diff --git a/src/OpenTelemetry.Shims.OpenTracing/CHANGELOG.md b/src/OpenTelemetry.Shims.OpenTracing/CHANGELOG.md
index 30a4b655820..aff4eaa3b30 100644
--- 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
index 483549e8f97..5cdbe55e85d 100644
--- 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,7 +70,7 @@ 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);
@@ -87,7 +78,6 @@ public SpanBuilderShim(Tracer tracer, string spanName, IList<string> rootOperati
             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
index c149928d78a..27f9cc11cd3 100644
--- 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);
+        }
     }
 }