feat(integrations): openai-agents streaming support #5291
Conversation
Semver Impact of This PR🟡 Minor (new features) 📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨
Internal Changes 🔧
🤖 This preview updates automatically when you update the PR. |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
… streaming events - Introduced a `_close_workflow_span` function to ensure proper closure of workflow spans. - Wrapped `stream_events` and `cancel` methods in `run_result` to manage workflow spans during their execution. - This change improves resource management and error handling in the OpenAI agent integration.
alexander-alderman-webb
left a comment
alexander-alderman-webb
left a comment
There was a problem hiding this comment.
looks good overall, only a few comments
| except Exception: | ||
| _close_streaming_workflow_span(agent) | ||
| raise |
There was a problem hiding this comment.
why do we need to close the workflow span here?
my understanding is that a new agent takes over with a handoff, so terminating the agent in finally would make sense?
let me know if I am missing anything.
There was a problem hiding this comment.
As far as I can tell this is the correct handling. The failed handoff will cause the workflow to stop, but a successful handoff should not terminate the workflow span
There was a problem hiding this comment.
okay I understand, I was confusing workflow and agent invocation spans 😢
can you check if it is possible to to use the context manager in a with statement in run_streamed as well? I know we catch exceptions at various levels to close the workflow span, but these exceptions should bubble up to the top-level run_streamed if I have the correct mental model. Something like
with agent_workflow_span(agent):
...
instead of all the manual management 🙏 .
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 5 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| # If run_streamed itself fails (not the background task), clean up immediately | ||
| workflow_span.__exit__(*sys.exc_info()) | ||
| _capture_exception(exc) | ||
| raise exc from None |
There was a problem hiding this comment.
Exception handling missing capture_internal_exceptions wrapper
Medium Severity
The _capture_exception(exc) call isn't wrapped in capture_internal_exceptions(), unlike the consistent pattern used elsewhere in the codebase (lines 43, 61). If _capture_exception raises an exception, it will replace the user's original exception, potentially hiding the root cause of the failure.
| # If run_streamed itself fails (not the background task), clean up immediately | ||
| workflow_span.__exit__(*sys.exc_info()) | ||
| _capture_exception(exc) | ||
| raise exc from None |
There was a problem hiding this comment.
Inconsistent exception reraising loses traceback information
Low Severity
Using raise exc from None suppresses the exception chain and doesn't preserve the full traceback, while the non-streaming version uses reraise(*exc_info) to maintain traceback integrity. This inconsistency makes debugging streaming failures more difficult compared to non-streaming failures.
|
|
||
| except Exception: | ||
| _close_streaming_workflow_span(agent) | ||
| raise |
There was a problem hiding this comment.
Missing exception safety wrapper in handoff error handling
High Severity
The _close_streaming_workflow_span(agent) call in the except block isn't wrapped in capture_internal_exceptions(), unlike the identical pattern at line 226. If closing the workflow span fails, it will replace the original handoff exception, hiding the root cause. The PR discussion explicitly flagged this issue and marked it as "Fixed", but the fix wasn't applied here.
| if agent and context_wrapper and _has_active_agent_span(context_wrapper): | ||
| end_invoke_agent_span(context_wrapper, agent, final_output) | ||
| # For streaming, close the workflow span (non-streaming uses context manager in _create_run_wrapper) | ||
| _close_streaming_workflow_span(agent) |
There was a problem hiding this comment.
Missing exception safety in final output cleanup
Medium Severity
The _close_streaming_workflow_span(agent) call in the finally block isn't wrapped in capture_internal_exceptions(). If span cleanup fails during exception handling (e.g., when original_execute_final_output at line 164 raises), the cleanup exception will replace the original exception, hiding the actual failure cause.
Description
Adds instrumentation for
Runner.run_streamedIssues
Closes https://linear.app/getsentry/issue/TET-1705/openai-agents-streaming-support