ref(metrics): Disable span metrics #4931

phacops · 2025-07-11T03:03:12Z

We migrated all parts of the frontend using span metrics to querying the EAP. We do not need to collect them anymore.

Dav1dde

As mentioned in a comment, the count_per_root is still used, also the usage metric might still be used in a billing consumer.

relay-server/src/metrics_extraction/event.rs

phacops · 2025-07-11T19:40:01Z

tests/integration/test_metrics.py

@@ -835,8 +832,6 @@ def assert_transaction():
        "received_at": time_after(timestamp),
    }

-    assert metrics["c:spans/usage@none"]["value"] == 2


@Dav1dde Should we still count the metric is the span extraction is not activated? I don't think so.

Is there a case where we don't have span extraction?
In any case, if we still have transaction only flows, these metrics should be fine to not emit.

So this is now gone, because it was tied to the span metric extraction feature flag, which no longer is set. Which brings the question, is there any transaction only product surface (AM1/AM2?) which requires span metrics to work (as they won't have span extraction)?

Okay so talking with Jan, I think we will always have to extract count_per_root and usage metrics, unconditionally, if we don't want to hide it behind another feature flag (which I don't think we should, let's just always extract them), as we don't know if they are later required for the product (dynamic sampling/outcomes) or not.

We still need to gate them behind ExtractSpansFromEvent otherwise we'd also extract this for AM1 organizations (which we don't want), but we shouldn't gate this behind any span metrics extraction flag since we'll want to extract them regardless.

phacops · 2025-07-11T20:02:33Z

tests/integration/test_spans.py

+        outcomes = outcomes_consumer.get_outcomes(timeout=10, n=7)
+        assert summarize_outcomes(outcomes) == {
+            (16, 0): 6,  # SpanIndexed, Accepted
+            (15, 1): 2,  # Metric, Filtered


@Dav1dde Can't quite say if this is right.

We likely started to emit metrics for usage and count_per_root all the time, which might itself be a bug, and they get filtered? What's the correct behavior here?

Yeah then we should fix that, either not emit the metrics when they shouldn't be emitted or if they should be emitted make sure they aren't dropped/filtered.

I don't know if we need the span based metrics in circumstances where there is no span extraction, will need to find that out, but I don't think so 🤔

I don't really get this failure. We get disabled-namespace outcomes for project 43 but I enable span ingestion for that project, no outcome gets emitted.

I'm not sure I understand what the failure is here, can you take a look please?

That makes sense, because the Spans metrics namespace is gated behind the span ingestion feature flag here.

Right but, why do we have a disabled-namespace outcome if it's disabled and not having ANY outcomes when it's enabled. Not having any outcomes when it's enabled suggests it doesn't try to emit a metric, but somehow, when it's disabled, it tries to emit metrics?

Since project 43 is the root and has dynamic sampling removing things, the organizations:standalone-span-ingestion should be enabled on both projects as they are part of the same organization. Then the test is well set up and passes (as it should).

tests/integration/test_metrics_encoding.py

relay-dynamic-config/src/defaults.rs

Dav1dde · 2025-07-15T08:09:11Z

tests/integration/test_metrics.py

@@ -835,8 +832,6 @@ def assert_transaction():
        "received_at": time_after(timestamp),
    }

-    assert metrics["c:spans/usage@none"]["value"] == 2


Is there a case where we don't have span extraction?
In any case, if we still have transaction only flows, these metrics should be fine to not emit.

