diff --git a/CHANGELOG.md b/CHANGELOG.md index 8dde8cadb33..401f2b84481 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ - Temporarily add metric summaries on spans and top-level transaction events to link DDM with performance monitoring. ([#2757](https://github.com/getsentry/relay/pull/2757)) - Add size limits on metric related envelope items. ([#2800](https://github.com/getsentry/relay/pull/2800)) - Include the size offending item in the size limit error message. ([#2801](https://github.com/getsentry/relay/pull/2801)) +- Allow ingestion of metrics summary on spans. ([#2823](https://github.com/getsentry/relay/pull/2823)) - Add metric_bucket data category. ([#2824](https://github.com/getsentry/relay/pull/2824)) ## 23.11.2 diff --git a/relay-event-normalization/src/normalize/mod.rs b/relay-event-normalization/src/normalize/mod.rs index b8d648fb8ec..1fb1d4d6a0f 100644 --- a/relay-event-normalization/src/normalize/mod.rs +++ b/relay-event-normalization/src/normalize/mod.rs @@ -2265,6 +2265,7 @@ mod tests { sentry_tags: ~, received: ~, measurements: ~, + _metrics_summary: ~, other: {}, }, ] @@ -2307,6 +2308,7 @@ mod tests { sentry_tags: ~, received: ~, measurements: ~, + _metrics_summary: ~, other: {}, }, ] @@ -2349,6 +2351,7 @@ mod tests { sentry_tags: ~, received: ~, measurements: ~, + _metrics_summary: ~, other: {}, }, ] diff --git a/relay-event-schema/src/protocol/span.rs b/relay-event-schema/src/protocol/span.rs index 3b68adceb9d..840ce350d6a 100644 --- a/relay-event-schema/src/protocol/span.rs +++ b/relay-event-schema/src/protocol/span.rs @@ -81,6 +81,13 @@ pub struct Span { #[metastructure(omit_from_schema)] // we only document error events for now pub measurements: Annotated, + /// Temporary protocol support for metric summaries. + /// + /// This shall move to a stable location once we have stabilized the + /// interface. This is intentionally not typed today. + #[metastructure(skip_serialization = "empty")] + pub _metrics_summary: Annotated, + // TODO remove retain when the api stabilizes /// Additional arbitrary fields for forwards compatibility. #[metastructure(additional_properties, retain = "true", pii = "maybe")] @@ -95,6 +102,7 @@ impl From<&Event> for Span { received: event.received.clone(), start_timestamp: event.start_timestamp.clone(), timestamp: event.timestamp.clone(), + _metrics_summary: event._metrics_summary.clone(), ..Default::default() }; @@ -245,6 +253,19 @@ mod tests { "span_id": "FA90FDEAD5F74052", "type": "trace" } + }, + "_metrics_summary": { + "some_metric": [ + { + "min": 1.0, + "max": 2.0, + "sum": 3.0, + "count": 2, + "tags": { + "environment": "test" + } + } + ] } }"#, ) @@ -280,6 +301,37 @@ mod tests { sentry_tags: ~, received: ~, measurements: ~, + _metrics_summary: Object( + { + "some_metric": Array( + [ + Object( + { + "count": I64( + 2, + ), + "max": F64( + 2.0, + ), + "min": F64( + 1.0, + ), + "sum": F64( + 3.0, + ), + "tags": Object( + { + "environment": String( + "test", + ), + }, + ), + }, + ), + ], + ), + }, + ), other: {}, } "###); diff --git a/relay-server/src/actors/processor/span/processing.rs b/relay-server/src/actors/processor/span/processing.rs index 23e8c81410f..bfb93499ae0 100644 --- a/relay-server/src/actors/processor/span/processing.rs +++ b/relay-server/src/actors/processor/span/processing.rs @@ -122,14 +122,6 @@ pub fn extract_from_event(state: &mut ProcessEnvelopeState) { return; }; - // Check feature flag. - if !state - .project_state - .has_feature(Feature::SpanMetricsExtraction) - { - return; - }; - let mut add_span = |span: Annotated| { let span = match validate(span) { Ok(span) => span, @@ -150,52 +142,65 @@ pub fn extract_from_event(state: &mut ProcessEnvelopeState) { state.managed_envelope.envelope_mut().add_item(item); }; + let span_metrics_extraction_enabled = state + .project_state + .has_feature(Feature::SpanMetricsExtraction); + let custom_metrics_enabled = state.project_state.has_feature(Feature::CustomMetrics); + let Some(event) = state.event.value() else { return; }; + let extract_transaction_span = span_metrics_extraction_enabled + || (custom_metrics_enabled && !event._metrics_summary.is_empty()); + let extract_child_spans = span_metrics_extraction_enabled; + // Extract transaction as a span. let mut transaction_span: Span = event.into(); - let all_modules_enabled = state - .project_state - .has_feature(Feature::SpanMetricsExtractionAllModules); - - // Add child spans as envelope items. - if let Some(child_spans) = event.spans.value() { - for span in child_spans { - let Some(inner_span) = span.value() else { - continue; - }; - // HACK: filter spans based on module until we figure out grouping. - if !all_modules_enabled && !is_allowed(inner_span) { - continue; + if extract_child_spans { + let all_modules_enabled = state + .project_state + .has_feature(Feature::SpanMetricsExtractionAllModules); + + // Add child spans as envelope items. + if let Some(child_spans) = event.spans.value() { + for span in child_spans { + let Some(inner_span) = span.value() else { + continue; + }; + // HACK: filter spans based on module until we figure out grouping. + if !all_modules_enabled && !is_allowed(inner_span) { + continue; + } + // HACK: clone the span to set the segment_id. This should happen + // as part of normalization once standalone spans reach wider adoption. + let mut new_span = inner_span.clone(); + new_span.is_segment = Annotated::new(false); + new_span.received = transaction_span.received.clone(); + new_span.segment_id = transaction_span.segment_id.clone(); + + // If a profile is associated with the transaction, also associate it with its + // child spans. + new_span.profile_id = transaction_span.profile_id.clone(); + + add_span(Annotated::new(new_span)); } - // HACK: clone the span to set the segment_id. This should happen - // as part of normalization once standalone spans reach wider adoption. - let mut new_span = inner_span.clone(); - new_span.is_segment = Annotated::new(false); - new_span.received = transaction_span.received.clone(); - new_span.segment_id = transaction_span.segment_id.clone(); - - // If a profile is associated with the transaction, also associate it with its - // child spans. - new_span.profile_id = transaction_span.profile_id.clone(); - - add_span(Annotated::new(new_span)); } } - // Extract tags to add to this span as well - let shared_tags = tag_extraction::extract_shared_tags(event); - transaction_span.sentry_tags = Annotated::new( - shared_tags - .clone() - .into_iter() - .map(|(k, v)| (k.sentry_tag_key().to_owned(), Annotated::new(v))) - .collect(), - ); - add_span(transaction_span.into()); + if extract_transaction_span { + // Extract tags to add to this span as well + let shared_tags = tag_extraction::extract_shared_tags(event); + transaction_span.sentry_tags = Annotated::new( + shared_tags + .clone() + .into_iter() + .map(|(k, v)| (k.sentry_tag_key().to_owned(), Annotated::new(v))) + .collect(), + ); + add_span(transaction_span.into()); + } } /// Config needed to normalize a standalone span. 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 a047712697a..52487507a36 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 @@ -40,6 +40,7 @@ expression: "(&event.value().unwrap().spans, metrics)" }, received: ~, measurements: ~, + _metrics_summary: ~, other: {}, }, Span { @@ -78,6 +79,7 @@ expression: "(&event.value().unwrap().spans, metrics)" }, received: ~, measurements: ~, + _metrics_summary: ~, other: {}, }, ], diff --git a/relay-server/src/metrics_extraction/transactions/mod.rs b/relay-server/src/metrics_extraction/transactions/mod.rs index 3f1d846555c..eb9ee94eac7 100644 --- a/relay-server/src/metrics_extraction/transactions/mod.rs +++ b/relay-server/src/metrics_extraction/transactions/mod.rs @@ -573,6 +573,7 @@ mod tests { sentry_tags: ~, received: ~, measurements: ~, + _metrics_summary: ~, other: {}, }, ] diff --git a/tests/integration/test_store.py b/tests/integration/test_store.py index 85fe066189a..9ea68309c04 100644 --- a/tests/integration/test_store.py +++ b/tests/integration/test_store.py @@ -1580,3 +1580,67 @@ def test_span_ingestion( "retention_days": 90, }, ] + + +def test_span_extraction_with_ddm( + mini_sentry, + relay_with_processing, + spans_consumer, +): + spans_consumer = spans_consumer() + + relay = relay_with_processing() + project_id = 42 + project_config = mini_sentry.add_full_project_config(project_id) + project_config["config"]["spanAttributes"] = ["exclusive-time"] + project_config["config"]["features"] = [ + "organizations:custom-metrics", + ] + + event = make_transaction({"event_id": "cbf6960622e14a45abc1f03b2055b186"}) + metrics_summary = { + "c:spans/some_metric@none": [ + { + "min": 1.0, + "max": 2.0, + "sum": 3.0, + "count": 4, + "tags": { + "environment": "test", + }, + }, + ], + } + event["_metrics_summary"] = metrics_summary + + relay.send_event(project_id, event) + + transaction_span = spans_consumer.get_span() + del transaction_span["start_time"] + del transaction_span["span"]["received"] + assert transaction_span == { + "event_id": "cbf6960622e14a45abc1f03b2055b186", + "project_id": 42, + "organization_id": 1, + "retention_days": 90, + "span": { + "description": "hi", + "exclusive_time": 2000.0, + "is_segment": True, + "op": "hi", + "segment_id": "968cff94913ebb07", + "sentry_tags": {"transaction": "hi", "transaction.op": "hi"}, + "span_id": "968cff94913ebb07", + "start_timestamp": datetime.fromisoformat(event["start_timestamp"]) + .replace(tzinfo=timezone.utc) + .timestamp(), + "status": "unknown", + "timestamp": datetime.fromisoformat(event["timestamp"]) + .replace(tzinfo=timezone.utc) + .timestamp(), + "trace_id": "a0fa8803753e40fd8124b21eeb2986b5", + "_metrics_summary": metrics_summary, + }, + } + + spans_consumer.assert_empty()