fix(telemetry): shut down the retired tracer provider off async workers

[BrynCooke]

Problem

A router that hot-reloads often, for example on every supergraph publish, can occasionally keep a retired pipeline in memory for good. The router keeps serving traffic, but memory steps up by the size of the old pipeline (plan cache, Redis connection pools and so on) and never comes back. The retired pipeline's Redis pool keeps heartbeating, and connections for the old schema drain to zero. So nothing legitimate is keeping it alive.

The cause is where the replaced OpenTelemetry tracer provider gets shut down.

• In opentelemetry_sdk 0.31, every SdkTracer holds a clone of its SdkTracerProvider, and so does every SDK Span through its tracer. The provider shuts itself down when its last clone is dropped.
• Our tracing layer builds and drops a short-lived SDK span on the closing thread for each finished tracing span (OpenTelemetryLayer::on_close). If that span is built just before a reload swaps the tracer, and dropped just after, it holds the last reference to the old provider.
• The shutdown therefore runs on an async worker. The async-runtime BatchSpanProcessor shutdown sends a message to its background task and then calls futures_executor::block_on to wait for the reply, ignoring its timeout. Tokio puts the woken background task in that worker's LIFO slot, which other workers cannot steal. The worker waits for a task that only it could run, and parks forever.
• The parked worker's stack still holds the request future, and through it the retired pipeline.

The reload path already tried to guard against this. It wrapped set_tracer_provider in block_in_place, and its comment claimed the old provider "is returned and must be dropped in a blocking task". In 0.31, set_tracer_provider returns (): it drops the replaced value itself, and nothing was ever handed back. block_in_place made the global's own drop safe, but it never covered clones held by in-flight spans. The same race reproduces with opentelemetry 0.24 and the earlier guard, so it predates the 0.31 upgrade.

Fix

The router now keeps its own reference to the tracer provider it has installed. Installing a new one hands back the provider it replaces, and the reload shuts that provider down explicitly, on a blocking thread, before any straddling span lets go of it. Once a provider is shut down, TracerProviderInner::drop sees is_shutdown and does nothing, whichever thread drops the last clone.

Where the installed provider lives

The installed provider is process-wide, like everything it feeds. The tracing subscriber is installed once per process, and so are the OpenTelemetry global and the hot-swappable tracer the subscriber exports through (OPENTELEMETRY_TRACER_HANDLE). So the provider is kept in that existing hot tracer handle, next to the tracer built from it, and no new static is added. The handle is a small type, TracerHandle, in reload/otel.rs:

flowchart LR
    S[tracing subscriber, once per process] -->|exports spans through| R[ReloadTracer]
    H[OPENTELEMETRY_TRACER_HANDLE: TracerHandle] -->|owns| R
    H -->|keeps| P[installed SdkTracerProvider]
    R -->|current tracer built from| P
    G[OTel global provider] -->|clone of| P
    A[Activation, one per reload] -->|install new provider, keep returned one| H
    E[executable exit] -->|take and shut down| H

Loading

• TracerHandle::install(provider) is the only way to install a tracer provider. It builds the scoped tracer and hot-swaps it into the subscriber, records the provider, and sets the global in block_in_place as before. It returns the provider it replaced.
• Activation::reload_tracing calls install and keeps the returned provider as the retired one. Activation's existing Drop, which already runs its cleanup in spawn_blocking (block_in_place under cfg(test)), now calls shutdown() on it rather than just dropping it. Whoever installs a provider is handed the old one to shut down, so this holds for every activation, however its Telemetry plugin was built.
• At exit, the executable's existing awaited spawn_blocking takes the final provider out of the handle and shuts it down, next to the meter provider shutdown. Before, it swapped in a default provider for the same purpose.
• The span read path through ReloadTracer is unchanged.

Reload lifecycle

sequenceDiagram
    participant A as Activation (reload)
    participant H as TracerHandle
    participant G as OTel global
    participant B as Blocking thread
    participant W as Async worker (straddling span)
    alt tracing config changed
        A->>H: install(new provider)
        H->>H: hot-swap tracer, record new provider
        H->>G: set_tracer_provider(new) in block_in_place
        Note over G: drops its clone of old, not the last one
        H-->>A: returns old provider
        A->>B: Activation drop: old.shutdown()
        W->>W: drops span, last clone of old
        Note over W: already shut down, returns immediately
    else tracing unchanged
        Note over A,H: nothing to install, handle keeps the current provider
    else reload fails before commit
        A->>B: Activation drop: shuts down its never-installed provider
        Note over H: installed provider untouched
    end
    Note over B,H: at exit, executable takes the installed provider from H and shuts it down in spawn_blocking

