fix(observability): isolate SSE stream spans and continue ingress traceparent - #390
Merged
cyntwang99 merged 6 commits intoAug 5, 2026
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>
Merge the stream generator's two try blocks into one so the fatal
except handlers cover the setup phase (task resolution, stream tail
read, initial status read, terminal replay) as well as the read loop.
Previously a setup failure escaped the generator instead of producing
the established SSE error frame; the streaming response wrapper then
raised StreamResponseError, so clients saw a broken stream. Now such a
failure yields a `data: {"type":"error",...}` frame, ends the stream
cleanly, and marks the span stream.outcome=error / disconnect.reason
with the exception type.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Reconcile the SSE stream-lifecycle metrics from main with the per-stream tracing on this branch. Both refactored stream_task_events and unified its try block, so the conflict in streams_use_case.py is resolved by sharing a single StreamOutcome value across the close metric and the span's stream.outcome attribute, keeping disconnect.reason as span-only detail. Task resolution stays inside the try (setup failures yield an SSE error frame and mark the span errored); the opened metric is recorded only after resolution. Test fixtures gain SSE_STREAM_STALL_THRESHOLD_SECONDS. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…eed-fix-per-stream-span-ingress
harvhan
approved these changes
Aug 5, 2026
| @@ -0,0 +1,238 @@ | |||
| """Tracing tests for the SSE task-event stream. | |||
|
|
|||
| These lock in the AGX1-617 fixes: | |||
Contributor
There was a problem hiding this comment.
This docstring references an internal ticket ID (line 3). This repo is public, so internal tracker IDs shouldn't land in anything that gets pushed, per the repo guidelines in CLAUDE.md. Suggest rewording to something like "These lock in the SSE trace context bleed fixes:". A follow up commit plus the eventual squash merge keeps main clean; if you want it fully scrubbed from the branch history too, amend and force push with lease.
| assert span.attributes["stream.outcome"] == "client_disconnect" | ||
| assert span.attributes["disconnect.reason"] == "client_disconnect" | ||
|
|
||
| async def test_span_ends_exactly_once_and_detaches_context(self, span_exporter): |
Contributor
There was a problem hiding this comment.
nit: the test name promises the span ends exactly once, but the body only asserts the context was detached. Adding a _only_stream_span(span_exporter) call here would cover the other half.
Remove an internal tracker ID from the tracing test docstring (this repo is public) and assert exactly one stream span is finished in the context-detach test so it matches its name. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…eed-fix-per-stream-span-ingress
cyntwang99
deleted the
cynthiawang/agx1-617-2b-sse-trace-context-bleed-fix-per-stream-span-ingress
branch
August 5, 2026 18:50
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.
Testing
Local trace validation - Exercised the real stream_task_events code against a local OpenTelemetry collector to confirm the per-stream span is emitted with the right lineage and lifecycle.
What was run
What it shows — two stream task events spans (Kind: Server):
The context-isolation behavior (a stream must not inherit a leftover ambient span from an unrelated request) can't be reproduced by this out-of-process harness and is covered by the
unit tests in tests/unit/use_cases/test_streams_use_case_tracing.py.
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
The PR isolates each SSE task-event stream in its own OpenTelemetry span and continues valid inbound W3C trace context.
Confidence Score: 5/5
The PR appears safe to merge.
The previously reported setup failures are now handled inside the stream recovery boundary and produce an SSE error frame, with no blocking failure remaining.
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 Task/stream repositories Client->>Route: "GET /tasks/.../stream<br/>optional traceparent" Route->>Stream: "stream_task_events(carrier=headers)" Stream->>OTel: Extract using isolated Context Stream->>OTel: Start and attach per-stream span Stream->>Repo: Resolve task and stream cursor alt Setup succeeds Stream-->>Client: connected and event frames else Setup fails Stream->>OTel: Record exception and error status Stream-->>Client: SSE error frame end Stream->>OTel: Add close metadata, end span, detach contextReviews (5): Last reviewed commit: "Merge branch 'main' into cynthiawang/agx..." | Re-trigger Greptile