fix(client): release the global OTel tracer provider on shutdown - #126
Merged
Merged
Conversation
`shutdown()` dropped its provider handle but left OpenTelemetry's global tracer provider registered. That global is once-guarded — a second `set_tracer_provider` logs "Overriding of current TracerProvider is not allowed" and keeps the provider already in place — so an init/shutdown/init cycle left every later span routed to the provider that had just been shut down, and exported nothing. Release both the global slot and the `Once` that guards it on teardown; clearing the slot alone leaves the guard tripped and the next set a no-op. Only released when telemetry actually started, so a process where `_setup_telemetry` bailed keeps whatever another library registered. The global text map propagator needs no equivalent — `set_global_textmap` is a plain assignment, so the next setup overwrites it. `_reset_for_testing` does the same, so a suite that initializes more than once does not leave every later span on the first test's provider. Also covers the BYOC idempotency this SDK already had: `_resolve_client` checks the singleton ahead of the pre-initialized-client path, so a repeat `init_client(options, client)` neither re-runs telemetry setup nor swaps the stored client. The JS SDK had that check below the BYOC branch and was re-registering OTel on every call; these tests pin the ordering here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
XieX
marked this pull request as ready for review
October 1, 2026 20:13
knfreemLD
approved these changes
Oct 1, 2026
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit d5ee277. Configure here.
…tered Addresses Bugbot on #126. The previous commit gated the global teardown on `_tracer_provider` being set, which says we *built* a provider, not that we own the global. `_setup_telemetry` assigns the handle after `set_tracer_provider`, whose set is refused when another library got there first — so in a process with an existing provider (auto-instrumentation, an APM agent, or an app that configures its own), `shutdown()` cleared that provider and reset the `Once`, leaving the global a no-op proxy and silently killing the host application's tracing. Track whether our set actually took, via `trace.get_tracer_provider() is provider`, and gate the release on that. The provider is still shut down either way since we built it and it owns an exporter and a batch timer. Also warn when the set is refused: the caller's `otlpEndpoint`/`serviceName` cannot take effect, and the only existing signal is OTel's own terse warning. The same flaw was in the JS change this ports from; fixed there too. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
XieX
added a commit
to launchdarkly/js-ai-sdk
that referenced
this pull request
Oct 1, 2026
The previous commit gated the global teardown on `tracerProvider` being non-null, which says we *built* a provider, not that we own the global registration. `setupTelemetry` assigns the handle and then calls `register()`, whose global set is refused when another library got there first — so in a process with an existing provider (auto-instrumentation, an APM agent, or an app that configures its own), `shutdownTelemetry()` wiped that provider and left the global a noop, silently killing the host application's tracing. Read the delegate back after `register()` to learn whether the set actually took, and gate the release on that. The provider is still shut down either way since we built it and it owns an exporter and a batch timer. Also warn when the registration is refused: the caller's `otlpEndpoint`/`serviceName` cannot take effect, and OTel's own diag error is swallowed by default. Caught by Bugbot on the Python port of this change (launchdarkly/python-ai-sdk#126); the same flaw was here. The test mock now models OTel's first-registration-wins semantics rather than reporting a fixed delegate, including the unregistered noop provider that carries no `_delegate` at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`_client is not None` is `_resolve_client`'s idempotency guard, but `_client` was assigned before `_setup_telemetry` ran. When setup raised — a malformed `OTEL_EXPORTER_OTLP_TIMEOUT` does, with a ValueError from the OTLP exporter — `_client` stayed set. The next `init_client()` returned that half-initialized client as a silent success with no telemetry, hiding the config error, and on the SDK-key path the LD client's connection was never closed. That contradicts `init_client`'s documented promise that a call which raises leaves no global state behind. Assign `_client` only after setup succeeds, on both paths. On the SDK-key path close the client we built when setup fails; on the BYOC path leave it open, since the caller owns it. Found in a pass over the lifecycle guards following the ownership fix; the JS SDK's counterpart was a cached failed-init promise, fixed in launchdarkly/js-ai-sdk#103. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Resolves the shutdown() docstring conflict with the base's comment trim (744199f): keep the base's reworded skill-store sentence and this branch's paragraph on releasing the process-global tracer provider. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Companion to launchdarkly/js-ai-sdk#103, which fixes the same lifecycle bugs in JS. Two of them exist here, and one has a Python-specific counterpart.
1. An init/shutdown/init cycle exported nothing
shutdown()dropped its provider handle but left OpenTelemetry's global tracer provider registered. That global is once-guarded — a secondset_tracer_providerlogsOverriding of current TracerProvider is not allowedand keeps the provider already in place — so after a re-init, every span routed to the provider that had just been shut down. Observed across two cycles: the globalservice.namestayedcycle1and now correctly becomescycle2.Releasing it means resetting both the global slot and the
Oncethat guards it; clearing the slot alone leaves the guard tripped, so the next set is a silent no-op. Both are private, sinceopentelemetry-pythonhas no public way to unset them, so_release_otel_globals's docstring explains the reach. The propagator needs no reset —set_global_textmapis a plain assignment.The release only happens when our
set_tracer_provideractually took effect (trace.get_tracer_provider() is provider). Bugbot caught that the first version gated it on having built a provider, which would have wiped a host app's already-registered provider; the JS PR had the same flaw. A refused set now logs a warning.2. A failed telemetry setup left a half-initialized client behind
_client is not Noneis the idempotency guard, but_clientwas assigned before_setup_telemetryran. A malformedOTEL_EXPORTER_OTLP_TIMEOUTmakes setup raise, and_clientstayed set: the nextinit_client()returned the client as a silent success with no telemetry, and on the SDK-key path its connection was never closed._clientis now assigned only after setup succeeds. On the SDK-key path the client we built is closed on failure; a BYOC client is left open, since the caller owns it. JS's counterpart was a cached failed-init promise.Not a bug here: BYOC idempotency
JS checked the singleton below the pre-initialized-client branch and re-registered OTel on every
initClient(client)._resolve_clientchecks it first — 1_setup_telemetrycall across three inits — and two tests pin that ordering.Verification
_setup_telemetry, with the OTLP exporter stubbed or failing before any network use.🤖 Generated with Claude Code
Note
Overview
Fixes Python client lifecycle bugs aligned with the JS SDK: init/shutdown/init no longer leaves spans routed to a shut-down tracer provider, and failed telemetry setup no longer leaves a half-initialized singleton client.
shutdown()now clears OpenTelemetry’s once-guarded global tracer registration (via_release_otel_globals) only when this SDK’sset_tracer_provideractually won (_owns_otel_globals). If another library registered first, setup logs a warning and shutdown does not tear down the host app’s provider. Test reset mirrors the same release so multi-init suites do not leak globals.init_clientassigns_clientonly after_setup_telemetrysucceeds. On the SDK-key path, a telemetry failure closes the newly created LD client; BYOC clients are left open for the caller. Repeat inits still run telemetry setup at most once (idempotency check first).Docs in
agents.mddescribe the shutdown/release behavior;test_lifecycle.pyadds coverage for cycles, foreign providers, failed setup, and idempotency.Reviewed by Cursor Bugbot for commit ebb6c1e. Bugbot is set up for automated code reviews on this repo. Configure here.