Loading

An earlier revision of this PR kept the provider in the router factory and passed a handle to it through PluginInit, add_plugin, create_plugins and a new factory shutdown() method. That tied a process-wide provider's lifetime to one factory: dropping a temporary factory (as TestHarness does) shut down the live provider, and plugins built elsewhere got a handle nobody owned. All of that is removed, including the YamlRouterFactory::default() refactor, which is reverted in its own commit. Replies posted on the earlier review threads about the factory handle, the factory shutdown() and its tests, the drop guard and the factory doc comment describe that removed design.

Meter providers

I audited meter providers for the same last-clone pattern, and they are not affected. SDK meters and instruments hold the provider's pipelines (Arc<Pipelines>), not SdkMeterProviderInner, and only that type's Drop triggers shutdown. The router's AggregateMeterProvider is the only holder of each SdkMeterProvider, and set hands the replaced one back to the Activation, which drops it on a blocking thread. No request path can own the last reference, so meter providers are unchanged apart from a comment recording this.

How it was verified

The tests use a span processor that counts how many times its provider is shut down, and a local TracerHandle, not the process-wide one.

• installing_a_tracer_provider_retires_the_one_it_replaces installs a first provider (nothing is returned), starts a span on it to stand in for the straddling span, and commits a second provider through an Activation. The first provider must be shut down exactly once while the span is still alive, the new one must stay live, and taking and shutting down the installed provider at exit must then stop the new one.
• retired_tracer_provider_is_shut_down_while_a_span_still_holds_it checks that dropping an Activation shuts down its retired provider while a span holds it, and that the span's later drop doesn't shut it down again.
• activations_that_do_not_replace_the_installed_provider_leave_it_running covers an unchanged-tracing reload and a failed reload. Neither may shut down the installed provider.

Each key assertion was checked by breaking the code: install not returning the replaced provider, the activation retiring the new provider instead of the old one, and Drop only dropping the retired provider. Each makes the tests fail.

The apollo_reports, apollo_otel_traces and apollo_otel_http_proxy integration tests, which failed on the previous revision because a dropped temporary factory shut down the live provider, now behave as they do on dev. Only the process-wide hot tracer reload and the global install are not unit-tested, because they are one-time process globals. An earlier standalone model of the reload path, built outside the router, retained no pipelines across 60,000 reloads with explicit shutdown of the retired provider. Without it, a pipeline was retained within a few reloads.

Risk

• Spans that finish on the retired provider after it has shut down are discarded. That only affects SDK spans built in the brief window between the tracer swap and the explicit shutdown. Request-scoped tracing spans build their SDK span on close with whichever tracer is current, so long-running requests that straddle a reload still export through the new provider.
• At exit, the installed provider is shut down rather than replaced with a default, so the global keeps a shut-down provider and is inert. The exit shutdown runs where it did before, in the same awaited blocking task as the meter provider shutdown.
• Only routers that run under the executable's telemetry setup install providers, as before. Routers built with TestHarness or embedded without that setup don't touch the hot tracer handle.
• A failure to shut down a tracer provider is logged at warn.
• This doesn't change the router's graceful-shutdown timeout, or the connection force-close logic that runs after a reload.
• Backing out means reverting this PR's commits. There is no configuration or wire-format change.

Where to look first

• TracerHandle::install in reload/otel.rs. It records the new provider before setting the global, and it returns the replaced provider. That order is what guarantees that the global's drop of the old provider is never its last reference and that no span can hold the last clone of a provider that hasn't been shut down.
• Activation::install_tracer_provider and Drop for Activation in reload/activation.rs, which keep the returned provider and shut it down off the async workers.

[apollo-librarian]

✅ Docs preview has no changes

The preview was not built because there were no changes.

Build ID: 8f6544d4484ffc819b7d5892
Build Logs: View logs

---

✅ AI Style Review — No Changes Detected

No MDX files were changed in this pull request.

Review Log: View detailed log

This review is AI-generated. Please use common sense when accepting these suggestions, as they may not always be accurate or appropriate for your specific context.

[rohan-b99]

