Conversation
Add span-streaming support to the Starlette integration so middleware spans and active-thread tracking work under the `trace_lifecycle: "stream"` experiment, while preserving the legacy transaction-based behavior. When span streaming is enabled, `_enable_span_for_middleware` starts middleware spans via `sentry_sdk.traces.start_span` with `sentry.op`, `sentry.origin`, and `starlette.middleware_name` attributes instead of the legacy `start_span(op=..., origin=...)` + tag pattern. In `patch_request_response`, when the current scope holds a `StreamedSpan` (and not a `NoOpStreamedSpan`), the profiler hook now calls `_segment._update_active_thread()`; otherwise the legacy `current_scope.transaction.update_active_thread()` path is preserved. Tests are parametrized across streaming and static modes for `test_middleware_spans`, `test_middleware_spans_disabled`, `test_middleware_callback_spans`, and `test_span_origin`. A new `test_active_thread_id_span_streaming` verifies the segment's `thread.id` attribute under streaming. `auto_enabling_integrations` is disabled in tests where auto-instrumented spans would leak into the captured span stream. Refs PY-2362 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Codecov Results 📊✅ 13 passed | Total: 13 | Pass Rate: 100% | Execution Time: 8.75s All tests are passing successfully. ❌ Patch coverage is 9.09%. Project has 14790 uncovered lines. Files with missing lines (1)
Generated by Codecov Action |
sentrivana
left a comment
There was a problem hiding this comment.
Nice, left a few comments -- we'll need to migrate the event processors as well.
| attributes={ | ||
| "sentry.op": op, | ||
| "sentry.origin": StarletteIntegration.origin, | ||
| "starlette.middleware_name": middleware_name, |
There was a problem hiding this comment.
starlette.middleware_name is not in Sentry semantic conventions -- anything we set as an attribute on a streamed span needs to be there. So we either need to add it or not set it at all.
In this case, if I read the code right, it seems to be the same as the span name? In that case I'd omit the attribute. If we want to keep it, I'd propose a new attribute name in Sentry conventions that's not tied to Starlette specifically, so that we can use it across different frameworks and languages.
| sentry_scope.add_event_processor( | ||
| _make_request_event_processor(request, integration) | ||
| ) |
There was a problem hiding this comment.
We're adding an event processor here. In legacy mode, it would enrich any transaction event with additional data. Since streamed spans are not events, it'll not apply, so anytime there's an event processor in an integration, we need to come up with an equivalent way to apply the data to streaming segments as well.
The gist is basically:
- Leave the event processor in place since it should continue working for other event types, like errors, as well, in the future.
- The span streaming version of this will basically be us setting equivalent attributes (which need to be in conventions) on the segment span.
- Event processors are normally run at the very end of the transaction lifecycle. Depending on what data they set, the span streaming equivalent might also need to be run towards the end of the segment's lifetime -- it depends on whether any of the data we set can be expected to be populated only later (like, for instance, the response status code).
Have a look at ASGI for prior art.
Ideally, this would've popped up in the tests. Looks like we're not really testing that the event processor applies the correct data, since the tests are passing even without it. That's not great. But we can also address that at a later point since it's out of scope of this PR.
| sentry_scope.add_event_processor( | ||
| _make_request_event_processor(request, integration) | ||
| ) |
Bring the Starlette integration to span-streaming parity with the FastAPI change that landed in #6118.
Under the
trace_lifecycle: "stream"experiment the scope'stransactionisNoneand the scope instead holds aStreamedSpan, so the existing middleware instrumentation (which calledstart_span(op=..., origin=...) + set_tag) and the profiler'scurrent_scope.transaction.update_active_thread()hook inpatch_request_responsedidn't function under streaming. This PR routes both through the streaming APIs when streaming is enabled while leaving the legacy transaction-based path untouched:_enable_span_for_middlewarenow uses a_start_middleware_spanhelper that callssentry_sdk.traces.start_spanwithsentry.op,sentry.origin, andstarlette.middleware_nameattributes under streaming, and the existingsentry_sdk.start_span(op=..., origin=...) + set_tagotherwise.StreamedSpan(excludingNoOpStreamedSpan) and calls_segment._update_active_thread(); otherwise it takes the legacycurrent_scope.transaction.update_active_thread()branch.Tests are parametrized across streaming and static modes for
test_middleware_spans,test_middleware_spans_disabled,test_middleware_callback_spans, andtest_span_origin. A newtest_active_thread_id_span_streamingcovers the segment'sthread.idattribute under streaming.auto_enabling_integrations=Falseis set in streaming tests where auto-instrumented spans would otherwise leak into the captured span stream.Refs PY-2362