diff --git a/relay-event-schema/src/protocol/span.rs b/relay-event-schema/src/protocol/span.rs index 09bae7c485..badf42f49f 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 b03a5aa8d6..f2c5ac1dd3 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; + let mut event = Event::default(); 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() - }; + $( + if let Some(value) = read_value!(span, $($span_path)+) { + *event_write_path!(&mut event, $($event_path)+) = Annotated::new(value.clone()).map_value(Into::into) + } + )+ + $( + *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,41 @@ 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: "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: ~, + transaction: ~, + ui_component_name: ~, + url_scheme: ~, + user: ~, + replay_id: ~, + other: {}, + }, sentry_tags: ~, received: ~, measurements: Measurements( diff --git a/relay-spans/src/span.rs b/relay-spans/src/span.rs index e1f69cca3d..31b5e92667 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 0900360ab4..08df52847a 100644 --- a/tests/integration/test_spans.py +++ b/tests/integration/test_spans.py @@ -497,7 +497,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", @@ -553,7 +552,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",