Skip to content

User span discard - #276

Merged
TheJokr merged 3 commits into
cloudflare:mainfrom
mar-cf:user-span-discard
Oct 9, 2026
Merged

TheJokr merged 3 commits into
cloudflare:mainfrom
mar-cf:user-span-discard

Conversation

@mar-cf

@mar-cf mar-cf commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

cloudflare/rustracing#13

Some user spans only turn out to be uninteresting after the work they measure, e.g. a routing
span when nothing matched. Creating them lazily would lose the real start time, and there was no
way to drop a span once created.

Add UserSpan::discard and the ambient user_tracing::discard_span. Discarding replaces the span
with an inactive one under its write lock, using Span::discard, so every handle stops recording
and nothing is reported. The sampling flag is cached per handle, so only the discarding handle's
cache is reset; other handles may still report being sampled while recording nothing. A deferred
root that wasn't activated yet is unaffected and can still be activated.

@mar-cf
mar-cf force-pushed the user-span-discard branch from 5c6868a to 2bcb1da Compare October 9, 2026 11:47
mar-cf and others added 2 commits October 9, 2026 13:50
Discarding user spans needs `Span::discard`, which cf-rustracing 1.4.1 adds.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Some user spans only turn out to be uninteresting after the work they measure, e.g. a routing
span when nothing matched. Creating them lazily would lose the real start time, and there was no
way to drop a span once created.

Add `UserSpan::discard` and the ambient `user_tracing::discard_span`. Discarding replaces the span
with an inactive one under its write lock, using `Span::discard`, so every handle stops recording
and nothing is reported. The sampling flag is cached per handle, so only the discarding handle's
cache is reset; other handles may still report being sampled while recording nothing. A deferred
root that wasn't activated yet is unaffected and can still be activated.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Comment thread foundations/src/telemetry/tracing/internal.rs Outdated
`SharedSpan` cached whether a span is sampled in each handle, and cloning a handle copied the
cache. Discarding a user span only reset the handle it was called on, so other handles kept
reporting it as sampled through `UserSpan::is_sampled`. `discard_span` discards through a clone
taken from the scope stack, which left even the caller's `UserSpan` reporting the span as sampled.
Deferred roots avoided stale copies with a sentinel that made every check take the span's lock
until the root was activated.

Keep the flag next to the span instead, in a `UserSpanSlot` shared by every handle to a user span.
Activation and discard update it under the span's write lock, and reading it never takes the lock.
Internal spans never change after creation, so the handle variant implies their flag, or `Tracked`
stores it for `track_all_spans`. Internal handles keep their `Arc<RwLock<Span>>`, which
`rustracing_span` returns.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@mar-cf
mar-cf force-pushed the user-span-discard branch from 2bcb1da to 34ef259 Compare October 9, 2026 14:09
}
}

pub(crate) fn with_read<R>(&self, f: impl FnOnce(&Span) -> R) -> R {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Should we check sampling in with_read and with_write to avoid taking the span's lock when it isn't sampled?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

wouldn't hurt but lets do that in a separate PR

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

i can do this as part of the release PR

@TheJokr
TheJokr merged commit af28600 into cloudflare:main Oct 9, 2026
27 checks passed
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