fix(attachments): Store the attachment file name in objectstore

CHANGELOG.md

@@ -6,6 +6,10 @@
 
 - Extract nvgpu dumps and create GPU events. ([#6242](https://github.com/getsentry/relay/pull/6242))
 
+**Bug Fixes**:
+
+- Store the attachment file name in objectstore so that downloads are named after the attachment. ([#6276](https://github.com/getsentry/relay/pull/6276))
+
 ## 26.7.2
 
 **Features**:

Cargo.lock

@@ -3360,9 +3360,9 @@ dependencies = [
 
 [[package]]
 name = "objectstore-client"
-version = "0.1.12"
+version = "0.2.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "2713028097b2f0f00696d7d389a877f7370c02813548124c4422941f669732f3"
+checksum = "ae3b20b3a34e77e1ca87bdd4b1020fab90d7563f8acac3645e70fb7bb845c4cf"
 dependencies = [
  "async-compression",
  "base64",
@@ -3385,10 +3385,12 @@ dependencies = [
 
 [[package]]
 name = "objectstore-types"
-version = "0.1.12"
+version = "0.2.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "c83aedfcc050322caefbfdba0372518f316624995f04f97a1f00cafb08ce1d0c"
+checksum = "452bd363377008a82744aaafa5416caafc34fd7c2f40c2ba3635bfa7af6b2e58"
 dependencies = [
+ "base64",
+ "ed25519-dalek",
  "http",
  "humantime",
  "humantime-serde",
@@ -6562,7 +6564,6 @@ dependencies = [
  "bytes",
  "futures-core",
  "futures-sink",
- "futures-util",
  "pin-project-lite",
  "tokio",
 ]

Cargo.toml

@@ -159,8 +159,8 @@ metrics = "0.24"
 metrics-exporter-dogstatsd = "0.9"
 num-traits = "0.2"
 num_cpus = "1"
-objectstore-client = { version = "0.1", features = ["multipart"] }
-objectstore-types = { version = "0.1" }
+objectstore-client = { version = "0.2.0", features = ["multipart"] }
+objectstore-types = { version = "0.2.0" }
 opentelemetry-semantic-conventions = "0.31"
 opentelemetry-proto = { version = "0.31", default-features = false }
 papaya = "0.2"

relay-server/src/services/objectstore.rs

@@ -560,14 +560,18 @@ impl ObjectstoreServiceInner {
         });
 
         for mut attachment in attachments.split(|e| e) {
+            let attributes = ObjectAttributes {
+                filename: attachment.filename().map(String::from),
+                ..Default::default()
+            };
+
             let result = self
                 .upload_bytes(
                     MessageKind::Event,
                     &session,
                     attachment.payload(),
                     event.retention_days,
-                    None,
-                    None,
+                    attributes,
                 )
                 .await;
 
@@ -613,13 +617,17 @@ impl ObjectstoreServiceInner {
         let upload_result = match session {
             Err(error) => Err(error),
             Ok(session) => {
+                let attributes = ObjectAttributes {
+                    filename: attachment.attachment.filename().map(String::from),
+                    ..Default::default()
+                };
+
                 self.upload_bytes(
                     MessageKind::EventAttachment,
                     &session,
                     attachment.attachment.payload(),
                     attachment.retention,
-                    None,
-                    None,
+                    attributes,
                 )
                 .await
             }
@@ -683,14 +691,18 @@ impl ObjectstoreServiceInner {
             #[cfg(debug_assertions)]
             let original_key = key.clone();
 
+            let attributes = ObjectAttributes {
+                key: Some(key),
+                ..Default::default()
+            };
+
             let _stored_key = self
                 .upload_bytes(
                     MessageKind::TraceAttachment,
                     &session,
                     body,
                     retention,
-                    Some(key),
-                    None,
+                    attributes,
                 )
                 .await
                 .reject(&trace_item)?;
@@ -754,14 +766,18 @@ impl ObjectstoreServiceInner {
             .for_project(scoping.organization_id.value(), scoping.project_id.value())
             .session(&self.objectstore_client)?;
 
+        let attributes = ObjectAttributes {
+            content_type: Some(content_type),
+            ..Default::default()
+        };
+
         let stored_key = self
             .upload_bytes(
                 MessageKind::RawProfile,
                 &session,
                 payload,
                 retention,
-                None,
-                Some(content_type),
+                attributes,
             )
             .await?;
 
@@ -818,9 +834,13 @@ impl ObjectstoreServiceInner {
         session: &Session,
         payload: Bytes,
         retention: u16,
-        key: Option<String>,
-        content_type: Option<ContentType>,
+        attributes: ObjectAttributes,
     ) -> Result<ObjectstoreKey, Error> {
+        let ObjectAttributes {
+            key,
+            content_type,
+            filename,
+        } = attributes;
         let retention_hours = retention.checked_mul(24);
         self.upload(
             kind,
@@ -830,6 +850,7 @@ impl ObjectstoreServiceInner {
                 key,
                 retention_hours,
                 content_type,
+                filename,
             },
         )
         .await
@@ -896,11 +917,15 @@ impl ObjectstoreServiceInner {
                 key,
                 retention_hours,
                 content_type,
+                filename,
             } => {
                 let mut request = session.put(body);
                 if let Some(content_type) = content_type {
                     request = request.content_type(content_type.as_str());
                 }
+                if let Some(filename) = filename {
+                    request = request.filename(filename);
+                }
                 if let Some(retention_hours) = retention_hours {
                     request = request.expiration_policy(ExpirationPolicy::TimeToLive(
                         Duration::from_hours(retention_hours.into()),
@@ -994,6 +1019,22 @@ impl ObjectstoreServiceInner {
     }
 }
 
+/// Optional attributes of an object stored in objectstore.
+#[derive(Debug, Default)]
+struct ObjectAttributes {
+    /// The key to store the object under.
+    ///
+    /// If this is `None`, objectstore assigns a random key.
+    key: Option<String>,
+    /// The content type of the payload.
+    content_type: Option<ContentType>,
+    /// The file name that downloads of this object are named after.
+    ///
+    /// Objectstore only sends a `Content-Disposition` header for objects that were stored with a
+    /// file name.
+    filename: Option<String>,
+}
+
 /// Common interface for calls to [`ObjectstoreServiceInner::upload`].
 ///
 /// This type is shared across retries.
@@ -1003,6 +1044,7 @@ enum Upload {
         key: Option<String>,
         retention_hours: Option<u16>,
         content_type: Option<ContentType>,
+        filename: Option<String>,
     },
     Stream {
         body: TakeOnce<BoundedStream<MeteredStream<ByteStream>>>,
@@ -1018,11 +1060,13 @@ impl Upload {
                 key,
                 retention_hours,
                 content_type,
+                filename,
             } => Some(UploadAttempt::Bytes {
                 body: body.clone(),
                 key: key.clone(),
                 retention_hours: *retention_hours,
                 content_type: *content_type,
+                filename: filename.clone(),
             }),
             Self::Stream { body, upload_ref } => {
                 RetryableStream::new(body.clone()).map(|body| UploadAttempt::Stream {
@@ -1043,6 +1087,7 @@ enum UploadAttempt {
         key: Option<String>,
         retention_hours: Option<u16>,
         content_type: Option<ContentType>,
+        filename: Option<String>,
     },
     Stream {
         body: RetryableStream<BoundedStream<MeteredStream<ByteStream>>>,

tests/integration/test_attachments.py

@@ -650,6 +650,7 @@ def test_event_with_attachment(
         Item(
             type="attachment",
             payload=PayloadRef(bytes=b"event attachment"),
+            filename="event.txt",
         )
     )
 
@@ -664,7 +665,7 @@ def test_event_with_attachment(
     assert event_message["attachments"][0].pop("id")
     assert list(event_message["attachments"]) == [
         {
-            "name": "Unnamed Attachment",
+            "name": "event.txt",
             "rate_limited": False,
             "content_type": "application/octet-stream",
             "attachment_type": "event.attachment",
@@ -676,7 +677,10 @@ def test_event_with_attachment(
 
     if use_objectstore:
         stored_id = event_message["attachments"][0]["stored_id"]
-        assert objectstore.get(stored_id).payload.read() == b"event attachment"
+        stored = objectstore.get(stored_id)
+        assert stored.payload.read() == b"event attachment"
+        # The file name is required for `Content-Disposition` on downloads.
+        assert stored.metadata.filename == "event.txt"
 
     # transaction attachments are sent as individual attachments,
     # either using chunks by default, or contents inlined
@@ -686,13 +690,14 @@ def test_event_with_attachment(
         Item(
             type="attachment",
             payload=PayloadRef(bytes=b"transaction attachment"),
+            filename="transaction.txt",
         )
     )
 
     relay.send_envelope(project_id, envelope)
 
     expected_attachment = {
-        "name": "Unnamed Attachment",
+        "name": "transaction.txt",
         "rate_limited": False,
         "content_type": "application/octet-stream",
         "attachment_type": "event.attachment",
@@ -710,7 +715,9 @@ def test_event_with_attachment(
 
     if use_objectstore:
         stored_id = attachment["attachment"]["stored_id"]
-        assert objectstore.get(stored_id).payload.read() == b"transaction attachment"
+        stored = objectstore.get(stored_id)
+        assert stored.payload.read() == b"transaction attachment"
+        assert stored.metadata.filename == "transaction.txt"
 
     assert attachment == {
         "type": "attachment",