feat(general): Don't fail events containing a span with missing times…

…tamp (#1690) If theres a missing timestamp on a span, it will get the timestamp of the event it belongs to, and it will get the status of DeadlineExceeded. Check issue for more info
getsentry · Dec 14, 2022 · efbdb32 · efbdb32
1 parent 026536a
commit efbdb32
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 51 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,10 +6,11 @@
 
 - The level of events created from Unreal Crash Reports now depends on whether it was an actual crash or an assert. ([#1677](https://github.com/getsentry/relay/pull/1677))
 - Dynamic sampling is now based on the volume received by Relay by default and does not include the original volume dropped by client-side sampling in SDKs. This is required for the final dynamic sampling feature in the latest Sentry plans. ([#1591](https://github.com/getsentry/relay/pull/1591))
-- Add OpenTelemetry Context ([#1617](https://github.com/getsentry/relay/pull/1617))
+- Add OpenTelemetry Context. ([#1617](https://github.com/getsentry/relay/pull/1617))
 - Add `app.in_foreground` and `thread.main` flag to protocol. ([#1578](https://github.com/getsentry/relay/pull/1578))
 - Add support for View Hierarchy attachment_type. ([#1642](https://github.com/getsentry/relay/pull/1642))
 - Add invalid replay recording outcome. ([#1684](https://github.com/getsentry/relay/pull/1684))
+- Stop rejecting spans without a timestamp, instead giving them their respective event timestamp and setting their status to DeadlineExceeded. ([#1690](https://github.com/getsentry/relay/pull/1690))
 - Add max replay size configuration parameter. ([#1694](https://github.com/getsentry/relay/pull/1694))
 
 **Bug Fixes**:

diff --git a/relay-general/src/store/normalize.rs b/relay-general/src/store/normalize.rs
@@ -2337,11 +2337,6 @@ mod tests {
               "span_id": "fa90fdead5f74053",
               "trace_id": "4c79f60c11214eb38604f4ae0781bfb2"
             }"#,
-            r#"{
-              "start_timestamp": 946684800.0,
-              "span_id": "fa90fdead5f74053",
-              "trace_id": "4c79f60c11214eb38604f4ae0781bfb2"
-            }"#,
             r#"{
               "timestamp": 946684810.0,
               "span_id": "fa90fdead5f74053",

diff --git a/relay-general/src/store/transactions.rs b/relay-general/src/store/transactions.rs
@@ -66,11 +66,7 @@ pub fn validate_timestamps(
     }
 }
 
-pub fn validate_transaction(event: &mut Event) -> ProcessingResult {
-    if event.ty.value() != Some(&EventType::Transaction) {
-        return Ok(());
-    }
-
+fn validate_transaction(event: &mut Event) -> ProcessingResult {
     validate_timestamps(event)?;
 
     let err_trace_context_required = Err(ProcessingAction::InvalidTransaction(
@@ -280,7 +276,13 @@ impl Processor for TransactionsProcessor {
         let spans = event.spans.value_mut().get_or_insert_with(|| Vec::new());
 
         for span in spans {
-            if span.value().is_none() {
+            if let Some(span) = span.value_mut() {
+                if span.timestamp.value().is_none() {
+                    // event timestamp guaranteed to be `Some` due to validate_transaction call
+                    span.timestamp.set_value(event.timestamp.value().cloned());
+                    span.status = Annotated::new(SpanStatus::DeadlineExceeded);
+                }
+            } else {
                 return Err(ProcessingAction::InvalidTransaction(
                     "spans must be valid in transaction event",
                 ));
@@ -344,6 +346,7 @@ impl Processor for TransactionsProcessor {
 
 #[cfg(test)]
 mod tests {
+
     use chrono::offset::TimeZone;
     use chrono::Utc;
     use similar_asserts::assert_eq;
@@ -421,6 +424,42 @@ mod tests {
         );
     }
 
+    #[test]
+    fn test_replace_missing_timestamp() {
+        let span = Span {
+            start_timestamp: Annotated::new(Utc.ymd(1968, 1, 1).and_hms_nano(0, 0, 1, 0).into()),
+            trace_id: Annotated::new(TraceId("4c79f60c11214eb38604f4ae0781bfb2".into())),
+            span_id: Annotated::new(SpanId("fa90fdead5f74053".into())),
+            ..Default::default()
+        };
+
+        let mut event = new_test_event().0.unwrap();
+        event.spans = Annotated::new(vec![Annotated::new(span)]);
+
+        TransactionsProcessor::default()
+            .process_event(
+                &mut event,
+                &mut Meta::default(),
+                &ProcessingState::default(),
+            )
+            .unwrap();
+
+        assert_eq!(
+            event.spans.value().unwrap()[0].value().unwrap().timestamp,
+            event.timestamp
+        );
+
+        assert_eq!(
+            event.spans.value().unwrap()[0]
+                .value()
+                .unwrap()
+                .status
+                .value()
+                .unwrap(),
+            &SpanStatus::DeadlineExceeded
+        );
+    }
+
     #[test]
     fn test_discards_when_missing_start_timestamp() {
         let mut event = Annotated::new(Event {
@@ -775,45 +814,6 @@ mod tests {
         );
     }
 
-    #[test]
-    fn test_discards_transaction_event_with_span_with_missing_timestamp() {
-        let mut event = Annotated::new(Event {
-            ty: Annotated::new(EventType::Transaction),
-            timestamp: Annotated::new(Utc.ymd(2000, 1, 1).and_hms(0, 0, 0).into()),
-            start_timestamp: Annotated::new(Utc.ymd(2000, 1, 1).and_hms(0, 0, 0).into()),
-            contexts: Annotated::new(Contexts({
-                let mut contexts = Object::new();
-                contexts.insert(
-                    "trace".to_owned(),
-                    Annotated::new(ContextInner(Context::Trace(Box::new(TraceContext {
-                        trace_id: Annotated::new(TraceId(
-                            "4c79f60c11214eb38604f4ae0781bfb2".into(),
-                        )),
-                        span_id: Annotated::new(SpanId("fa90fdead5f74053".into())),
-                        op: Annotated::new("http.server".to_owned()),
-                        ..Default::default()
-                    })))),
-                );
-                contexts
-            })),
-            spans: Annotated::new(vec![Annotated::new(Span {
-                ..Default::default()
-            })]),
-            ..Default::default()
-        });
-
-        assert_eq!(
-            process_value(
-                &mut event,
-                &mut TransactionsProcessor::default(),
-                ProcessingState::root()
-            ),
-            Err(ProcessingAction::InvalidTransaction(
-                "span is missing timestamp"
-            ))
-        );
-    }
-
     #[test]
     fn test_discards_transaction_event_with_span_with_missing_start_timestamp() {
         let mut event = Annotated::new(Event {