From 12c5a463c95534bc9cd914ba344528f33c2ba3d3 Mon Sep 17 00:00:00 2001 From: Joris Bayer Date: Wed, 17 Apr 2024 14:11:38 +0200 Subject: [PATCH 01/20] feat(spans): Convert data fields --- .../src/protocol/span/convert.rs | 300 ++++++++++-------- relay-event-schema/src/protocol/types.rs | 14 + relay-protocol/src/value.rs | 15 + 3 files changed, 197 insertions(+), 132 deletions(-) diff --git a/relay-event-schema/src/protocol/span/convert.rs b/relay-event-schema/src/protocol/span/convert.rs index b03a5aa8d6e..45bb949218f 100644 --- a/relay-event-schema/src/protocol/span/convert.rs +++ b/relay-event-schema/src/protocol/span/convert.rs @@ -1,124 +1,130 @@ //! This module defines bidirectional field mappings between spans and transactions. -use crate::protocol::{ - ContextInner, Contexts, DefaultContext, Event, ProfileContext, Span, TraceContext, -}; +use crate::protocol::{Contexts, Event, ProfileContext, Span, TraceContext}; use relay_base_schema::events::EventType; use relay_protocol::Annotated; -use std::collections::BTreeMap; + +macro_rules! write_path( + ($root:expr, $path_root:ident $($path_segment:ident)*) => { + { + let annotated = &mut ($root).$path_root; + $( + let value = annotated.get_or_insert_with(Default::default); + let annotated = &mut value.$path_segment; + )* + annotated + } + }; +); + +macro_rules! read_value( + ($span:expr, $path_root:ident $($path_segment:ident)*) => { + { + let value = ($span).$path_root.value(); + $( + let value = value.and_then(|value| value.$path_segment.value()); + )* + value + } + }; +); + +macro_rules! context_write_path ( + ($event:expr, $ContextType:ty, $context_field:ident) => { + { + let contexts = $event.contexts.get_or_insert_with(Contexts::default); + let context = contexts.get_or_default::<$ContextType>(); + write_path!(context, $context_field) + } + }; +); + +macro_rules! event_write_path( + ($event:expr, contexts trace $context_field:ident) => { + context_write_path!($event, TraceContext, $context_field) + }; + ($event:expr, contexts profile $context_field:ident) => { + context_write_path!($event, ProfileContext, $context_field) + }; + ($event:expr, $path_root:ident $(. $path_segment:ident)*) => { + { + write_path!($event, $path_root $(. $path_segment:ident)*) + } + }; +); + +macro_rules! context_value ( + ($event:expr, $ContextType:ty, $context_field:ident) => { + { + let context = &($event).context::<$ContextType>(); + context.map_or(None, |ctx|ctx.$context_field.value()) + } + }; +); + +macro_rules! event_value( + ($event:expr, contexts trace $context_field:ident) => { + context_value!($event, TraceContext, $context_field) + }; + ($event:expr, contexts profile $context_field:ident) => { + context_value!($event, ProfileContext, $context_field) + }; + ($event:expr, $path_root:ident $(. $path_segment:ident)*) => { + { + let value = ($event).$path_root.value(); + $( + let value = value.and_then(|value|&value.$path_segment.value()); + )* + value + } + }; +); /// Implements the conversion between transaction events and segment spans. /// /// Invoking this macro implements both `From<&Event> for Span` and `From<&Span> for Event`. -/// -/// Example: -/// -/// ```ignore -/// map_fields!( -/// top_level { -/// span.received <=> event.received -/// } -/// contexts { -/// TraceContext { -/// span.trace_id <=> context.trace_id -/// } -/// } -/// fixed_for_span { -/// // ... -/// } -/// fixed_for_event { -/// // ... -/// } -/// ); -/// ``` -#[macro_export] macro_rules! map_fields { ( - top_level { - $(span.$span_field:ident <=> event.$event_field:ident), * - } - contexts { - $( - $ContextType:ident { - $(span.$primary_span_field:ident $(, $(span.$additional_span_field:ident),+)? <=> context.$context_field:ident), * - } - )* - } - fixed_for_span { - $(span.$fixed_span_field:ident <= $fixed_span_value:expr), * - } - fixed_for_event { - $($fixed_event_value:expr => event.$fixed_event_field:ident), * - } + $(span $(. $span_path:ident)+ <=> event $(. $event_path:ident)+),+ + ; + $(span . $fixed_span_path:tt <= $fixed_span_field:expr),+ + ; + $($fixed_event_field:expr => event . $fixed_event_path:tt),+ ) => { - #[allow(clippy::needless_update)] impl From<&Event> for Span { fn from(event: &Event) -> Self { - Self { - $( - $span_field: event.$event_field.clone().map_value(Into::into), - )* - $( - $( - $primary_span_field: event.context::<$ContextType>() - .map_or(None, |ctx|ctx.$context_field.value().cloned()).into(), - $( - $( - $additional_span_field: event.context::<$ContextType>() - .map_or(None, |ctx|ctx.$context_field.value().cloned()).into(), - )+ - )? - )* - )* - $( - $fixed_span_field: $fixed_span_value.into(), - )* - ..Default::default() - } + let mut span = Span::default(); + $( + if let Some(value) = event_value!(event, $($event_path)+) { + *write_path!(&mut span, $($span_path)+) = Annotated::new(value.clone()).map_value(Into::into); + } + )+ + $( + *write_path!(&mut span, $fixed_span_path) = Annotated::new($fixed_span_field); + )+ + span } } - #[allow(clippy::needless_update)] impl TryFrom<&Span> for Event { type Error = (); fn try_from(span: &Span) -> Result { - use relay_protocol::Empty; + // use relay_protocol::Empty; - if !span.is_segment.value().unwrap_or(&false) { - // Only segment spans can become transactions. - return Err(()); - } - let event = Self { - $( - $event_field: span.$span_field.clone().map_value(Into::into), - )* - $( - $fixed_event_field: $fixed_event_value.into(), - )* - contexts: Annotated::new( - Contexts({ - let mut contexts = BTreeMap::new(); - $( - let mut context = $ContextType::default(); - let mut has_fields = false; - $( - if !span.$primary_span_field.is_empty() { - context.$context_field = span.$primary_span_field.clone(); - has_fields = true; - } - )* - if has_fields { - let context_key = <$ContextType as DefaultContext>::default_key().into(); - contexts.insert(context_key, ContextInner(context.into_context()).into()); - } - )* - contexts - }) - ), - ..Default::default() - }; + let mut event = Event::default(); + $( + if let Some(value) = read_value!(span, $($span_path)+) { + *event_write_path!(&mut event, $($event_path)+) = match value.clone().try_into() { + Ok(value) => Annotated::new(value), + Err(_) => {return Err(())} + }; + } + )+ + $( + *event_write_path!(&mut event, $fixed_event_path) = Annotated::new($fixed_event_field); + )+ Ok(event) } @@ -129,38 +135,30 @@ macro_rules! map_fields { // This macro call implements a bidirectional mapping between transaction event and segment spans, // allowing users to call both `Event::from(&span)` and `Span::from(&event)`. map_fields!( - top_level { - span._metrics_summary <=> event._metrics_summary, - span.description <=> event.transaction, - span.measurements <=> event.measurements, - span.platform <=> event.platform, - span.received <=> event.received, - span.start_timestamp <=> event.start_timestamp, - span.tags <=> event.tags, - span.timestamp <=> event.timestamp - } - contexts { - TraceContext { - span.exclusive_time <=> context.exclusive_time, - span.op <=> context.op, - span.parent_span_id <=> context.parent_span_id, - // A transaction corresponds to a segment span, so span_id and segment_id have the same value: - span.span_id, span.segment_id <=> context.span_id, - span.status <=> context.status, - span.trace_id <=> context.trace_id - } - ProfileContext { - span.profile_id <=> context.profile_id - } - } - fixed_for_span { - // A transaction event corresponds to a segment span. - span.is_segment <= true, - span.was_transaction <= true - } - fixed_for_event { - EventType::Transaction => event.ty - } + span._metrics_summary <=> event._metrics_summary, + span.description <=> event.transaction, + span.measurements <=> event.measurements, + span.platform <=> event.platform, + span.received <=> event.received, + span.start_timestamp <=> event.start_timestamp, + span.tags <=> event.tags, + span.timestamp <=> event.timestamp, + span.exclusive_time <=> event.contexts.trace.exclusive_time, + span.op <=> event.contexts.trace.op, + span.parent_span_id <=> event.contexts.trace.parent_span_id, + // A transaction corresponds to a segment span, so span_id and segment_id have the same value: + span.span_id <=> event.contexts.trace.span_id, + span.segment_id <=> event.contexts.trace.span_id, + span.status <=> event.contexts.trace.status, + span.trace_id <=> event.contexts.trace.trace_id, + span.profile_id <=> event.contexts.profile.profile_id, + span.data.release <=> event.release, + span.data.environment <=> event.environment + ; + span.is_segment <= true, + span.was_transaction <= true + ; + EventType::Transaction => event.ty ); #[cfg(test)] @@ -174,6 +172,8 @@ mod tests { let event = Annotated::::from_json( r#"{ "type": "transaction", + "release": "myapp@1.0.0", + "environment": "prod", "contexts": { "profile": {"profile_id": "a0aaaaaaaaaaaaaaaaaaaaaaaaaaaaab"}, "trace": { @@ -238,7 +238,43 @@ mod tests { profile_id: EventId( a0aaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaab, ), - data: ~, + data: SpanData { + app_start_type: ~, + browser_name: ~, + code_filepath: ~, + code_lineno: ~, + code_function: ~, + code_namespace: ~, + db_operation: ~, + db_system: ~, + environment: String( + "prod", + ), + release: String( + "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: ~, + transaction: ~, + ui_component_name: ~, + url_scheme: ~, + user: ~, + replay_id: ~, + other: {}, + }, sentry_tags: ~, received: ~, measurements: Measurements( diff --git a/relay-event-schema/src/protocol/types.rs b/relay-event-schema/src/protocol/types.rs index c423a31d5d3..a501a4dd403 100644 --- a/relay-event-schema/src/protocol/types.rs +++ b/relay-event-schema/src/protocol/types.rs @@ -849,6 +849,20 @@ impl FromValue for LenientString { } } +impl From for Value { + fn from(value: LenientString) -> Self { + value.into_value() + } +} + +impl TryFrom for LenientString { + type Error = (); + + fn try_from(value: Value) -> Result { + Self::from_value(value.into()).into_value().ok_or(()) + } +} + /// A "into-string" type of value. All non-string values are serialized as JSON. #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Empty, IntoValue, ProcessValue)] #[cfg_attr(feature = "jsonschema", derive(JsonSchema))] diff --git a/relay-protocol/src/value.rs b/relay-protocol/src/value.rs index be774eddfc7..40ed6e226ea 100644 --- a/relay-protocol/src/value.rs +++ b/relay-protocol/src/value.rs @@ -132,6 +132,21 @@ impl TryFrom<&Value> for String { } } +impl TryFrom for String { + type Error = (); + + fn try_from(value: Value) -> Result { + Ok(match value { + Value::Bool(v) => v.to_string(), + Value::I64(v) => v.to_string(), + Value::U64(v) => v.to_string(), + Value::F64(v) => v.to_string(), + Value::String(v) => v, + _ => return Err(()), + }) + } +} + impl Serialize for Value { fn serialize(&self, serializer: S) -> Result where From b089c2503f5527892dddf6c9df62b65ec54abe17 Mon Sep 17 00:00:00 2001 From: Joris Bayer Date: Thu, 18 Apr 2024 10:59:05 +0200 Subject: [PATCH 02/20] fix: parent span and is_segment condition --- relay-event-schema/src/protocol/contexts/trace.rs | 3 ++- relay-event-schema/src/protocol/span/convert.rs | 7 +++++-- relay-server/src/services/processor/event.rs | 2 +- relay-spans/src/span.rs | 13 ++++++++----- tests/integration/test_spans.py | 2 -- 5 files changed, 16 insertions(+), 11 deletions(-) diff --git a/relay-event-schema/src/protocol/contexts/trace.rs b/relay-event-schema/src/protocol/contexts/trace.rs index c9e2e86ec50..5bfa1f297e4 100644 --- a/relay-event-schema/src/protocol/contexts/trace.rs +++ b/relay-event-schema/src/protocol/contexts/trace.rs @@ -64,9 +64,10 @@ impl fmt::Display for SpanId { impl FromValue for SpanId { fn from_value(value: Annotated) -> Annotated { - match value { + match dbg!(value) { Annotated(Some(Value::String(value)), mut meta) => { if !is_hex_string(&value, 16) || value.bytes().all(|x| x == b'0') { + dbg!("error", &value); meta.add_error(Error::invalid("not a valid span id")); meta.set_original_value(Some(value)); Annotated(None, meta) diff --git a/relay-event-schema/src/protocol/span/convert.rs b/relay-event-schema/src/protocol/span/convert.rs index 45bb949218f..edf57b5ff02 100644 --- a/relay-event-schema/src/protocol/span/convert.rs +++ b/relay-event-schema/src/protocol/span/convert.rs @@ -110,10 +110,13 @@ macro_rules! map_fields { type Error = (); fn try_from(span: &Span) -> Result { - // use relay_protocol::Empty; - let mut event = Event::default(); + if !span.is_segment.value().unwrap_or(&false) { + // Only segment spans can become transactions. + return Err(()); + } + $( if let Some(value) = read_value!(span, $($span_path)+) { *event_write_path!(&mut event, $($event_path)+) = match value.clone().try_into() { diff --git a/relay-server/src/services/processor/event.rs b/relay-server/src/services/processor/event.rs index 0d5fa060f4f..7e3733065f7 100644 --- a/relay-server/src/services/processor/event.rs +++ b/relay-server/src/services/processor/event.rs @@ -84,7 +84,7 @@ pub fn extract( metric!(timer(RelayTimers::EventProcessingDeserialize), { // Transaction items can only contain transaction events. Force the event type to // hint to normalization that we're dealing with a transaction now. - event_from_json_payload(item, Some(EventType::Transaction))? + event_from_json_payload(dbg!(item), Some(EventType::Transaction))? }) } else if let Some(item) = user_report_v2_item { relay_log::trace!("processing user_report_v2"); diff --git a/relay-spans/src/span.rs b/relay-spans/src/span.rs index e1f69cca3db..31b5e926673 100644 --- a/relay-spans/src/span.rs +++ b/relay-spans/src/span.rs @@ -83,12 +83,15 @@ pub fn otel_to_sentry_span(otel_span: OtelSpan) -> EventSpan { let span_id = hex::encode(span_id); let trace_id = hex::encode(trace_id); - let parent_span_id = hex::encode(parent_span_id); + let parent_span_id = match parent_span_id.as_slice() { + &[] => None, + _ => Some(hex::encode(parent_span_id)), + }; - // TODO: This is wrong, a segment could still have a parent in the trace. - let segment_id = if parent_span_id.is_empty() { + let segment_id = if parent_span_id.is_none() { Annotated::new(SpanId(span_id.clone())) } else { + // TODO: derive from attributes Annotated::empty() }; @@ -167,7 +170,7 @@ pub fn otel_to_sentry_span(otel_span: OtelSpan) -> EventSpan { } // TODO: This is wrong, a segment could still have a parent in the trace. - let is_segment = parent_span_id.is_empty().into(); + 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); @@ -178,7 +181,7 @@ pub fn otel_to_sentry_span(otel_span: OtelSpan) -> EventSpan { description: description.into(), data: SpanData::from_value(Annotated::new(data.into())), exclusive_time: exclusive_time_ms.into(), - parent_span_id: SpanId(parent_span_id).into(), + parent_span_id: parent_span_id.map(SpanId).into(), segment_id, span_id: Annotated::new(SpanId(span_id)), start_timestamp: Timestamp(start_timestamp).into(), diff --git a/tests/integration/test_spans.py b/tests/integration/test_spans.py index 01ba7edba12..8d9196b8db5 100644 --- a/tests/integration/test_spans.py +++ b/tests/integration/test_spans.py @@ -496,7 +496,6 @@ def test_span_ingestion( "exclusive_time_ms": 500.0, "is_segment": True, "organization_id": 1, - "parent_span_id": "", "project_id": 42, "retention_days": 90, "segment_id": "a342abb1214ca181", @@ -552,7 +551,6 @@ def test_span_ingestion( "exclusive_time_ms": 500.0, "is_segment": True, "organization_id": 1, - "parent_span_id": "", "project_id": 42, "retention_days": 90, "segment_id": "d342abb1214ca182", From 0cfeb26f108160c8838c5fc02ca9d1dee48e2fc2 Mon Sep 17 00:00:00 2001 From: Joris Bayer Date: Thu, 18 Apr 2024 12:06:48 +0200 Subject: [PATCH 03/20] ref: dbg --- relay-event-schema/src/protocol/contexts/trace.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/relay-event-schema/src/protocol/contexts/trace.rs b/relay-event-schema/src/protocol/contexts/trace.rs index 5bfa1f297e4..c9e2e86ec50 100644 --- a/relay-event-schema/src/protocol/contexts/trace.rs +++ b/relay-event-schema/src/protocol/contexts/trace.rs @@ -64,10 +64,9 @@ impl fmt::Display for SpanId { impl FromValue for SpanId { fn from_value(value: Annotated) -> Annotated { - match dbg!(value) { + match value { Annotated(Some(Value::String(value)), mut meta) => { if !is_hex_string(&value, 16) || value.bytes().all(|x| x == b'0') { - dbg!("error", &value); meta.add_error(Error::invalid("not a valid span id")); meta.set_original_value(Some(value)); Annotated(None, meta) From d429e7f848980f6dffde892b388c699e22695e46 Mon Sep 17 00:00:00 2001 From: Joris Bayer Date: Thu, 18 Apr 2024 12:59:59 +0200 Subject: [PATCH 04/20] dbg again --- relay-server/src/services/processor/event.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/relay-server/src/services/processor/event.rs b/relay-server/src/services/processor/event.rs index 7e3733065f7..0d5fa060f4f 100644 --- a/relay-server/src/services/processor/event.rs +++ b/relay-server/src/services/processor/event.rs @@ -84,7 +84,7 @@ pub fn extract( metric!(timer(RelayTimers::EventProcessingDeserialize), { // Transaction items can only contain transaction events. Force the event type to // hint to normalization that we're dealing with a transaction now. - event_from_json_payload(dbg!(item), Some(EventType::Transaction))? + event_from_json_payload(item, Some(EventType::Transaction))? }) } else if let Some(item) = user_report_v2_item { relay_log::trace!("processing user_report_v2"); From 0a12ca675562f78c703fa0f11fe9402cd1a47fc3 Mon Sep 17 00:00:00 2001 From: Joris Bayer Date: Thu, 18 Apr 2024 13:13:39 +0200 Subject: [PATCH 05/20] Type out span.data --- relay-event-schema/src/protocol/span.rs | 10 +++++----- relay-event-schema/src/protocol/span/convert.rs | 5 +---- relay-event-schema/src/protocol/types.rs | 14 -------------- relay-protocol/src/value.rs | 15 --------------- 4 files changed, 6 insertions(+), 38 deletions(-) diff --git a/relay-event-schema/src/protocol/span.rs b/relay-event-schema/src/protocol/span.rs index 09bae7c4854..badf42f49f7 100644 --- a/relay-event-schema/src/protocol/span.rs +++ b/relay-event-schema/src/protocol/span.rs @@ -6,8 +6,8 @@ use relay_protocol::{Annotated, Empty, FromValue, Getter, IntoValue, Object, Val use crate::processor::ProcessValue; use crate::protocol::{ - EventId, JsonLenientString, Measurements, MetricsSummary, OperationType, OriginType, SpanId, - SpanStatus, Timestamp, TraceId, + EventId, JsonLenientString, LenientString, Measurements, MetricsSummary, OperationType, + OriginType, SpanId, SpanStatus, Timestamp, TraceId, }; #[derive(Clone, Debug, Default, PartialEq, Empty, FromValue, IntoValue, ProcessValue)] @@ -201,11 +201,11 @@ pub struct SpanData { /// The sentry environment. #[metastructure(field = "environment")] - pub environment: Annotated, + pub environment: Annotated, /// The release version of the project. #[metastructure(field = "release")] - pub release: Annotated, + pub release: Annotated, /// The decoded body size of the response (in bytes). #[metastructure(field = "http.decoded_response_content_length")] @@ -308,7 +308,7 @@ impl Getter for SpanData { "code\\.namespace" => self.code_namespace.value()?.into(), "db.operation" => self.db_operation.value()?.into(), "db\\.system" => self.db_system.value()?.into(), - "environment" => self.environment.value()?.into(), + "environment" => self.environment.as_str()?.into(), "http\\.decoded_response_content_length" => { self.http_decoded_response_content_length.value()?.into() } diff --git a/relay-event-schema/src/protocol/span/convert.rs b/relay-event-schema/src/protocol/span/convert.rs index edf57b5ff02..6f3320f8719 100644 --- a/relay-event-schema/src/protocol/span/convert.rs +++ b/relay-event-schema/src/protocol/span/convert.rs @@ -119,10 +119,7 @@ macro_rules! map_fields { $( if let Some(value) = read_value!(span, $($span_path)+) { - *event_write_path!(&mut event, $($event_path)+) = match value.clone().try_into() { - Ok(value) => Annotated::new(value), - Err(_) => {return Err(())} - }; + *event_write_path!(&mut event, $($event_path)+) = Annotated::new(value.clone()).map_value(Into::into) } )+ $( diff --git a/relay-event-schema/src/protocol/types.rs b/relay-event-schema/src/protocol/types.rs index a501a4dd403..c423a31d5d3 100644 --- a/relay-event-schema/src/protocol/types.rs +++ b/relay-event-schema/src/protocol/types.rs @@ -849,20 +849,6 @@ impl FromValue for LenientString { } } -impl From for Value { - fn from(value: LenientString) -> Self { - value.into_value() - } -} - -impl TryFrom for LenientString { - type Error = (); - - fn try_from(value: Value) -> Result { - Self::from_value(value.into()).into_value().ok_or(()) - } -} - /// A "into-string" type of value. All non-string values are serialized as JSON. #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Empty, IntoValue, ProcessValue)] #[cfg_attr(feature = "jsonschema", derive(JsonSchema))] diff --git a/relay-protocol/src/value.rs b/relay-protocol/src/value.rs index 40ed6e226ea..be774eddfc7 100644 --- a/relay-protocol/src/value.rs +++ b/relay-protocol/src/value.rs @@ -132,21 +132,6 @@ impl TryFrom<&Value> for String { } } -impl TryFrom for String { - type Error = (); - - fn try_from(value: Value) -> Result { - Ok(match value { - Value::Bool(v) => v.to_string(), - Value::I64(v) => v.to_string(), - Value::U64(v) => v.to_string(), - Value::F64(v) => v.to_string(), - Value::String(v) => v, - _ => return Err(()), - }) - } -} - impl Serialize for Value { fn serialize(&self, serializer: S) -> Result where From 0ad1b7892e12cd50f86230ae805cfc488e5cbc3d Mon Sep 17 00:00:00 2001 From: Joris Bayer Date: Thu, 18 Apr 2024 13:26:00 +0200 Subject: [PATCH 06/20] test: update snapshot --- relay-event-schema/src/protocol/span/convert.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/relay-event-schema/src/protocol/span/convert.rs b/relay-event-schema/src/protocol/span/convert.rs index 6f3320f8719..f2c5ac1dd39 100644 --- a/relay-event-schema/src/protocol/span/convert.rs +++ b/relay-event-schema/src/protocol/span/convert.rs @@ -247,10 +247,8 @@ mod tests { code_namespace: ~, db_operation: ~, db_system: ~, - environment: String( - "prod", - ), - release: String( + environment: "prod", + release: LenientString( "myapp@1.0.0", ), http_decoded_response_content_length: ~, From b409f25ceb0421bdac9070ed7018cc8e46e00a4f Mon Sep 17 00:00:00 2001 From: Joris Bayer Date: Thu, 18 Apr 2024 15:27:43 +0200 Subject: [PATCH 07/20] feat(spans): Add more fields to span <-> transaction conversion --- .../src/normalize/span/tag_extraction.rs | 15 +++--- relay-event-schema/src/protocol/span.rs | 17 +++--- .../src/protocol/span/convert.rs | 53 ++++++++++++------- .../src/services/processor/span/processing.rs | 4 +- 4 files changed, 52 insertions(+), 37 deletions(-) diff --git a/relay-event-normalization/src/normalize/span/tag_extraction.rs b/relay-event-normalization/src/normalize/span/tag_extraction.rs index b0726014241..6720781d7d8 100644 --- a/relay-event-normalization/src/normalize/span/tag_extraction.rs +++ b/relay-event-normalization/src/normalize/span/tag_extraction.rs @@ -505,8 +505,10 @@ pub fn extract_tags( } if let Some(measurements) = span.measurements.value() { if span_op.starts_with("ui.interaction.") && measurements.contains_key("inp") { - if let Some(transaction) = - span.data.value().and_then(|data| data.transaction.as_str()) + if let Some(transaction) = span + .data + .value() + .and_then(|data| data.segment_name.as_str()) { span_tags.insert(SpanTagKey::Transaction, transaction.into()); } @@ -572,13 +574,8 @@ pub fn extract_tags( } } - if let Some(browser_name) = span - .data - .value() - .and_then(|data| data.browser_name.value()) - .and_then(|browser_name| browser_name.as_str()) - { - span_tags.insert(SpanTagKey::BrowserName, browser_name.into()); + if let Some(browser_name) = span.data.value().and_then(|data| data.browser_name.value()) { + span_tags.insert(SpanTagKey::BrowserName, browser_name.clone()); } span_tags diff --git a/relay-event-schema/src/protocol/span.rs b/relay-event-schema/src/protocol/span.rs index badf42f49f7..4138e6b2856 100644 --- a/relay-event-schema/src/protocol/span.rs +++ b/relay-event-schema/src/protocol/span.rs @@ -165,7 +165,7 @@ pub struct SpanData { /// The client's browser name. #[metastructure(field = "browser.name")] - pub browser_name: Annotated, + pub browser_name: Annotated, /// The source code file name that identifies the code unit as uniquely as possible. #[metastructure(field = "code.filepath", pii = "maybe")] @@ -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). @@ -274,7 +274,8 @@ pub struct SpanData { /// Origin Transaction name of the span. /// /// For INP spans, this is the route name where the interaction occurred. - pub transaction: Annotated, + #[metastructure(field = "sentry.segment.name", legacy_alias = "transaction")] + pub segment_name: Annotated, /// Name of the UI component (e.g. React). #[metastructure(field = "ui.component_name")] @@ -292,6 +293,10 @@ pub struct SpanData { #[metastructure(field = "replay_id")] pub replay_id: Annotated, + #[metastructure(field = "sentry.sdk.name")] + /// The sentry SDK (see [`crate::protocol::ClientSdkInfo`]). + pub sdk_name: Annotated, + /// Other fields in `span.data`. #[metastructure(additional_properties, pii = "true", retain = "true")] other: Object, @@ -301,7 +306,7 @@ impl Getter for SpanData { fn get_value(&self, path: &str) -> Option> { Some(match path { "app_start_type" => self.app_start_type.value()?.into(), - "browser\\.name" => self.browser_name.value()?.into(), + "browser\\.name" => self.browser_name.as_str()?.into(), "code\\.filepath" => self.code_filepath.value()?.into(), "code\\.function" => self.code_function.value()?.into(), "code\\.lineno" => self.code_lineno.value()?.into(), @@ -327,7 +332,7 @@ impl Getter for SpanData { "thread\\.name" => self.thread_name.value()?.into(), "ui\\.component_name" => self.ui_component_name.value()?.into(), "url\\.scheme" => self.url_scheme.value()?.into(), - "transaction" => self.transaction.as_str()?.into(), + "transaction" => self.segment_name.as_str()?.into(), _ => { let escaped = path.replace("\\.", "\0"); let mut path = escaped.split('.').map(|s| s.replace('\0', ".")); diff --git a/relay-event-schema/src/protocol/span/convert.rs b/relay-event-schema/src/protocol/span/convert.rs index f2c5ac1dd39..5581db962f6 100644 --- a/relay-event-schema/src/protocol/span/convert.rs +++ b/relay-event-schema/src/protocol/span/convert.rs @@ -1,5 +1,5 @@ //! This module defines bidirectional field mappings between spans and transactions. -use crate::protocol::{Contexts, Event, ProfileContext, Span, TraceContext}; +use crate::protocol::{BrowserContext, Contexts, Event, ProfileContext, Span, TraceContext}; use relay_base_schema::events::EventType; use relay_protocol::Annotated; @@ -40,15 +40,18 @@ macro_rules! context_write_path ( ); macro_rules! event_write_path( + ($event:expr, contexts browser $context_field:ident) => { + context_write_path!($event, BrowserContext, $context_field) + }; ($event:expr, contexts trace $context_field:ident) => { context_write_path!($event, TraceContext, $context_field) }; ($event:expr, contexts profile $context_field:ident) => { context_write_path!($event, ProfileContext, $context_field) }; - ($event:expr, $path_root:ident $(. $path_segment:ident)*) => { + ($event:expr, $path_root:ident $($path_segment:ident)*) => { { - write_path!($event, $path_root $(. $path_segment:ident)*) + write_path!($event, $path_root $($path_segment)*) } }; ); @@ -63,17 +66,20 @@ macro_rules! context_value ( ); macro_rules! event_value( + ($event:expr, contexts browser $context_field:ident) => { + context_value!($event, BrowserContext, $context_field) + }; ($event:expr, contexts trace $context_field:ident) => { context_value!($event, TraceContext, $context_field) }; ($event:expr, contexts profile $context_field:ident) => { context_value!($event, ProfileContext, $context_field) }; - ($event:expr, $path_root:ident $(. $path_segment:ident)*) => { + ($event:expr, $path_root:ident $($path_segment:ident)*) => { { let value = ($event).$path_root.value(); $( - let value = value.and_then(|value|&value.$path_segment.value()); + let value = value.and_then(|value|value.$path_segment.value()); )* value } @@ -136,24 +142,26 @@ macro_rules! map_fields { // allowing users to call both `Event::from(&span)` and `Span::from(&event)`. map_fields!( span._metrics_summary <=> event._metrics_summary, - span.description <=> event.transaction, + span.data.browser_name <=> event.contexts.browser.name, + span.data.environment <=> event.environment, + span.data.sdk_name <=> event.client_sdk.name, + span.data.release <=> event.release, + span.data.segment_name <=> event.transaction, + span.exclusive_time <=> event.contexts.trace.exclusive_time, span.measurements <=> event.measurements, + span.op <=> event.contexts.trace.op, + span.parent_span_id <=> event.contexts.trace.parent_span_id, span.platform <=> event.platform, + span.profile_id <=> event.contexts.profile.profile_id, span.received <=> event.received, + span.segment_id <=> event.contexts.trace.span_id, + span.span_id <=> event.contexts.trace.span_id, span.start_timestamp <=> event.start_timestamp, + span.status <=> event.contexts.trace.status, span.tags <=> event.tags, span.timestamp <=> event.timestamp, - span.exclusive_time <=> event.contexts.trace.exclusive_time, - span.op <=> event.contexts.trace.op, - span.parent_span_id <=> event.contexts.trace.parent_span_id, - // A transaction corresponds to a segment span, so span_id and segment_id have the same value: - span.span_id <=> event.contexts.trace.span_id, - span.segment_id <=> event.contexts.trace.span_id, - span.status <=> event.contexts.trace.status, - span.trace_id <=> event.contexts.trace.trace_id, - span.profile_id <=> event.contexts.profile.profile_id, - span.data.release <=> event.release, - span.data.environment <=> event.environment + span.trace_id <=> event.contexts.trace.trace_id + ; span.is_segment <= true, span.was_transaction <= true @@ -172,9 +180,13 @@ mod tests { let event = Annotated::::from_json( r#"{ "type": "transaction", + "platform": "php", + "sdk": {"name": "sentry.php"}, "release": "myapp@1.0.0", "environment": "prod", + "transaction": "my 1st transaction", "contexts": { + "browser": {"name": "Chrome"}, "profile": {"profile_id": "a0aaaaaaaaaaaaaaaaaaaaaaaaaaaaab"}, "trace": { "trace_id": "4C79F60C11214EB38604F4AE0781BFB2", @@ -240,7 +252,7 @@ mod tests { ), data: SpanData { app_start_type: ~, - browser_name: ~, + browser_name: "Chrome", code_filepath: ~, code_lineno: ~, code_function: ~, @@ -266,11 +278,12 @@ mod tests { ai_total_tokens_used: ~, ai_responses: ~, thread_name: ~, - transaction: ~, + segment_name: "my 1st transaction", ui_component_name: ~, url_scheme: ~, user: ~, replay_id: ~, + sdk_name: "sentry.php", other: {}, }, sentry_tags: ~, @@ -300,7 +313,7 @@ mod tests { ], }, ), - platform: ~, + platform: "php", was_transaction: true, other: {}, } diff --git a/relay-server/src/services/processor/span/processing.rs b/relay-server/src/services/processor/span/processing.rs index dc1f58c2c91..f8b80f1d293 100644 --- a/relay-server/src/services/processor/span/processing.rs +++ b/relay-server/src/services/processor/span/processing.rs @@ -484,7 +484,7 @@ fn normalize( .and_then(|v| v.name.value()) { let data = span.data.value_mut().get_or_insert_with(SpanData::default); - data.browser_name = Annotated::new(browser_name.to_owned().into()); + data.browser_name = Annotated::new(browser_name.to_owned()); } if let Annotated(Some(ref mut measurement_values), ref mut meta) = span.measurements { @@ -504,7 +504,7 @@ fn normalize( .data .value_mut() .as_mut() - .map(|data| &mut data.transaction) + .map(|data| &mut data.segment_name) { normalize_transaction_name(transaction, &project_config.tx_name_rules); } From c761aac7aef5acbd4627c46c335642c633c397e4 Mon Sep 17 00:00:00 2001 From: Joris Bayer Date: Thu, 18 Apr 2024 16:15:00 +0200 Subject: [PATCH 08/20] test --- Cargo.lock | 1 + relay-spans/Cargo.toml | 9 ++- relay-spans/src/span.rs | 165 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 174 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 6a1532d0c49..75a62ae0c64 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4203,6 +4203,7 @@ dependencies = [ "chrono", "enumset", "hex", + "insta", "once_cell", "opentelemetry-proto", "relay-event-schema", diff --git a/relay-spans/Cargo.toml b/relay-spans/Cargo.toml index 5e70d8c67d5..41ccf28741c 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/span.rs b/relay-spans/src/span.rs index 31b5e926673..a9487254cf1 100644 --- a/relay-spans/src/span.rs +++ b/relay-spans/src/span.rs @@ -391,4 +391,169 @@ 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_everything() { + let json = r#"{ + "traceId": "4c79f60c11214eb38604f4ae0781bfb2", + "spanId": "fa90fdead5f74052", + "parentSpanId": "fa90fdead5f74051", + "startTimeUnixNano": 123000000000, + "endTimeUnixNano": 123500000000, + "attributes": [ + { + "key" : "sentry.op", + "value": { + "stringValue": "myop" + } + }, + { + "key" : "sentry.segment.id", + "value": { + "stringValue": "fa90fdead5f74052" + } + }, + { + "key" : "sentry.segment.name", + "value": { + "stringValue": "my 1st transaction" + } + }, + { + "key" : "sentry.profile.id", + "value": { + "stringValue": "a0aaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaab" + } + }, + { + "key" : "sentry.browser.name", + "value": { + "stringValue": "Chrome" + } + }, + { + "key" : "sentry.environment", + "value": { + "stringValue": "prod" + } + }, + { + "key" : "sentry.release", + "value": { + "stringValue": "myapp@1.0.0" + } + }, + { + "key" : "sentry.sdk.name", + "value": { + "stringValue": "sentry.php" + } + }, + { + "key" : "sentry.platform", + "value": { + "stringValue": "php" + } + } + ] + }"#; + 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###" + Span { + timestamp: ~, + start_timestamp: ~, + exclusive_time: 123.4, + description: ~, + op: "myop", + span_id: SpanId( + "fa90fdead5f74052", + ), + parent_span_id: SpanId( + "fa90fdead5f74051", + ), + trace_id: TraceId( + "4c79f60c11214eb38604f4ae0781bfb2", + ), + segment_id: SpanId( + "fa90fdead5f74052", + ), + is_segment: true, + 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: Measurements( + { + "memory": Measurement { + value: 9001.0, + unit: Information( + Byte, + ), + }, + }, + ), + _metrics_summary: MetricsSummary( + { + "some_metric": [ + MetricSummary { + min: 1.0, + max: 2.0, + sum: 3.0, + count: 2, + tags: { + "environment": "test", + }, + }, + ], + }, + ), + platform: "php", + was_transaction: true, + other: {}, + } + "###); + } } From d2ffc6829eca28fc3cd7fba5f8a7785ac1895822 Mon Sep 17 00:00:00 2001 From: Joris Bayer Date: Thu, 18 Apr 2024 16:28:14 +0200 Subject: [PATCH 09/20] test --- relay-spans/src/span.rs | 65 +++++++++++++---------------------------- 1 file changed, 21 insertions(+), 44 deletions(-) diff --git a/relay-spans/src/span.rs b/relay-spans/src/span.rs index a9487254cf1..a3d2b51df69 100644 --- a/relay-spans/src/span.rs +++ b/relay-spans/src/span.rs @@ -394,7 +394,7 @@ mod tests { /// Intended to be synced with `relay-event-schema::protocol::span::convert::tests::roundtrip`. #[test] - fn parse_everything() { + fn parse_sentry_attributes() { let json = r#"{ "traceId": "4c79f60c11214eb38604f4ae0781bfb2", "spanId": "fa90fdead5f74052", @@ -403,39 +403,33 @@ mod tests { "endTimeUnixNano": 123500000000, "attributes": [ { - "key" : "sentry.op", - "value": { - "stringValue": "myop" - } - }, - { - "key" : "sentry.segment.id", + "key" : "sentry.browser.name", "value": { - "stringValue": "fa90fdead5f74052" + "stringValue": "Chrome" } }, { - "key" : "sentry.segment.name", + "key" : "sentry.environment", "value": { - "stringValue": "my 1st transaction" + "stringValue": "prod" } }, { - "key" : "sentry.profile.id", + "key" : "sentry.op", "value": { - "stringValue": "a0aaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaab" + "stringValue": "myop" } }, { - "key" : "sentry.browser.name", + "key" : "sentry.platform", "value": { - "stringValue": "Chrome" + "stringValue": "php" } }, { - "key" : "sentry.environment", + "key" : "sentry.profile.id", "value": { - "stringValue": "prod" + "stringValue": "a0aaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaab" } }, { @@ -451,9 +445,15 @@ mod tests { } }, { - "key" : "sentry.platform", + "key" : "sentry.segment.id", "value": { - "stringValue": "php" + "stringValue": "fa90fdead5f74052" + } + }, + { + "key" : "sentry.segment.name", + "value": { + "stringValue": "my 1st transaction" } } ] @@ -461,6 +461,8 @@ mod tests { 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: ~, @@ -525,31 +527,6 @@ mod tests { }, sentry_tags: ~, received: ~, - measurements: Measurements( - { - "memory": Measurement { - value: 9001.0, - unit: Information( - Byte, - ), - }, - }, - ), - _metrics_summary: MetricsSummary( - { - "some_metric": [ - MetricSummary { - min: 1.0, - max: 2.0, - sum: 3.0, - count: 2, - tags: { - "environment": "test", - }, - }, - ], - }, - ), platform: "php", was_transaction: true, other: {}, From 14cfbdbb0c40df8c368454b5c1ff96d10d1c244b Mon Sep 17 00:00:00 2001 From: Joris Bayer Date: Thu, 18 Apr 2024 16:50:39 +0200 Subject: [PATCH 10/20] revert rename (will be follow up) --- relay-event-schema/src/protocol/span.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/relay-event-schema/src/protocol/span.rs b/relay-event-schema/src/protocol/span.rs index 4138e6b2856..8356d9114c8 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 = "sentry.environment", legacy_alias = "environment")] + #[metastructure(field = "environment")] pub environment: Annotated, /// The release version of the project. - #[metastructure(field = "sentry.release", legacy_alias = "release")] + #[metastructure(field = "release")] pub release: Annotated, /// The decoded body size of the response (in bytes). @@ -274,7 +274,7 @@ pub struct SpanData { /// Origin Transaction name of the span. /// /// For INP spans, this is the route name where the interaction occurred. - #[metastructure(field = "sentry.segment.name", legacy_alias = "transaction")] + #[metastructure(field = "segment.name", legacy_alias = "transaction")] pub segment_name: Annotated, /// Name of the UI component (e.g. React). @@ -293,7 +293,7 @@ pub struct SpanData { #[metastructure(field = "replay_id")] pub replay_id: Annotated, - #[metastructure(field = "sentry.sdk.name")] + #[metastructure(field = "sdk.name")] /// The sentry SDK (see [`crate::protocol::ClientSdkInfo`]). pub sdk_name: Annotated, @@ -534,11 +534,12 @@ mod tests { ai_total_tokens_used: ~, ai_responses: ~, thread_name: ~, - transaction: ~, + segment_name: ~, ui_component_name: ~, url_scheme: ~, user: ~, replay_id: ~, + sdk_name: ~, other: { "bar": String( "3", From d03e8617824572e697db1c9687be8684efd71552 Mon Sep 17 00:00:00 2001 From: Joris Bayer Date: Thu, 18 Apr 2024 16:59:39 +0200 Subject: [PATCH 11/20] test: update another snapshot --- ...traction__event__tests__extract_span_metrics_mobile.snap | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) 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 7aa673ccb77..9de0b77f1cb 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 @@ -339,11 +339,12 @@ expression: "(&event.value().unwrap().spans, metrics)" ai_total_tokens_used: ~, ai_responses: ~, thread_name: ~, - transaction: ~, + segment_name: ~, ui_component_name: ~, url_scheme: ~, user: ~, replay_id: ~, + sdk_name: ~, other: {}, }, sentry_tags: { @@ -421,11 +422,12 @@ expression: "(&event.value().unwrap().spans, metrics)" ai_total_tokens_used: ~, ai_responses: ~, thread_name: ~, - transaction: ~, + segment_name: ~, ui_component_name: ~, url_scheme: ~, user: ~, replay_id: ~, + sdk_name: ~, other: {}, }, sentry_tags: { From bec18541f8496272e25a049f96477a89bb9deb62 Mon Sep 17 00:00:00 2001 From: Joris Bayer Date: Thu, 18 Apr 2024 17:17:31 +0200 Subject: [PATCH 12/20] simplify --- relay-event-schema/src/protocol/span.rs | 10 +-- relay-spans/src/lib.rs | 1 - relay-spans/src/otel_to_sentry_tags.rs | 34 ------- relay-spans/src/span.rs | 112 ++++++++++++------------ 4 files changed, 63 insertions(+), 94 deletions(-) delete mode 100644 relay-spans/src/otel_to_sentry_tags.rs diff --git a/relay-event-schema/src/protocol/span.rs b/relay-event-schema/src/protocol/span.rs index 8356d9114c8..d8929394286 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). @@ -274,7 +274,7 @@ pub struct SpanData { /// Origin Transaction name of the span. /// /// 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). @@ -290,10 +290,10 @@ 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, - #[metastructure(field = "sdk.name")] + #[metastructure(field = "sentry.sdk.name")] /// The sentry SDK (see [`crate::protocol::ClientSdkInfo`]). pub sdk_name: Annotated, diff --git a/relay-spans/src/lib.rs b/relay-spans/src/lib.rs index 028cea0e11f..c4dce6ce7d7 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 d8e2c47a9a2..00000000000 --- 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 a3d2b51df69..29a75f64bc4 100644 --- a/relay-spans/src/span.rs +++ b/relay-spans/src/span.rs @@ -8,7 +8,6 @@ use relay_event_schema::protocol::{ }; 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; @@ -103,64 +102,69 @@ pub fn otel_to_sentry_span(otel_span: OtelSpan) -> EventSpan { let mut grpc_status_code = 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 = statement; + } } - OtelValue::BytesValue(v) => { - if let Ok(v) = String::from_utf8(v) { + } + "http.method" | "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); + } + _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())); - } - }; + } } } } From ac94ab0544b857177c67945bfe8aab6a45ed4361 Mon Sep 17 00:00:00 2001 From: Joris Bayer Date: Thu, 18 Apr 2024 17:49:06 +0200 Subject: [PATCH 13/20] fix --- relay-spans/src/span.rs | 70 ++++++++++++++++++++++++++--------------- 1 file changed, 45 insertions(+), 25 deletions(-) diff --git a/relay-spans/src/span.rs b/relay-spans/src/span.rs index 29a75f64bc4..628bb5d7450 100644 --- a/relay-spans/src/span.rs +++ b/relay-spans/src/span.rs @@ -3,14 +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_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, @@ -63,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; @@ -87,19 +94,15 @@ 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) { match attribute.key.as_str() { @@ -110,7 +113,7 @@ pub fn otel_to_sentry_span(otel_span: OtelSpan) -> EventSpan { op = op.or(Some("db".to_string())); if key == "db.statement" { if let Some(statement) = otel_value_to_string(value) { - description = statement; + description = Some(statement); } } } @@ -141,6 +144,15 @@ pub fn otel_to_sentry_span(otel_span: OtelSpan) -> EventSpan { "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 { @@ -173,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 { @@ -186,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), @@ -196,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() } } @@ -405,9 +418,10 @@ mod tests { "parentSpanId": "fa90fdead5f74051", "startTimeUnixNano": 123000000000, "endTimeUnixNano": 123500000000, + "status": {"code": 0, "message": "foo"}, "attributes": [ { - "key" : "sentry.browser.name", + "key" : "browser.name", "value": { "stringValue": "Chrome" } @@ -469,9 +483,13 @@ mod tests { insta::assert_debug_snapshot!(span_from_otel, @r###" Span { - timestamp: ~, - start_timestamp: ~, - exclusive_time: 123.4, + 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( @@ -486,7 +504,7 @@ mod tests { segment_id: SpanId( "fa90fdead5f74052", ), - is_segment: true, + is_segment: ~, status: Ok, tags: ~, origin: ~, @@ -531,8 +549,10 @@ mod tests { }, sentry_tags: ~, received: ~, + measurements: ~, + _metrics_summary: ~, platform: "php", - was_transaction: true, + was_transaction: ~, other: {}, } "###); From f9218a9e730fca7bca44112ef0bd3252fc2dfaf5 Mon Sep 17 00:00:00 2001 From: Joris Bayer Date: Thu, 18 Apr 2024 17:56:42 +0200 Subject: [PATCH 14/20] fix: transaction description --- relay-event-schema/src/protocol/span/convert.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/relay-event-schema/src/protocol/span/convert.rs b/relay-event-schema/src/protocol/span/convert.rs index 5581db962f6..9d567185736 100644 --- a/relay-event-schema/src/protocol/span/convert.rs +++ b/relay-event-schema/src/protocol/span/convert.rs @@ -146,7 +146,7 @@ map_fields!( span.data.environment <=> event.environment, span.data.sdk_name <=> event.client_sdk.name, span.data.release <=> event.release, - span.data.segment_name <=> event.transaction, + span.description <=> event.transaction, span.exclusive_time <=> event.contexts.trace.exclusive_time, span.measurements <=> event.measurements, span.op <=> event.contexts.trace.op, From 3b1f4b494f8fc3484901fa008678017a97bd99bc Mon Sep 17 00:00:00 2001 From: Joris Bayer Date: Thu, 18 Apr 2024 18:07:16 +0200 Subject: [PATCH 15/20] fix: double write transaction --- .../src/protocol/span/convert.rs | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/relay-event-schema/src/protocol/span/convert.rs b/relay-event-schema/src/protocol/span/convert.rs index 9d567185736..9820699e068 100644 --- a/relay-event-schema/src/protocol/span/convert.rs +++ b/relay-event-schema/src/protocol/span/convert.rs @@ -147,6 +147,7 @@ map_fields!( span.data.sdk_name <=> event.client_sdk.name, span.data.release <=> event.release, span.description <=> event.transaction, + span.data.segment_name <=> event.transaction, span.exclusive_time <=> event.contexts.trace.exclusive_time, span.measurements <=> event.measurements, span.op <=> event.contexts.trace.op, @@ -173,6 +174,8 @@ map_fields!( mod tests { use relay_protocol::Annotated; + use crate::protocol::SpanData; + use super::*; #[test] @@ -229,7 +232,7 @@ mod tests { timestamp: ~, start_timestamp: ~, exclusive_time: 123.4, - description: ~, + description: "my 1st transaction", op: "myop", span_id: SpanId( "fa90fdead5f74052", @@ -323,6 +326,23 @@ mod tests { assert_eq!(event, roundtripped); } + #[test] + fn segment_name_takes_precedence() { + let span = Span { + is_segment: true.into(), + description: "This is the description".to_owned().into(), + data: SpanData { + segment_name: "This is the segment name".to_owned().into(), + ..Default::default() + } + .into(), + ..Default::default() + }; + let event = Event::try_from(&span).unwrap(); + + assert_eq!(event.transaction.as_str(), Some("This is the segment name")); + } + #[test] fn no_empty_profile_context() { let span = Span { From bec3ac2975950ccb2bcb8e84100e6bc0946c72e4 Mon Sep 17 00:00:00 2001 From: Joris Bayer Date: Thu, 18 Apr 2024 18:12:06 +0200 Subject: [PATCH 16/20] Restore order --- .../src/protocol/span/convert.rs | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/relay-event-schema/src/protocol/span/convert.rs b/relay-event-schema/src/protocol/span/convert.rs index 9820699e068..bb6afdae1f7 100644 --- a/relay-event-schema/src/protocol/span/convert.rs +++ b/relay-event-schema/src/protocol/span/convert.rs @@ -142,27 +142,27 @@ macro_rules! map_fields { // allowing users to call both `Event::from(&span)` and `Span::from(&event)`. map_fields!( span._metrics_summary <=> event._metrics_summary, - span.data.browser_name <=> event.contexts.browser.name, - span.data.environment <=> event.environment, - span.data.sdk_name <=> event.client_sdk.name, - span.data.release <=> event.release, span.description <=> event.transaction, span.data.segment_name <=> event.transaction, - span.exclusive_time <=> event.contexts.trace.exclusive_time, span.measurements <=> event.measurements, - span.op <=> event.contexts.trace.op, - span.parent_span_id <=> event.contexts.trace.parent_span_id, span.platform <=> event.platform, - span.profile_id <=> event.contexts.profile.profile_id, span.received <=> event.received, - span.segment_id <=> event.contexts.trace.span_id, - span.span_id <=> event.contexts.trace.span_id, span.start_timestamp <=> event.start_timestamp, - span.status <=> event.contexts.trace.status, span.tags <=> event.tags, span.timestamp <=> event.timestamp, - span.trace_id <=> event.contexts.trace.trace_id - + span.exclusive_time <=> event.contexts.trace.exclusive_time, + span.op <=> event.contexts.trace.op, + span.parent_span_id <=> event.contexts.trace.parent_span_id, + // A transaction corresponds to a segment span, so span_id and segment_id have the same value: + span.span_id <=> event.contexts.trace.span_id, + span.segment_id <=> event.contexts.trace.span_id, + span.status <=> event.contexts.trace.status, + span.trace_id <=> event.contexts.trace.trace_id, + span.profile_id <=> event.contexts.profile.profile_id, + span.data.release <=> event.release, + span.data.environment <=> event.environment, + span.data.browser_name <=> event.contexts.browser.name, + span.data.sdk_name <=> event.client_sdk.name ; span.is_segment <= true, span.was_transaction <= true From 980b6d30c7407e9cbfc8cdf243dd5609a80d9a00 Mon Sep 17 00:00:00 2001 From: Joris Bayer Date: Fri, 19 Apr 2024 11:03:35 +0200 Subject: [PATCH 17/20] fix: Assume default, key name --- relay-server/src/services/store.rs | 1 + relay-spans/src/span.rs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/relay-server/src/services/store.rs b/relay-server/src/services/store.rs index 259763e8167..c3f98d477cd 100644 --- a/relay-server/src/services/store.rs +++ b/relay-server/src/services/store.rs @@ -1357,6 +1357,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/src/span.rs b/relay-spans/src/span.rs index 628bb5d7450..13a7696e660 100644 --- a/relay-spans/src/span.rs +++ b/relay-spans/src/span.rs @@ -117,7 +117,7 @@ pub fn otel_to_sentry_span(otel_span: OtelSpan) -> EventSpan { } } } - "http.method" | "request.method" => { + "http.method" | "http.request.method" => { let http_op = match kind { 2 => "http.server", 3 => "http.client", From e47a11ffce144a1beef4a8ecf046ff2ce91dfadc Mon Sep 17 00:00:00 2001 From: Joris Bayer Date: Mon, 22 Apr 2024 10:42:36 +0200 Subject: [PATCH 18/20] Update relay-spans/src/span.rs Co-authored-by: David Herberth --- relay-spans/src/span.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/relay-spans/src/span.rs b/relay-spans/src/span.rs index 13a7696e660..723f85cd795 100644 --- a/relay-spans/src/span.rs +++ b/relay-spans/src/span.rs @@ -186,7 +186,7 @@ pub fn otel_to_sentry_span(otel_span: OtelSpan) -> EventSpan { } if let (Some(http_method), Some(http_route)) = (http_method, http_route) { - description = Some(format!("{} {}", http_method, http_route)); + description = Some(format!("{http_method} {http_route}")); } EventSpan { From 3f6c1ed4a82ffdcb869e3c98214f8c6dc6760133 Mon Sep 17 00:00:00 2001 From: Joris Bayer Date: Mon, 22 Apr 2024 10:44:39 +0200 Subject: [PATCH 19/20] doc: changelog --- CHANGELOG.md | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a1900aadfe..897e871b95b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,10 +1,16 @@ # Changelog +## Unreleased + +**Features**: + +- Use same keys for Otel span attributes and Sentry span data. ([#3457](https://github.com/getsentry/relay/pull/3457)) + ## 24.4.1 **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:** From c9c6dde7baf28edffb9c1d9347380be8a5ec106f Mon Sep 17 00:00:00 2001 From: Joris Bayer Date: Mon, 22 Apr 2024 11:06:29 +0200 Subject: [PATCH 20/20] fix: merge --- relay-event-schema/src/protocol/span.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/relay-event-schema/src/protocol/span.rs b/relay-event-schema/src/protocol/span.rs index 1a9380f6821..1e1932f8fd1 100644 --- a/relay-event-schema/src/protocol/span.rs +++ b/relay-event-schema/src/protocol/span.rs @@ -296,7 +296,7 @@ pub struct SpanData { 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`.