fix(tracing): close and recover the default exporter safely - #4737
fix(tracing): close and recover the default exporter safely#4737seratch wants to merge 1 commit into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3ad8ca3da2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
3ad8ca3 to
9ac6e49
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9ac6e493b8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return False | ||
| provider._set_processors(processors) | ||
| default_processor = _DEFAULT_PROCESSOR | ||
| if default_processor is not None and default_processor not in processors: |
There was a problem hiding this comment.
When a custom TracingProcessor implements value equality that returns true for the SDK processor, default_processor not in processors is false even though the default instance was removed from the provider. The provider then contains only the custom processor, while _DEFAULT_PROCESSOR and _global_processor still identify the detached default as active; its exporter is never retired, and atexit also skips closing it because the cached processor remains marked as attached. Use an identity check for this ownership decision.
AGENTS.md reference: AGENTS.md:L149-L149
Useful? React with 👍 / 👎.
Co-authored-by: abhay-codes07 <abhaysingh0293@gmail.com> Co-authored-by: rajarshidattapy <rayan05rio@gmail.com>
9ac6e49 to
7bda67a
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7bda67a3b8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| _DEFAULT_PROCESSOR = None | ||
| _SDK_DEFAULT_PROCESSOR = None |
There was a problem hiding this comment.
When callers save the current processor list, temporarily call set_trace_processors([custom]), and later restore the saved list, these assignments discard both ownership pointers while retirement makes the saved default processor terminal. Restoring that list then follows the _DEFAULT_PROCESSOR is None path and installs the shut-down instance unchanged, so every subsequent default trace or span is silently dropped by _enqueue; the repository already uses this save/restore pattern in integration_tests/openai/test_tracing.py:74-95. Recognize the retired SDK default during restoration and replace it with the live recovered processor.
AGENTS.md reference: AGENTS.md:L147-L147
Useful? React with 👍 / 👎.
1 participant
This pull request supersedes #4712 and replaces the overlapping #4732 lifecycle work with one coherent default tracing shutdown model.
The module-owned default
BackendSpanExporternow closes exactly once after its final export, including timed shutdowns with a surviving worker. Caller-injected exporters remain caller-owned. Terminal default processors and exporters recover through the existing provider pipeline while preserving exporter configuration, disabled state, custom processors, and provider identity.The change also makes repeated shutdown safe, retires defaults removed through either top-level or direct provider processor replacement, serializes API-key updates with recovery, and prevents atexit callbacks from resurrecting a fresh default tracing stack.
This pull request resolves #4681.
This pull request resolves #4683.