From 1742c10493615a942552f0b966a53196aef8035a Mon Sep 17 00:00:00 2001 From: David Herberth Date: Tue, 29 Jul 2025 10:00:24 +0200 Subject: [PATCH 1/4] ref(spans): Always extract span metrics --- relay-server/src/metrics_extraction/event.rs | 27 ++++++++++++-------- relay-server/src/services/processor.rs | 2 +- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/relay-server/src/metrics_extraction/event.rs b/relay-server/src/metrics_extraction/event.rs index 7c1e4948bf6..4df2c9ab6c8 100644 --- a/relay-server/src/metrics_extraction/event.rs +++ b/relay-server/src/metrics_extraction/event.rs @@ -57,6 +57,7 @@ pub fn extract_metrics( target_project_id: ProjectId, max_tag_value_size: usize, extract_spans: bool, + produces_spans: bool, ) -> ExtractedMetrics { let mut metrics = ExtractedMetrics { project_metrics: generic::extract_metrics(event, config), @@ -70,6 +71,7 @@ pub fn extract_metrics( sampling_decision, target_project_id, max_tag_value_size, + produces_spans, &mut metrics, ); } @@ -83,6 +85,7 @@ fn extract_span_metrics_for_event( sampling_decision: SamplingDecision, target_project_id: ProjectId, max_tag_value_size: usize, + produces_spans: bool, output: &mut ExtractedMetrics, ) { relay_statsd::metric!(timer(RelayTimers::EventProcessingSpanMetricsExtraction), { @@ -104,17 +107,18 @@ fn extract_span_metrics_for_event( } } - // This function assumes it is only called when span metrics should be extracted, hence we - // extract the span root counter unconditionally. - let transaction = transactions::get_transaction_name(event); - let bucket = create_span_root_counter( - event, - transaction, - span_count, - sampling_decision, - target_project_id, - ); - output.sampling_metrics.extend(bucket); + // Extract the count per root metric, only if Relay will also produce standalone spans. + if produces_spans { + let transaction = transactions::get_transaction_name(event); + let bucket = create_span_root_counter( + event, + transaction, + span_count, + sampling_decision, + target_project_id, + ); + output.sampling_metrics.extend(bucket); + } }); } @@ -1270,6 +1274,7 @@ mod tests { ProjectId::new(4711), 200, true, + true, ) } diff --git a/relay-server/src/services/processor.rs b/relay-server/src/services/processor.rs index 7187b8adef5..633cb8a26b5 100644 --- a/relay-server/src/services/processor.rs +++ b/relay-server/src/services/processor.rs @@ -1427,7 +1427,6 @@ impl EnvelopeProcessorService { // If spans were already extracted for an event, we rely on span processing to extract metrics. let extract_spans = !spans_extracted.0 - && project_info.config.features.produces_spans() && utils::sample(global.options.span_extraction_sample_rate.unwrap_or(1.0)).is_keep(); let metrics = crate::metrics_extraction::event::extract_metrics( @@ -1440,6 +1439,7 @@ impl EnvelopeProcessorService { .aggregator_config_for(MetricNamespace::Spans) .max_tag_value_length, extract_spans, + project_info.config.features.produces_spans(), ); extracted_metrics.extend(metrics, Some(sampling_decision)); From 319d2e2c24c9b263e5b6d468338cd1ba86fe05b6 Mon Sep 17 00:00:00 2001 From: David Herberth Date: Tue, 29 Jul 2025 10:14:07 +0200 Subject: [PATCH 2/4] make count per root not optional --- relay-server/src/metrics_extraction/event.rs | 28 +++++++++----------- relay-server/src/services/processor.rs | 1 - 2 files changed, 12 insertions(+), 17 deletions(-) diff --git a/relay-server/src/metrics_extraction/event.rs b/relay-server/src/metrics_extraction/event.rs index 4df2c9ab6c8..28fd05d1247 100644 --- a/relay-server/src/metrics_extraction/event.rs +++ b/relay-server/src/metrics_extraction/event.rs @@ -57,7 +57,6 @@ pub fn extract_metrics( target_project_id: ProjectId, max_tag_value_size: usize, extract_spans: bool, - produces_spans: bool, ) -> ExtractedMetrics { let mut metrics = ExtractedMetrics { project_metrics: generic::extract_metrics(event, config), @@ -71,7 +70,6 @@ pub fn extract_metrics( sampling_decision, target_project_id, max_tag_value_size, - produces_spans, &mut metrics, ); } @@ -85,7 +83,6 @@ fn extract_span_metrics_for_event( sampling_decision: SamplingDecision, target_project_id: ProjectId, max_tag_value_size: usize, - produces_spans: bool, output: &mut ExtractedMetrics, ) { relay_statsd::metric!(timer(RelayTimers::EventProcessingSpanMetricsExtraction), { @@ -107,18 +104,18 @@ fn extract_span_metrics_for_event( } } - // Extract the count per root metric, only if Relay will also produce standalone spans. - if produces_spans { - let transaction = transactions::get_transaction_name(event); - let bucket = create_span_root_counter( - event, - transaction, - span_count, - sampling_decision, - target_project_id, - ); - output.sampling_metrics.extend(bucket); - } + // We unconditionally run metric extraction for spans. The count per root, is technically + // only required for configurations which do have dynamic sampling enabled. But for the + // sake of simplicity we always add it here. + let transaction = transactions::get_transaction_name(event); + let bucket = create_span_root_counter( + event, + transaction, + span_count, + sampling_decision, + target_project_id, + ); + output.sampling_metrics.extend(bucket); }); } @@ -1274,7 +1271,6 @@ mod tests { ProjectId::new(4711), 200, true, - true, ) } diff --git a/relay-server/src/services/processor.rs b/relay-server/src/services/processor.rs index 633cb8a26b5..5c27f542614 100644 --- a/relay-server/src/services/processor.rs +++ b/relay-server/src/services/processor.rs @@ -1439,7 +1439,6 @@ impl EnvelopeProcessorService { .aggregator_config_for(MetricNamespace::Spans) .max_tag_value_length, extract_spans, - project_info.config.features.produces_spans(), ); extracted_metrics.extend(metrics, Some(sampling_decision)); From 83cc660519d175bd56d93b9e513b204d5e5e9124 Mon Sep 17 00:00:00 2001 From: David Herberth Date: Tue, 29 Jul 2025 10:36:34 +0200 Subject: [PATCH 3/4] allow metrics in spans namespace --- relay-server/src/services/processor/metrics.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/relay-server/src/services/processor/metrics.rs b/relay-server/src/services/processor/metrics.rs index ff084a2f848..15979b3772b 100644 --- a/relay-server/src/services/processor/metrics.rs +++ b/relay-server/src/services/processor/metrics.rs @@ -60,7 +60,7 @@ fn is_metric_namespace_valid(state: &ProjectInfo, namespace: MetricNamespace) -> match namespace { MetricNamespace::Sessions => true, MetricNamespace::Transactions => true, - MetricNamespace::Spans => state.config.features.produces_spans(), + MetricNamespace::Spans => true, MetricNamespace::Custom => state.has_feature(Feature::CustomMetrics), MetricNamespace::Stats => true, MetricNamespace::Unsupported => false, From 6793def0770c990f0cf547dbc4517f0c2b7dd731 Mon Sep 17 00:00:00 2001 From: David Herberth Date: Tue, 29 Jul 2025 11:23:01 +0200 Subject: [PATCH 4/4] fix integration tests --- tests/integration/test_metrics.py | 20 +++++++++++--------- tests/integration/test_outcome.py | 8 ++++---- tests/integration/test_store.py | 8 ++++++++ 3 files changed, 23 insertions(+), 13 deletions(-) diff --git a/tests/integration/test_metrics.py b/tests/integration/test_metrics.py index 788d5751c33..bbcdabb2cb2 100644 --- a/tests/integration/test_metrics.py +++ b/tests/integration/test_metrics.py @@ -1024,7 +1024,7 @@ def test_transaction_metrics_extraction_external_relays( assert len(metrics_envelope.items) == 1 payload = json.loads(metrics_envelope.items[0].get_bytes().decode()) - assert len(payload) == 4 + assert len(payload) == 6 by_name = {m["name"]: m for m in payload} light_metric = by_name["d:transactions/duration_light@millisecond"] @@ -1087,7 +1087,7 @@ def test_transaction_metrics_extraction_processing_relays( tx_consumer.assert_empty() if expect_metrics_extraction: - metrics = metrics_by_name(metrics_consumer, 4, timeout=3) + metrics = metrics_by_name(metrics_consumer, 6, timeout=3) metric_usage = metrics["c:transactions/usage@none"] assert metric_usage["tags"] == {} assert metric_usage["value"] == 1.0 @@ -1332,20 +1332,22 @@ def test_limit_custom_measurements( event, _ = transactions_consumer.get_event() assert len(event["measurements"]) == 2 - # Expect exactly 5 metrics: - # (transaction.duration, transaction.duration_light, transactions.count_per_root_project, 1 builtin, 1 custom) - metrics = metrics_by_name(metrics_consumer, 6) - metrics.pop("headers") - - assert metrics.keys() == { + expected_metrics = { "c:transactions/usage@none", "d:transactions/duration@millisecond", "d:transactions/duration_light@millisecond", "c:transactions/count_per_root_project@none", "d:transactions/measurements.foo@none", "d:transactions/measurements.bar@none", + "c:spans/usage@none", + "c:spans/count_per_root_project@none", } + metrics = metrics_by_name(metrics_consumer, len(expected_metrics)) + metrics.pop("headers") + + assert metrics.keys() == expected_metrics + @pytest.mark.parametrize("has_measurements_config", [True, False]) def test_do_not_drop_custom_measurements_in_static( @@ -1856,7 +1858,7 @@ def test_metrics_extraction_with_computed_context_filters( ] # Verify that all three metrics were extracted - metrics = metrics_by_name(metrics_consumer, 7) + metrics = metrics_by_name(metrics_consumer, 9) # Check each extracted metric for metric_name in metric_names: diff --git a/tests/integration/test_outcome.py b/tests/integration/test_outcome.py index c7c2e4d5d60..9cb3530d8e1 100644 --- a/tests/integration/test_outcome.py +++ b/tests/integration/test_outcome.py @@ -1452,11 +1452,11 @@ def make_envelope(): "source": "pop-relay", "timestamp": time_within_delta(), } - for category in [6, 11] # Profile, ProfileIndexed + for category in [DataCategory.PROFILE.value, DataCategory.PROFILE_INDEXED.value] ] # Make sure the profile will not be counted as accepted: - metrics = metrics_by_name(metrics_consumer, 4) + metrics = metrics_by_name(metrics_consumer, 7) assert "has_profile" not in metrics["d:transactions/duration@millisecond"]["tags"] assert "has_profile" not in metrics["c:transactions/usage@none"]["tags"] @@ -1621,10 +1621,10 @@ def make_envelope(): "source": "processing-relay", "timestamp": time_within_delta(), } - for category in [6, 11] # Profile, ProfileIndexed + for category in [DataCategory.PROFILE.value, DataCategory.PROFILE_INDEXED.value] ] - metrics = metrics_by_name(metrics_consumer, 4) + metrics = metrics_by_name(metrics_consumer, 7) assert "has_profile" not in metrics["d:transactions/duration@millisecond"]["tags"] assert "has_profile" not in metrics["c:transactions/usage@none"]["tags"] diff --git a/tests/integration/test_store.py b/tests/integration/test_store.py index a5dfb80fbc4..3ec2a60756a 100644 --- a/tests/integration/test_store.py +++ b/tests/integration/test_store.py @@ -916,6 +916,14 @@ def test_processing_quota_transaction_indexing( timeout=3, ) + # Ignore span metrics, they may be emitted because rate limits from transactions are not + # currently enforced for spans, which they should be. See: https://github.com/getsentry/relay/issues/4961. + metrics = {metric["name"] for (metric, _) in metrics_consumer.get_metrics()} + assert metrics == { + "c:spans/count_per_root_project@none", + "c:spans/usage@none", + } + def test_events_buffered_before_auth(relay, mini_sentry): evt = threading.Event()