-
Notifications
You must be signed in to change notification settings - Fork 273
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
Registering instrumentation libraries more than once adds duplicate Activity listeners #1971
Comments
when `.AddAspNetCoreInstrumentation()` is called more than 1x. ref: open-telemetry#1971
Possibly happening with Need to do more debugging, but have definitely noticed duplicate trace spans for a given call to Redis. Our app has multiple But we haven't noticed duplicate spans for other instrumentations so far (just Redis). |
Hi @YayBurritos - in the cases that I reported, the duplicate listeners don't result in duplicate spans, because the Activities are created/started/stopped by AspNetCore or HttpClient - instead all of the instrumentation logic is just running more than once, and if any of that logic is not idempotent (including filters or enrichers) then that's a bigger problem than happens by default. If the Activity were created/started/stopped by the instrumentation library, then that could result in duplicate spans, and the root cause would be this same bug. |
@johncrim - Is the purpose for calling Options can also configured as below: builder.Services.Configure<AspNetCoreTraceInstrumentationOptions>(options =>
{
options.RecordException = true;
...
...
}); |
@vishweshbankwar - The reason it's called more than once is to ensure that AspNetCoreInstrumentation is included. Configuring in multiple locations is a secondary requirement. I'm aware that you can configure options using But that doesn't work b/c we have different setup code in different places that each need to ensure that AspNetCoreInstrumentation is registered. For example, if I'm adding request telemetry filters and request enrichers in a library, I need to ensure that AspNetCoreInstrumentation is included (b/c otherwise the filters won't be called). But the app itself also needs to call But also - this is a bug - the API violates expectations. |
Component
OpenTelemetry.Instrumentation.AspNetCore
OpenTelemetry.Instrumentation.Http
(and possibly more)
Package Version
Runtime Version
net8.0;net7.0;net6.0
Description
When an instrumentation library is added more than once, it results in duplicate Activity listeners, in which case:
This is true at least for
OpenTelemetry.Instrumentation.AspNetCore
and forOpenTelemetry.Instrumentation.Http
- it may be true for other instrumentation libraries as well.It's probably unnecessary for me to explain this, but most of the otel setup is idempotent, and most .NET framework setup is idempotent. Also, given that calls to
builder.AddAspNetCoreInstrumentation()
are included in egbuilder.UseAzureMonitor()
, any calls to customize the AspNetCore instrumentation, like:will result in duplicate listeners, duplicate instrumentation, and duplicate callbacks for each request.
Steps to Reproduce
This test runs in
OpenTelemetry.Instrumentation.AspNetCore.Tests
:Note that measuring the number of filter and callback calls is an easy and accessible way to determine how many Activity listeners are running. The test fails b/c there 2 calls each to filter and enrich for the single request.
Expected Result
Calling
.AddAspNetCoreInstrumentation()
and.AddHttpClientInstrumentation()
in multiple places during startup results in effectively a single instance of the instrumentation, and a single Activity listener for each Activity source that is instrumented.Actual Result
1 additional Activity listener for each time
.AddAspNetCoreInstrumentation()
and.AddHttpClientInstrumentation()
are called.Additional Context
The part of
.AddAspNetCoreInstrumentation()
that causes the problem is:https://github.com/open-telemetry/opentelemetry-dotnet-contrib/blob/6f3ae2dd61a3fa8e520b1eb626d6de2873bab9f0/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationTracerProviderBuilderExtensions.cs#L75C1-L81C12
Which always creates and adds a new listener.
I don't think
builder.AddInstrumentation()
can be made idempotent on the instrumentation type without breaking some valid use-cases. Potentially object equality could be used to determine equivalency; or another option is to add abuilder.TryAddInstrumentation()
method that only creates the instrumentation if the instrumentation type hasn't already been instantiated.The text was updated successfully, but these errors were encountered: