Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 22 commits into from
Jan 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
6d2c7ee
Do not create sibling activity when format is W3C
ngruson Dec 6, 2023
e516460
Formatted temp comments properly in unit tests
ngruson Dec 6, 2023
3e1af43
Conditional using statement
ngruson Dec 6, 2023
cb50735
Create sibling activity when format != W3C or trace flags is Recorded
ngruson Dec 6, 2023
a3c6d65
Leave trace flags out of comparison to create sibling activity
ngruson Dec 6, 2023
a76e470
Use WebApplicationFactory
ngruson Dec 7, 2023
0ab1901
Added Microsoft.Extensions.Features to Directory.Packages.props
ngruson Dec 7, 2023
f0c8152
Merge branch 'main' into 4466-bug-httpactivityfeature
ngruson Dec 7, 2023
d582a92
Updated trace id/span id/trace state comparison
ngruson Dec 8, 2023
0e26e0b
Merge branch 'main' into 4466-bug-httpactivityfeature
ngruson Dec 8, 2023
98c8feb
Add comment and compare Activity.Current to IHttpFeatureActivityFeatu…
ngruson Dec 8, 2023
83ce8b3
Merge branch 'main' into 4466-bug-httpactivityfeature
cijothomas Dec 8, 2023
ba7b218
Merge branch 'main' into 4466-bug-httpactivityfeature
ngruson Dec 19, 2023
f1df768
Merge branch '4466-bug-httpactivityfeature' of https://github.com/ngr…
ngruson Dec 19, 2023
462287e
Merge branch 'main' into 4466-bug-httpactivityfeature
ngruson Dec 20, 2023
2aa9fd9
Don't create ActivityContext struct for comparison
ngruson Dec 21, 2023
219ceed
Merge branch 'main' into 4466-bug-httpactivityfeature
ngruson Dec 27, 2023
b3cb1e2
Added PR #5136 to CHANGELOG.md
ngruson Jan 8, 2024
61a3bcb
Fixed trailing spaces
ngruson Jan 8, 2024
9837992
Merge branch 'main' into 4466-bug-httpactivityfeature
CodeBlanch Jan 8, 2024
e0b013e
Merge branch 'main' into 4466-bug-httpactivityfeature
alanwest Jan 16, 2024
f842030
Merge branch 'main' into 4466-bug-httpactivityfeature
alanwest Jan 16, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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))
utpilla marked this conversation as resolved.
Show resolved Hide resolved
&& !((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
Expand Down
6 changes: 6 additions & 0 deletions src/OpenTelemetry/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions test/Directory.Packages.props
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Original file line number Diff line number Diff line change
Expand Up @@ -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);
cijothomas marked this conversation as resolved.
Show resolved Hide resolved
}
#endif

public void Dispose()
{
this.tracerProvider?.Dispose();
Expand Down
10 changes: 10 additions & 0 deletions test/TestApp.AspNetCore/Controllers/ChildActivityController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// SPDX-License-Identifier: Apache-2.0

using System.Diagnostics;
using Microsoft.AspNetCore.Http.Features;
using Microsoft.AspNetCore.Mvc;
using OpenTelemetry;

Expand Down Expand Up @@ -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;
}
}
1 change: 1 addition & 0 deletions test/TestApp.AspNetCore/TestApp.AspNetCore.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
</PropertyGroup>

<ItemGroup>
<PackageReference Include="Microsoft.Extensions.Features" />
<PackageReference Include="Swashbuckle.AspNetCore" />
</ItemGroup>

Expand Down