-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
[service] Allow users to disable the tracer provider via the feature gate service.noopTracerProvider
#10859
[service] Allow users to disable the tracer provider via the feature gate service.noopTracerProvider
#10859
Conversation
service::telemetry::trace
configuration to disable all trace providers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The previous approach of using the configured service::telemetry::trace::providers
wouldn't work because it would break the default behaviour of the zpages extension.
My suggestion in this PR is to move to a similar model as we will be moving to with metrics, where different meters can be configured for the different levels.
Alternatively, we could break the behaviour of the zpages extension, but that doesn't sound great either
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #10859 +/- ##
==========================================
+ Coverage 91.64% 91.65% +0.01%
==========================================
Files 406 406
Lines 19023 19025 +2
==========================================
+ Hits 17434 17438 +4
+ Misses 1228 1227 -1
+ Partials 361 360 -1 ☔ View full report in Codecov by Sentry. |
service/telemetry/config.go
Outdated
// - "basic" is the recommended and covers the basics of the service telemetry. | ||
// - "normal" adds some additional spans top of basic. | ||
// - "detailed" adds all available spans to the previous levels. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are any of them utilized now?
Also, configtelemetry.Level
defaults to Basic. Should we have None by default here?
@dmitryax I'm going to update this PR to use a feature gate as a short term solution for the original issue |
Sounds good to me. Maybe we can figure out how to make it work with the zpages extension as a follow-up after the release |
…are configured Previously the service was returning an instance of a SDK tracer provider regardless of whether there were any processors configured causing resources to be consumed unnecessarily. Fixes open-telemetry#10858 Signed-off-by: Alex Boten <[email protected]>
Signed-off-by: Alex Boten <[email protected]>
Signed-off-by: Alex Boten <[email protected]>
4497578
to
5c3c329
Compare
Signed-off-by: Alex Boten <[email protected]>
service::telemetry::trace
configuration to disable all trace providersservice.noopTracerProvider
Previously the service was returning an instance of a SDK tracer provider regardless of whether there were any processors configured causing resources to be consumed unnecessarily.
Fixes #10858