Commit-time provider swap is never exercised by tests. Neither new test runs reload_tracing. The first one sets activation.new_trace_provider = Some(retired) by hand, and OPENTELEMETRY_TRACER_HANDLE is unset in unit tests, so the installed.replace(...) plus self.new_trace_provider = retired swap never runs. If this line were dropped, or the wrong provider went into the handle, both tests would still pass. The old provider would then leak, or the live one would be shut down on every reload.

[BrynCooke]

Fix it.

[BrynCooke]

Fixed. The handle swap is now in its own method, Activation::swap_installed_tracer_provider. It takes the new provider, records it in the handle and keeps the replaced one as the retired provider. reload_tracing calls it and only adds the hot-tracer reload and the global set_tracer_provider, which rely on process globals that unit tests can't set up. No statics were added.

The new test committing_a_tracer_provider_retires_the_installed_one runs that swap against a handle that already holds an installed provider, then drops the activation. It checks that the old provider was shut down exactly once, that the new one is still live, and that the handle's own shutdown later stops the new one. I checked that it fails for both mutations you described: if the handle isn't updated, or if the new provider is retired instead of the old one, the test fails with "the newly installed provider must stay live".

Resolved by 86b841be9767.

[rohan-b99]

No test checks that the state machine calls factory shutdown. Nothing asserts that process_events calls router_configurator.shutdown() or that OrbiterRouterSuperServiceFactory passes the call on. MockMyRouterConfigurator quietly uses the empty default. executable.rs no longer resets the global tracer provider, so this call is now the only exit-time flush. If it regresses, the last batch of spans is lost on every shutdown and no test fails.

[BrynCooke]

Added coverage for both parts.

• router_factory_is_shut_down_when_the_state_machine_stops runs process_events with a factory that passes create through to the existing mock and counts shutdown calls. It has two cases: a clean stop (startup, then Shutdown, returning Ok) and an error exit (the factory fails on startup, returning Err). Both cases assert that shutdown ran exactly once.
• orbiter::test::shutdown_is_forwarded_to_the_router_factory wraps a YamlRouterFactory holding an installed tracer provider in OrbiterRouterSuperServiceFactory. It calls shutdown() and asserts the provider was shut down before the factory is dropped. That covers both Orbiter's forwarding and the real factory's shutdown.

With the state machine call or the Orbiter delegation removed, all three tests fail.

Resolved by fd69101c4099.

[rohan-b99]

The exit-time tracer flush now depends on process_events finishing. With the unconditional teardown removed, spans are flushed only if the state machine reaches its post-loop shutdown() call. If the state machine task panics (router/mod.rs turns the JoinError into StartupError), or the RouterHttpServer future is dropped early, YamlRouterFactory::shutdown never runs. Only the meter provider is shut down here, so buffered BatchSpanProcessor spans are never flushed.

[BrynCooke]

Fix it

[BrynCooke]

Fixed with a drop guard owned by the lifecycle, without statics. YamlRouterFactory now holds its handle through a TracerProviderOwner, and dropping the owner shuts down whatever provider the handle still holds, on a blocking thread. The explicit shutdown() at the end of process_events still runs first on normal exits and empties the handle, so the guard then does nothing. If the state machine task panics or is cancelled first, the future is dropped, so the factory and its owner are dropped too, and the final provider is still shut down.

owner_shuts_down_the_installed_provider_if_its_task_panics_or_is_cancelled puts an owner inside a spawned task and checks that the provider is shut down exactly once, both when the task panics and when it is aborted. With the guard's shutdown removed, the test fails.

An early drop of RouterHttpServer sends the shutdown signal and leaves the task running, so it still reaches the explicit shutdown. If the runtime itself is shutting down, the task is cancelled and the guard takes over. One limit: the guard's blocking task isn't awaited, so if the process exits immediately afterwards the flush can be cut short. The normal exit path still awaits it.

Resolved by 8d3118eafd00.

[rohan-b99]

"After its last router has been dropped" isn't enforced. The Reloading path discards all_connections_stopped_signals ("Connections from before this attempt will not be awaited on shutdown"), so old pipelines can still be serving requests when this runs. Their spans then end on a provider that is already shut down and are silently dropped. Either enforce this or relax the doc comment.

[BrynCooke]

I'm Ok with dropping some span if it is very complex to to the right thing.
Think about this and make a judgment call.

[BrynCooke]

