ref: Introduce inline type check for whether a span is StreamedSpan by ericapisani · Pull Request #6180 · getsentry/sentry-python

ericapisani · 2026-05-01T16:10:19Z

The isinstance(span, StreamedSpan) and not isinstance(span, NoOpStreamedSpan) pattern was repeated in multiple places. Omitting the NoOpStreamedSpan guard is a frequent code review catch — it silently treats unsampled spans as sampled.

Introduce _is_streamed_span() in traces.py with a TypeGuard[StreamedSpan] return type for proper type narrowing, and replace all combined-check instances in scope.py, starlette.py, and fastapi.py.

Locations where _span can also be a regular Span (not just StreamedSpan) were intentionally left unchanged, as the semantics differ there.

Fixes PY-2397
Fixes #6182

…ed check The `isinstance(span, StreamedSpan) and not isinstance(span, NoOpStreamedSpan)` pattern was scattered across the codebase and easy to get wrong — forgetting the `NoOpStreamedSpan` guard is a recurring code review catch. Extract it into a single `_is_sampled_streamed_span()` helper in `traces.py` with a `TypeGuard[StreamedSpan]` return type for proper type narrowing, and replace all instances of the combined check in `scope.py`, `starlette.py`, and `fastapi.py`.

ericapisani · 2026-05-01T16:10:35Z

bugbot run

ericapisani · 2026-05-01T16:10:41Z

@sentry review

github-actions · 2026-05-01T16:11:56Z

Codecov Results 📊

✅ 13 passed | Total: 13 | Pass Rate: 100% | Execution Time: 8.90s

All tests are passing successfully.

❌ Patch coverage is 75.00%. Project has 15002 uncovered lines.

Files with missing lines (4)

File	Patch %	Lines
`starlette.py`	5.84%	⚠️ 371 Missing
`scope.py`	71.73%	⚠️ 253 Missing and 84 partials
`traces.py`	69.28%	⚠️ 94 Missing and 21 partials
`fastapi.py`	15.96%	⚠️ 79 Missing

Generated by Codecov Action

cursor

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 10e96aa. Configure here.}

linear-code · 2026-05-01T16:30:54Z

PY-2397 Create helper method to replace frequent "is streamed span but not a noop streamed span" check

alexander-alderman-webb

It looks like in blocks gated by not isinstance(current_span, NoOpStreamedSpan) we rely on private attributes and functions of StreamedSpan such as _segment and _is_segment().

If we're relying on private details that we expect to only exist on StreamedSpan (and not subclasses) it would be more appropriate to replace logic like

isinstance(span, StreamedSpan) and not isinstance(span, NoOpStreamedSpan)

with

type(span) is StreamedSpan

sentrivana

Wondering if it'd be better to replace

isinstance(span, StreamedSpan) and not isinstance(span, NoOpStreamedSpan)

with

isinstance(span, StreamedSpan) and span.sampled

But I assume that might cause mypy to complain about nonexistent methods on NoOpStreamedSpans since it won't be able to realize the condition effectively means it can't be a NoOpStreamedSpan?

Edit: Only saw Alex's comment above now. That also sounds good.

ericapisani · 2026-05-04T12:43:52Z

Re: using the type approach rather than isinstance (i.e.: type(span) is StreamedSpan)

I think it depends on how strict we want to be with ensuring that the only spans that pass this are StreamedSpan classes specifically.

I wrote this check with isinstance in case there are additional children introduced in the future of StreamedSpan that we would want to handle similar to their parent, and that we ultimately didn't want to filter out with this guard - using type would unfortunately cause that to happen.

Re: using the sampled property:

But I assume that might cause mypy to complain about nonexistent methods on NoOpStreamedSpans since it won't be able to realize the condition effectively means it can't be a NoOpStreamedSpan?

We would achieve the same outcome using the sampled property as is written here because it exists on both the StreamedSpan and the NoOpStreamedSpan, but I think it doesn't make the conditional as explicitly clear that it's more about removing any instances of NoOpStreamedSpan due to the reason that you mentioned in that they lack the methods that StreamedSpan has.

alexander-alderman-webb · 2026-05-04T14:18:27Z

I think it depends on how strict we want to be with ensuring that the only spans that pass this are StreamedSpan classes specifically.

The spans touched by the filtering logic are created by the SDK, so we know that they are of type StreamedSpan and not a subclass other than NoOpStreamedSpan. If we introduce other subclasses in the future, I think we have full flexibility to change 😄.

ericapisani · 2026-05-05T11:57:13Z

@alexander-alderman-webb @sentrivana Made the change in 203c816

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 203c816. Configure here.}

alexander-alderman-webb · 2026-05-05T15:13:23Z

-                    if isinstance(current_span, StreamedSpan) and not isinstance(
-                        current_span, NoOpStreamedSpan
-                    ):
+                    if _is_streamed_span(current_span):


Let's inline the method now since it's only one condition. Removes one indirection.

Suggested change

if _is_streamed_span(current_span):

if type(span) is StreamedSpan:

alexander-alderman-webb · 2026-05-05T15:26:51Z

-        and isinstance(current_span, StreamedSpan)
-        and not isinstance(current_span, NoOpStreamedSpan)
-    ):
+    if info and "data" in info and _is_streamed_span(current_span):