So this is now gone, because it was tied to the span metric extraction feature flag, which no longer is set. Which brings the question, is there any transaction only product surface (AM1/AM2?) which requires span metrics to work (as they won't have span extraction)?

tests/integration/test_metrics_encoding.py

Dav1dde · 2025-07-15T08:12:49Z

tests/integration/test_spans.py

+        outcomes = outcomes_consumer.get_outcomes(timeout=10, n=7)
+        assert summarize_outcomes(outcomes) == {
+            (16, 0): 6,  # SpanIndexed, Accepted
+            (15, 1): 2,  # Metric, Filtered


Yeah then we should fix that, either not emit the metrics when they shouldn't be emitted or if they should be emitted make sure they aren't dropped/filtered.

I don't know if we need the span based metrics in circumstances where there is no span extraction, will need to find that out, but I don't think so 🤔

Dav1dde · 2025-07-15T09:04:36Z

tests/integration/test_metrics.py

@@ -835,8 +832,6 @@ def assert_transaction():
        "received_at": time_after(timestamp),
    }

-    assert metrics["c:spans/usage@none"]["value"] == 2


Okay so talking with Jan, I think we will always have to extract count_per_root and usage metrics, unconditionally, if we don't want to hide it behind another feature flag (which I don't think we should, let's just always extract them), as we don't know if they are later required for the product (dynamic sampling/outcomes) or not.

relay-dynamic-config/src/feature.rs

…-metrics

Dav1dde · 2025-07-17T11:32:46Z

@sentry review

Dav1dde

Looks good, gets rid of a bunch of old stuff, usage metric and count per root metrics aren't changed which are the important bits.

Dav1dde · 2025-07-17T11:50:32Z

relay-dynamic-config/src/feature.rs

-    /// Enables metric extraction from spans for addon modules.
-    ///
-    /// Serialized as `projects:span-metrics-extraction-addons`.
+    /// This feature has graduated and is hard-coded for external Relays.


Graduated features are hardcoded at the top of the file in the GRADUATED_FEATURE_FLAGS slice, but I don't think we can graduate it, as that would unconditionally enable it.

Should call out that this is deprecated and just kept around for external Relays, we can also maybe just graduate them if the span extraction one is enabled, then we can remove them from Sentry.

phacops requested a review from a team as a code owner July 11, 2025 03:03

Dav1dde reviewed Jul 11, 2025

View reviewed changes

relay-server/src/metrics_extraction/event.rs Outdated Show resolved Hide resolved

Dav1dde assigned phacops Jul 11, 2025

phacops added 5 commits July 11, 2025 09:49

chore(metrics): Disable span metrics

df7dba7

Remove feature flags

9a804ec

Update snapshots

5841a87

Remove unneded tests

8f19191

Remove more tests

e431ebc

phacops force-pushed the pierre/disable-span-metrics branch from 91bd476 to e431ebc Compare July 11, 2025 17:00

phacops added 3 commits July 11, 2025 10:00

Add a CHANGELOG entry

b1ca3cf

Remove reference to inexistent feature flag

762596d

Fix test_rate_limit_spans_in_envelope

4d425eb

phacops requested a review from Dav1dde July 11, 2025 17:19

phacops added 2 commits July 11, 2025 11:29

Fix integration tests

619f0b5

Fix more tests

b33acb5

phacops commented Jul 11, 2025

View reviewed changes

Fix test_dynamic_sampling

e59cb87

phacops commented Jul 11, 2025

View reviewed changes

Remove more references to span metrics in tests

f9482ca

phacops commented Jul 11, 2025

View reviewed changes

tests/integration/test_metrics_encoding.py Show resolved Hide resolved

phacops added 3 commits July 14, 2025 13:56

Restore transaction metrics extracted from spans

fd5259d

Fix test_rate_limit_spans_in_envelope

b638447

Fix test_get_global_config

f5e39cf

Dav1dde reviewed Jul 15, 2025

View reviewed changes

phacops added 5 commits July 15, 2025 16:00

Fix test_transaction_metrics

b414c90

Deprecate features instead of removing them

970896b

Fix test_metric_bucket_encoding_dynamic_global_config_option

9d2baf6

Merge remote-tracking branch 'origin/master' into pierre/disable-span…

4cbaa2b

…-metrics

Remove print

6f22a67

Fix test_dynamic_sampling

eee7701

phacops requested a review from Dav1dde July 16, 2025 01:24

phacops added 2 commits July 16, 2025 11:01

Merge branch 'master' into pierre/disable-span-metrics

7bed444

Fix CHANGELOG entry

b6ac63e

Dav1dde changed the title ~~chore(metrics): Disable span metrics~~ ref(metrics): Disable span metrics Jul 17, 2025

Dav1dde approved these changes Jul 17, 2025

View reviewed changes

Rename deprecated features and fix comments

208f723

phacops added this pull request to the merge queue Jul 17, 2025

Merged via the queue into master with commit 0bec9cc Jul 17, 2025
28 checks passed

phacops deleted the pierre/disable-span-metrics branch July 17, 2025 17:32

ref(metrics): Disable span metrics #4931

ref(metrics): Disable span metrics #4931

Uh oh!

Conversation

phacops commented Jul 11, 2025

Uh oh!

Dav1dde left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

phacops Jul 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Dav1dde commented Jul 17, 2025

Uh oh!

Dav1dde left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

phacops Jul 11, 2025 •

edited

Loading