fix(attachments): Serialize ID without hyphens (#5501)

Author: jjbayer

relay-event-schema/src/protocol/trace_attachment.rs

@@ -1,4 +1,8 @@
-use relay_protocol::{Annotated, Empty, FromValue, IntoValue, Object, Value};
+use std::fmt;
+use std::ops::Deref;
+
+use relay_protocol::{Annotated, Empty, FromValue, IntoValue, Object, SkipSerialization, Value};
+use serde::Serializer;
 
 use crate::processor::ProcessValue;
 use crate::protocol::{Attributes, Timestamp, TraceId};
@@ -14,7 +18,7 @@ pub struct TraceAttachmentMeta {
 
     /// Unique identifier for this attachment.
     #[metastructure(required = true, nonempty = true, trim = false)]
-    pub attachment_id: Annotated<Uuid>,
+    pub attachment_id: Annotated<AttachmentId>,
 
     /// Timestamp when the attachment was created.
     #[metastructure(required = true, trim = false)]
@@ -36,3 +40,50 @@ pub struct TraceAttachmentMeta {
     #[metastructure(additional_properties, pii = "maybe")]
     pub other: Object<Value>,
 }
+
+#[derive(Clone, Default, PartialEq, Empty, FromValue, ProcessValue)]
+pub struct AttachmentId(Uuid);
+
+impl AttachmentId {
+    /// Creates a new attachment ID. Only used in tests.
+    pub fn random() -> Self {
+        Self(Uuid::now_v7())
+    }
+}
+
+impl Deref for AttachmentId {
+    type Target = Uuid;
+
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
+
+impl fmt::Display for AttachmentId {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "{}", self.0.as_simple())
+    }
+}
+
+impl fmt::Debug for AttachmentId {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "AttachmentId(\"{}\")", self.0.as_simple())
+    }
+}
+
+impl IntoValue for AttachmentId {
+    fn into_value(self) -> Value
+    where
+        Self: Sized,
+    {
+        Value::String(self.to_string())
+    }
+
+    fn serialize_payload<S>(&self, s: S, _behavior: SkipSerialization) -> Result<S::Ok, S::Error>
+    where
+        Self: Sized,
+        S: Serializer,
+    {
+        s.collect_str(self)
+    }
+}

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

