feat(outcomes): Have relay generate metric billing outcomes by klochek · Pull Request #6066 · getsentry/relay

klochek · 2026-06-09T15:46:38Z

No description provided.

linear-code · 2026-06-09T15:46:42Z

loewenheim

For the tests, I think it's preferable if they are run with the new feature both on and off. Although I do realize that makes it much more annoying to define what the outcome should be for every combination.

loewenheim · 2026-06-10T07:42:42Z

+
+    /// Tracks billing-related outcomes for the list of buckets, adding the
+    /// "billing_outcome_accepted" tag to the bucket if that bucket is accepted.
+    pub fn track_billing_outcome(&self, scoping: Scoping, buckets: &mut [Bucket]) {


Is this intentionally only implemented for spans?

yes, since this only serves the billing outcomes path.

tobias-wilfert · 2026-06-10T08:40:25Z

+PYTEST_N ?= auto
+TEST ?= tests
+test-custom: build setup-venv ## run arbitrary tests
+	.venv/bin/pytest -n $(PYTEST_N) $(TEST) -vv
+.PHONY: test-custom


Nice. One question (as a makefile novice) do we need PYTEST_N ?= auto on line 78 since it is already on line 73?

Nope, that's me copy-pasting a little too fast :P Per David's comment, I'll remove this from this PR, and then move the variables up to the top so we don't wind up with them everywhere.

tobias-wilfert · 2026-06-10T08:42:09Z


+    /// Enable relay billing outcome generation.
+    #[serde(rename = "organizations:relay-generate-billing-outcome")]
+    GenerateBillingOutcome,


I think we would want this just under MinidumpUploads, since now it is in the Deprecated* group.

Also must admit that I am not sure if there is a functional difference these days between "organizations:... and "projects:...

Yep, ideally it's not in the deprecated group.

these days between "organizations:... and "projects:...

You can also roll out a projects: flag to all projects in an org, but you can't roll out a organizations: flag to a single project. Either is fine here.

tobias-wilfert · 2026-06-10T09:06:44Z

    )  # should be kept by dynamic sampling

-    outcomes = outcomes_consumer.get_outcomes()
+    outcomes = outcomes_consumer.get_aggregated_outcomes(timeout=5)


@Dav1dde you did this a while back (#5908) with the same rational we would not want the timeout here?

Correct, no timeouts, only counts.

Suggested change

outcomes = outcomes_consumer.get_aggregated_outcomes(timeout=5)

outcomes = outcomes_consumer.get_aggregated_outcomes(n={HOW MANY INDIVIDUAL MESSAGES ARE EXPECTED})

The n= is optional, but speeds up tests as we don't have to wait timeout for no more messages to arrive. A missed message will still fail the test, as we assert empty on teardown.

I did this because the outcome aggregator is pretty timing sensitive; I was seeing both fully-aggregated results, and dis-aggregated results (the same kinds of spans broken into two buckets due to timing.) I've added a new method to make this faster.

Isn't the default timeout already 5 seconds?

I did this because the outcome aggregator is pretty timing sensitive;

Tests run with minimal/no aggregation:

"outcomes": { "batch_size": 1, "batch_interval": 1, "aggregator": { "bucket_interval": 1, "flush_interval": 0, }, },

Sounds like there is something else going on we should figure out instead. Especially since this works fine for all other tests.

Have you tried specifying the exact amount of outcomes you want to wait for n=?

Yes, I did exactly that--sometimes I would get unaggregated results, othertimes I would see some aggregation. I'm not super clear on the 'biased' bit in the aggregator loop, if it means it always prefers the timeout, or just weights things differently, such that handling a message first could be possible. I'll do a few runs with more logging to make sure of what I saw.

Dav1dde · 2026-06-10T09:12:02Z


+    /// Enable relay billing outcome generation.
+    #[serde(rename = "organizations:relay-generate-billing-outcome")]
+    GenerateBillingOutcome,


Yep, ideally it's not in the deprecated group.

these days between "organizations:... and "projects:...

You can also roll out a projects: flag to all projects in an org, but you can't roll out a organizations: flag to a single project. Either is fine here.

Dav1dde · 2026-06-10T09:14:15Z

+                        });
+                    }
+                }
+                _ => continue,


