fix(telemetry): treat QueueShutDown gracefully, not an error#1075
fix(telemetry): treat QueueShutDown gracefully, not an error#1075sokoliva wants to merge 3 commits into
Conversation
🧪 Code Coverage (vs
|
| Base | PR | Delta | |
|---|---|---|---|
| src/a2a/utils/telemetry.py | 90.63% | 90.77% | 🟢 +0.14% |
| Total | 93.07% | 93.07% | ⚪️ 0.00% |
Generated by coverage-comment.yml
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the telemetry utility to gracefully handle QueueShutDown exceptions, ensuring they are recorded on spans without marking them as errors, and includes compatibility logic for Python 3.13. Feedback was provided regarding the test suite, noting that the lazy import helper is currently executed during the pytest collection phase, which defeats its intended purpose; using a fixture or dynamic parametrization was suggested as a solution.
Inline the parametrize list at the decorator and import QueueShutDown at the top of the test module. Drops the misleading "lazy import" helper flagged by code review (it ran at collection time anyway).
| if sys.version_info >= (3, 13): | ||
| from asyncio import QueueShutDown as _QueueShutDown | ||
| else: | ||
| from culsans import AsyncQueueShutDown as _QueueShutDown | ||
|
|
||
| _GRACEFUL_ASYNC_EXCEPTIONS: tuple[type[BaseException], ...] = ( | ||
| asyncio.CancelledError, | ||
| _QueueShutDown, | ||
| ) |
There was a problem hiding this comment.
Given that we have the same check in event_queue.py wdyt about extracting it in an internal utility module (i.e. _async_queue_compat.py) and share it between two?
| # Graceful end-of-operation signals (currently asyncio.CancelledError, | ||
| # QueueShutDown). |
There was a problem hiding this comment.
nit: I'd say this comment is not needed here and the list itself should be used as a source of truth (as we don't want to update this comment for each new item added into the list)
| except asyncio.CancelledError as ce: | ||
| # Graceful end-of-operation signals (currently asyncio.CancelledError, | ||
| # QueueShutDown). | ||
| except _GRACEFUL_ASYNC_EXCEPTIONS as ge: |
2 participants
Description
Summary
trace_function's async wrapper marks any exception other thanCancelledErrorasStatusCode.ERROR. This wrongly flagsQueueShutDown, the normal signal raised when a producer cleanly closes an event queue, as a failure.Change
QueueShutDownto the same graceful-exception allowlist asCancelledErrorinsrc/a2a/utils/telemetry.py. The class is resolved per Python version (stdlib on 3.13+,culsansback-port on <3.13), mirroringevent_queue.py.CancelledErrorandQueueShutDown.Fixes #1065🦕