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

Make it possible to disable specific signals when using auto-instrumentation #4395

Open
MaxShvets opened this issue Jan 23, 2025 · 5 comments
Labels
auto-instrumentation related to auto-instrumentation of the sdk feature-request

Comments

@MaxShvets
Copy link

Is your feature request related to a problem?

I'll describe this in terms of disabling metrics, because that's the issue that vexes me the most, but this should be applicable to other signals as well

I use OpenTelemetry purely for tracing and I'd like as little as possible instrumentation code to run for other signals. This is both for performance reasons (though the impact is not that big) and because certain instrumentations can emit warnings related to the other signals, which I'd have to silence (a good example of that is the aiohttp-server instrumentation, which gets the same meters for each request, resulting in "meter already exists warnings").

Describe the solution you'd like

opentelemetry-instrument has the --meter_provider arguments(and corresponding env var), which is ignored, as far as I can see (configuration code uses MeterProvider unconditionally). These could accept the none value, and the auto-instrumentation code would react by selecting the NoOpMeterProvider. As is, this wouldn't help with the aiohttp-server issue described above, because NoOpMeter still emits warnings, so I think it should also be changed, to emit no warnings.

Describe alternatives you've considered

I don't think there are better alternatives, since it seems like some instrumentations (again aiohttp-server is an example) are implemented in a way where you can't choose which signals to instrument. So, to continue to work they should at least be able to get meters (and corresponding objects for other signals) and create metric objects. Which is why, I think, using the no-op versions of these objects, and having the no-op objects emit no warnings is the best we can do

Additional Context

No response

Would you like to implement a fix?

Yes

@aabmass aabmass added the auto-instrumentation related to auto-instrumentation of the sdk label Jan 23, 2025
@aabmass
Copy link
Member

aabmass commented Jan 23, 2025

opentelemetry-instrument has the --meter_provider arguments(and corresponding env var), which is ignored, as far as I can see (configuration code uses MeterProvider unconditionally). These could accept the none value, and the auto-instrumentation code would react by selecting the NoOpMeterProvider.

Ya this seems like an oversight. We actually already have entry points for the noop implementations that you should be able to specify:

[project.entry-points.opentelemetry_meter_provider]
default_meter_provider = "opentelemetry.metrics:NoOpMeterProvider"

As is, this wouldn't help with the aiohttp-server issue described above, because NoOpMeter still emits warnings, so I think it should also be changed, to emit no warnings.

not sure which warnings, but yes that might be appropriate.

@MaxShvets
Copy link
Author

Yeah, I've seen the entry points. Will try to implement a fix in the coming week

The warnings are all of this form:

An instrument with name http.server.duration, type Histogram, unit ms and description Measures the duration of inbound HTTP requests. has been created already.

@xrmx
Copy link
Contributor

xrmx commented Jan 27, 2025

The warnings are all of this form:

An instrument with name http.server.duration, type Histogram, unit ms and description Measures the duration of inbound HTTP requests. has been created already.

The warning will be fixed in #4361

@MaxShvets
Copy link
Author

Won't touch the warnings in this PR then, thanks for the heads up @xrmx

@MaxShvets
Copy link
Author

MaxShvets commented Feb 2, 2025

Finally got to work on this, and there’s an issue that I’m not sure how best to resolve. Using traces as an example, the current configuration process requires a narrower interface than the generic TracerProvider. It requires the class to accept a sampler, id generator, and a resource at initialization, and also to have an add_span_processor method, which makes it incompatible with, for example, NoOpTracerProvider. I'm currently thinking of updating the configuration process to inspect the provider class it got, and only create the required objects based on the argument types (maybe falling back to names in the case of untyped arguments?). Are you ok with that direction, or do you have other ways of addressing this in mind, @aabmass?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-instrumentation related to auto-instrumentation of the sdk feature-request
Projects
None yet
Development

No branches or pull requests

3 participants