-
Notifications
You must be signed in to change notification settings - Fork 893
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
Add "none" as a possible value to OTEL_PROPAGATORS #2052
Conversation
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.
Please explain the need / document the use case for this setting.
Nit: given that this is a list of propagators, the value "none" seems a bit odd. It's fine when it is standalone value, but this setting allows composing propagators, such that none can be combined with something else. "noop" might be more accurate description of it, since it's not the absence of a propagator, but one that does nothing.
@yurishkuro Same use case like for span and metrics exporters. I am proposing Later I can try improving the description |
@yurishkuro Description updated |
My concern is that by making this a requirement in the spec we're adding cumulatively hours to days of work for SDK maintainers, and doing it "for consistency" is not a strong enough argument. Is there a strong demand for this feature from end users? Also, I find the wording (not introduced in this PR) of "these are known values" rather confusing - do SDKs must support all those values and the respective algorithms? We may want to book a separate issue to discuss that. |
Adding this to the next Maintainers call for further discussion. |
is this a |
oh yes, what you said 👍 |
Since And would argue if there is a configuration that has a default that is used if it isn't set then there needs to be a way to set it to be a noop, even if many users haven't asked for it yet :) |
in js this won't be a problem with propagator equal to |
@open-telemetry/proto-go-maintainers A last look perhaps? |
Per #2045 (comment)
Changes
Add
none
as a possible value forOTEL_PROPAGATORS
to disable context propagation. Because the default istracecontext,baggage
, there is no way to disable signal export via env vars.The same approach has been specified for
OTEL_TRACES_EXPORTER
andOTEL_METRICS_EXPORTER
here: #1439It is also important given the fact that an env var with empty env var should be interpreted in the same way as an usnset env var. More: #2045