open-telemetry / open-telemetry/opentelemetry-python
opentelemetry-sdk: stop coercing instrumentation options once instrumentors read ConfigProvider
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 2.6k
- Forks
- 1k
- Avg merge
- 4d 15h
- Merged PRs (30d)
- 19
Description
Is your feature request related to a problem?
configure_instrumentation() in opentelemetry-sdk/src/opentelemetry/sdk/_configuration/instrumentation.py, added in https://github.com/open-telemetry/opentelemetry-python/pull/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 https://github.com/open-telemetry/opentelemetry-python-contrib/pull/4766#issuecomment-5356515506.
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 https://github.com/open-telemetry/opentelemetry-python/pull/5486. - Depends on the
BaseInstrumentorread hook tracked in https://github.com/open-telemetry/opentelemetry-python-contrib/issues/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
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with opentelemetry-sdk/src/opentelemetry/sdk/_configuration/instrumentation.py and its activation tests in opentelemetry-sdk/tests/_configuration/test_instrumentation.py. Check the ConfigProvider and BaseInstrumentor dependencies in the linked work before changing behavior. Done means activation remains covered, instrumentation options are read through ConfigProvider, docs/sdk/configuration.rst is updated, and coercion tests move to contrib.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- backend, observability
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100