Skip to content

fix(spans): Mark root spans as segments in OTel conversion - #5532

Merged
mjq merged 7 commits into
masterfrom
mjq/2026-01-13/otel-segment-definition
Jan 13, 2026
Merged

fix(spans): Mark root spans as segments in OTel conversion#5532
mjq merged 7 commits into
masterfrom
mjq/2026-01-13/otel-segment-definition

Conversation

@mjq

@mjq mjq commented Jan 13, 2026

Copy link
Copy Markdown
Member

Previously, the is_segment flag was only set when is_remote was true. However, root spans should also be marked as segments. This fix adds a check for empty parent_span_id to correctly identify root spans as segments.

Previously, the is_segment flag was only set when is_remote was true.  However,
root spans should also be marked as segments. This fix adds a check for empty
parent_span_id to correctly identify root spans as segments.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@mjq mjq changed the title fix(spans): Mark root spans as segments in OTEL conversion fix(spans): Mark root spans as segments in OTel conversion Jan 13, 2026
@mjq
mjq marked this pull request as ready for review January 13, 2026 17:56
@mjq
mjq requested a review from a team as a code owner January 13, 2026 17:56
Comment thread relay-spans/src/otel_to_sentry_v2.rs Outdated
Comment thread relay-spans/src/otel_to_sentry_v2.rs Outdated
// A remote span is a segment span, but not every segment span is remote:
// A span is also a segment if it has no parent span (i.e., it's a root span).
let is_root_span = parent_span_id.value().is_none();
let is_segment = match is_remote {

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.

Could simplify the match to is_root_span || is_remote.unwrap_or(false)

@mjq mjq Jan 13, 2026

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.

@Dav1dde 👍 Did this, but it does have the side effect of making is_segment explicitly false rather than None. Still cool?

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.

Oh I was thinking to keep the match just instead match on true/false, but alternatively you can also do (is_root_span || is_remote.unwrap_or(false)).then_some(true) if you're feeling fancy.

@mjq
mjq enabled auto-merge January 13, 2026 18:55
@mjq
mjq added this pull request to the merge queue Jan 13, 2026
Merged via the queue into master with commit 5353c59 Jan 13, 2026
29 checks passed
@mjq
mjq deleted the mjq/2026-01-13/otel-segment-definition branch January 13, 2026 19:25
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