Only create a sibling activity when trace id, span id or trace state differ (#5136)

Co-authored-by: Cijo Thomas <cijo.thomas@gmail.com>
Co-authored-by: Mikel Blanchard <mblanchard@macrosssoftware.com>
Co-authored-by: Alan West <3676547+alanwest@users.noreply.github.com>

Author: 4 people

src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs

@@ -105,7 +105,7 @@ public void OnStartActivity(Activity activity, object payload)
         // By this time, samplers have already run and
         // activity.IsAllDataRequested populated accordingly.
 
-        HttpContext context = payload as HttpContext;
+        var context = payload as HttpContext;
         if (context == null)
         {
             AspNetCoreInstrumentationEventSource.Log.NullPayload(nameof(HttpInListener), nameof(this.OnStartActivity), activity.OperationName);
@@ -118,9 +118,10 @@ public void OnStartActivity(Activity activity, object payload)
         if (textMapPropagator is not TraceContextPropagator)
         {
             var ctx = textMapPropagator.Extract(default, request, HttpRequestHeaderValuesGetter);
-
             if (ctx.ActivityContext.IsValid()
-                && ctx.ActivityContext != new ActivityContext(activity.TraceId, activity.ParentSpanId, activity.ActivityTraceFlags, activity.TraceStateString, true))
+                && !((ctx.ActivityContext.TraceId == activity.TraceId)
+                    && (ctx.ActivityContext.SpanId == activity.ParentSpanId)
+                    && (ctx.ActivityContext.TraceState == activity.TraceStateString)))
             {
                 // Create a new activity with its parent set from the extracted context.
                 // This makes the new activity as a "sibling" of the activity created by

src/OpenTelemetry/CHANGELOG.md

@@ -9,6 +9,12 @@
   upgrading. For details see:
   [#5169](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5169)
 
+* Fixed an issue where the created activity from ASP.NET Core was replaced
+  with a new one. This replacement should only happen when the activity context
+  from the used propagator has a different trace id, parent span id or trace
+  state compared to the current activity. For details see:
+  [#5136](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5136)
+
 ## 1.7.0
 
 Released 2023-Dec-08

test/Directory.Packages.props

@@ -4,5 +4,6 @@
     <PackageVersion Update="System.Text.Json" Version="7.0.1" />
     <PackageVersion Include="System.Runtime.InteropServices.RuntimeInformation" Version="4.3.0" />
     <PackageVersion Include="Microsoft.Coyote" Version="1.7.10" />
+    <PackageVersion Include="Microsoft.Extensions.Features" Version="8.0.0" />
   </ItemGroup>
 </Project>

test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs

@@ -1024,6 +1024,43 @@ public async Task DiagnosticSourceExceptionCallBackIsNotReceivedForExceptionsHan
         Assert.True(exceptionHandled);
     }
 
+#if NET6_0_OR_GREATER
+    [Fact]
+    public async Task NoSiblingActivityCreatedWhenTraceFlagsNone()
+    {
+        this.tracerProvider = Sdk.CreateTracerProviderBuilder()
+            .SetSampler(new AlwaysOnSampler())
+            .AddAspNetCoreInstrumentation()
+            .Build();
+
+        using var testFactory = this.factory
+            .WithWebHostBuilder(builder =>
+            {
+                builder.ConfigureTestServices(services =>
+                {
+                    this.tracerProvider = Sdk.CreateTracerProviderBuilder()
+                    .AddAspNetCoreInstrumentation()
+                    .Build();
+                });
+
+                builder.ConfigureLogging(loggingBuilder => loggingBuilder.ClearProviders());
+            });
+        using var client = testFactory.CreateClient();
+        var request = new HttpRequestMessage(HttpMethod.Get, "/api/GetActivityEquality");
+        var traceId = ActivityTraceId.CreateRandom();
+        var spanId = ActivitySpanId.CreateRandom();
+        request.Headers.Add("traceparent", $"00-{traceId}-{spanId}-00");
+
+        var response = await client.SendAsync(request);
+        var result = bool.Parse(await response.Content.ReadAsStringAsync());
+
+        Assert.True(response.IsSuccessStatusCode);
+
+        // Confirm that Activity.Current and IHttpActivityFeature activity are same
+        Assert.True(result);
+    }
+#endif
+
     public void Dispose()
     {
         this.tracerProvider?.Dispose();

test/TestApp.AspNetCore/Controllers/ChildActivityController.cs

@@ -2,6 +2,7 @@
 // SPDX-License-Identifier: Apache-2.0
 
 using System.Diagnostics;
+using Microsoft.AspNetCore.Http.Features;
 using Microsoft.AspNetCore.Mvc;
 using OpenTelemetry;
 
@@ -34,4 +35,13 @@ public IReadOnlyDictionary<string, string> GetChildActivityBaggageContext()
         var result = Baggage.Current.GetBaggage();
         return result;
     }
+
+    [HttpGet]
+    [Route("api/GetActivityEquality")]
+    public bool GetActivityEquality()
+    {
+        var activity = this.HttpContext.Features.GetRequiredFeature<IHttpActivityFeature>().Activity;
+        var equal = Activity.Current == activity;
+        return equal;
+    }
 }

test/TestApp.AspNetCore/TestApp.AspNetCore.csproj

@@ -5,6 +5,7 @@
   </PropertyGroup>
 
   <ItemGroup>
+    <PackageReference Include="Microsoft.Extensions.Features" />
     <PackageReference Include="Swashbuckle.AspNetCore" />
   </ItemGroup>