From 4c9bcc077fd36b107a94a1dac4e44b98560681a1 Mon Sep 17 00:00:00 2001 From: Paulo Janotti Date: Fri, 29 Jul 2022 14:22:18 -0700 Subject: [PATCH 1/3] Fix OpenTracing shim under AspNetCore instr. span --- .../SpanBuilderShim.cs | 18 +----------- .../SpanBuilderShimTests.cs | 28 +++++++++++++++++-- 2 files changed, 27 insertions(+), 19 deletions(-) diff --git a/src/OpenTelemetry.Shims.OpenTracing/SpanBuilderShim.cs b/src/OpenTelemetry.Shims.OpenTracing/SpanBuilderShim.cs index 483549e8f97..c2af33f384f 100644 --- a/src/OpenTelemetry.Shims.OpenTracing/SpanBuilderShim.cs +++ b/src/OpenTelemetry.Shims.OpenTracing/SpanBuilderShim.cs @@ -50,14 +50,6 @@ internal sealed class SpanBuilderShim : ISpanBuilder /// private readonly List> attributes = new(); - /// - /// The set of operation names for System.Diagnostics.Activity based automatic instrumentations that indicate a root span. - /// - private readonly IList rootOperationNamesForActivityBasedAutoInstrumentations = new List - { - "Microsoft.AspNetCore.Hosting.HttpRequestIn", - }; - /// /// The parent as an TelemetrySpan, if any. /// @@ -79,7 +71,7 @@ internal sealed class SpanBuilderShim : ISpanBuilder private bool error; - public SpanBuilderShim(Tracer tracer, string spanName, IList rootOperationNamesForActivityBasedAutoInstrumentations = null) + public SpanBuilderShim(Tracer tracer, string spanName) { Guard.ThrowIfNull(tracer); Guard.ThrowIfNull(spanName); @@ -87,7 +79,6 @@ public SpanBuilderShim(Tracer tracer, string spanName, IList rootOperati this.tracer = tracer; this.spanName = spanName; this.ScopeManager = new ScopeManagerShim(this.tracer); - this.rootOperationNamesForActivityBasedAutoInstrumentations = rootOperationNamesForActivityBasedAutoInstrumentations ?? this.rootOperationNamesForActivityBasedAutoInstrumentations; } private IScopeManager ScopeManager { get; } @@ -172,13 +163,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 { "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 { "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); + } } } From f74e5aef20b8724595f217e011565de227999dd8 Mon Sep 17 00:00:00 2001 From: Paulo Janotti Date: Fri, 29 Jul 2022 16:57:17 -0700 Subject: [PATCH 2/3] Add CHANGELOG.md entry --- src/OpenTelemetry.Shims.OpenTracing/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) 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 From 0d44397dc48a221db0fde2c73cb025a5a09d07fd Mon Sep 17 00:00:00 2001 From: Paulo Janotti Date: Fri, 29 Jul 2022 17:01:37 -0700 Subject: [PATCH 3/3] Remove unused using directive --- src/OpenTelemetry.Shims.OpenTracing/SpanBuilderShim.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/OpenTelemetry.Shims.OpenTracing/SpanBuilderShim.cs b/src/OpenTelemetry.Shims.OpenTracing/SpanBuilderShim.cs index c2af33f384f..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;