ref(processing): Use new transactions processor #5379

jjbayer · 2025-11-14T08:30:21Z

linear · 2025-11-14T08:30:24Z

INGEST-610 Migrate `process_transactions`

jjbayer · 2025-11-20T13:03:41Z

relay-server/src/processing/transactions/mod.rs

+pub struct ExpandedTransaction<C> {
    headers: EnvelopeHeaders,
-    transaction: T, // might be empty
+    event: Annotated<Event>,


I changed this from a wrapper type that implements Counted to a phantom-data-style field, for three reasons:

Quantities for the wrapper type could not be inferred correctly because we need to check flags.spans_extracted to get the quantities.

Consistency with the spans processor.

Simpler code.

relay-server/src/processing/transactions/mod.rs

relay-server/src/processing/transactions/process.rs

jjbayer · 2025-11-20T15:12:26Z

relay-server/src/processing/transactions/profile.rs

-    } else if work.transaction.0.value().is_none() && profile_item.sampled() {
-        // A profile with `sampled=true` should never be without a transaction
-        record_keeper.reject_err(
-            Outcome::Invalid(DiscardReason::Profiling("missing_transaction")),
-            work.profile.take(),
-        );


This is leftover code, there is always a transaction.

jjbayer · 2025-11-20T15:13:08Z

tests/integration/test_metrics.py

+        {"reason": "release-version", "category": "span", "quantity": 2},
+        {"reason": "release-version", "category": "span_indexed", "quantity": 2},


This now counts quantities correctly when the transaction is filtered.

jjbayer · 2025-11-20T15:13:22Z

tests/integration/fixtures/mini_sentry.py

                },
                "blacklistedIps": ["127.43.33.22"],
                "trustedRelays": [],
+                "transactionMetrics": {"version": 3},


This is now a requirement, otherwise debug assertions will fail.

jjbayer · 2025-11-20T15:13:54Z

tests/integration/test_outcome.py

+            (DataCategory.TRANSACTION, 1, "invalid_transaction"),
+            (DataCategory.TRANSACTION_INDEXED, 1, "invalid_transaction"),
+            (DataCategory.SPAN, 1, "invalid_span"),
+            (DataCategory.SPAN, 2, "invalid_transaction"),


This now counts embedded spans correctly.

Dav1dde · 2025-11-21T07:17:40Z

relay-server/src/processing/transactions/extraction.rs

        metrics_extracted,
        spans_extracted,
    } = ctx;
+    // TODO(follow-up): this function should always extract metrics. Dynamic sampling should validate


Wanna link to a GH issue here?

Done: #5403

Dav1dde · 2025-11-21T07:18:52Z

relay-server/src/processing/transactions/mod.rs

+        #[cfg(debug_assertions)]
+        event.ensure_span_count();


Doesn't that mess with the Managed assertions and we may get misaccounting in prod, but not in tests because this is ran in tests but not in production?

Or is this there to specifically defuse these Managed assertions and it's okay if we misaccount in prod until the transaction is parsed?

Or is this there to specifically defuse these Managed assertions and it's okay if we misaccount in prod until the transaction is parsed?

Yes, I believe it's not worth shallow-parsing in prod, because it gets properly parsed immediately afterwards.

Dav1dde · 2025-11-21T07:22:32Z

relay-server/src/processing/transactions/mod.rs

+            category: _,
+        } = self;
+        debug_assert!(flags.metrics_extracted);
+        let mut quantities = smallvec![(DataCategory::TransactionIndexed, 1),];


Suggested change

let mut quantities = smallvec![(DataCategory::TransactionIndexed, 1),];

let mut quantities = smallvec![(DataCategory::TransactionIndexed, 1)];

Dav1dde · 2025-11-21T07:24:46Z

relay-server/src/processing/transactions/mod.rs

+        let limits = rate_limiter
+            .try_consume(scoping.item(DataCategory::Transaction), 1)
+            .await;


If the transaction is limited, we can already drop the entire payload and don't need to consider attachments and profiles, right?

Yes, I thought I had an early return here, good catch.

Dav1dde · 2025-11-21T07:29:55Z

relay-server/src/processing/transactions/mod.rs

+        // If there is a transaction limit, drop everything.
+        // This also affects profiles that lost their transaction due to sampling.
+        let limits = rate_limiter
+            .try_consume(scoping.item(DataCategory::TransactionIndexed), 1)


I think you need to check both categories here, that's what the EnvelopeLimiter does.

Note fn is_event_active() checks both categories, indexed and non-indexed as well.

Naivly I would now expect transaction rate limits never to apply, what is strange, I would've expected a test to fail here. Am I missing something?

The spans processor always only rate limits total + indexed, it extracts the metrics only after rate limiting. I think this is also what we should do here.

Then you also don't have to implement RateLimited twice.

Dav1dde · 2025-11-21T07:33:30Z

relay-server/src/processing/common.rs

    ($($variant:ident => $ty:ty,)*) => {
        /// All known [`Processor`] outputs.
        #[derive(Debug)]
+        #[allow(clippy::large_enum_variant)]


Suggested change

#[allow(clippy::large_enum_variant)]

#[expect(clippy::large_enum_variant), reason = "..."]

Alternatively we can also box the transaction output, I think that kinda depends why the lint triggers, if it's just because of a small-ish amount it's fine, if there is a large discrepancy maybe it's worth boxing.

relay-server/src/processing/transactions/mod.rs

relay-server/src/metrics/rate_limits.rs

cursor · 2025-11-26T12:56:20Z

relay-server/src/processing/transactions/mod.rs

        }

+        Ok(())
+    }


Bug: Rate limiting doesn't check indexed transaction quota

