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

[Instrumentation.AspNetCore] Fix issue causing Activity is not exported when used with custom propagator/sampler. #4637

Merged
merged 9 commits into from
Jul 10, 2023
4 changes: 4 additions & 0 deletions src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@
which attributes are emitted by setting the environment variable
`OTEL_SEMCONV_STABILITY_OPT_IN`.

* Fixed an issue affecting NET 7.0+. If custom propagation is being used
and tags are added to an Activity during sampling then that Activity would be dropped.
([#4637](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4637))

## 1.5.0-beta.1

Released 2023-Jun-05
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,12 @@ public void OnStopActivity(Activity activity, object payload)
}
}

#if NET7_0_OR_GREATER
var tagValue = activity.GetTagValue("IsCreatedByInstrumentation");
if (ReferenceEquals(tagValue, bool.TrueString))
#else
if (activity.TryCheckFirstTag("IsCreatedByInstrumentation", out var tagValue) && ReferenceEquals(tagValue, bool.TrueString))
#endif
{
// If instrumentation started a new Activity, it must
// be stopped here.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -194,8 +194,10 @@ public async Task SuccessfulTemplateControllerCallUsesParentContext()
ValidateAspNetCoreActivity(activity, "/api/values/2");
}

[Fact]
public async Task CustomPropagator()
[Theory]
[InlineData(true)]
[InlineData(false)]
public async Task CustomPropagator(bool addSampler)
{
try
{
Expand All @@ -219,7 +221,18 @@ public async Task CustomPropagator()
builder.ConfigureTestServices(services =>
{
Sdk.SetDefaultTextMapPropagator(propagator.Object);
this.tracerProvider = Sdk.CreateTracerProviderBuilder().AddAspNetCoreInstrumentation().AddInMemoryExporter(exportedItems).Build();
var tracerProviderBuilder = Sdk.CreateTracerProviderBuilder();

if (addSampler)
{
tracerProviderBuilder
.SetSampler(new TestSampler(SamplingDecision.RecordAndSample, new Dictionary<string, object> { { "SomeTag", "SomeKey" }, }));
}

this.tracerProvider = tracerProviderBuilder
.AddAspNetCoreInstrumentation()
.AddInMemoryExporter(exportedItems)
.Build();
});
builder.ConfigureLogging(loggingBuilder => loggingBuilder.ClearProviders());
}))
Expand Down Expand Up @@ -1134,15 +1147,17 @@ public override void Inject<T>(PropagationContext context, T carrier, Action<T,
private class TestSampler : Sampler
{
private readonly SamplingDecision samplingDecision;
private readonly IEnumerable<KeyValuePair<string, object>> attributes;

public TestSampler(SamplingDecision samplingDecision)
public TestSampler(SamplingDecision samplingDecision, IEnumerable<KeyValuePair<string, object>> attributes = null)
{
this.samplingDecision = samplingDecision;
this.attributes = attributes;
}

public override SamplingResult ShouldSample(in SamplingParameters samplingParameters)
{
return new SamplingResult(this.samplingDecision);
return new SamplingResult(this.samplingDecision, this.attributes);
}
}

Expand Down