feat(replays): Add error and trace uuid validation (#2931)

`annotated_struct.field.meta().has_errors()` does not work as expected. I had to access `struct.field.0`.
getsentry · Jan 11, 2024 · 16fd6db · 16fd6db
1 parent da081f4
commit 16fd6db
Show file tree

Hide file tree

Showing 5 changed files with 105 additions and 12 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -19,6 +19,7 @@
 - Make Kafka spans compatible with the Snuba span schema. ([#2917](https://github.com/getsentry/relay/pull/2917), [#2926](https://github.com/getsentry/relay/pull/2926))
 - Only extract span metrics / tags when they are needed. ([#2907](https://github.com/getsentry/relay/pull/2907), [#2923](https://github.com/getsentry/relay/pull/2923), [#2924](https://github.com/getsentry/relay/pull/2924))
 - Normalize metric resource identifiers in `event._metrics_summary` and `span._metrics_summary`. ([#2914](https://github.com/getsentry/relay/pull/2914))
+- Validate error_id and trace_id vectors in replay deserializer. ([#2931](https://github.com/getsentry/relay/pull/2931))
 
 ## 23.12.1
 

diff --git a/py/CHANGELOG.md b/py/CHANGELOG.md
@@ -4,6 +4,7 @@
 
 - This release requires Python 3.9 or later. There are no intentionally breaking changes included in this release, but we stopped testing against Python 3.8.
 - Normalize event timestamps before validating them, fixing cases where Relay would drop valid events with reason "invalid_transaction". ([#2878](https://github.com/getsentry/relay/pull/2878))
+- Normalize error and trace-ids. Values must be valid UUIDs. ([#2931](https://github.com/getsentry/relay/pull/2931))
 
 ## 0.8.39
 

diff --git a/relay-event-normalization/src/replay.rs b/relay-event-normalization/src/replay.rs
@@ -55,6 +55,30 @@ pub fn validate(replay: &Replay) -> Result<(), ReplayError> {
         ));
     }
 
+    if replay
+        .error_ids
+        .value()
+        .into_iter()
+        .flat_map(|v| v.iter())
+        .any(|v| v.meta().has_errors())
+    {
+        return Err(ReplayError::InvalidPayload(
+            "Invalid error-id specified.".to_string(),
+        ));
+    }
+
+    if replay
+        .trace_ids
+        .value()
+        .into_iter()
+        .flat_map(|v| v.iter())
+        .any(|v| v.meta().has_errors())
+    {
+        return Err(ReplayError::InvalidPayload(
+            "Invalid trace-id specified.".to_string(),
+        ));
+    }
+
     Ok(())
 }
 
@@ -137,6 +161,7 @@ mod tests {
     use std::net::{IpAddr, Ipv4Addr};
 
     use chrono::{TimeZone, Utc};
+    use uuid::Uuid;
 
     use relay_event_schema::protocol::{
         BrowserContext, Context, DeviceContext, EventId, OsContext, TagEntry, Tags,
@@ -182,10 +207,10 @@ mod tests {
             ),
             urls: Annotated::new(vec![Annotated::new("localhost:9000".to_string())]),
             error_ids: Annotated::new(vec![Annotated::new(
-                "52df9022835246eeb317dbd739ccd059".to_string(),
+                Uuid::parse_str("52df9022835246eeb317dbd739ccd059").unwrap(),
             )]),
             trace_ids: Annotated::new(vec![Annotated::new(
-                "52df9022835246eeb317dbd739ccd059".to_string(),
+                Uuid::parse_str("52df9022835246eeb317dbd739ccd059").unwrap(),
             )]),
             platform: Annotated::new("myplatform".to_string()),
             release: Annotated::new("myrelease".to_string().into()),
@@ -309,12 +334,12 @@ mod tests {
             .map(|_| Annotated::new("localhost:9000".to_string()))
             .collect();
 
-        let error_ids: Vec<Annotated<String>> = (0..101)
-            .map(|_| Annotated::new("52df9022835246eeb317dbd739ccd059".to_string()))
+        let error_ids: Vec<Annotated<Uuid>> = (0..101)
+            .map(|_| Annotated::new(Uuid::parse_str("52df9022835246eeb317dbd739ccd059").unwrap()))
             .collect();
 
-        let trace_ids: Vec<Annotated<String>> = (0..101)
-            .map(|_| Annotated::new("52df9022835246eeb317dbd739ccd059".to_string()))
+        let trace_ids: Vec<Annotated<Uuid>> = (0..101)
+            .map(|_| Annotated::new(Uuid::parse_str("52df9022835246eeb317dbd739ccd059").unwrap()))
             .collect();
 
         let mut replay = Annotated::new(Replay {
@@ -373,4 +398,68 @@ mod tests {
         assert!(replay_value.trace_ids.value().unwrap().is_empty());
         assert!(replay_value.urls.value().unwrap().is_empty());
     }
+
+    #[test]
+    fn test_error_id_validation() {
+        // NOTE: Interfaces will be tested separately.
+        let json = r#"{
+  "event_id": "52df9022835246eeb317dbd739ccd059",
+  "replay_id": "52df9022835246eeb317dbd739ccd059",
+  "segment_id": 0,
+  "replay_type": "session",
+  "error_sample_rate": 0.5,
+  "session_sample_rate": 0.5,
+  "timestamp": 946684800.0,
+  "replay_start_timestamp": 946684800.0,
+  "urls": ["localhost:9000"],
+  "error_ids": ["test"],
+  "trace_ids": [],
+  "platform": "myplatform",
+  "release": "myrelease",
+  "dist": "mydist",
+  "environment": "myenv",
+  "tags": [
+    [
+      "tag",
+      "value"
+    ]
+  ]
+}"#;
+
+        let mut replay = Annotated::<Replay>::from_json(json).unwrap();
+        let validation_result = validate(replay.value_mut().as_mut().unwrap());
+        assert!(validation_result.is_err());
+    }
+
+    #[test]
+    fn test_trace_id_validation() {
+        // NOTE: Interfaces will be tested separately.
+        let json = r#"{
+  "event_id": "52df9022835246eeb317dbd739ccd059",
+  "replay_id": "52df9022835246eeb317dbd739ccd059",
+  "segment_id": 0,
+  "replay_type": "session",
+  "error_sample_rate": 0.5,
+  "session_sample_rate": 0.5,
+  "timestamp": 946684800.0,
+  "replay_start_timestamp": 946684800.0,
+  "urls": ["localhost:9000"],
+  "error_ids": [],
+  "trace_ids": ["123"],
+  "platform": "myplatform",
+  "release": "myrelease",
+  "dist": "mydist",
+  "environment": "myenv",
+  "tags": [
+    [
+      "tag",
+      "value"
+    ]
+  ]
+}"#;
+
+        let mut replay = Annotated::<Replay>::from_json(json).unwrap();
+        let validation_result = validate(replay.value_mut().as_mut().unwrap());
+        assert!(validation_result.is_err());
+    }
 }
diff --git a/relay-event-schema/src/protocol/replay.rs b/relay-event-schema/src/protocol/replay.rs
@@ -33,6 +33,7 @@ use crate::processor::ProcessValue;
 use crate::protocol::{
     ClientSdkInfo, Contexts, EventId, LenientString, Request, Tags, Timestamp, User,
 };
+use uuid::Uuid;
 
 #[derive(Clone, Debug, Default, PartialEq, Empty, FromValue, IntoValue, ProcessValue)]
 #[cfg_attr(feature = "jsonschema", derive(JsonSchema))]
@@ -136,11 +137,11 @@ pub struct Replay {
 
     /// A list of error-ids discovered during the lifetime of the segment.
     #[metastructure(bag_size = "medium")]
-    pub error_ids: Annotated<Array<String>>,
+    pub error_ids: Annotated<Array<Uuid>>,
 
     /// A list of trace-ids discovered during the lifetime of the segment.
     #[metastructure(bag_size = "medium")]
-    pub trace_ids: Annotated<Array<String>>,
+    pub trace_ids: Annotated<Array<Uuid>>,
 
     /// Contexts describing the environment (e.g. device, os or browser).
     #[metastructure(skip_serialization = "empty")]
@@ -265,10 +266,10 @@ mod tests {
             ),
             urls: Annotated::new(vec![Annotated::new("localhost:9000".to_string())]),
             error_ids: Annotated::new(vec![Annotated::new(
-                "52df9022835246eeb317dbd739ccd059".to_string(),
+                Uuid::parse_str("52df9022835246eeb317dbd739ccd059").unwrap(),
             )]),
             trace_ids: Annotated::new(vec![Annotated::new(
-                "52df9022835246eeb317dbd739ccd059".to_string(),
+                Uuid::parse_str("52df9022835246eeb317dbd739ccd059").unwrap(),
             )]),
             platform: Annotated::new("myplatform".to_string()),
             release: Annotated::new("myrelease".to_string().into()),

diff --git a/tests/integration/test_replay_events.py b/tests/integration/test_replay_events.py
@@ -1,4 +1,5 @@
 import json
+import uuid
 
 
 def generate_replay_sdk_event():
@@ -11,8 +12,8 @@ def generate_replay_sdk_event():
         "timestamp": 1597977777.6189718,
         "replay_start_timestamp": 1597976392.6542819,
         "urls": ["sentry.io"],
-        "error_ids": ["1", "2"],
-        "trace_ids": ["3", "4"],
+        "error_ids": [str(uuid.uuid4())],
+        "trace_ids": [str(uuid.uuid4())],
         "dist": "1.12",
         "platform": "javascript",
         "environment": "production",