-
Notifications
You must be signed in to change notification settings - Fork 710
Remove SDK's dependency on opentelemetry-instrumentation #2184
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
Comments
I would support Option 2, I think the dependency on the instrumentation package is problematic. |
@open-telemetry/python-approvers @open-telemetry/python-maintainers thoughts? |
It does feel wrong to have Maybe installing all these “regular use case” packages is what |
Each distro can solve whatever it wants :) but yes this is something that all distros would obviously want to solve. The upcoming |
Today the SDK depends on opentelemetry-instrumentation package because it needs to import
opentelemetry.instrumentation.utils._SUPPRESS_INSTRUMENTATION_KEY
in order to suppress instrumentation in its own exporters. We decided in last SIG call to get rid of this dependency as it does not make sense to have SDK depend on the instrumentation package and it complicates things in general.Right now the _SUPRESS_INSTRUMENTATION variable is a an opentelemetry context key i.e, it is a randomly generated uuid. We have two options to fix this:
Make _SUPPRESS_INSTRUMENTATION_KEY a static symbol e.g, a well-known string value that is used by convention by SDK and opentelemetry-instrumentation both without importing it from anywhere. This is the simplest and quite robust but it technically violates the spec as context keys should not be static strings.
Move the _SUPRESS_INSTRUMENTATION_KEY to
opentelemetry.context._SUPRESS_INSTRUMENTATION_KEY
. We'll place it in the API package but it'll not be part of the public context API just like it is not a part of public instrumentation API today. It'll continue to be strictly used by internal package just like today. Both SDK and instrumention packages will import it fromopentelemetry.context._SUPPRESS_INSTRUMENTATION_KEY
. Once the spec recommends a way to implement this officially, we can get rid of this or make it part of the public API.3rd options is to have SDK or instrumentation package "register" a global key like this in
opentelemetry.context.
namespace just like we register propagators or providers but I feel it'd be overkill for this problem. I think we should go with 2.The text was updated successfully, but these errors were encountered: