diff --git a/relay-server/src/metrics_extraction/event.rs b/relay-server/src/metrics_extraction/event.rs index 7c1e4948bf6..28fd05d1247 100644 --- a/relay-server/src/metrics_extraction/event.rs +++ b/relay-server/src/metrics_extraction/event.rs @@ -104,8 +104,9 @@ 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. + // 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, diff --git a/relay-server/src/services/processor.rs b/relay-server/src/services/processor.rs index 7187b8adef5..5c27f542614 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( 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, 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()