The rate limiting implementation for ExpandedTransaction<TotalAndIndexed> only checks the Transaction category quota, but the quantities() method reports both Transaction and TransactionIndexed categories. This means TransactionIndexed quota limits are never enforced for transactions before metrics extraction. In contrast, the ExpandedTransaction<Indexed> rate limiting correctly checks both categories (lines 537-546). Without checking the indexed category, rate limits on indexed transactions can be bypassed.

Additional Locations (1)

relay-server/src/processing/transactions/mod.rs#L467-L478

cursor · 2025-12-03T14:10:02Z

relay-server/src/processing/transactions/mod.rs

+            let main = match self.limiter.enforce_quotas(&mut indexed, ctx).await {
+                Err(e) => {
+                    if let Error::RateLimited(rate_limits) = e.inner() {
+                        if rate_limits.is_any_limited(&[scoping.item(DataCategory::Transaction)]) {


Bug: Transaction rate limit check missing TransactionIndexed category

The enforce_quotas error handling for transactions only checks DataCategory::Transaction to decide if an event should be dropped. It misses DataCategory::TransactionIndexed limits, which the RateLimited implementation correctly checks. This causes transactions to not be dropped when only TransactionIndexed is rate-limited, and related integration tests are currently disabled.

Additional Locations (1)

tests/integration/test_store.py#L868-L897

ref(processing): Use new transactions processor

9fd743a

jjbayer added 28 commits November 17, 2025 09:36

fixes

4fd0055

refactor profile test

cea5ff1

ref

989b031

Log error on extraction failure

b962ef8

bookkeeping in metrics extraction

f063ad6

fix except rate limiting

add0dd8

fix

40b1326

split rate limiting code

ce20017

rust tests pass

4ccc557

feat(spans): Add span_count item header

d8a903a

Add TODO

4737ed3

ref: auto-ensure

9c27ac4

simplify

a1b42a2

ensure

6f242b8

ref

fb2c132

test

799103e

rust test

0ce93c7

fix integration test

1ebc03f

update test

0120631

doc: changelog

a2600b2

review comments

a9beb7c

test

5acad09

Merge branch 'feat/span-count' into ref/use-new-processor

8031075

Count spans in item quantities

976c7cd

instr: only measure actual parsing

886dbe7

fix tests

c095090

fix integration test

b1a4136

Fix all the tests

a336c2c

jjbayer added 6 commits November 20, 2025 13:17

Merge remote-tracking branch 'origin/master' into ref/use-new-processor

49d5fcb

fix: duplicate span counting

753c2e9

fix test assumptions

0263642

minor stuff

747a9a2

fix bookkeeping

6f1c486

ref: transaction at the end

e785f23

jjbayer commented Nov 20, 2025

View reviewed changes

jjbayer added 2 commits November 20, 2025 16:15

Update relay-server/src/processing/transactions/process.rs

cf08b4f

Update relay-server/src/processing/transactions/mod.rs

5256ccf

jjbayer marked this pull request as ready for review November 20, 2025 15:15

jjbayer requested a review from a team as a code owner November 20, 2025 15:15

Dav1dde reviewed Nov 21, 2025

View reviewed changes

jjbayer mentioned this pull request Nov 24, 2025

If metrics extraction fails, dynamic sampling should also fail #5403

Open

AbhiPrasad and others added 5 commits November 25, 2025 11:13

feat(ourlogs): Add event.name to otlp logs transform (#5396)

50c95a7

Merge remote-tracking branch 'origin/master' into ref/use-new-processor

b54d161

check both categories for indexed rate limits

ef89349

fix count on serialize

c432508

Accounting for attachment item

c0f78f9

cursor bot reviewed Nov 25, 2025

View reviewed changes

relay-server/src/processing/transactions/mod.rs Show resolved Hide resolved

test: add transaction metrics config in basic conf

903f969

cursor bot reviewed Nov 25, 2025

View reviewed changes

relay-server/src/metrics/rate_limits.rs Outdated Show resolved Hide resolved

jjbayer added 3 commits November 25, 2025 16:14

fix one test

c180a9f

fix: Remove debug assertions to maintain regression tests

cbc73f4

fix: consume transaction quota

72062fe

cursor bot reviewed Nov 26, 2025

View reviewed changes

jjbayer added 3 commits December 3, 2025 13:24

Merge remote-tracking branch 'origin/master' into ref/use-new-processor

1030517

fix: profile accounting

7c61e54

hacky version of indexed metrics extraction

ef1e88f

cursor bot reviewed Dec 3, 2025

View reviewed changes

jjbayer marked this pull request as draft December 4, 2025 13:43

		{"reason": "release-version", "category": "span", "quantity": 2},
		{"reason": "release-version", "category": "span_indexed", "quantity": 2},

	let mut quantities = smallvec![(DataCategory::TransactionIndexed, 1),];
	let mut quantities = smallvec![(DataCategory::TransactionIndexed, 1)];

	#[allow(clippy::large_enum_variant)]
	#[expect(clippy::large_enum_variant), reason = "..."]

                       }
+                      Ok(())
+                  }

ref(processing): Use new transactions processor #5379

Are you sure you want to change the base?

ref(processing): Use new transactions processor #5379

Uh oh!

Conversation

jjbayer commented Nov 14, 2025

Uh oh!

linear bot commented Nov 14, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

Dav1dde Nov 21, 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!

Uh oh!

Uh oh!

cursor bot Nov 26, 2025

Choose a reason for hiding this comment

Bug: Rate limiting doesn't check indexed transaction quota

Uh oh!

cursor bot Dec 3, 2025

Choose a reason for hiding this comment

Bug: Transaction rate limit check missing TransactionIndexed category

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Dav1dde Nov 21, 2025 •

edited

Loading