[ocelotl]

Description

Fixes #5640

PeriodicExportingMetricReader starts a daemon ticker thread in its constructor, and nothing reaps a daemon thread between tests. A reader that is never shut down leaves that thread 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 guilty test has passed, so the guilty test is green and the damage shows up somewhere else, naming the victim rather than the cause.

There was no check for this. This PR adds one, and fixes the four leaks that exist today so it can land on a clean suite.

Commits

Each commit touches exactly one file.

1. test_exporter_not_called_concurrently registers a reader on a MeterProvider and never shuts either down. This is the most consequential of the four: the provider holds an observable counter, so every later tick runs the callback and feeds a measurement back into the storage.
2. test_register_metric_readers builds two readers to check that one cannot be registered on two providers, and shuts down neither. Two leaked threads.
3. The two exporter preference tests each build a reader purely to read its preference mapping. These readers are never registered on a provider, so their ticks return early at the self._collect is None guard and cannot corrupt another test's state, but each still burns a thread and logs a warning every interval into whatever test is running.
4. The detector, an autouse fixture in opentelemetry-sdk/tests/conftest.py.

All four fixes use addCleanup so the shutdown happens even when the test body fails.

How the detector works

It snapshots the live threads named OtelPeriodicExportingMetricReader before the test and fails if a new one is still alive afterwards.

Diffing against a before-snapshot is the important part. It attributes a leak to the test that caused it, and it stops one leak from cascading into a failure in every test that runs after it.

There is a short grace period before it gives up, because shutdown() joins the ticker with a timeout and a thread on its way out can still be alive when the test body returns. Only a test that actually leaks pays that cost.

Testing

• pytest opentelemetry-sdk/tests before the fixes, with the detector in place: 4 errors, naming exactly the four tests above.
• pytest opentelemetry-sdk/tests after: 888 passed, 47 subtests passed, 0 errors.
• ruff format --check and ruff check clean on all four changed files with the pinned ruff==0.16.1.

I also checked that the detector fires and does not cascade, using a scratch module with two tests: one that builds a reader and drops it, one that builds a reader and shuts it down. Result was 2 passed, 1 error, the error naming only the leaking test. The well-behaved test ran afterwards and passed, confirming the before-snapshot prevents the cascade.

Scope and overlap

Test-only. No production code touched.

Commit 3 overlaps with #5622, which fixes the same two preference tests with try/finally rather than addCleanup. Happy to drop that commit if #5622 lands first, though the detector needs those two fixed one way or the other to stay green.

Relationship to #5157

I want to be careful not to overclaim. I have not proven that any of these four leaks produced the traceback in #5157; the CI logs from that run have expired, and the surviving excerpt does not pin it down. What this gives is a mechanical guard, so that if it 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 and #5639.

[opentelemetry-pr-dashboard]

Pull request dashboard status

Waiting on the author · refreshed 2026-10-20 23:06 UTC

Respond to 2 review items (e.g. link a commit, explain why not, ask a follow-up):

• Inline threads: 1, 2

Status above doesn't look right?

• Just replied or pushed? Anything around or after the refresh time above may not be picked up yet — give it a few minutes.
• Should this be with reviewers? Comment /dashboard route:reviewers to route it to them.
• Anything wrong — including the routing? Report it with what you expected; it helps us improve the dashboard.

[hectorhdzg]

These cleanups shut down the readers directly, but the MeterProvider instances still register atexit handlers that call them again. Could the provider constructions below use shutdown_on_exit=False to avoid leaving those callbacks behind?

[hectorhdzg]

Suggested change

	`opentelemetry-sdk`: fail any test that leaves a `PeriodicExportingMetricReader` ticker thread running, and shut down the four readers that were leaking one into the rest of the test session
	`opentelemetry-sdk`: detect and clean up leaked periodic metric reader threads in tests

[opentelemetry-pr-dashboard]

Hi @ocelotl — just a friendly reminder that this pull request is waiting on you. The dashboard status comment has the open items and is kept current.

• Replying is enough to hand it off — answer, explain why no change is needed, or ask a follow-up. The dashboard routes it onward once nothing on the list is waiting on you.
• To hand it back for any other reason, including the dashboard getting this wrong, comment /dashboard route:reviewers.

[github-actions]

This PR has been automatically marked as stale because it has not had any activity for 14 days. It will be closed if no further activity occurs within 14 days of this comment.
If you're still working on this, please add a comment or push new commits.