open-telemetry / open-telemetry/opentelemetry-python
Nothing detects a test that leaks a PeriodicExportingMetricReader ticker thread, and four tests leak one today
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 2.6k
- Forks
- 1k
- Avg merge
- 4d 15h
- Merged PRs (30d)
- 19
Description
Describe your environment
main (5aa2f8f), opentelemetry-sdk test suite. Not environment specific, though the consequences only show up on slow runners.
What happened?
PeriodicExportingMetricReader.__init__ starts a daemon ticker thread immediately:
Nothing reaps a daemon thread between tests. Not the end of the test, not the end of the class, not the end of the module. The only thing that stops it is shutdown(), which sets _shutdown_event:
So a test that builds one and does not shut it down leaves a thread that keeps waking on its own clock, for the rest of the session, calling collect() inside unrelated tests. With the default 60 second interval its first tick lands long after the test that created it has passed, so the guilty test is green and the damage shows up somewhere else entirely.
There is currently no check for this. A leak is invisible until it happens to break something, and when it does the failure names the victim rather than the cause. #5157 is an instance: the reported failure is in test_metric_reader_storage.py and the accompanying thread exception is attributed to a test in exponential_histogram/, neither of which creates a metric reader.
There are four leaking tests on main today. Found by adding the fixture proposed below and running pytest opentelemetry-sdk/tests:
opentelemetry-sdk/tests/metrics/integration_test/test_exporter_concurrency.py::TestExporterConcurrency::test_exporter_not_called_concurrently 1 thread
opentelemetry-sdk/tests/metrics/test_metrics.py::TestMeterProvider::test_register_metric_readers 2 threads
opentelemetry-sdk/tests/metrics/test_periodic_exporting_metric_reader.py::TestPeriodicExportingMetricReader::test_exporter_aggregation_preference 1 thread
opentelemetry-sdk/tests/metrics/test_periodic_exporting_metric_reader.py::TestPeriodicExportingMetricReader::test_exporter_temporality_preference 1 thread
The first is the most consequential of the four: the reader is registered on a MeterProvider that holds an observable counter, so each later tick runs the callback and feeds a measurement back into the storage. The two preference tests build readers that are never registered, so their ticks return early at the self._collect is None guard and cannot corrupt another test's state, but they still burn a thread and log a warning every interval into whatever test is running.
What did you expect to see?
A test that leaks a ticker thread should fail, naming itself, at the moment it leaks.
Suggested fix
An autouse fixture in opentelemetry-sdk/tests/conftest.py that snapshots the live threads named OtelPeriodicExportingMetricReader before the test and fails if any new one is still alive afterwards.
Diffing against a before-snapshot matters: it attributes the leak to the test that caused it, and stops a single leak from cascading into a failure in every test that runs after it.
A short grace period covers the case where shutdown() is in flight, since it joins the ticker with a timeout. Only a test that actually leaks pays that cost.
Relationship to #5157
This does not by itself explain the #5157 traceback, and I want to be careful not to overclaim: I have not proven that any of the four leaks above produced it. The original CI logs have expired, so the evidence is limited to the excerpt in that issue.
What this does give is a mechanical guard. If the failure recurs, the leaking test fails by name instead of the symptom surfacing three files later. The other half of #5157, the tests asserting on process-global mock state, is #5638.
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/tests/conftest.py and run pytest opentelemetry-sdk/tests to reproduce the listed leaks. Add the autouse thread-leak check described in the issue, then verify that the four named tests fail by name until their readers are shut down and the full SDK test suite passes.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- testing-qa
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 76/100