feat(normalization): Remove stale spans from transactions (#2627)

getsentry · Oct 20, 2023 · c535093 · c535093
1 parent 0602533
commit c535093
Show file tree

Hide file tree

Showing 3 changed files with 109 additions and 6 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,7 @@
 **Features**:
 
 - Update Docker Debian image from 10 to 12. ([#2622](https://github.com/getsentry/relay/pull/2622))
+- Remove from events spans starting or ending before January 1, 1970 UTC. ([#2627](https://github.com/getsentry/relay/pull/2627))
 
 **Internal**:
 

diff --git a/py/CHANGELOG.md b/py/CHANGELOG.md
@@ -4,6 +4,7 @@
 
 - Add `scraping_attempts` field to the event schema. ([#2575](https://github.com/getsentry/relay/pull/2575))
 - Drop events starting or ending before January 1, 1970 UTC. ([#2613](https://github.com/getsentry/relay/pull/2613))
+- Remove from events spans starting or ending before January 1, 1970 UTC. ([#2627](https://github.com/getsentry/relay/pull/2627))
 
 ## 0.8.31
 

diff --git a/relay-event-normalization/src/timestamp.rs b/relay-event-normalization/src/timestamp.rs
@@ -1,8 +1,8 @@
 use relay_event_schema::processor::{
-    ProcessingAction, ProcessingResult, ProcessingState, Processor,
+    ProcessValue, ProcessingAction, ProcessingResult, ProcessingState, Processor,
 };
-use relay_event_schema::protocol::Event;
-use relay_protocol::Meta;
+use relay_event_schema::protocol::{Event, Span};
+use relay_protocol::{Error, Meta};
 
 /// Ensures an event's timestamps are not stale.
 ///
@@ -22,7 +22,7 @@ impl Processor for TimestampProcessor {
         &mut self,
         event: &mut Event,
         _: &mut Meta,
-        _: &ProcessingState,
+        state: &ProcessingState,
     ) -> ProcessingResult {
         if let Some(end_timestamp) = event.timestamp.value() {
             if end_timestamp.into_inner().timestamp_millis() < 0 {
@@ -39,15 +39,45 @@ impl Processor for TimestampProcessor {
             }
         }
 
+        event.process_child_values(self, state)?;
+
+        Ok(())
+    }
+
+    fn process_span(
+        &mut self,
+        span: &mut Span,
+        meta: &mut Meta,
+        _: &ProcessingState<'_>,
+    ) -> ProcessingResult {
+        if let Some(start_timestamp) = span.start_timestamp.value() {
+            if start_timestamp.into_inner().timestamp_millis() < 0 {
+                meta.add_error(Error::invalid(format!(
+                    "start_timestamp is too stale: {}",
+                    start_timestamp
+                )));
+                return Err(ProcessingAction::DeleteValueHard);
+            }
+        }
+        if let Some(end_timestamp) = span.timestamp.value() {
+            if end_timestamp.into_inner().timestamp_millis() < 0 {
+                meta.add_error(Error::invalid(format!(
+                    "timestamp is too stale: {}",
+                    end_timestamp
+                )));
+                return Err(ProcessingAction::DeleteValueHard);
+            }
+        }
+
         Ok(())
     }
 }
 
 #[cfg(test)]
 mod tests {
     use relay_event_schema::processor::{process_value, ProcessingState};
-    use relay_event_schema::protocol::Event;
-    use relay_protocol::Annotated;
+    use relay_event_schema::protocol::{Event, Span};
+    use relay_protocol::{assert_annotated_snapshot, get_value, Annotated};
 
     use crate::timestamp::TimestampProcessor;
 
@@ -61,6 +91,7 @@ mod tests {
         assert!(
             process_value(&mut error, &mut TimestampProcessor, ProcessingState::root()).is_ok()
         );
+        assert_eq!(get_value!(error.timestamp!).into_inner().timestamp(), 1);
     }
 
     #[test]
@@ -133,4 +164,74 @@ mod tests {
             "invalid transaction event: timestamp is too stale"
         );
     }
+
+    #[test]
+    fn test_accept_recent_span() {
+        let json = r#"{
+      "span_id": "52df9022835246eeb317dbd739ccd050",
+      "start_timestamp": 1,
+      "timestamp": 2
+    }"#;
+        let mut span = Annotated::<Span>::from_json(json).unwrap();
+        assert!(process_value(&mut span, &mut TimestampProcessor, ProcessingState::root()).is_ok());
+        assert_eq!(
+            get_value!(span.start_timestamp!).into_inner().timestamp(),
+            1
+        );
+        assert_eq!(get_value!(span.timestamp!).into_inner().timestamp(), 2);
+    }
+
+    #[test]
+    fn test_reject_stale_span() {
+        let json = r#"{
+      "span_id": "52df9022835246eeb317dbd739ccd050",
+      "start_timestamp": -2,
+      "timestamp": -1
+    }"#;
+        let mut span = Annotated::<Span>::from_json(json).unwrap();
+        assert!(process_value(&mut span, &mut TimestampProcessor, ProcessingState::root()).is_ok());
+        assert_annotated_snapshot!(&span, @r###"
+        {
+          "_meta": {
+            "": {
+              "err": [
+                [
+                  "invalid_data",
+                  {
+                    "reason": "start_timestamp is too stale: 1969-12-31 23:59:58 UTC"
+                  }
+                ]
+              ]
+            }
+          }
+        }
+        "###);
+    }
+
+    #[test]
+    fn test_reject_long_running_span() {
+        let json = r#"{
+      "span_id": "52df9022835246eeb317dbd739ccd050",
+      "start_timestamp": -1,
+      "timestamp": 1
+    }"#;
+        let mut span = Annotated::<Span>::from_json(json).unwrap();
+        assert!(process_value(&mut span, &mut TimestampProcessor, ProcessingState::root()).is_ok());
+        assert_annotated_snapshot!(&span, @r###"
+        {
+          "_meta": {
+            "": {
+              "err": [
+                [
+                  "invalid_data",
+                  {
+                    "reason": "start_timestamp is too stale: 1969-12-31 23:59:59 UTC"
+                  }
+                ]
+              ]
+            }
+          }
+        }
+        "###);
+    }
 }