Skip to content
Closed
Show file tree
Hide file tree
Changes from 5 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
3 changes: 2 additions & 1 deletion relay-event-normalization/src/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,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 @@ -347,6 +347,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 fn extract_segment_name_from_event(event: &mut Event) {
Comment thread
cursor[bot] marked this conversation as resolved.
Outdated
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
2 changes: 1 addition & 1 deletion relay-spans/src/v1_to_v2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,7 @@ mod tests {
.attributes
.value()
.unwrap()
.get_value("sentry.segment.name")
.get_value(SENTRY__SEGMENT__NAME)
.and_then(Value::as_str),
Some("hi")
);
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 @@ -139,6 +139,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