Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

- Emit more precise outcome discard reasons for the Playstation, Minidump, and Attachments endpoints. ([#5950](https://github.com/getsentry/relay/pull/5950))
- Set the name based on the transaction name when converting a transaction into a segment span. ([#5961](https://github.com/getsentry/relay/pull/5961))
- Set the segment name for child spans based on the transaction name. ([#5959](https://github.com/getsentry/relay/pull/5959))

**Internal**:

Expand Down
3 changes: 2 additions & 1 deletion relay-event-normalization/src/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ use uuid::Uuid;

use crate::normalize::request;
use crate::span::ai::enrich_ai_event_data;
use crate::span::tag_extraction::extract_span_tags_from_event;
use crate::span::tag_extraction::{extract_segment_name_from_event, extract_span_tags_from_event};
use crate::utils::{self, MAX_DURATION_MOBILE_MS, get_event_user_tag};
use crate::{
BorrowedSpanOpDefaults, BreakdownsConfig, CombinedMeasurementsConfig, GeoIpLookup, MaxChars,
Expand Down Expand Up @@ -350,6 +350,7 @@ fn normalize(event: &mut Event, meta: &mut Meta, config: &NormalizationConfig) {
config.max_tag_value_length,
config.span_allowed_hosts,
);
extract_segment_name_from_event(event);
}

if let Some(context) = event.context_mut::<TraceContext>() {
Expand Down
24 changes: 24 additions & 0 deletions relay-event-normalization/src/normalize/span/tag_extraction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,30 @@ impl std::fmt::Display for RenderBlockingStatus {
}
}

/// Extracts the `transaction` field and writes it into child spans
/// as `data.segment_name`.
pub(crate) fn extract_segment_name_from_event(event: &mut Event) {
let Some(transaction) = event.transaction.value() else {
return;
};

let Some(spans) = event.spans.value_mut() else {
return;
};

for span in spans {
let Some(span) = span.value_mut() else {
continue;
};

let data = span.data.get_or_insert_with(Default::default);

if data.segment_name.is_empty() {
data.segment_name = Annotated::new(transaction.clone());
}
Comment on lines +80 to +82
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

}
}

/// Wrapper for [`extract_span_tags`].
///
/// Tags longer than `max_tag_value_size` bytes will be truncated.
Expand Down
9 changes: 9 additions & 0 deletions tests/integration/test_ai.py
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,7 @@ def test_ai_spans_example_transaction(
"sentry.sdk.name": {"type": "string", "value": "raven-node"},
"sentry.sdk.version": {"type": "string", "value": "2.6.3"},
"sentry.segment.id": {"type": "string", "value": "657cf984a6a4e59b"},
"sentry.segment.name": {"type": "string", "value": "main"},
"sentry.status": {"type": "string", "value": "ok"},
"sentry.trace.status": {"type": "string", "value": "ok"},
"sentry.transaction": {"type": "string", "value": "main"},
Expand Down Expand Up @@ -543,6 +544,7 @@ def test_ai_spans_example_transaction(
"sentry.sdk.name": {"type": "string", "value": "raven-node"},
"sentry.sdk.version": {"type": "string", "value": "2.6.3"},
"sentry.segment.id": {"type": "string", "value": "657cf984a6a4e59b"},
"sentry.segment.name": {"type": "string", "value": "main"},
"sentry.status": {"type": "string", "value": "ok"},
"sentry.trace.status": {"type": "string", "value": "ok"},
"sentry.transaction": {"type": "string", "value": "main"},
Expand Down Expand Up @@ -646,6 +648,7 @@ def test_ai_spans_example_transaction(
"sentry.sdk.name": {"type": "string", "value": "raven-node"},
"sentry.sdk.version": {"type": "string", "value": "2.6.3"},
"sentry.segment.id": {"type": "string", "value": "657cf984a6a4e59b"},
"sentry.segment.name": {"type": "string", "value": "main"},
"sentry.status": {"type": "string", "value": "ok"},
"sentry.status_code": {"type": "string", "value": "200"},
"sentry.trace.status": {"type": "string", "value": "ok"},
Expand Down Expand Up @@ -729,6 +732,7 @@ def test_ai_spans_example_transaction(
"sentry.sdk.name": {"type": "string", "value": "raven-node"},
"sentry.sdk.version": {"type": "string", "value": "2.6.3"},
"sentry.segment.id": {"type": "string", "value": "657cf984a6a4e59b"},
"sentry.segment.name": {"type": "string", "value": "main"},
"sentry.status": {"type": "string", "value": "ok"},
"sentry.trace.status": {"type": "string", "value": "ok"},
"sentry.transaction": {"type": "string", "value": "main"},
Expand Down Expand Up @@ -795,6 +799,7 @@ def test_ai_spans_example_transaction(
"sentry.sdk.name": {"type": "string", "value": "raven-node"},
"sentry.sdk.version": {"type": "string", "value": "2.6.3"},
"sentry.segment.id": {"type": "string", "value": "657cf984a6a4e59b"},
"sentry.segment.name": {"type": "string", "value": "main"},
"sentry.status": {"type": "string", "value": "ok"},
"sentry.status_code": {"type": "string", "value": "200"},
"sentry.trace.status": {"type": "string", "value": "ok"},
Expand Down Expand Up @@ -871,6 +876,7 @@ def test_ai_spans_example_transaction(
"sentry.sdk.name": {"type": "string", "value": "raven-node"},
"sentry.sdk.version": {"type": "string", "value": "2.6.3"},
"sentry.segment.id": {"type": "string", "value": "657cf984a6a4e59b"},
"sentry.segment.name": {"type": "string", "value": "main"},
"sentry.status": {"type": "string", "value": "ok"},
"sentry.trace.status": {"type": "string", "value": "ok"},
"sentry.transaction": {"type": "string", "value": "main"},
Expand Down Expand Up @@ -937,6 +943,7 @@ def test_ai_spans_example_transaction(
"sentry.sdk.name": {"type": "string", "value": "raven-node"},
"sentry.sdk.version": {"type": "string", "value": "2.6.3"},
"sentry.segment.id": {"type": "string", "value": "657cf984a6a4e59b"},
"sentry.segment.name": {"type": "string", "value": "main"},
"sentry.status": {"type": "string", "value": "ok"},
"sentry.status_code": {"type": "string", "value": "200"},
"sentry.trace.status": {"type": "string", "value": "ok"},
Expand Down Expand Up @@ -1043,6 +1050,7 @@ def test_ai_spans_example_transaction(
"sentry.sdk.name": {"type": "string", "value": "raven-node"},
"sentry.sdk.version": {"type": "string", "value": "2.6.3"},
"sentry.segment.id": {"type": "string", "value": "657cf984a6a4e59b"},
"sentry.segment.name": {"type": "string", "value": "main"},
"sentry.status": {"type": "string", "value": "ok"},
"sentry.trace.status": {"type": "string", "value": "ok"},
"sentry.transaction": {"type": "string", "value": "main"},
Expand Down Expand Up @@ -1143,6 +1151,7 @@ def test_ai_spans_example_transaction(
"sentry.sdk.name": {"type": "string", "value": "raven-node"},
"sentry.sdk.version": {"type": "string", "value": "2.6.3"},
"sentry.segment.id": {"type": "string", "value": "657cf984a6a4e59b"},
"sentry.segment.name": {"type": "string", "value": "main"},
"sentry.status": {"type": "string", "value": "ok"},
"sentry.status_code": {"type": "string", "value": "200"},
"sentry.trace.status": {"type": "string", "value": "ok"},
Expand Down
1 change: 1 addition & 0 deletions tests/integration/test_span_links.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ def test_event_with_span_link_in_transaction(relay, mini_sentry):
"start_timestamp": 1624366926.0,
"timestamp": 1624366927.0,
"sentry_tags": mock.ANY,
"data": {"sentry.segment.name": "/users"},
"links": [
{
"trace_id": "4c79f60c11214eb38604f4ae0781bfb2",
Expand Down
1 change: 1 addition & 0 deletions tests/integration/test_spans.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ def test_span_extraction(
"sentry.sdk.name": {"type": "string", "value": "raven-node"},
"sentry.sdk.version": {"type": "string", "value": "2.6.3"},
"sentry.status": {"type": "string", "value": "ok"},
"sentry.segment.name": {"type": "string", "value": "hi"},
"sentry.trace.status": {"type": "string", "value": "ok"},
"sentry.transaction": {"type": "string", "value": "hi"},
"sentry.transaction.op": {"type": "string", "value": "hi"},
Expand Down
Loading