fix(sdk): spawn periodic reader worker at reader build time

[karan-vk]

Fixes #3601

Changes

periodic_reader_with_async_runtime::PeriodicReader stored its spawn closure and only ran it inside register_pipeline, so the worker task landed on whatever tokio runtime was ambient when the meter provider was built rather than when the reader, which takes the runtime argument, was built. Building the reader and the provider in different contexts could deadlock or panic.

The worker is now spawned eagerly in build(), matching the async-runtime BatchSpanProcessor and BatchLogProcessor, and ProducerOrWorker is reduced to a plain Option producer. Added a regression test that builds the reader inside a runtime and the provider outside of one; it panics without this fix.

Behavior note: a reader that is built but never attached to a provider now runs its interval ticker and logs collect errors, where before the worker was never spawned. This matches the existing behavior of the async-runtime batch processors.

Merge requirement checklist

• CONTRIBUTING guidelines followed
• Unit tests added/updated (if applicable)
• Appropriate CHANGELOG.md files updated for non-trivial, user-facing changes
• Changes in public API reviewed (if applicable) - no public API changes

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.4%. Comparing base (aa0b81a) to head (ac5c658).

Additional details and impacted files

@@           Coverage Diff           @@
##            main   #3605     +/-   ##
=======================================
+ Coverage   82.3%   82.4%   +0.1%     
=======================================
  Files        130     130             
  Lines      28697   28737     +40     
=======================================
+ Hits       23626   23707     +81     
+ Misses      5071    5030     -41     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[scottgerring]

I think #3413 should help out here also and we should aim to get that in soon to clear up all of these weird unexpected deadlocks issues at once

[karan-vk]

Took a proper look through #3413 and I think it's a really nice piece of work. Consolidating Tokio/TokioCurrentThread into a single runtime that detects the flavor at spawn time (and treating a single-worker multi_thread runtime as starvation-prone too, which is what any 1-vCPU container gets by default) removes a whole class of footguns, and the SpawnHandle associated type keeps the trait open for custom runtimes pinned to a specific Handle. Shutdown joining the worker directly and surfacing the final export error, and even worker panics, is a big step up from the oneshot signalling, and it's great to see the #2056 test un-ignored. Fully agree it should go in soon. Happy to help there, whether that's review, testing it against the #3601 repros, or rebase gruntwork.

For this PR specifically, I think the two compose rather than overlap: #3413 fixes how and where the spawn executes and how shutdown waits, while this one fixes when the worker is captured (at reader build, where the runtime argument is supplied, matching the async-runtime batch processors). As #3413 stands, the worker closure is still invoked inside register_pipeline, so the flavor detection runs against whatever context is ambient at provider build. The case in pydantic/logfire-rust#141 (reader built inside a dedicated runtime to keep hyper/reqwest's own tracing events out of the exported telemetry) would still spawn onto the app runtime whenever that one is a healthy multi-thread runtime.

A nice side effect of landing this one first is that the #3413 rebase of this file gets simpler: with the spawn in build(), the handle exists at construction, so worker_handle can start out as Running(handle), and the WorkerState::Complete(Ok(())) placeholder, the placeholder worker closure, and ProducerOrWorker itself all fall away. It's a small change with no public API impact and it's rebased on latest main. Happy to go whichever way you prefer, though.

[Copilot]

Pull request overview

This PR fixes periodic_reader_with_async_runtime::PeriodicReader spawning its background worker during meter provider construction (via register_pipeline) instead of at reader build time, which could capture the wrong ambient Tokio runtime and lead to deadlocks or panics when reader/provider are built in different runtime contexts.

Changes:

• Spawn the periodic reader worker eagerly in PeriodicReaderBuilder::build(), so it runs on the runtime that’s ambient at reader construction time.
• Simplify internal state by removing the deferred “spawn worker” closure and storing only an optional producer (sdk_producer) for collection.
• Add a regression test covering the “build reader inside runtime, build provider outside runtime” scenario and update the SDK changelog.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
opentelemetry-sdk/src/metrics/periodic_reader_with_async_runtime.rs	Eagerly spawns the periodic worker at reader build time, simplifies registration state, and adds a regression test for the captured-runtime behavior.
opentelemetry-sdk/CHANGELOG.md	Documents the behavior change for PeriodicReader worker spawning timing and its implications.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[scottgerring]

Hey @karan-vk , thanks for looking more into this!
I've looked again and I agree. Almost, neither my clanker nor my actual human review could find anything to fault; thanks for the minimal, clean improvement. LGTM.

@cijothomas would be great to merge this when you have a moment!