diff --git a/CHANGELOG.md b/CHANGELOG.md index 4eaca985f3..20822d48fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ - Respect country code TLDs when scrubbing span tags. ([#3458](https://github.com/getsentry/relay/pull/3458)) +**Features**: + +- Use same keys for OTel span attributes and Sentry span data. ([#3457](https://github.com/getsentry/relay/pull/3457)) + **Internal**: - Emit gauges for total and self times for spans. ([#3448](https://github.com/getsentry/relay/pull/3448)) @@ -14,7 +18,7 @@ **Features**: -- Add inbound filters for Annotated types. ([#3420](https://github.com/getsentry/relay/pull/3420)) +- Add inbound filters for `Annotated` types. ([#3420](https://github.com/getsentry/relay/pull/3420)) - Add Linux distributions to os context. ([#3443](https://github.com/getsentry/relay/pull/3443)) **Internal:** diff --git a/Cargo.lock b/Cargo.lock index ff027b8062..387ebe7fed 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4198,6 +4198,7 @@ dependencies = [ "chrono", "enumset", "hex", + "insta", "once_cell", "opentelemetry-proto", "relay-event-schema", diff --git a/relay-event-schema/src/protocol/span.rs b/relay-event-schema/src/protocol/span.rs index d24424feb6..1e1932f8fd 100644 --- a/relay-event-schema/src/protocol/span.rs +++ b/relay-event-schema/src/protocol/span.rs @@ -200,11 +200,11 @@ pub struct SpanData { pub db_system: Annotated, /// The sentry environment. - #[metastructure(field = "environment")] + #[metastructure(field = "sentry.environment", legacy_alias = "environment")] pub environment: Annotated, /// The release version of the project. - #[metastructure(field = "release")] + #[metastructure(field = "sentry.release", legacy_alias = "release")] pub release: Annotated, /// The decoded body size of the response (in bytes). @@ -276,7 +276,7 @@ pub struct SpanData { /// This corresponds to the transaction name in the transaction-based model. /// /// For INP spans, this is the route name where the interaction occurred. - #[metastructure(field = "segment.name", legacy_alias = "transaction")] + #[metastructure(field = "sentry.segment.name", legacy_alias = "transaction")] pub segment_name: Annotated, /// Name of the UI component (e.g. React). @@ -292,11 +292,11 @@ pub struct SpanData { pub user: Annotated, /// Replay ID - #[metastructure(field = "replay_id")] + #[metastructure(field = "sentry.replay.id", legacy_alias = "replay_id")] pub replay_id: Annotated, /// The sentry SDK (see [`crate::protocol::ClientSdkInfo`]). - #[metastructure(field = "sdk.name")] + #[metastructure(field = "sentry.sdk.name")] pub sdk_name: Annotated, /// Other fields in `span.data`. diff --git a/relay-server/src/services/store.rs b/relay-server/src/services/store.rs index 0ebd1896ae..4fab6f3f26 100644 --- a/relay-server/src/services/store.rs +++ b/relay-server/src/services/store.rs @@ -1359,6 +1359,7 @@ struct SpanKafkaMessage<'a> { event_id: Option, #[serde(rename(deserialize = "exclusive_time"))] exclusive_time_ms: f64, + #[serde(default)] is_segment: bool, #[serde(borrow, default, skip_serializing_if = "Option::is_none")] diff --git a/relay-spans/Cargo.toml b/relay-spans/Cargo.toml index 5815362def..ee3eb480f1 100644 --- a/relay-spans/Cargo.toml +++ b/relay-spans/Cargo.toml @@ -17,9 +17,16 @@ chrono = { workspace = true } enumset = { workspace = true } hex = { workspace = true } once_cell = { workspace = true } -opentelemetry-proto = { workspace = true, features = ["gen-tonic", "with-serde", "trace"] } +opentelemetry-proto = { workspace = true, features = [ + "gen-tonic", + "with-serde", + "trace", +] } relay-event-schema = { workspace = true } relay-protocol = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } serde_repr = { workspace = true } + +[dev-dependencies] +insta = { workspace = true } diff --git a/relay-spans/src/lib.rs b/relay-spans/src/lib.rs index 028cea0e11..c4dce6ce7d 100644 --- a/relay-spans/src/lib.rs +++ b/relay-spans/src/lib.rs @@ -10,6 +10,5 @@ pub use crate::span::otel_to_sentry_span; pub use opentelemetry_proto::tonic::trace::v1 as otel_trace; -mod otel_to_sentry_tags; mod span; mod status_codes; diff --git a/relay-spans/src/otel_to_sentry_tags.rs b/relay-spans/src/otel_to_sentry_tags.rs deleted file mode 100644 index d8e2c47a9a..0000000000 --- a/relay-spans/src/otel_to_sentry_tags.rs +++ /dev/null @@ -1,34 +0,0 @@ -use std::collections::BTreeMap; - -use once_cell::sync::Lazy; - -pub static OTEL_TO_SENTRY_TAGS: Lazy> = Lazy::new(|| { - BTreeMap::from([ - ("enduser.id", "user.id"), - ("http.request.cookies", "request.cookies"), - ("http.request.env", "request.env"), - ( - "http.request.headers.content-type", - "request.headers.content-type", - ), - ("http.request.method", "request.method"), - ("sentry.environment", "environment"), - ("sentry.origin", "origin"), - ("sentry.release", "release"), - ("sentry.sample_rate", "sample_rate"), - ("sentry.sdk.integrations", "sdk.integrations"), - ("sentry.sdk.name", "sdk.name"), - ("sentry.sdk.packages", "sdk.packages"), - ("sentry.sdk.version", "sdk.version"), - ("sentry.source", "source"), - ("sentry.user.email", "user.email"), - ("sentry.user.geo.city", "user.geo.city"), - ("sentry.user.geo.country_code", "user.geo.country_code"), - ("sentry.user.geo.region", "user.geo.region"), - ("sentry.user.ip_address", "user.ip_address"), - ("sentry.user.segment", "user.segment"), - ("sentry.user.username", "user.username"), - ("url.full", "request.url"), - ("url.query_string", "request.query_string"), - ]) -}); diff --git a/relay-spans/src/span.rs b/relay-spans/src/span.rs index 31b5e92667..723f85cd79 100644 --- a/relay-spans/src/span.rs +++ b/relay-spans/src/span.rs @@ -3,15 +3,13 @@ use std::str::FromStr; use chrono::{TimeZone, Utc}; use opentelemetry_proto::tonic::common::v1::any_value::Value as OtelValue; +use crate::otel_trace::{status::StatusCode as OtelStatusCode, Span as OtelSpan}; +use crate::status_codes; use relay_event_schema::protocol::{ - Span as EventSpan, SpanData, SpanId, SpanStatus, Timestamp, TraceId, + EventId, Span as EventSpan, SpanData, SpanId, SpanStatus, Timestamp, TraceId, }; use relay_protocol::{Annotated, FromValue, Object}; -use crate::otel_to_sentry_tags::OTEL_TO_SENTRY_TAGS; -use crate::otel_trace::{status::StatusCode as OtelStatusCode, Span as OtelSpan}; -use crate::status_codes; - /// convert_from_otel_to_sentry_status returns a status as defined by Sentry based on the OTel status. fn convert_from_otel_to_sentry_status( status_code: Option, @@ -64,6 +62,14 @@ fn otel_value_to_string(value: OtelValue) -> Option { } } +fn otel_value_to_span_id(value: OtelValue) -> Option { + match value { + OtelValue::StringValue(s) => Some(s), + OtelValue::BytesValue(b) => Some(hex::encode(b)), + _ => None, + } +} + /// Transform an OtelSpan to a Sentry span. pub fn otel_to_sentry_span(otel_span: OtelSpan) -> EventSpan { let mut exclusive_time_ms = 0f64; @@ -88,79 +94,89 @@ pub fn otel_to_sentry_span(otel_span: OtelSpan) -> EventSpan { _ => Some(hex::encode(parent_span_id)), }; - let segment_id = if parent_span_id.is_none() { - Annotated::new(SpanId(span_id.clone())) - } else { - // TODO: derive from attributes - Annotated::empty() - }; - let mut op = None; - let mut description = name; + let mut description = if name.is_empty() { None } else { Some(name) }; let mut http_method = None; let mut http_route = None; let mut http_status_code = None; let mut grpc_status_code = None; + let mut platform = None; + let mut segment_id = None; + let mut profile_id = None; for attribute in attributes.into_iter() { if let Some(value) = attribute.value.and_then(|v| v.value) { - let key: String = if let Some(key) = OTEL_TO_SENTRY_TAGS.get(attribute.key.as_str()) { - key.to_string() - } else { - attribute.key - }; - if key == "sentry.op" { - op = otel_value_to_string(value); - } else 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 = statement; - } + match attribute.key.as_str() { + "sentry.op" => { + op = otel_value_to_string(value); } - } else if key == "http.method" || key == "request.method" { - let http_op = match kind { - 2 => "http.server", - 3 => "http.client", - _ => "http", - }; - op = op.or(Some(http_op.to_string())); - http_method = otel_value_to_string(value); - } else if key == "http.route" || key == "url.path" { - http_route = otel_value_to_string(value); - } else if key.contains("exclusive_time_ns") { - let value = match value { - OtelValue::IntValue(v) => v as f64, - OtelValue::DoubleValue(v) => v, - OtelValue::StringValue(v) => v.parse::().unwrap_or_default(), - _ => 0f64, - }; - exclusive_time_ms = value / 1e6f64; - } else if key == "http.status_code" { - http_status_code = otel_value_to_i64(value); - } else if key == "rpc.grpc.status_code" { - grpc_status_code = otel_value_to_i64(value); - } else { - match value { - OtelValue::ArrayValue(_) => {} - OtelValue::BoolValue(v) => { - data.insert(key, Annotated::new(v.into())); + 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); + } } - OtelValue::BytesValue(v) => { - if let Ok(v) = String::from_utf8(v) { + } + "http.method" | "http.request.method" => { + let http_op = match kind { + 2 => "http.server", + 3 => "http.client", + _ => "http", + }; + op = op.or(Some(http_op.to_string())); + http_method = otel_value_to_string(value); + } + "http.route" | "url.path" => { + http_route = otel_value_to_string(value); + } + key if key.contains("exclusive_time_ns") => { + let value = match value { + OtelValue::IntValue(v) => v as f64, + OtelValue::DoubleValue(v) => v, + OtelValue::StringValue(v) => v.parse::().unwrap_or_default(), + _ => 0f64, + }; + exclusive_time_ms = value / 1e6f64; + } + "http.status_code" => { + http_status_code = otel_value_to_i64(value); + } + "rpc.grpc.status_code" => { + grpc_status_code = otel_value_to_i64(value); + } + "sentry.platform" => { + platform = otel_value_to_string(value); + } + "sentry.segment.id" => { + segment_id = otel_value_to_span_id(value); + } + "sentry.profile.id" => { + profile_id = otel_value_to_span_id(value); + } + _other => { + let key = attribute.key; + match value { + OtelValue::ArrayValue(_) => {} + OtelValue::BoolValue(v) => { + data.insert(key, Annotated::new(v.into())); + } + OtelValue::BytesValue(v) => { + if let Ok(v) = String::from_utf8(v) { + data.insert(key, Annotated::new(v.into())); + } + } + OtelValue::DoubleValue(v) => { + data.insert(key, Annotated::new(v.into())); + } + OtelValue::IntValue(v) => { + data.insert(key, Annotated::new(v.into())); + } + OtelValue::KvlistValue(_) => {} + OtelValue::StringValue(v) => { data.insert(key, Annotated::new(v.into())); } } - OtelValue::DoubleValue(v) => { - data.insert(key, Annotated::new(v.into())); - } - OtelValue::IntValue(v) => { - data.insert(key, Annotated::new(v.into())); - } - OtelValue::KvlistValue(_) => {} - OtelValue::StringValue(v) => { - data.insert(key, Annotated::new(v.into())); - } - }; + } } } } @@ -169,11 +185,8 @@ pub fn otel_to_sentry_span(otel_span: OtelSpan) -> EventSpan { (otel_span.end_time_unix_nano - otel_span.start_time_unix_nano) as f64 / 1e6f64; } - // TODO: This is wrong, a segment could still have a parent in the trace. - let is_segment = parent_span_id.is_none().into(); - if let (Some(http_method), Some(http_route)) = (http_method, http_route) { - description = format!("{} {}", http_method, http_route); + description = Some(format!("{http_method} {http_route}")); } EventSpan { @@ -182,8 +195,12 @@ pub fn otel_to_sentry_span(otel_span: OtelSpan) -> EventSpan { data: SpanData::from_value(Annotated::new(data.into())), exclusive_time: exclusive_time_ms.into(), parent_span_id: parent_span_id.map(SpanId).into(), - segment_id, + segment_id: segment_id.map(SpanId).into(), span_id: Annotated::new(SpanId(span_id)), + profile_id: profile_id + .as_deref() + .and_then(|s| EventId::from_str(s).ok()) + .into(), start_timestamp: Timestamp(start_timestamp).into(), status: Annotated::new(convert_from_otel_to_sentry_status( status.map(|s| s.code), @@ -192,7 +209,7 @@ pub fn otel_to_sentry_span(otel_span: OtelSpan) -> EventSpan { )), timestamp: Timestamp(end_timestamp).into(), trace_id: TraceId(trace_id).into(), - is_segment, + platform: platform.into(), ..Default::default() } } @@ -391,4 +408,153 @@ mod tests { Annotated::new("GET /api/search?q=foobar".into()) ); } + + /// Intended to be synced with `relay-event-schema::protocol::span::convert::tests::roundtrip`. + #[test] + fn parse_sentry_attributes() { + let json = r#"{ + "traceId": "4c79f60c11214eb38604f4ae0781bfb2", + "spanId": "fa90fdead5f74052", + "parentSpanId": "fa90fdead5f74051", + "startTimeUnixNano": 123000000000, + "endTimeUnixNano": 123500000000, + "status": {"code": 0, "message": "foo"}, + "attributes": [ + { + "key" : "browser.name", + "value": { + "stringValue": "Chrome" + } + }, + { + "key" : "sentry.environment", + "value": { + "stringValue": "prod" + } + }, + { + "key" : "sentry.op", + "value": { + "stringValue": "myop" + } + }, + { + "key" : "sentry.platform", + "value": { + "stringValue": "php" + } + }, + { + "key" : "sentry.profile.id", + "value": { + "stringValue": "a0aaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaab" + } + }, + { + "key" : "sentry.release", + "value": { + "stringValue": "myapp@1.0.0" + } + }, + { + "key" : "sentry.sdk.name", + "value": { + "stringValue": "sentry.php" + } + }, + { + "key" : "sentry.segment.id", + "value": { + "stringValue": "fa90fdead5f74052" + } + }, + { + "key" : "sentry.segment.name", + "value": { + "stringValue": "my 1st transaction" + } + } + ] + }"#; + let otel_span: OtelSpan = serde_json::from_str(json).unwrap(); + let span_from_otel = otel_to_sentry_span(otel_span); + + // TODO: measurements, metrics_summary + + insta::assert_debug_snapshot!(span_from_otel, @r###" + Span { + timestamp: Timestamp( + 1970-01-01T00:02:03.500Z, + ), + start_timestamp: Timestamp( + 1970-01-01T00:02:03Z, + ), + exclusive_time: 500.0, + description: ~, + op: "myop", + span_id: SpanId( + "fa90fdead5f74052", + ), + parent_span_id: SpanId( + "fa90fdead5f74051", + ), + trace_id: TraceId( + "4c79f60c11214eb38604f4ae0781bfb2", + ), + segment_id: SpanId( + "fa90fdead5f74052", + ), + is_segment: ~, + status: Ok, + tags: ~, + origin: ~, + profile_id: EventId( + a0aaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaab, + ), + data: SpanData { + app_start_type: ~, + browser_name: "Chrome", + code_filepath: ~, + code_lineno: ~, + code_function: ~, + code_namespace: ~, + db_operation: ~, + db_system: ~, + environment: "prod", + release: LenientString( + "myapp@1.0.0", + ), + http_decoded_response_content_length: ~, + http_request_method: ~, + http_response_content_length: ~, + http_response_transfer_size: ~, + resource_render_blocking_status: ~, + server_address: ~, + cache_hit: ~, + cache_item_size: ~, + http_response_status_code: ~, + ai_input_messages: ~, + ai_completion_tokens_used: ~, + ai_prompt_tokens_used: ~, + ai_total_tokens_used: ~, + ai_responses: ~, + thread_name: ~, + segment_name: "my 1st transaction", + ui_component_name: ~, + url_scheme: ~, + user: ~, + replay_id: ~, + sdk_name: "sentry.php", + other: {}, + }, + sentry_tags: ~, + received: ~, + measurements: ~, + _metrics_summary: ~, + platform: "php", + was_transaction: ~, + other: {}, + } + "###); + } }