I relaxed the doc comment rather than enforcing the guarantee. The comment now says that shutdown runs once when the state machine stops, whether it stopped cleanly or because of an error. It also says shutdown doesn't wait for every retired pipeline: connections from before a failed reload attempt aren't tracked, and a fatal reload error stops the router without draining it, so spans those pipelines end after this call are dropped.

Enforcing it would mean carrying the connection-stopped signals through failed reload attempts (try_start consumes them today) and draining on the fatal-error path too. That changes state machine behaviour that has nothing to do with this fix. The only cost is losing some late spans from old pipelines at process exit, which you said is acceptable.

Resolved by b12591cd6646.

[rohan-b99]

Shutdown failures are logged only at debug!. YamlRouterFactory::shutdown also only logs a JoinError. So a failed flush of the final batch at exit (exporter timeout or unreachable) loses spans with nothing logged at info or above. Meter provider shutdown in executable.rs logs the same kind of failure with tracing::error!.

[BrynCooke]

tracing::warn seems more appropriate for both of these.

[BrynCooke]

Both now log at warn!: the tracer provider shutdown error in activation.rs, and the JoinError from the exit shutdown task in YamlRouterFactory::shutdown.

Resolved by c774ba696149.

[rohan-b99]

Altitude: a static would do what this plumbing does. This threads a TracerProviderHandle through the factory, a new RouterSuperServiceFactory::shutdown trait method, Orbiter delegation and two lend call sites. activation.rs already keeps process-global state in a static (REGISTRY), and OPENTELEMETRY_TRACER_HANDLE is global too. A static INSTALLED_TRACER_PROVIDER: Mutex<Option<SdkTracerProvider>> swapped in reload_tracing, and shut down from executable.rs's existing spawn_blocking, would cover every activation path. Today, any activation that isn't lent a handle silently falls back to the original bug: the last span drop runs shutdown on an async worker.

[BrynCooke]

No, we do not want statics. Statics are a code smell.

[BrynCooke]

We're keeping the lifecycle-owned design and not adding a static. The installed provider belongs to the router's lifecycle: the factory that the state machine owns builds every router and outlives them. A process global would hide that ownership, and it would be shared by every router in the process, including test routers.

Your underlying concern was right, though: an activation that was never lent a handle quietly fell back to the original bug. That fallback is now gone structurally. The handle reaches Telemetry through its construction path, as a crate-internal PluginInit field that the router factory sets for every plugin it builds. Telemetry keeps it and passes it to Activation::commit, which now requires it. Nothing is lent afterwards, the activation has no optional handle, and there's no ordering requirement. The other thread has the details. The exit flush is also covered now if the state machine task panics or is cancelled, by a drop guard on the factory's handle, still without a static.

Resolved by 8d3118eafd00, a46deb5232a7.

[rohan-b99]

Two lend call sites. For the initial telemetry plugin this call does nothing, because create() already lent the handle and activated it. Any future path that builds and activates a Telemetry plugin has to remember to lend first, or it silently brings back the shutdown-on-async-worker bug. Lending in one place, such as at Telemetry construction or through a global, would remove that ordering requirement.

[BrynCooke]

Fix it

[BrynCooke]

Fixed. The handle is now supplied when the plugin is constructed, not lent afterwards.

• PluginInit has a crate-internal tracer_provider field. Nothing changes in the public builder API.
• The router factory sets it on every PluginInit it builds, both for the early telemetry plugin at startup and in add_plugin. create_plugins and add_plugin now take the handle as a required argument.
• Telemetry::new stores the handle, and Telemetry::activate passes it to Activation::commit(&handle), which requires it.

lend_installed_tracer_provider, Activation::with_installed_tracer_provider and the optional handle field are all removed. So is the redundant lend in inner_create_supergraph. A Telemetry plugin can no longer be activated without a handle, and nothing needs to happen between construction and activation. A PluginInit built outside the router factory, for example in a unit test, gets a fresh handle of its own.

Resolved by a46deb5232a7.

[rohan-b99]

This Drop is what's failing the 25 CI tests. TestHarness::build_common (test_harness.rs:321) builds through a temporary YamlRouterFactory::default(). inner_create_supergraph activates telemetry, which installs the provider into this owner's handle. The temporary is then dropped at the end of the statement, and this Drop shuts down the provider that the global and the hot tracer are still using. Every later span is discarded, which is why apollo_reports, apollo_otel_traces and apollo_otel_http_proxy time out with 0 reports. TestHarness is public, so downstream users lose spans too. The response_cache and connectors helpers have the same pattern, where the local factory drops when the helper returns.