Suggested change

if info and "data" in info and _is_streamed_span(current_span):

if info and "data" in info and type(current_span) is StreamedSpan:

alexander-alderman-webb · 2026-05-05T15:27:14Z

-                    if isinstance(current_span, StreamedSpan) and not isinstance(
-                        current_span, NoOpStreamedSpan
-                    ):
+                    if _is_streamed_span(current_span):


Suggested change

if _is_streamed_span(current_span):

if type(current_span) is StreamedSpan:

alexander-alderman-webb · 2026-05-05T15:27:47Z

-            and self.streamed_span is not None
-            and not isinstance(self.streamed_span, NoOpStreamedSpan)
-        ):
+        if span_streaming and _is_streamed_span(self.streamed_span):


Suggested change

if span_streaming and _is_streamed_span(self.streamed_span):

if type(self.streamed_span) is StreamedSpan:

alexander-alderman-webb · 2026-05-05T15:27:54Z

-            and self.streamed_span is not None
-            and not isinstance(self.streamed_span, NoOpStreamedSpan)
-        ):
+        if span_streaming and _is_streamed_span(self.streamed_span):


Suggested change

if span_streaming and _is_streamed_span(self.streamed_span):

if type(self.streamed_span) is StreamedSpan:

alexander-alderman-webb · 2026-05-05T15:28:00Z

-            and not isinstance(span, NoOpStreamedSpan)
-            and span._is_segment()
-        ):
+        if _is_streamed_span(span) and span._is_segment():


Suggested change

if _is_streamed_span(span) and span._is_segment():

if type(self.streamed_span) is StreamedSpan:

…ry/sentry-python into create-sampled-span-helper-0doa2

alexander-alderman-webb

Code LGTM! Change the title when you merge 🙏

cursor Bot reviewed May 1, 2026

View reviewed changes

ericapisani marked this pull request as ready for review May 1, 2026 16:16

ericapisani requested a review from a team as a code owner May 1, 2026 16:16

alexander-alderman-webb reviewed May 4, 2026

View reviewed changes

sentrivana reviewed May 4, 2026

View reviewed changes

use type method instead of isinstance

203c816

cursor Bot reviewed May 5, 2026

View reviewed changes

Comment thread sentry_sdk/traces.py Outdated

ericapisani changed the title ~~ref: Introduce _is_sampled_streamed_span() helper to replace repeated check~~ ref: Introduce _is_streamed_span() helper to replace repeated check May 5, 2026

Merge branch 'master' into create-sampled-span-helper-0doa2

d89fcbd

alexander-alderman-webb requested changes May 5, 2026

View reviewed changes

ericapisani added 2 commits May 5, 2026 11:58

inline all the things

701bc00

Merge branch 'create-sampled-span-helper-0doa2' of github.com:getsent…

97bd86e

…ry/sentry-python into create-sampled-span-helper-0doa2

alexander-alderman-webb approved these changes May 5, 2026

View reviewed changes

ericapisani changed the title ~~ref: Introduce _is_streamed_span() helper to replace repeated check~~ ref: Introduce inline type check for whether a span is StreamedSpan May 5, 2026

ericapisani merged commit d40313b into master May 5, 2026
156 checks passed

ericapisani deleted the create-sampled-span-helper-0doa2 branch May 5, 2026 16:18

	if _is_streamed_span(current_span):
	if type(span) is StreamedSpan:

	if info and "data" in info and _is_streamed_span(current_span):
	if info and "data" in info and type(current_span) is StreamedSpan:

	if _is_streamed_span(current_span):
	if type(current_span) is StreamedSpan:

	if span_streaming and _is_streamed_span(self.streamed_span):
	if type(self.streamed_span) is StreamedSpan:

	if _is_streamed_span(span) and span._is_segment():
	if type(self.streamed_span) is StreamedSpan:

Conversation

ericapisani commented May 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ericapisani commented May 1, 2026

Uh oh!

ericapisani commented May 1, 2026

Uh oh!

github-actions Bot commented May 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Results 📊

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

linear-code Bot commented May 1, 2026

Uh oh!

alexander-alderman-webb left a comment

Choose a reason for hiding this comment

Uh oh!

sentrivana left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ericapisani commented May 4, 2026

Uh oh!

alexander-alderman-webb commented May 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ericapisani commented May 5, 2026

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

alexander-alderman-webb May 5, 2026

Choose a reason for hiding this comment

Uh oh!

alexander-alderman-webb May 5, 2026

Choose a reason for hiding this comment

Uh oh!

alexander-alderman-webb May 5, 2026

Choose a reason for hiding this comment

Uh oh!

alexander-alderman-webb May 5, 2026

Choose a reason for hiding this comment

Uh oh!

alexander-alderman-webb May 5, 2026

Choose a reason for hiding this comment

Uh oh!

alexander-alderman-webb May 5, 2026

Choose a reason for hiding this comment

Uh oh!

alexander-alderman-webb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ericapisani commented May 1, 2026 •

edited

Loading

github-actions Bot commented May 1, 2026 •

edited

Loading

sentrivana left a comment •

edited

Loading

alexander-alderman-webb commented May 4, 2026 •

edited

Loading