Skip to content

fix(tracing): Fix unsampled/deferred trace propagation in span streaming#6757

Open
sentrivana wants to merge 24 commits into
masterfrom
ivana/traceparent-test
Open

fix(tracing): Fix unsampled/deferred trace propagation in span streaming#6757
sentrivana wants to merge 24 commits into
masterfrom
ivana/traceparent-test

Conversation

@sentrivana

@sentrivana sentrivana commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Description

The SDK didn't behave correctly in some trace propagation scenarios in stream mode.

The current SDK design was emitting headers with a deferred sampling decision if the sampling decision was negative (whether inherited or not), because it was falling back to TwP (tracing without performance) behavior too eagerly.

Additionally, some edge cases concerning ignore_spans and its interaction with sampling were clarified and the necessary changes included in this PR.

How it should work

In the following, tracing enabled means either a traces_sample_rate or a traces_sampler is defined. Tracing disabled means both are None and tracing without performance is active.

There are three kinds of sampling decisions: sampled (1), unsampled (0), and deferred (``), meaning no decision.

  1. No incoming trace headers
    • tracing enabled: make a sampling decision and propagate it
    • tracing disabled: do not make a decision, propagate a deferred decision (no sampling decision)
  2. Incoming trace headers with a sampled/unsampled decision
    • tracing enabled: respect and propagate incoming decision
    • tracing disabled: respect and propagate incoming decision
  3. Incoming trace headers with a deferred sampling decision (essentially same as 1)
    • tracing enabled: make a sampling decision and propagate it
    • tracing disabled: propagate deferred sampling decision

The problem and the fix

If a no-op span was active, we were falling back to the propagation context to generate propagation headers, which by default doesn't contain a sampling decision.

With this PR, NoOpStreamedSpans now carry their own trace and span ID information as well as the current propagation data (a snapshot of the propagation context, like StreamedSpans) and use it to emit the correct headers. This mirrors old behavior: spans would always be created, whether they'd be sampled or not, and they would be responsible for generating propagation headers; with the fallback to the propagation context only kicking in if there is no span.

I recommend checking out the new tests to see what behavior is now expected.

Edge cases

The following cases regarding ignore_spans were clarified and will be documented soon:

  • An outgoing request is made from an ignored segment: sentry-trace should have an unsampled decision (-0), parent_span_id is the ID of the segment (or any random span ID, since for all intents and purposes, the ignored segment doesn't actually exist for any downstream service)
  • An outgoing request is made from an ignored child span of a non-ignored segment: sentry-trace's sampling decision should be the segment's sampling decision; parent_span_id should be the first non-ignored span ancestor

Issues

Closes #6755

Reminders

@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Codecov Results 📊

91200 passed | ⏭️ 6302 skipped | Total: 97502 | Pass Rate: 93.54% | Execution Time: 312m 25s

📊 Comparison with Base Branch

Metric Change
Total Tests 📈 +180
Passed Tests 📈 +180
Failed Tests
Skipped Tests

All tests are passing successfully.

✅ Patch coverage is 97.22%. Project has 2444 uncovered lines.
✅ Project coverage is 89.77%. Comparing base (base) to head (head).

Files with missing lines (1)
File Patch % Lines
sentry_sdk/traces.py 93.75% ⚠️ 1 Missing and 1 partials
Coverage diff
@@            Coverage Diff             @@
##          main       #PR       +/-##
==========================================
+ Coverage    89.76%    89.77%    +0.01%
==========================================
  Files          193       193         —
  Lines        23888     23898       +10
  Branches      8250      8256        +6
==========================================
+ Hits         21442     21454       +12
- Misses        2446      2444        -2
- Partials      1353      1352        -1

Generated by Codecov Action

Comment thread sentry_sdk/scope.py
Comment thread sentry_sdk/tracing_utils.py Outdated
Comment thread sentry_sdk/tracing_utils.py Outdated
Comment thread tests/tracing/test_span_streaming.py
@sentrivana sentrivana marked this pull request as ready for review July 6, 2026 12:01
@sentrivana sentrivana requested a review from a team as a code owner July 6, 2026 12:01
Comment thread sentry_sdk/traces.py

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Fix All in Cursor

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

Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 46b410a. Configure here.

Comment thread sentry_sdk/scope.py
@sentrivana sentrivana marked this pull request as draft July 6, 2026 12:09
@sentrivana

Copy link
Copy Markdown
Contributor Author

Converting to draft to address bot comments

Comment thread sentry_sdk/tracing_utils.py
Comment thread sentry_sdk/traces.py Outdated
@sentrivana sentrivana marked this pull request as ready for review July 10, 2026 11:34
Comment thread sentry_sdk/traces.py

@ericapisani ericapisani left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haven't had a chance to look at the tests yet, but noticed a couple things on my first pass

Comment thread sentry_sdk/scope.py
self._update_sample_rate(sample_rate)

if sampled is False:
if sampled is False or sampled is None:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can also be written as to remove the extra conditional:

Suggested change
if sampled is False or sampled is None:
if bool(sampled):

Comment thread sentry_sdk/traces.py
Comment on lines +613 to +616
"_trace_id",
"_span_id",
"_sampled",
"_segment",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of these, only _sampled needs to be added as the other 3 already exist on the parent 's __slots__ and will still be accessible by instances of NoOpStreamedSpan.

As an example:

>>> class Foo():
...     __slots__ = ("x")
...
>>> class Bar(Foo):
...     __slots__ = ("y")
...
>>> b = Bar()
>>> b.x = "hi"
>>> b.y = "world"
>>> print(b.x)
hi
>>> b.z
Traceback (most recent call last):
  File "<python-input-7>", line 1, in <module>
    b.z
AttributeError: 'Bar' object has no attribute 'z'

One thing to be wary of that I found when looking into the slots docs - duplicating the variable name in __slots__ as shown here creates a shadow of the parent value.

@ericapisani ericapisani left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than the slots thing that I mentioned earlier, this LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sampling decision is deferred when it should be unsampled

2 participants