fix(tracing): avoid busy-polling in BatchTraceProcessor - #4703
fix(tracing): avoid busy-polling in BatchTraceProcessor#4703mikemikimike wants to merge 1 commit into
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: 8df9c0db6e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| time.sleep(0.2) | ||
| # Wait until the next scheduled export or shutdown, whichever comes first. | ||
| timeout = max(0.0, min(0.2, self._next_export_time - time.monotonic())) | ||
| self._shutdown_event.wait(timeout) |
There was a problem hiding this comment.
When shutdown(timeout=...) is called while the worker is waiting with queued work below the trigger threshold, setting _shutdown_event wakes this wait before shutdown() assigns _shutdown_deadline. The worker can therefore enter the final drain with deadline=None and call BackendSpanExporter.export() without the requested deadline; a stalled HTTP request may then keep the worker alive for the client's normal 60-second timeout. Assign the deadline before signaling the event so the awakened worker always observes the finite shutdown budget.
AGENTS.md reference: AGENTS.md:L149-L149
Useful? React with 👍 / 👎.
| # Sleep a short interval so we don't busy-wait. | ||
| time.sleep(0.2) | ||
| # Wait until the next scheduled export or shutdown, whichever comes first. | ||
| timeout = max(0.0, min(0.2, self._next_export_time - time.monotonic())) |
There was a problem hiding this comment.
With the default schedule_delay=5.0 and a queue below the export threshold, this timeout remains capped at 0.2 seconds, so Event.wait() expires and the worker rechecks the queue five times per second even when completely idle—exactly the same wakeup cadence as the replaced time.sleep(0.2). This improves shutdown latency but does not remove the idle polling targeted by the change; the worker needs an enqueue/shutdown notification or a blocking queue wait so it can sleep until actual work or the scheduled export.
Useful? React with 👍 / 👎.
|
Please refer to #4688 (comment) |
2 participants
Fixes #4688
nnnBatchTraceProcessor now waits on its shutdown event until the next scheduled export instead of polling with time.sleep(0.2). This removes idle wakeups while preserving the existing 200 ms queue-trigger responsiveness and makes shutdown interrupt the wait immediately.nnTests:n- uv run --frozen pytest tests/test_trace_processor.py::test_batch_trace_processor_shutdown_interrupts_idle_worker -vvn- uv run --frozen ruff format --check src/agents/tracing/processors.py tests/test_trace_processor.pyn- uv run --frozen ruff check src/agents/tracing/processors.py tests/test_trace_processor.pyn- uv run --frozen mypy src/agents/tracing/processors.pyn- git diff --checknnThe full test file was attempted but hung after 13 tests on Windows and was stopped.