@@ -168,10 +168,9 @@ pub fn scrub_attachment<'a>(
 
 #[cfg(test)]
 mod tests {
-    use relay_event_schema::protocol::TraceAttachmentMeta;
+    use relay_event_schema::protocol::{AttachmentId, TraceAttachmentMeta};
     use relay_pii::PiiConfig;
     use relay_protocol::SerializableAnnotated;
-    use uuid::Uuid;
 
     use crate::envelope::ParentId;
     use crate::services::projects::project::ProjectInfo;
@@ -189,7 +188,7 @@ mod tests {
         let mut attachment = ExpandedAttachment {
             parent_id: None,
             meta: Annotated::new(TraceAttachmentMeta {
-                attachment_id: Annotated::new(Uuid::new_v4()),
+                attachment_id: Annotated::new(AttachmentId::random()),
                 filename: Annotated::new("data.txt".to_owned()),
                 content_type: Annotated::new("text/plain".to_owned()),
                 ..Default::default()
@@ -235,7 +234,7 @@ mod tests {
         let mut attachment = ExpandedAttachment {
             parent_id: Some(ParentId::SpanId(None)),
             meta: Annotated::new(TraceAttachmentMeta {
-                attachment_id: Annotated::new(Uuid::new_v4()),
+                attachment_id: Annotated::new(AttachmentId::random()),
                 filename: Annotated::new("data.txt".to_owned()),
                 content_type: Annotated::new("text/plain".to_owned()),
                 attributes: Annotated::new(attributes),

relay-server/src/services/upload.rs

@@ -336,6 +336,7 @@ impl UploadServiceInner {
             let key = Uuid::from_slice(&trace_item.trace_item.item_id)
                 .map_err(Error::from)
                 .reject(&trace_item)?
+                .as_simple()
                 .to_string();
 
             #[cfg(debug_assertions)]

tests/integration/test_attachmentsv2.py

@@ -24,7 +24,7 @@
 def create_attachment_metadata():
     return {
         "trace_id": uuid.uuid4().hex,
-        "attachment_id": str(uuid.uuid4()),
+        "attachment_id": uuid.uuid4().hex,
         "timestamp": 1760520026.781239,
         "filename": "myfile.txt",
         "content_type": "text/plain",
@@ -37,7 +37,6 @@ def create_attachment_metadata():
 def create_attachment_envelope(project_config):
     return Envelope(
         headers={
-            "event_id": "515539018c9b4260a6f999572f1661ee",
             "trace": {
                 "trace_id": "5b8efff798038103d269b633813fc60c",
                 "public_key": project_config["publicKeys"][0]["publicKey"],
@@ -203,7 +202,7 @@ def test_standalone_attachment_store(
         ),
         pytest.param(
             {"meta_length": None},
-            273,
+            269,
             "invalid_trace_attachment",
             id="missing_meta_length",
         ),
@@ -341,9 +340,9 @@ def test_attachment_with_matching_span(mini_sentry, relay):
     assert attachment.payload.bytes == combined_payload
     assert attachment.headers == {
         "type": "attachment",
-        "length": 260,
+        "length": 256,
         "content_type": "application/vnd.sentry.trace-attachment",
-        "meta_length": 237,
+        "meta_length": 233,
         "span_id": span_id,
     }
 
@@ -544,9 +543,9 @@ def test_two_attachments_mapping_to_same_span(mini_sentry, relay):
         assert item.payload.bytes == combined_payload
         assert item.headers == {
             "type": "attachment",
-            "length": 260,
+            "length": 256,
             "content_type": "application/vnd.sentry.trace-attachment",
-            "meta_length": 237,
+            "meta_length": 233,
             "span_id": span_id,
         }
 
@@ -1260,7 +1259,7 @@ def test_attachment_default_pii_scrubbing_meta(
     )
 
     metadata = {
-        "attachment_id": str(uuid.uuid4()),
+        "attachment_id": uuid.uuid4().hex,
         "timestamp": ts.timestamp(),
         "filename": "data.txt",
         "content_type": "text/plain",
@@ -1304,7 +1303,7 @@ def test_attachment_default_pii_scrubbing_meta(
     metadata_part = json.loads(payload[:meta_length].decode("utf-8"))
 
     assert metadata_part == {
-        "attachment_id": mock.ANY,
+        "attachment_id": metadata["attachment_id"],
         "timestamp": time_within_delta(ts),
         "filename": "data.txt",
         "content_type": "text/plain",
@@ -1364,7 +1363,7 @@ def test_attachment_pii_scrubbing_meta_attribute(
     )
 
     metadata = {
-        "attachment_id": str(uuid.uuid4()),
+        "attachment_id": uuid.uuid4().hex,
         "timestamp": ts.timestamp(),
         "filename": "data.txt",
         "content_type": "text/plain",
@@ -1398,7 +1397,7 @@ def test_attachment_pii_scrubbing_meta_attribute(
     metadata_part = json.loads(payload[:meta_length].decode("utf-8"))
 
     assert metadata_part == {
-        "attachment_id": mock.ANY,
+        "attachment_id": metadata["attachment_id"],
         "timestamp": time_within_delta(ts),
         "filename": "data.txt",
         "content_type": "text/plain",
@@ -1455,7 +1454,7 @@ def test_attachment_pii_scrubbing_body(mini_sentry, relay):
     )
 
     metadata = {
-        "attachment_id": str(uuid.uuid4()),
+        "attachment_id": uuid.uuid4().hex,
         "timestamp": ts.timestamp(),
         "filename": "log.txt",
         "content_type": "text/plain",