feat(transaction): Propagate segment_name to spans#5959
feat(transaction): Propagate segment_name to spans#5959loewenheim wants to merge 6 commits intomasterfrom
Conversation
This propagates an event's `transaction` field to the event's spans as `data.segment_name`. Previously, we were only doing this when converting a transaction into a span, which meant that only the root span had this field set.
There was a problem hiding this comment.
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 a48d977. Configure here.
| if data.segment_name.is_empty() { | ||
| data.segment_name = Annotated::new(transaction.clone()); | ||
| } |
There was a problem hiding this comment.
Bug: The check data.segment_name.is_empty() is incorrect for an Annotated type, as it can return false if metadata exists, even when the value itself is None.
Severity: LOW
Suggested Fix
To correctly check for a missing value in an Annotated field, regardless of its metadata, change the condition from data.segment_name.is_empty() to data.segment_name.value().is_none(). This ensures the check is performed on the value itself.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: relay-event-normalization/src/normalize/span/tag_extraction.rs#L80-L82
Potential issue: The code uses `data.segment_name.is_empty()` to check if a child span's
segment name is missing. However, for an `Annotated<T>` type, `is_empty()` returns
`false` if metadata is present, even if the underlying value is `None`. This scenario
occurs if an SDK sends a malformed `segment_name` (e.g., an integer), which results in a
`None` value but with validation error metadata. Consequently, the check incorrectly
evaluates to `false`, and the code fails to populate the `segment_name` from the parent
transaction's name as intended.
Did we get this right? 👍 / 👎 to inform future reviews.
|
Per discussion on Slack, this is the wrong approach: Filling the child spans' |

This propagates an event's
transactionfield to the event's spans asdata.segment_name. Previously, we were only doing this when converting a transaction into a span, which meant that only the root span had this field set.