feat(attachments): Migrate standalone logic by tobias-wilfert · Pull Request #5703 · getsentry/relay

tobias-wilfert · 2026-03-06T14:29:43Z

Moves the standalone attachment logic out of process_standalone, migrating the logic to the new processor architecture.

Ref: https://linear.app/getsentry/issue/INGEST-796/extract-attachment-logic-from-process-standalone

linear-code · 2026-03-06T14:30:20Z

INGEST-796 Extract attachment logic from `process_standalone`

tobias-wilfert · 2026-03-06T14:33:41Z

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

+    ) -> Option<Managed<Self::UnitOfWork>> {
+        // For now only extract the standalone attachments.
+        if envelope.envelope().items().any(Item::creates_event) {
+            return None;
+        };


Not sure if we can just grab attachments here, since I would assume that this break in the future.

I think it's fine to remove the check, but instead maybe explicitly turn it into a debug_assert!

…achments

…github.com:getsentry/relay into tobias-wilfert/feat/migrate-standalone-attachments

tobias-wilfert · 2026-03-09T15:54:37Z

tests/integration/test_attachments.py

-    attachments = [
-        ("att_1", "foo.txt", chunked_contents),
-        ("att_2", "foobar.txt", b""),
-    ]
-    relay.send_attachments(project_id, event_id, attachments)
+    relay.send_attachments(
+        project_id, event_id, [("att_1", "foo.txt", chunked_contents)]
+    )


Split this into 2 tests since the 'wrong' attachment arrived first now, causing the test to fail.
Not sure how this never flaked before.

tobias-wilfert · 2026-03-09T15:55:14Z

relay-server/src/services/objectstore.rs

-pub struct StoreAttachment {
+pub struct StoreTraceAttachment {


Renamed this to avoid naming collisions.

tobias-wilfert · 2026-03-09T15:56:04Z

relay-server/src/services/objectstore.rs

+    /// Uploads the attachment.
+    ///
+    /// This mutates the attachment item in-place, setting the `stored_key` field to the key in the
+    /// objectstore.
+    async fn handle_attachment(&self, mut attachment: Managed<StoreAttachment>) {


Logic taken over from handle_envelope with minor changes.

…achments

…github.com:getsentry/relay into tobias-wilfert/feat/migrate-standalone-attachments

tobias-wilfert · 2026-03-09T16:13:24Z

Current plan is to deploy this without a feature flag, if there are objections to it I can still add one.

relay-server/src/services/processor.rs

cursor

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

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

✅ Fixed: Standalone attachments missing PII scrubbing for untyped items
- Changed filter to use unwrap_or_default() so attachments without explicit AttachmentType headers are correctly routed to StandaloneAttachments group for PII scrubbing.

Or push these changes by commenting:

@cursor push fc2208ed3e

Preview (fc2208ed3e)

diff --git a/relay-server/src/services/processor.rs b/relay-server/src/services/processor.rs
--- a/relay-server/src/services/processor.rs
+++ b/relay-server/src/services/processor.rs
@@ -380,7 +380,7 @@
         if !envelope.items().any(Item::creates_event) {
             let standalone_attachments = envelope.take_items_by(|i| {
                 i.requires_event()
-                    && matches!(i.attachment_type(), Some(AttachmentType::Attachment))
+                    && i.attachment_type().unwrap_or_default() == AttachmentType::Attachment
             });
             if !standalone_attachments.is_empty() {
                 grouped_envelopes.push((

_{This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.}

relay-server/src/services/processor.rs

relay-server/src/processing/attachments/forward.rs

Dav1dde · 2026-03-10T12:59:39Z

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

+                Some(Outcome::RateLimited(reason_code))
+            }
+            #[cfg(feature = "processing")]
+            Self::NoEventId => Some(Outcome::Invalid(DiscardReason::Internal)),


I assume Relay somehow already ensures there is always an event id?

What happens if the there is a standalone attachment sent in an envelope without event id? Does it make sense to ingest it? And does that actually mean it's Internal?

The old logic checks it in the store here, so this check "just moved into the processor".

relay/relay-server/src/services/store.rs

Line 371 in bf82189

event_id.ok_or(StoreError::NoEventId)?,

The outcome is the same as that of the store error (but we can ofcourse improve it):

relay/relay-server/src/services/store.rs

Lines 52 to 67 in bf82189

pub enum StoreError {

#[error("failed to send the message to kafka: {0}")]

SendFailed(#[from] ClientError),

#[error("failed to encode data: {0}")]

EncodingFailed(std::io::Error),

#[error("failed to store event because event id was missing")]

NoEventId,

}

impl OutcomeError for StoreError {

type Error = Self;

fn consume(self) -> (Option<Outcome>, Self::Error) {

(Some(Outcome::Invalid(DiscardReason::Internal)), self)

}

}

Dav1dde · 2026-03-10T13:00:22Z

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

+    ) -> Option<Managed<Self::UnitOfWork>> {
+        // For now only extract the standalone attachments.
+        if envelope.envelope().items().any(Item::creates_event) {
+            return None;
+        };


I think it's fine to remove the check, but instead maybe explicitly turn it into a debug_assert!

Dav1dde · 2026-03-10T13:01:22Z

relay-server/src/services/objectstore.rs

+            Err(error) => {
+                relay_log::error!(error = &error as &dyn std::error::Error, "session error");
+                relay_statsd::metric!(
+                    counter(RelayCounters::AttachmentUpload) += 1,
+                    result = error.to_string().as_str(),
+                    type = "attachment",
+                );
+            }


This match is pretty large, maybe an early return makes it more readable.

session.inspect_err(|_| <analytics here>)?;

The function is a bit sneaky in the sense that at the end we still do self.store.send(attachment) no matter what match arm we end up in.

Co-authored-by: David Herberth <david.herberth@sentry.io>

…achments

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.}

relay-server/src/services/processor.rs

initial logic

8d61d82

tobias-wilfert self-assigned this Mar 6, 2026

tobias-wilfert commented Mar 6, 2026

View reviewed changes

tobias-wilfert and others added 4 commits March 6, 2026 15:36

Merge branch 'master' into tobias-wilfert/feat/migrate-standalone-att…

3080083

…achments

split logic into different files & send attachment to object store

9d30699

Merge branch 'tobias-wilfert/feat/migrate-standalone-attachments' of …

e31ba26

…github.com:getsentry/relay into tobias-wilfert/feat/migrate-standalone-attachments

split strange test into two

f2af288

tobias-wilfert commented Mar 9, 2026

View reviewed changes

tobias-wilfert and others added 5 commits March 9, 2026 16:59

Merge branch 'master' into tobias-wilfert/feat/migrate-standalone-att…

4990f24

…achments

remove old code

3e99a9e

add changelog entry

2c0674b

Merge branch 'tobias-wilfert/feat/migrate-standalone-attachments' of …

c3a1bc5

…github.com:getsentry/relay into tobias-wilfert/feat/migrate-standalone-attachments

small fixes

b22e064

tobias-wilfert marked this pull request as ready for review March 9, 2026 16:13

tobias-wilfert requested a review from a team as a code owner March 9, 2026 16:13

sentry bot reviewed Mar 9, 2026

View reviewed changes

relay-server/src/services/processor.rs Outdated Show resolved Hide resolved

cursor bot reviewed Mar 9, 2026

View reviewed changes

relay-server/src/services/processor.rs Show resolved Hide resolved

Dav1dde approved these changes Mar 10, 2026

View reviewed changes

tobias-wilfert and others added 4 commits March 10, 2026 14:15

match on the item type rather than attachment type

51816ad

Update relay-server/src/processing/attachments/forward.rs

3e542be

Co-authored-by: David Herberth <david.herberth@sentry.io>

Merge branch 'master' into tobias-wilfert/feat/migrate-standalone-att…

60269da

…achments

change if to debug assert

7600e86

cursor bot reviewed Mar 10, 2026

View reviewed changes

relay-server/src/services/processor.rs Show resolved Hide resolved

add lenient for ViewHierarchyScrubbing

3c7ccca

tobias-wilfert requested a review from jjbayer March 10, 2026 14:04

		pub struct StoreAttachment {
		pub struct StoreTraceAttachment {

	pub enum StoreError {
	#[error("failed to send the message to kafka: {0}")]
	SendFailed(#[from] ClientError),
	#[error("failed to encode data: {0}")]
	EncodingFailed(std::io::Error),
	#[error("failed to store event because event id was missing")]
	NoEventId,
	}

	impl OutcomeError for StoreError {
	type Error = Self;

	fn consume(self) -> (Option<Outcome>, Self::Error) {
	(Some(Outcome::Invalid(DiscardReason::Internal)), self)
	}
	}

Conversation

tobias-wilfert commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

linear-code bot commented Mar 6, 2026

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!

tobias-wilfert commented Mar 9, 2026

Uh oh!

Uh oh!

cursor bot left a comment • 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!

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!

cursor bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

tobias-wilfert commented Mar 6, 2026 •

edited

Loading

cursor bot left a comment •

edited

Loading