[BrynCooke]

Needs fixing

[BrynCooke]

Fixed by the redesign. The installed tracer provider no longer belongs to a router factory, so dropping one can't shut it down. TracerProviderOwner and TracerProviderHandle are gone. The provider now lives in the hot tracer handle behind OPENTELEMETRY_TRACER_HANDLE, next to the tracer it feeds, and that handle lives exactly as long as the process-wide subscriber. TestHarness and the response_cache and connectors helpers build their temporary YamlRouterFactory as before, with no effect on the live provider.

Locally, apollo_reports, apollo_otel_traces and apollo_otel_http_proxy now match dev. Out of 42 tests, the previous head failed 33, with timeouts; this head fails only the same 10 that also fail on unchanged dev, and those fail because this sandbox can't resolve the demo subgraph hosts.

Resolved by 2a620bdaaf49, 6a7cf666a76a.

[rohan-b99]

The production cancel path isn't covered. off_async_workers uses block_in_place under cfg(test) but spawn_blocking in release builds, so the new panic/cancel test only exercises the test-only branch. In production the state machine future is usually dropped during runtime shutdown, and spawn_blocking then returns a cancelled handle without running the closure. That means the "cancelled" case described here still doesn't flush.

[BrynCooke]

You're right that the guard's spawn_blocking doesn't run during runtime shutdown. The redesign removes the drop guard and off_async_workers entirely. The final provider is shut down once more by the executable's existing awaited spawn_blocking at exit, next to the meter provider shutdown, as it was before this PR. The only difference is that it now takes the installed provider out of the hot tracer handle and calls shutdown() on it, where before it swapped in a default provider. So there's no untested production-only cancel path any more.

Resolved by 2a620bdaaf49.

[rohan-b99]

An unowned default handle silently brings back the original bug. builder, try_builder and fake_builder all default this to a fresh, unowned handle, and the "telemetry plugin ONLY" note isn't enforced. A Telemetry plugin built through any other path retires nothing on commit, so set_tracer_provider drops the previously installed provider. If a span holds the last reference, shutdown runs on an async worker, which is the stall this PR is fixing.

[BrynCooke]

Fixed by removing the field. PluginInit is back to how it is on dev, with no tracer provider handle and no builder defaults. There is now only one place that can install a tracer provider: TracerHandle::install, on the process-wide hot tracer handle. It returns the provider it replaces, and Activation keeps that as the retired provider and shuts it down off the async workers. However a Telemetry plugin is built, its activation goes through that same install operation, so there's no unowned handle to fall back to.

Resolved by 2a620bdaaf49.

[rohan-b99]

This ties a global provider's lifetime to one factory instance. The installed provider is process-global (it lives in opentelemetry::global and OPENTELEMETRY_TRACER_HANDLE), but its lifetime now depends on whichever factory installed it. The plumbing has spread to PluginInit, add_plugin, create_plugins and test fixtures, and any shorter-lived factory kills the live provider, which is what broke CI. A static INSTALLED_TRACER_PROVIDER next to REGISTRY, swapped in reload_tracing and shut down at state-machine or executable teardown, would match the real scope and remove the handle from PluginInit.

[BrynCooke]

Agreed on scope: the provider is process-wide, and tying it to one factory was the mistake. This follows your scope point, but rather than adding a new static it uses the existing hot tracer handle behind OPENTELEMETRY_TRACER_HANDLE. That handle is already set once per process, alongside the subscriber. It now holds both the reloadable tracer and the provider it came from. TracerHandle::install builds the scoped tracer, hot-swaps it, sets the global in block_in_place and returns the replaced provider. The executable shuts the final provider down at exit.

All the plumbing is gone: the handle in PluginInit, the arguments to add_plugin and create_plugins, the handle stored in Telemetry, the argument to commit, RouterSuperServiceFactory::shutdown with the state machine call and the Orbiter forwarding, and the fixture edits, including the YamlRouterFactory::default() refactor. Against dev, the PR now touches only reload/otel.rs, reload/activation.rs, executable.rs and the changeset.

Resolved by 2a620bdaaf49, 6a7cf666a76a.

[mergify]

This pull request does not currently match the merge queue conditions, so it cannot be queued from here. The box comes back if it matches again.