open-telemetry / open-telemetry/opentelemetry-python-contrib
Do not configure instrumentation package config/env vars globally
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 1.1k
- Forks
- 1.1k
- Avg merge
- 4d 15h
- Merged PRs (30d)
- 16
Description
Most instrumentation packages support some form of configuration via env vars and right now all packages read this configuration at import time into a global var. For example, https://github.com/open-telemetry/opentelemetry-python-contrib/blob/main/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py#L76
This has a few obvious disadvantages such as:
- harder to test as it needs patching in multiple places.
- hard to run two tests that need different values in parallel.
- code is executed on import even if instrumentation is not used.
- force instrumentation to use global vars.
I suggest we fix this by moving such lines to instrumentor constructors so the value is read once per instrumentor application. This would simplify testing and localize all config logic related to an instrumentor inside its constructor.
We already did this for Falcon and as a result significantly simplified some test code: https://github.com/open-telemetry/opentelemetry-python-contrib/pull/650
We should do it for all other affected instrumentation packages.
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 the Flask instrumentation init.py location linked in the issue, then compare the Falcon change in pull request #650. Identify other instrumentation packages that read configuration into globals at import time, and review their instrumentor constructors and tests. Done means affected packages read configuration per instrumentor application and their tests still pass without shared global state.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- observability-sre
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 38/100