From 2716ef36e029d704dc78610c4015152110cf782e Mon Sep 17 00:00:00 2001 From: Ash Anand Date: Mon, 3 Mar 2025 17:18:32 -0500 Subject: [PATCH 1/7] feat(otlp): Infer span description for `db` spans Co-authored-by: Matt Quinn ` --- relay-event-schema/src/protocol/span.rs | 7 +++ .../src/services/processor/span/processing.rs | 46 +++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/relay-event-schema/src/protocol/span.rs b/relay-event-schema/src/protocol/span.rs index 89fe08fc765..54e6cd6c034 100644 --- a/relay-event-schema/src/protocol/span.rs +++ b/relay-event-schema/src/protocol/span.rs @@ -400,6 +400,13 @@ pub struct SpanData { #[metastructure(field = "db.operation")] pub db_operation: Annotated, + /// The database query being executed. + /// + /// E.g. SELECT * FROM wuser_table where username = ?; SET mykey ? + /// See [OpenTelemetry docs for a list of well-known identifiers](https://opentelemetry.io/docs/specs/semconv/database/database-spans/#common-attributes). + #[metastructure(field = "db.query.text", legacy_alias = "db.statement")] + pub db_statement: Annotated, + /// An identifier for the database management system (DBMS) product being used. /// /// See [OpenTelemetry docs for a list of well-known identifiers](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/database.md#notes-and-well-known-identifiers-for-dbsystem). diff --git a/relay-server/src/services/processor/span/processing.rs b/relay-server/src/services/processor/span/processing.rs index e3cf0eac82c..857b180c0cd 100644 --- a/relay-server/src/services/processor/span/processing.rs +++ b/relay-server/src/services/processor/span/processing.rs @@ -644,6 +644,10 @@ fn normalize( ); span.sentry_tags = Annotated::new(tags); + if span.description.value().is_empty() { + span.description = infer_span_description(span).into(); + } + normalize_performance_score(span, performance_score); if let Some(model_costs_config) = ai_model_costs { extract_ai_measurements(span, model_costs_config); @@ -803,6 +807,19 @@ fn validate(span: &mut Annotated) -> Result<(), ValidationError> { Ok(()) } +fn infer_span_description(span: &Span) -> Option { + let category = span.sentry_tags.value().and_then(|v| v.category.value()); + + match category.map(|v| v.as_ref()) { + Some("db") => span.data.value().and_then(|v| { + v.db_statement + .value() + .map(|db_statement| db_statement.to_owned()) + }), + _ => None, + } +} + #[cfg(test)] mod tests { use std::collections::BTreeMap; @@ -1431,4 +1448,33 @@ mod tests { &EventId("480ffcc911174ade9106b40ffbd822f5".parse().unwrap()) ); } + + #[test] + fn infers_span_description() { + let mut span = Annotated::from_json( + r#"{ + "start_timestamp": 0, + "timestamp": 1, + "trace_id": "922dda2462ea4ac2b6a4b339bee90863", + "span_id": "922dda2462ea4ac2" + "sentry_tags": { + "category": "db" + } + }"#, + ) + .unwrap(); + + normalize(&mut span, normalize_config()).unwrap(); + + let data = get_value!(span.data!); + + assert_eq!(data.exclusive_time, Annotated::empty()); + assert_eq!(*get_value!(span.exclusive_time!), 128.0); + + assert_eq!(data.profile_id, Annotated::empty()); + assert_eq!( + get_value!(span.profile_id!), + &EventId("480ffcc911174ade9106b40ffbd822f5".parse().unwrap()) + ); + } } From cf723e1afc6203eb358a12cf283838b2da369310 Mon Sep 17 00:00:00 2001 From: Ash Anand Date: Tue, 4 Mar 2025 17:08:27 -0500 Subject: [PATCH 2/7] attempt to get the test working --- .../src/services/processor/span/processing.rs | 46 +++++++++++++------ 1 file changed, 33 insertions(+), 13 deletions(-) diff --git a/relay-server/src/services/processor/span/processing.rs b/relay-server/src/services/processor/span/processing.rs index 857b180c0cd..035bee51b75 100644 --- a/relay-server/src/services/processor/span/processing.rs +++ b/relay-server/src/services/processor/span/processing.rs @@ -809,7 +809,6 @@ fn validate(span: &mut Annotated) -> Result<(), ValidationError> { fn infer_span_description(span: &Span) -> Option { let category = span.sentry_tags.value().and_then(|v| v.category.value()); - match category.map(|v| v.as_ref()) { Some("db") => span.data.value().and_then(|v| { v.db_statement @@ -828,7 +827,7 @@ mod tests { use bytes::Bytes; use once_cell::sync::Lazy; use relay_event_schema::protocol::{ - Context, ContextInner, EventId, SpanId, Timestamp, TraceContext, TraceId, + Context, ContextInner, EventId, SentryTags, SpanId, Timestamp, TraceContext, TraceId, }; use relay_event_schema::protocol::{Contexts, Event, Span}; use relay_protocol::get_value; @@ -1450,13 +1449,17 @@ mod tests { } #[test] - fn infers_span_description() { - let mut span = Annotated::from_json( + fn infers_db_span_description() { + let mut span: Annotated = Annotated::from_json( r#"{ "start_timestamp": 0, "timestamp": 1, "trace_id": "922dda2462ea4ac2b6a4b339bee90863", - "span_id": "922dda2462ea4ac2" + "span_id": "922dda2462ea4ac2", + "data": { + "db.query.text": "SELECT * FROM users WHERE id = 1", + "db.statement": "SELECT * FROM users WHERE id = 1" + }, "sentry_tags": { "category": "db" } @@ -1464,17 +1467,34 @@ mod tests { ) .unwrap(); - normalize(&mut span, normalize_config()).unwrap(); - - let data = get_value!(span.data!); + span.value_mut().as_mut().unwrap().sentry_tags = SentryTags { + category: Some("db".to_string()).into(), + ..Default::default() + } + .into(); - assert_eq!(data.exclusive_time, Annotated::empty()); - assert_eq!(*get_value!(span.exclusive_time!), 128.0); + normalize(&mut span, normalize_config()).unwrap(); - assert_eq!(data.profile_id, Annotated::empty()); + // let mut span: Annotated = Span { + // sentry_tags: SentryTags { + // category: Some("db".to_string()).into(), + // ..Default::default() + // }.into(), + // data: SpanData { + // db_statement: Some("SELECT * FROM users WHERE id = 1".to_string()).into(), + // ..Default::default() + // }.into(), + // start_timestamp: Timestamp::fro.into(), + // timestamp: Timestamp(1).into(), + // ..Default::default() + // }.into(); + + // let description = get_value!(span.sentry_tags.user!); + // assert_eq!(); assert_eq!( - get_value!(span.profile_id!), - &EventId("480ffcc911174ade9106b40ffbd822f5".parse().unwrap()) + get_value!(span.description!).as_str(), + "SELECT * FROM users WHERE id = 1" ); + // assert_eq!(get_value!(span.description!), "SELECT * FROM users WHERE id = 1"); } } From 6d163f4557c476745a2f1c514945e4509352cbba Mon Sep 17 00:00:00 2001 From: Matt Quinn Date: Wed, 5 Mar 2025 10:05:20 -0500 Subject: [PATCH 3/7] passing test --- relay-event-schema/src/protocol/span.rs | 4 +- .../src/protocol/span/convert.rs | 1 + ...t__tests__extract_span_metrics_mobile.snap | 5 +++ .../src/services/processor/span/processing.rs | 42 +++---------------- relay-spans/src/span.rs | 5 ++- 5 files changed, 18 insertions(+), 39 deletions(-) diff --git a/relay-event-schema/src/protocol/span.rs b/relay-event-schema/src/protocol/span.rs index 54e6cd6c034..b03305e9cad 100644 --- a/relay-event-schema/src/protocol/span.rs +++ b/relay-event-schema/src/protocol/span.rs @@ -405,7 +405,7 @@ pub struct SpanData { /// E.g. SELECT * FROM wuser_table where username = ?; SET mykey ? /// See [OpenTelemetry docs for a list of well-known identifiers](https://opentelemetry.io/docs/specs/semconv/database/database-spans/#common-attributes). #[metastructure(field = "db.query.text", legacy_alias = "db.statement")] - pub db_statement: Annotated, + pub db_query_text: Annotated, /// An identifier for the database management system (DBMS) product being used. /// @@ -1041,6 +1041,7 @@ mod tests { let data = r#"{ "foo": 2, "bar": "3", + "db.query.text": "SELECT * FROM table", "db.system": "mysql", "code.filepath": "task.py", "code.lineno": 123, @@ -1085,6 +1086,7 @@ mod tests { "ns", ), db_operation: ~, + db_query_text: "SELECT * FROM table", db_system: String( "mysql", ), diff --git a/relay-event-schema/src/protocol/span/convert.rs b/relay-event-schema/src/protocol/span/convert.rs index bc6bb117075..9f5a4713acd 100644 --- a/relay-event-schema/src/protocol/span/convert.rs +++ b/relay-event-schema/src/protocol/span/convert.rs @@ -170,6 +170,7 @@ mod tests { code_function: ~, code_namespace: ~, db_operation: ~, + db_query_text: ~, db_system: ~, db_collection_name: ~, environment: "prod", diff --git a/relay-server/src/metrics_extraction/snapshots/relay_server__metrics_extraction__event__tests__extract_span_metrics_mobile.snap b/relay-server/src/metrics_extraction/snapshots/relay_server__metrics_extraction__event__tests__extract_span_metrics_mobile.snap index 51b50d15908..9dd0d419fc6 100644 --- a/relay-server/src/metrics_extraction/snapshots/relay_server__metrics_extraction__event__tests__extract_span_metrics_mobile.snap +++ b/relay-server/src/metrics_extraction/snapshots/relay_server__metrics_extraction__event__tests__extract_span_metrics_mobile.snap @@ -123,6 +123,7 @@ expression: "(&event.value().unwrap().spans, metrics.project_metrics)" code_function: ~, code_namespace: ~, db_operation: ~, + db_query_text: ~, db_system: ~, db_collection_name: ~, environment: ~, @@ -645,6 +646,7 @@ expression: "(&event.value().unwrap().spans, metrics.project_metrics)" code_function: ~, code_namespace: ~, db_operation: ~, + db_query_text: ~, db_system: ~, db_collection_name: ~, environment: ~, @@ -802,6 +804,7 @@ expression: "(&event.value().unwrap().spans, metrics.project_metrics)" code_function: ~, code_namespace: ~, db_operation: ~, + db_query_text: ~, db_system: ~, db_collection_name: ~, environment: ~, @@ -1041,6 +1044,7 @@ expression: "(&event.value().unwrap().spans, metrics.project_metrics)" code_function: ~, code_namespace: ~, db_operation: ~, + db_query_text: ~, db_system: ~, db_collection_name: ~, environment: ~, @@ -1198,6 +1202,7 @@ expression: "(&event.value().unwrap().spans, metrics.project_metrics)" code_function: ~, code_namespace: ~, db_operation: ~, + db_query_text: ~, db_system: ~, db_collection_name: ~, environment: ~, diff --git a/relay-server/src/services/processor/span/processing.rs b/relay-server/src/services/processor/span/processing.rs index 035bee51b75..c8fbd5f4bea 100644 --- a/relay-server/src/services/processor/span/processing.rs +++ b/relay-server/src/services/processor/span/processing.rs @@ -808,13 +808,9 @@ fn validate(span: &mut Annotated) -> Result<(), ValidationError> { } fn infer_span_description(span: &Span) -> Option { - let category = span.sentry_tags.value().and_then(|v| v.category.value()); - match category.map(|v| v.as_ref()) { - Some("db") => span.data.value().and_then(|v| { - v.db_statement - .value() - .map(|db_statement| db_statement.to_owned()) - }), + let category = span.sentry_tags.value()?.category.value()?; + match category.as_str() { + "db" => span.data.value()?.db_query_text.value()?.to_owned().into(), _ => None, } } @@ -827,7 +823,7 @@ mod tests { use bytes::Bytes; use once_cell::sync::Lazy; use relay_event_schema::protocol::{ - Context, ContextInner, EventId, SentryTags, SpanId, Timestamp, TraceContext, TraceId, + Context, ContextInner, EventId, SpanId, Timestamp, TraceContext, TraceId, }; use relay_event_schema::protocol::{Contexts, Event, Span}; use relay_protocol::get_value; @@ -1450,7 +1446,7 @@ mod tests { #[test] fn infers_db_span_description() { - let mut span: Annotated = Annotated::from_json( + let mut span = Annotated::from_json( r#"{ "start_timestamp": 0, "timestamp": 1, @@ -1458,43 +1454,17 @@ mod tests { "span_id": "922dda2462ea4ac2", "data": { "db.query.text": "SELECT * FROM users WHERE id = 1", - "db.statement": "SELECT * FROM users WHERE id = 1" - }, - "sentry_tags": { - "category": "db" + "sentry.category": "db" } }"#, ) .unwrap(); - span.value_mut().as_mut().unwrap().sentry_tags = SentryTags { - category: Some("db".to_string()).into(), - ..Default::default() - } - .into(); - normalize(&mut span, normalize_config()).unwrap(); - // let mut span: Annotated = Span { - // sentry_tags: SentryTags { - // category: Some("db".to_string()).into(), - // ..Default::default() - // }.into(), - // data: SpanData { - // db_statement: Some("SELECT * FROM users WHERE id = 1".to_string()).into(), - // ..Default::default() - // }.into(), - // start_timestamp: Timestamp::fro.into(), - // timestamp: Timestamp(1).into(), - // ..Default::default() - // }.into(); - - // let description = get_value!(span.sentry_tags.user!); - // assert_eq!(); assert_eq!( get_value!(span.description!).as_str(), "SELECT * FROM users WHERE id = 1" ); - // assert_eq!(get_value!(span.description!), "SELECT * FROM users WHERE id = 1"); } } diff --git a/relay-spans/src/span.rs b/relay-spans/src/span.rs index 13a2f665d6b..72c8b768b52 100644 --- a/relay-spans/src/span.rs +++ b/relay-spans/src/span.rs @@ -549,7 +549,7 @@ mod tests { let otel_span: OtelSpan = serde_json::from_str(json).unwrap(); let span_from_otel = otel_to_sentry_span(otel_span); - insta::assert_debug_snapshot!(span_from_otel, @r#" + insta::assert_debug_snapshot!(span_from_otel, @r###" Span { timestamp: Timestamp( 1970-01-01T00:02:03.500Z, @@ -591,6 +591,7 @@ mod tests { code_function: ~, code_namespace: ~, db_operation: ~, + db_query_text: ~, db_system: ~, db_collection_name: ~, environment: "prod", @@ -662,7 +663,7 @@ mod tests { was_transaction: ~, other: {}, } - "#); + "###); } #[test] From c47a7dbf6cad2ce0e88810b9530eac93c23041b4 Mon Sep 17 00:00:00 2001 From: Matt Quinn Date: Wed, 5 Mar 2025 10:50:37 -0500 Subject: [PATCH 4/7] remove db description inference in otel parsing --- relay-spans/src/span.rs | 47 ----------------------------------------- 1 file changed, 47 deletions(-) diff --git a/relay-spans/src/span.rs b/relay-spans/src/span.rs index 72c8b768b52..cd67f91955a 100644 --- a/relay-spans/src/span.rs +++ b/relay-spans/src/span.rs @@ -123,11 +123,6 @@ pub fn otel_to_sentry_span(otel_span: OtelSpan) -> EventSpan { } key if key.starts_with("db") => { op = op.or(Some("db".to_string())); - if key == "db.statement" { - if let Some(statement) = otel_value_to_string(value) { - description = Some(statement); - } - } } "http.method" | "http.request.method" => { let http_op = match kind { @@ -346,48 +341,6 @@ mod tests { assert_eq!(event_span.exclusive_time, Annotated::new(0.0788)); } - #[test] - fn parse_span_with_db_attributes() { - let json = r#"{ - "traceId": "89143b0763095bd9c9955e8175d1fb23", - "spanId": "e342abb1214ca181", - "parentSpanId": "0c7a7dea069bf5a6", - "name": "database query", - "kind": 3, - "startTimeUnixNano": "1697620454980000000", - "endTimeUnixNano": "1697620454980078800", - "attributes": [ - { - "key" : "db.name", - "value": { - "stringValue": "database" - } - }, - { - "key" : "db.type", - "value": { - "stringValue": "sql" - } - }, - { - "key" : "db.statement", - "value": { - "stringValue": "SELECT \"table\".\"col\" FROM \"table\" WHERE \"table\".\"col\" = %s" - } - } - ] - }"#; - let otel_span: OtelSpan = serde_json::from_str(json).unwrap(); - let event_span: EventSpan = otel_to_sentry_span(otel_span); - assert_eq!(event_span.op, Annotated::new("db".into())); - assert_eq!( - event_span.description, - Annotated::new( - "SELECT \"table\".\"col\" FROM \"table\" WHERE \"table\".\"col\" = %s".into() - ) - ); - } - #[test] fn parse_span_with_http_attributes() { let json = r#"{ From 68bca0d70c313aa0bca6bc63ce7616ce60fed7ff Mon Sep 17 00:00:00 2001 From: Ash Anand Date: Wed, 5 Mar 2025 15:34:01 -0500 Subject: [PATCH 5/7] update `CHANGELOG` --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a14979799ce..9e223ac3575 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ - Adopt new `AsyncPool` for the `EnvelopeProcessorService` and `StoreService`. ([#4520](https://github.com/getsentry/relay/pull/4520)) - Update mapping of OTLP spans to Sentry spans in the experimental OTL traces endpoint. ([#4505](https://github.com/getsentry/relay/pull/4505)) - Expose metrics for the `AsyncPool`. ([#4538](https://github.com/getsentry/relay/pull/4538)) +- Infer span `description` for spans with `category` set to `db` . ([#4541](https://github.com/getsentry/relay/pull/4541)) ## 25.2.0 From 6a3ad6d41de9db91041e662aa1f045e5580d0ac2 Mon Sep 17 00:00:00 2001 From: Matt Quinn Date: Thu, 6 Mar 2025 14:26:40 -0500 Subject: [PATCH 6/7] apply David's changelog suggestion Co-authored-by: David Herberth --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e223ac3575..103bbebd9e5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,7 +21,7 @@ - Adopt new `AsyncPool` for the `EnvelopeProcessorService` and `StoreService`. ([#4520](https://github.com/getsentry/relay/pull/4520)) - Update mapping of OTLP spans to Sentry spans in the experimental OTL traces endpoint. ([#4505](https://github.com/getsentry/relay/pull/4505)) - Expose metrics for the `AsyncPool`. ([#4538](https://github.com/getsentry/relay/pull/4538)) -- Infer span `description` for spans with `category` set to `db` . ([#4541](https://github.com/getsentry/relay/pull/4541)) +- Infer span `description` for spans with `category` set to `db` from query attribute. ([#4541](https://github.com/getsentry/relay/pull/4541)) ## 25.2.0 From 9257e6a5c940449cc5c45f8bb98ed344a80d60db Mon Sep 17 00:00:00 2001 From: Matt Quinn Date: Mon, 10 Mar 2025 16:43:20 -0400 Subject: [PATCH 7/7] document order dependencies --- relay-server/src/services/processor/span/processing.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/relay-server/src/services/processor/span/processing.rs b/relay-server/src/services/processor/span/processing.rs index c8fbd5f4bea..03f8de00509 100644 --- a/relay-server/src/services/processor/span/processing.rs +++ b/relay-server/src/services/processor/span/processing.rs @@ -644,6 +644,8 @@ fn normalize( ); span.sentry_tags = Annotated::new(tags); + // This inference depends on `sentry_tags.category`, which is set during tag + // extraction above. Please be careful if trying to reorder this operation. if span.description.value().is_empty() { span.description = infer_span_description(span).into(); } @@ -807,6 +809,9 @@ fn validate(span: &mut Annotated) -> Result<(), ValidationError> { Ok(()) } +// Infer a span's `description` based on its attributes. Note that tag +// extraction must have already run before this is called, as it relies on the +// values of `span.sentry_tags`. fn infer_span_description(span: &Span) -> Option { let category = span.sentry_tags.value()?.category.value()?; match category.as_str() {