I'd exhaustively match on BucketSummary so there is a compiler error once a new variant is added. This unfortunately means though you need to move the if into the match body.

Dav1dde · 2026-06-10T09:16:02Z

+
+    /// Tracks billing-related outcomes for the list of buckets, adding the
+    /// "billing_outcome_accepted" tag to the bucket if that bucket is accepted.
+    pub fn track_billing_outcome(&self, scoping: Scoping, buckets: &mut [Bucket]) {


I assume you opted for this additional method because we need to temporarily add the tag to prevent double counting and longterm we can merge this into track?

The comment in track is now also invalid with that change, not a big deal as long as it eventually gets fixed:

// Never emit accepted outcomes for surrogate metrics. // These are handled from within Sentry. if !matches!(outcome, Outcome::Accepted) {

It felt right to keep them separate given the divergence in logic, at least for now. Comment updated.

Dav1dde · 2026-06-10T09:20:48Z

+                    let all_categories = [DataCategory::Span, DataCategory::Transaction];
+                    let num_categories = if is_segment { 2 } else { 1 };
+                    let categories = &all_categories[0..num_categories];


Suggested change

let all_categories = [DataCategory::Span, DataCategory::Transaction];

let num_categories = if is_segment { 2 } else { 1 };

let categories = &all_categories[0..num_categories];

match is_segment {

true => &[DataCategory::Span, DataCategory::Transaction],

false => &[DataCategory::Span],

}

Dav1dde · 2026-06-10T09:26:02Z

+    /// Tracks billing-related outcomes for the list of buckets, adding the
+    /// "billing_outcome_accepted" tag to the bucket if that bucket is accepted.


Suggested change

/// Tracks billing-related outcomes for the list of buckets, adding the

/// "billing_outcome_accepted" tag to the bucket if that bucket is accepted.

/// Emits accepted outcomes for the provided list of buckets.

///

/// Additionally adds a marker tag `billing_outcome_accepted` to all buckets for which an outcome

/// has been emitted.

Nit: billing-related would also include filtered outcomes (see more is_billing on TrackRawOutcome).

Dav1dde · 2026-06-10T09:26:23Z

+
+    /// Tracks billing-related outcomes for the list of buckets, adding the
+    /// "billing_outcome_accepted" tag to the bucket if that bucket is accepted.
+    pub fn track_billing_outcome(&self, scoping: Scoping, buckets: &mut [Bucket]) {


Suggested change

pub fn track_billing_outcome(&self, scoping: Scoping, buckets: &mut [Bucket]) {

pub fn track_accepted_outcomes(&self, scoping: Scoping, buckets: &mut [Bucket]) {

Dav1dde · 2026-06-10T09:28:24Z

    )  # should be kept by dynamic sampling

-    outcomes = outcomes_consumer.get_outcomes()
+    outcomes = outcomes_consumer.get_aggregated_outcomes(timeout=5)


Correct, no timeouts, only counts.

Suggested change

outcomes = outcomes_consumer.get_aggregated_outcomes(timeout=5)

outcomes = outcomes_consumer.get_aggregated_outcomes(n={HOW MANY INDIVIDUAL MESSAGES ARE EXPECTED})

The n= is optional, but speeds up tests as we don't have to wait timeout for no more messages to arrive. A missed message will still fail the test, as we assert empty on teardown.

Dav1dde · 2026-06-10T09:29:49Z

+PYTEST_N ?= auto
+TEST ?= tests
+test-custom: build setup-venv ## run arbitrary tests
+	.venv/bin/pytest -n $(PYTEST_N) $(TEST) -vv
+.PHONY: test-custom


Changes to build systems etc. ideally are split out into separate PRs.

Easier to review, also (which I don't expect to be a problem here), if we have to revert the PR it doesn't also revert these changes.

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 76c805b. Configure here.}

sentry · 2026-06-11T15:38:43Z

+                    let categories = match is_segment {
+                        true => [DataCategory::Span, DataCategory::Transaction].as_slice(),
+                        false => [DataCategory::Span].as_slice(),
+                    };


Bug: The track_accepted_outcome function incorrectly generates a transaction outcome for any segment span by only checking is_segment and ignoring was_transaction, leading to billing inaccuracies.
_{Severity: HIGH}

Suggested Fix

Modify the track_accepted_outcome function to align with the logic in extract_quantities. The function should check for both is_segment and was_transaction being true before emitting a DataCategory::Transaction outcome. This ensures that only segments originating from transactions are counted as such for billing.

Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not valid. Location: relay-server/src/metrics/outcomes.rs#L80-L83 Potential issue: The `track_accepted_outcome` function has logic that is inconsistent with `extract_quantities`. It emits a `DataCategory::Transaction` outcome for any span where `is_segment` is true, but it ignores the `was_transaction` flag. The `extract_quantities` function, however, requires both `is_segment` and `was_transaction` to be true to count a transaction. This discrepancy will cause over-billing, as segment spans that did not originate from a transaction (e.g., from raw span ingestion) will be incorrectly billed as transactions.

_{Did we get this right? 👍 / 👎 to inform future reviews.}

feat(outcomes): Have relay generate metric billing outcomes

1f110a9

klochek requested a review from a team as a code owner June 9, 2026 15:46

klochek mentioned this pull request Jun 9, 2026

feat(ingest-metrics): Ensure billing metrics consumer avoids outcome double-billing getsentry/sentry#117186

Open

loewenheim reviewed Jun 10, 2026

View reviewed changes

tobias-wilfert reviewed Jun 10, 2026

View reviewed changes

Dav1dde requested changes Jun 10, 2026

View reviewed changes

feedback

76c805b

cursor Bot reviewed Jun 11, 2026

View reviewed changes

Comment thread tests/integration/test_attachmentsv2.py Outdated

klochek requested a review from Dav1dde June 11, 2026 01:36

feedback

9bf748f

sentry Bot reviewed Jun 11, 2026

View reviewed changes

	outcomes = outcomes_consumer.get_aggregated_outcomes(timeout=5)
	outcomes = outcomes_consumer.get_aggregated_outcomes(n={HOW MANY INDIVIDUAL MESSAGES ARE EXPECTED})

		/// Tracks billing-related outcomes for the list of buckets, adding the
		/// "billing_outcome_accepted" tag to the bucket if that bucket is accepted.

-    /// Tracks billing-related outcomes for the list of buckets, adding the
-    /// "billing_outcome_accepted" tag to the bucket if that bucket is accepted.
+    /// Emits accepted outcomes for the provided list of buckets.
+    ///
+    /// Additionally adds a marker tag `billing_outcome_accepted` to all buckets for which an outcome
+    /// has been emitted.

	pub fn track_billing_outcome(&self, scoping: Scoping, buckets: &mut [Bucket]) {
	pub fn track_accepted_outcomes(&self, scoping: Scoping, buckets: &mut [Bucket]) {

Conversation

klochek commented Jun 9, 2026

Uh oh!

linear-code Bot commented Jun 9, 2026

Uh oh!

loewenheim left a comment

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!

tobias-wilfert Jun 10, 2026 • 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!

Dav1dde Jun 11, 2026 • 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!

Choose a reason for hiding this comment

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sentry Bot Jun 11, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

tobias-wilfert Jun 10, 2026 •

edited

Loading

Dav1dde Jun 11, 2026 •

edited

Loading