fix(tracing): avoid import-time tracing exporter/provider initialization#2490
fix(tracing): avoid import-time tracing exporter/provider initialization#2490OiPunk wants to merge 3 commits intoopenai:mainfrom
Conversation
![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: af1a0f9b58
ℹ️ 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".
|
Thanks for the CI signal. I pushed Local re-check on this follow-up commit:
|
|
Thanks for the review, this was a valid catch. I pushed commit 43e8b51 to make setup.get_trace_provider() lazily initialize the default provider and processor on first use. This keeps import-time behavior side-effect free, but restores expected runtime behavior for helper entry points like agents.trace(...), which now no longer fail when tracing has not been manually initialized yet. I also added regression coverage in tests/tracing/test_lazy_tracing_init.py for:
Local checks run on this update:
|
|
@codex review again |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 43e8b512d6
ℹ️ 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".
| if GLOBAL_TRACE_PROVIDER is None: | ||
| raise RuntimeError("Trace provider not set") | ||
| # Lazily initialize defaults on first tracing API usage to avoid | ||
| # import-time side effects while keeping historical call behavior. | ||
| from .processors import default_processor | ||
| from .provider import DefaultTraceProvider | ||
|
|
||
| provider = DefaultTraceProvider() | ||
| GLOBAL_TRACE_PROVIDER = provider | ||
| provider.register_processor(default_processor()) | ||
| atexit.register(provider.shutdown) |
There was a problem hiding this comment.
get_trace_provider() uses an unsynchronized check-then-set on GLOBAL_TRACE_PROVIDER. Two threads hitting first-use concurrently can each create a DefaultTraceProvider, register processors, and register separate atexit shutdown callbacks; one instance is then overwritten. This can return different providers to callers and cause duplicate shutdown/export behavior under concurrent startup.
Useful? React with 👍 / 👎.
| if _global_exporter is None: | ||
| _global_exporter = BackendSpanExporter() | ||
| return _global_exporter | ||
|
|
||
|
|
||
| def default_processor() -> BatchTraceProcessor: | ||
| """The default processor, which exports traces and spans to the backend in batches.""" | ||
| global _global_processor | ||
| if _global_processor is None: | ||
| _global_processor = BatchTraceProcessor(default_exporter()) |
There was a problem hiding this comment.
default_exporter() and default_processor() lazily initialize globals without synchronization. Concurrent first calls can allocate multiple BackendSpanExporter / BatchTraceProcessor instances. Extra exporters create their own httpx.Client objects and may not be explicitly closed, leading to avoidable resource usage and inconsistent singleton expectations.
Useful? React with 👍 / 👎.
|
Closing this in favor of #2499 |
2 participants
Summary
Fixes #2489 by removing import-time tracing initialization side effects.
This change makes default tracing objects lazy:
BackendSpanExporteris now created only when first needed.BatchTraceProcessoris now created only when first needed.TraceProvideris initialized on firstget_trace_provider()access instead of at module import.This avoids constructing
httpx.Clientand thread primitives during import, which is safer for fork-based runtimes and pre-fork server startup paths.Changes
src/agents/tracing/processors.pysrc/agents/tracing/__init__.pyset_trace_provider(...),add_trace_processor(...), andatexit.register(...)side effects.tests/tracing/test_lazy_tracing_init.pyValidation
uv run --with ruff ruff check src/agents/tracing/__init__.py src/agents/tracing/processors.py tests/tracing/test_lazy_tracing_init.pyenv -u ALL_PROXY -u all_proxy -u HTTPS_PROXY -u https_proxy -u HTTP_PROXY -u http_proxy -u NO_PROXY -u no_proxy uv run --with pytest pytest -q tests/tracing/test_lazy_tracing_init.py tests/test_trace_processor.py(22 passed)