fix(observability): isolate SSE stream spans and continue ingress traceparent - #390
Open
cyntwang99 wants to merge 1 commit into
Open
Conversation
…ceparent Each task-event stream now runs under its own OpenTelemetry span, parented only on the inbound W3C traceparent (or a fresh root when absent) and never the ambient context. The SSE body is pumped by the ASGI server after the request handler returns, so the ambient context could still carry a prior request's span; inheriting it made a stream's logs and child spans resolve to an unrelated, long-lived trace (cross-request context bleed). The span carries open/first-event/close lifecycle events and the task.id / stream.outcome / disconnect.reason attributes, and stays attached for the generator's lifetime so log lines correlate to the correct trace. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
| span.add_event("first-event") | ||
|
|
||
| span.add_event("open") | ||
| try: |
There was a problem hiding this comment.
Setup failures escape the SSE stream
When task lookup, Redis tail lookup, or the initial status read fails before the inner handler begins, the exception now exits the generator instead of producing the established SSE error event. The response wrapper then raises StreamResponseError, so clients receive a broken stream and a generic 500 rather than the stream error frame.
Prompt To Fix With AI
This is a comment left during a code review.
Path: agentex/src/domain/use_cases/streams_use_case.py
Line: 151
Comment:
**Setup failures escape the SSE stream**
When task lookup, Redis tail lookup, or the initial status read fails before the inner handler begins, the exception now exits the generator instead of producing the established SSE error event. The response wrapper then raises `StreamResponseError`, so clients receive a broken stream and a generic 500 rather than the stream error frame.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The SSE task-event stream reused the ambient OpenTelemetry context across requests. Because the SSE body is pumped by the ASGI server long after the request handler returns, the ambient context could still carry a previous request's span — so a stream's logs (
otelTraceID) and any child spans resolved to an unrelated, long-lived trace (cross-request context bleed).This change makes each stream self-contained:
stream_task_eventscall runs under its own span, parented only on the inbound W3Ctraceparent(viaextract(carrier, context=Context())) or a fresh root when absent — never the ambient context. The emptyContext()base is the isolation mechanism.traceparentthe stream span continues that trace as a child instead of starting a new root.open/first-event/closeevents andtask.id/stream.outcome/disconnect.reasonattributes, and stays attached for the generator's lifetime so log lines correlate to the correct trace. Afinallyblock classifies the in-flight exception (GeneratorExit/CancelledError→ client disconnect) so the ASGI disconnect path isn't mislabeledcompleted.Existing SSE behavior (keepalive pings, backoff, terminal detection, shared-topic non-deletion) is unchanged.
Changes
src/domain/use_cases/streams_use_case.py— module tracer + span lifecycle around the stream generator;carrierparam.src/api/routes/tasks.py— both stream routes injectRequestand passcarrier=dict(request.headers).tests/unit/use_cases/test_streams_use_case_tracing.py(new) — 7 tests covering isolation-from-ambient, traceparent continuation, ingress-wins-over-ambient, lifecycle event order/attributes, and clean context detach.Test plan
test_task_stream.py) — could not run locally (testcontainers/Docker env issue); calls are signature-compatible with the newcarrier=Nonedefault/streamtrace is a self-contained root without atraceparentheader, and a child of the caller when one is supplied🤖 Generated with Claude Code
Greptile Summary
Introduces isolated lifecycle tracing for task-event SSE streams.
Confidence Score: 4/5
The pre-loop SSE failure path needs to be fixed before merging because task or Redis lookup errors now terminate the response without an SSE error event.
Initial task resolution, stream-tail lookup, and status reads execute outside the exception handler that serializes stream failures, allowing those errors to escape through the response wrapper as a generic server failure.
Files Needing Attention: agentex/src/domain/use_cases/streams_use_case.py
Important Files Changed
Sequence Diagram
sequenceDiagram participant Client participant Route as Task stream route participant Stream as StreamsUseCase participant OTel as OpenTelemetry participant Repo as Stream repository Client->>Route: GET task event stream\noptional traceparent Route->>Stream: "stream_task_events(carrier=headers)" Stream->>OTel: Extract parent from isolated Context Stream->>OTel: Start and attach stream span Stream->>Repo: Read task events loop Stream lifetime Repo-->>Stream: Event or idle cycle Stream-->>Client: SSE event or keepalive end Stream->>OTel: Record outcome and close event Stream->>OTel: End span and detach contextPrompt To Fix All With AI
Reviews (1): Last reviewed commit: "fix(observability): isolate SSE stream s..." | Re-trigger Greptile