Repository navigation
opentelemetry-sdk: stop coercing instrumentation options once instrumentors read ConfigProvider #5619
Description
Is your feature request related to a problem?
configure_instrumentation() in opentelemetry-sdk/src/opentelemetry/sdk/_configuration/instrumentation.py, added in #5372, does two separate jobs:
- Activation. Load the
opentelemetry_instrumentorentry point by name, honorenabled: false, skip an already instrumented library, and isolate a failing instrumentor. - Option delivery. Read the instrumentor's
configurationdataclass attribute, run the raw options through_dict_to_dataclass, and forward the result as keyword arguments toinstrument().
Job 2 belongs on the instrumentation side. The Instrumentation Configuration API specification states that instrumentation libraries access ConfigProvider during initialization, either because it is passed to them or because they read it from a global. See https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/configuration/api.md.
Raised by @xrmx in open-telemetry/opentelemetry-python-contrib#4766 (comment).
Leaving job 2 in place after instrumentors can read configuration for themselves means two components coerce the same values, and the SDK keeps instrumentation specific knowledge it does not need.
Describe the solution you'd like
Once BaseInstrumentor reads its own configuration from ConfigProvider, remove the coercion branch from configure_instrumentation():
- Drop the
getattr(cls, "configuration", None)lookup, theis_dataclasscheck, and the_dict_to_dataclasscall. - Drop the rebuild of
optionsfrom the dataclass fields. - Keep activation unchanged: entry point loading,
enabled: false, the already instrumented check, and the error isolation. - Keep passing the remaining raw options to
instrument(), or decide explicitly that all options now arrive throughConfigProviderand the call becomesinstrument()with no arguments.
Update docs/sdk/configuration.rst to describe where instrumentation options are read.
Describe alternatives you've considered
Leave the branch in place. It becomes unreachable once instrumentation reads its own configuration, so it is dead code that still has to be maintained and tested.
Keep both paths permanently and define a precedence between them. This doubles the number of places a user has to look at to understand where an option came from, for no benefit.
Additional Context
Ordering, because this work spans two repositories:
- Depends on
ConfigProviderandConfigPropertiesfrom Implement declarative config ConfigProvider / ConfigProperties API #5486. - Depends on the
BaseInstrumentorread hook tracked in opentelemetry-instrumentation: read declarative configuration from ConfigProvider in BaseInstrumentor opentelemetry-python-contrib#5027. - Must land after that contrib work, not before.
If this removal shipped first, options set in a configuration file would be silently dropped for every instrumentor, because nothing would read them.
opentelemetry-sdk/tests/_configuration/test_instrumentation.py covers the coercion behavior today. The tests for configuration coercion, None field omission, and unknown field rejection move to the contrib repository along with the behavior. The activation tests stay here.
Would you like to implement a fix?
Yes
Activity