Skip to content

chore(core): Implement disabled tracing semantics - #1286

Open
szokeasaurusrex wants to merge 3 commits into
szokeasaurusrex/chore/deprecate-is-sampledfrom
szokeasaurusrex/fix/fully-distinguish-disabled-tracing
Open

chore(core): Implement disabled tracing semantics#1286
szokeasaurusrex wants to merge 3 commits into
szokeasaurusrex/chore/deprecate-is-sampledfrom
szokeasaurusrex/fix/fully-distinguish-disabled-tracing

Conversation

@szokeasaurusrex

@szokeasaurusrex szokeasaurusrex commented Aug 10, 2026

Copy link
Copy Markdown
Member

PR #1227 enabled us to represent disabled tracing differently from a 0.0 sample rate when the SDK is initialized but left the behavior for the two cases largely the same.

This change fundamentally alters the semantics around how disabled tracing is handled.

Spans/Transactions now keep track of a so-called TracingState which keeps track of both whether tracing is enabled and what the sampling decision is. When tracing is enabled, there will always be a sampling decision (including a corresponding sample rate); when tracing is disabled, a sampling decision may be available if the trace was continued from an incoming trace with a sampling decision. Traces started in the SDK when tracing is disabled never have a sampling decision.

We then separate the concepts of sampling decision and whether to capture traces. The sampling decision now can correctly represent the case where the decision has been deferred. Span/Transaction tracing headers will always include this sampling decision, regardless of whetehr the SDK is capturing the corresponding the Span/Transaction. For whether to capture traces, we now use a FinishAction with three possibilities: Send and Discard are both used when tracing is enabled and are used for sampled and unsampled traces, respectively, while Ignore is used for all traces when tracing is disabled. Both Discard and Ignore result in spans/transactions not being sent to Sentry, but only Discard sends a client report.

@szokeasaurusrex szokeasaurusrex changed the title ref(core): Correctly represent disabled tracing on spans/transactions chore(core): Represent disabled tracing on spans/transactions Aug 10, 2026
@szokeasaurusrex
szokeasaurusrex force-pushed the szokeasaurusrex/fix/fully-distinguish-disabled-tracing branch from 3287209 to 0392622 Compare August 10, 2026 15:32
@szokeasaurusrex
szokeasaurusrex marked this pull request as ready for review August 10, 2026 15:33
@szokeasaurusrex
szokeasaurusrex requested a review from a team as a code owner August 10, 2026 15:33
@szokeasaurusrex
szokeasaurusrex force-pushed the szokeasaurusrex/fix/fully-distinguish-disabled-tracing branch from 0392622 to 3b583f6 Compare August 10, 2026 15:49
@szokeasaurusrex
szokeasaurusrex marked this pull request as draft August 11, 2026 11:18
@szokeasaurusrex
szokeasaurusrex force-pushed the szokeasaurusrex/fix/fully-distinguish-disabled-tracing branch from 3b583f6 to 08ed195 Compare August 11, 2026 11:26
@szokeasaurusrex
szokeasaurusrex changed the base branch from master to szokeasaurusrex/feat/new-dsc-methods August 11, 2026 11:26
@szokeasaurusrex
szokeasaurusrex force-pushed the szokeasaurusrex/fix/fully-distinguish-disabled-tracing branch from 08ed195 to 9ed110b Compare August 11, 2026 11:43
@szokeasaurusrex szokeasaurusrex changed the title chore(core): Represent disabled tracing on spans/transactions chore(core): Implement disabled tracing semantics Aug 11, 2026
@linear-code

linear-code Bot commented Aug 11, 2026

Copy link
Copy Markdown

RUST-273

@szokeasaurusrex
szokeasaurusrex force-pushed the szokeasaurusrex/fix/fully-distinguish-disabled-tracing branch from 9ed110b to 0c73e73 Compare August 12, 2026 10:11
@szokeasaurusrex
szokeasaurusrex changed the base branch from szokeasaurusrex/feat/new-dsc-methods to szokeasaurusrex/chore/deprecate-is-sampled August 12, 2026 10:11
PR #1227 enabled us to represent disabled tracing differently from a 0.0 sample rate when the SDK is initialized but left the behavior for the two cases largely the same.

This change fundemantally alters the semantics around how disabled tracing is handled.

Spans/Transactions now keep track of a so-called `TracingState` which keeps track of both whether tracing is enabled and what the sampling decision is. When tracing is enabled, there will always be a sampling decision (including a corresponding sample rate); when tracing is disabled, a sampling decision may be available if the trace was continued from an incoming trace with a sampling decision. Traces started in the SDK when tracing is disabled never have a sampling decision.

We then separate the concepts of sampling decision and whether to capture traces. The sampling decision now can correctly represent the case where the decision has been deferred. Span/Transaction tracing headers will always include this sampling decision, regardless of whetehr the SDK is capturing the corresponding the Span/Transaction. For whether to capture traces, we now use a `FinishAction` with three possibilities: `Send` and `Discard` are both used when tracing is enabled and are used for sampled and unsampled traces, respectively, while `Ignore` is used for all traces when tracing is disabled. Both `Discard` and `Ignore` result in spans/transactions not being sent to Sentry, but only `Discard` sends a client report.

Resolves [#1289](#1289)
Resolves [RUST-273](https://linear.app/getsentry/issue/RUST-273)
Resolves [#1282](#1282)
Resolves [RUST-272](https://linear.app/getsentry/issue/RUST-272)
szokeasaurusrex added a commit that referenced this pull request Aug 12, 2026
Deprecate the `is_sampled` methods on `Transaction`, `Span`, and `TransactionOrSpan` because these methods no longer faithfully represent the sampling state of these objects now that the SDK can properly represent the tracing-disabled state. This will be even more true after the follow up PR #1286 is merged; that's because that PR changes these struct's internal `sampled` representation to accurately represent the disabled-tracing states.

This PR also removes `is_sampled` assertions from the trace continuation tests. These assertions are not needed because the tests' purpose is to check trace continuation, not sampling decision propagation. These `is_sampled` checks should probably never have been added there.

We are not adding a replacement for `is_sampled` because a review of code in the `getsentry` org and public GitHub repos did not reveal any usecases of `is_sampled` that could not be replaced with another reasonable existing alternative, e.g. the functions that return the trace propagation headers. In `getsentry`, no usages of these methods could be found outside the SDK itself.
@szokeasaurusrex
szokeasaurusrex force-pushed the szokeasaurusrex/fix/fully-distinguish-disabled-tracing branch from 152813a to ca9bbe7 Compare August 12, 2026 20:30
Comment on lines +995 to +996
.with_sample_rate(sample_rate)
.with_sampled(true);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

If we are sending the transaction, that means that the transaction is sampled.

@szokeasaurusrex

Copy link
Copy Markdown
Member Author

@giortzisg in case you took at this PR earlier, just a heads up that I have fundamentally changed it.

The previous implementation conflated sampling decision and tracing enabled/disabled by assuming that a deferred sampling decision is equivalent to tracing being disabled. This is true when we are the SDK starting the trace, but if we are continuing an incoming trace, we need to be able to correctly represent that tracing is disabled, but that there is still a sampling decision that needs to be propagated outwards.

@szokeasaurusrex
szokeasaurusrex marked this pull request as ready for review August 12, 2026 21:03
Comment on lines +135 to +136
/// Set the `sampled` field, accepting `Option` values.
pub(crate) fn with_maybe_sampled(self, sampled: Option<bool>) -> Self {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

l: found the name a bit confusing. Maybe with_sampled_option is a better one.

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.

2 participants