ref(errors): Unconditionally create a trace context

CHANGELOG.md

@@ -8,6 +8,10 @@
 - Obtain PII values for `SpanData` fields from `sentry-conventions`. ([#5997](https://github.com/getsentry/relay/pull/5997))
 - Add `sentry.dsc.transaction` and `sentry.dsc.trace_id` to all standalone spans. ([#6001](https://github.com/getsentry/relay/pull/6001), [#6004](https://github.com/getsentry/relay/pull/6004))
 
+**Internal**:
+
+- Unconditionally create a trace context with a trace id for errors. ([#6009](https://github.com/getsentry/relay/pull/6009)) 
+
 ## 26.5.0
 
 **Features**:

relay-cabi/src/processing.rs

@@ -274,7 +274,7 @@ pub unsafe extern "C" fn relay_store_normalizer_normalize_event(
         span_allowed_hosts: &[],              // only supported in relay
         span_op_defaults: Default::default(), // only supported in relay
         performance_issues_spans: Default::default(),
-        derive_trace_id: Default::default(),
+        force_trace_context: true, // always required for Sentry
     };
     normalize_event(&mut event, &normalization_config);
 

relay-dynamic-config/src/feature.rs

@@ -95,9 +95,6 @@ pub enum Feature {
     /// Upload non-prosperodmp playstation attachments via the upload endpoint.
     #[serde(rename = "projects:relay-playstation-uploads")]
     PlaystationUploads,
-    /// Add a random trace ID to events that lack one.
-    #[serde(rename = "organizations:relay-default-trace-id")]
-    AddDefaultTraceID,
     /// Enable experimental expansion of the unreal report in the endpoint rather than in the
     /// processor. Only enable for organizations with sufficient attachment quota.
     #[serde(rename = "organizations:relay-unreal-endpoint-expansion")]

relay-event-normalization/src/event.rs

@@ -24,10 +24,10 @@ use relay_conventions::measurements::{
 };
 use relay_event_schema::processor::{self, ProcessingAction, ProcessingState, Processor};
 use relay_event_schema::protocol::{
-    AsPair, Attributes, AutoInferSetting, ClientSdkInfo, Context, ContextInner, Contexts,
-    DebugImage, DeviceClass, Event, EventId, EventType, Exception, Headers, IpAddr, Level,
-    LogEntry, Measurement, Measurements, PerformanceScoreContext, ReplayContext, Request, Span,
-    SpanStatus, SpanV2, Tags, Timestamp, TraceContext, User, VALID_PLATFORMS,
+    AsPair, Attributes, AutoInferSetting, ClientSdkInfo, Contexts, DebugImage, DeviceClass, Event,
+    EventId, EventType, Exception, Headers, IpAddr, Level, LogEntry, Measurement, Measurements,
+    PerformanceScoreContext, ReplayContext, Request, Span, SpanId, SpanV2, Tags, Timestamp,
+    TraceContext, TraceId, User, VALID_PLATFORMS,
 };
 use relay_protocol::{
     Annotated, Empty, Error, ErrorKind, FiniteF64, FromValue, Getter, Meta, Object, Remark,
@@ -173,8 +173,17 @@ pub struct NormalizationConfig<'a> {
     /// Set a flag to enable performance issue detection on spans.
     pub performance_issues_spans: bool,
 
-    /// Should add a random trace ID to events that lack one.
-    pub derive_trace_id: bool,
+    /// Forces a valid trace context for error events.
+    ///
+    /// Sentry requires a valid trace context for events. This ensures a valid trace context always
+    /// exists.
+    ///
+    /// If the error does not contain a trace context, one will be created. If there is already an
+    /// existing trace context, it ensures it's valid and has a trace id.
+    ///
+    /// This is never applied to transaction events, which require a valid transaction context from
+    /// the SDK.
+    pub force_trace_context: bool,
 }
 
 impl Default for NormalizationConfig<'_> {
@@ -209,7 +218,7 @@ impl Default for NormalizationConfig<'_> {
             span_allowed_hosts: Default::default(),
             span_op_defaults: Default::default(),
             performance_issues_spans: Default::default(),
-            derive_trace_id: Default::default(),
+            force_trace_context: Default::default(),
         }
     }
 }
@@ -334,11 +343,12 @@ fn normalize(event: &mut Event, meta: &mut Meta, config: &NormalizationConfig) {
         Ok(())
     });
 
-    // We know this exists thanks to normalize_default_attributes.
-    let event_id = event.id.value().unwrap().0;
+    if config.force_trace_context && event.ty.value() != Some(&EventType::Transaction) {
+        normalize_force_trace_context(event);
+    }
 
     // Some contexts need to be normalized before metrics extraction takes place.
-    normalize_contexts(&mut event.contexts, event_id, config);
+    normalize_contexts(&mut event.contexts);
 
     if config.normalize_spans && event.ty.value() == Some(&EventType::Transaction) {
         crate::normalize::normalize_app_start_spans(event);
@@ -1358,34 +1368,34 @@ fn remove_logger_word(tokens: &mut Vec<&str>) {
     }
 }
 
-/// Normalizes incoming contexts for the downstream metric extraction.
-fn normalize_contexts(
-    contexts: &mut Annotated<Contexts>,
-    event_id: Uuid,
-    config: &NormalizationConfig,
-) {
-    if config.derive_trace_id {
-        // We will always need a TraceContext.
-        let _ = contexts.get_or_insert_with(Contexts::new);
-    }
+/// Creates a new trace context if it is missing and ensures the context has a valid trace and span id.
+///
+/// The function keeps existing meta on trace and span id intact, still surfacing user errors in the
+/// original payload.
+fn normalize_force_trace_context(event: &mut Event) {
+    let contexts = event.contexts.get_or_insert_with(Contexts::new);
+    let trace = contexts.get_or_default::<TraceContext>();
+
+    let trace_id = trace
+        .trace_id
+        .get_or_insert_with(|| TraceId::from(*event.id.get_or_insert_with(Default::default)));
+    let _ = trace.span_id.get_or_insert_with(|| {
+        // Derive the span id from the trace id, for a stable id.
+        // This is equally as good as creating an invented random id.
+        let [a, b, c, d, e, f, g, h, ..] = *trace_id.as_bytes();
+        SpanId([a, b, c, d, e, f, g, h])
+    });
+}
 
+/// Normalizes incoming contexts for the downstream metric extraction.
+fn normalize_contexts(contexts: &mut Annotated<Contexts>) {
     let _ = processor::apply(contexts, |contexts, _meta| {
         // Reprocessing context sent from SDKs must not be accepted, it is a Sentry-internal
         // construct.
         // [`normalize`] does not run on renormalization anyway.
         contexts.0.remove("reprocessing");
 
-        // We need a TraceId to ingest the event into EAP.
-        // If the event lacks a TraceContext, add a random one.
-
-        if config.derive_trace_id && !contexts.contains::<TraceContext>() {
-            contexts.add(TraceContext::random(event_id))
-        }
-
         for annotated in &mut contexts.0.values_mut() {
-            if let Some(ContextInner(Context::Trace(context))) = annotated.value_mut() {
-                context.status.get_or_insert_with(|| SpanStatus::Unknown);
-            }
             if let Some(context_inner) = annotated.value_mut() {
                 crate::normalize::contexts::normalize_context(&mut context_inner.0);
             }
@@ -4925,48 +4935,23 @@ mod tests {
     }
 
     #[test]
-    fn test_normalize_adds_trace_id() {
+    fn test_normalize_adds_trace_context() {
         let json = r#"
         {
-            "type": "transaction",
-            "timestamp": "2021-04-26T08:00:05+0100",
-            "start_timestamp": "2021-04-26T08:00:00+0100",
-            "measurements": {
-                "inp": {"value": 120.0}
+            "type": "error",
+            "exception": {
+                "values": [{"type": "ValueError", "value": "Should not happen"}]
             }
         }
         "#;
 
         let mut event = Annotated::<Event>::from_json(json).unwrap().0.unwrap();
 
-        let performance_score: PerformanceScoreConfig = serde_json::from_value(json!({
-            "profiles": [
-                {
-                    "name": "Desktop",
-                    "scoreComponents": [
-                        {
-                            "measurement": "inp",
-                            "weight": 1.0,
-                            "p10": 0.1,
-                            "p50": 0.25
-                        },
-                    ],
-                    "condition": {
-                        "op":"and",
-                        "inner": []
-                    },
-                    "version": "beta"
-                }
-            ]
-        }))
-        .unwrap();
-
         normalize(
             &mut event,
             &mut Meta::default(),
             &NormalizationConfig {
-                performance_score: Some(&performance_score),
-                derive_trace_id: true,
+                force_trace_context: true,
                 ..Default::default()
             },
         );
@@ -4977,67 +4962,14 @@ mod tests {
             ".trace.span_id" => "[span-id]"
         }, @r#"
         {
-          "performance_score": {
-            "score_profile_version": "beta",
-            "type": "performancescore",
-          },
           "trace": {
             "trace_id": "[trace-id]",
             "span_id": "[span-id]",
             "status": "unknown",
-            "exclusive_time": 5000.0,
             "type": "trace",
           },
-          "_meta": {
-            "trace": {
-              "span_id": {
-                "": Meta(Some(MetaInner(
-                  rem: [
-                    [
-                      "span_id.missing",
-                      s,
-                    ],
-                  ],
-                ))),
-              },
-              "trace_id": {
-                "": Meta(Some(MetaInner(
-                  rem: [
-                    [
-                      "trace_id.missing",
-                      s,
-                    ],
-                  ],
-                ))),
-              },
-            },
-          },
         }
         "#);
-        insta::assert_ron_snapshot!(SerializableAnnotated(&event.measurements), {}, @r###"
-        {
-          "inp": {
-            "value": 120.0,
-            "unit": "millisecond",
-          },
-          "score.inp": {
-            "value": 0.0,
-            "unit": "ratio",
-          },
-          "score.ratio.inp": {
-            "value": 0.0,
-            "unit": "ratio",
-          },
-          "score.total": {
-            "value": 0.0,
-            "unit": "ratio",
-          },
-          "score.weight.inp": {
-            "value": 1.0,
-            "unit": "ratio",
-          },
-        }
-        "###);
     }
 
     #[test]

relay-event-normalization/src/normalize/contexts.rs

@@ -4,6 +4,7 @@ use std::collections::HashMap;
 use std::sync::LazyLock;
 
 use regex::Regex;
+use relay_base_schema::spans::SpanStatus;
 use relay_event_schema::protocol::{
     BrowserContext, Context, Cookies, OsContext, ResponseContext, RuntimeContext,
 };
@@ -382,6 +383,9 @@ pub fn normalize_context(context: &mut Context) {
                 device.name.set_value(Some(product_name.to_string()))
             }
         }
+        Context::Trace(trace) => {
+            trace.status.get_or_insert_with(|| SpanStatus::Unknown);
+        }
         _ => {}
     }
 }

relay-event-schema/src/protocol/contexts/trace.rs

@@ -9,7 +9,7 @@ use std::str::FromStr;
 use uuid::Uuid;
 
 use crate::processor::ProcessValue;
-use crate::protocol::{OperationType, OriginType, SpanData, SpanLink, SpanStatus};
+use crate::protocol::{EventId, OperationType, OriginType, SpanData, SpanLink, SpanStatus};
 
 /// Represents a W3C Trace Context `trace-id`.
 ///
@@ -112,6 +112,12 @@ impl From<Uuid> for TraceId {
     }
 }
 
+impl From<EventId> for TraceId {
+    fn from(event_id: EventId) -> Self {
+        Self::from(event_id.0)
+    }
+}
+
 impl From<TraceId> for Uuid {
     fn from(trace_id: TraceId) -> Self {
         trace_id.0
@@ -170,7 +176,7 @@ impl IntoValue for TraceId {
 /// A 16-character hex string as described in the W3C trace context spec, stored
 /// internally as an array of 8 bytes.
 #[derive(Clone, Copy, Default, Eq, Hash, PartialEq, Ord, PartialOrd)]
-pub struct SpanId([u8; 8]);
+pub struct SpanId(pub [u8; 8]);
 
 relay_common::impl_str_serde!(SpanId, "a span identifier");
 
@@ -337,23 +343,6 @@ pub struct TraceContext {
     pub other: Object<Value>,
 }
 
-impl TraceContext {
-    /// Generates a random [`SpanId`] and takes `[TraceId]` from the event's UUID.
-    /// Leaves all other fields blank.
-    pub fn random(event_id: Uuid) -> Self {
-        let mut trace_meta = Meta::default();
-        trace_meta.add_remark(Remark::new(RemarkType::Substituted, "trace_id.missing"));
-
-        let mut span_meta = Meta::default();
-        span_meta.add_remark(Remark::new(RemarkType::Substituted, "span_id.missing"));
-        TraceContext {
-            trace_id: Annotated(Some(TraceId::from(event_id)), trace_meta),
-            span_id: Annotated(Some(SpanId::random()), span_meta),
-            ..Default::default()
-        }
-    }
-}
-
 impl super::DefaultContext for TraceContext {
     fn default_key() -> &'static str {
         "trace"
@@ -645,31 +634,4 @@ mod tests {
         let remark = annotated.meta().iter_remarks().next().unwrap();
         assert_eq!(remark.rule_id(), "trace_id.invalid");
     }
-
-    #[test]
-    fn test_random_trace_context() {
-        let rand_context = TraceContext::random(Uuid::new_v4());
-        assert!(rand_context.trace_id.value().is_some());
-        assert_eq!(
-            rand_context
-                .trace_id
-                .meta()
-                .iter_remarks()
-                .next()
-                .unwrap()
-                .rule_id(),
-            "trace_id.missing"
-        );
-        assert!(rand_context.span_id.value().is_some());
-        assert_eq!(
-            rand_context
-                .span_id
-                .meta()
-                .iter_remarks()
-                .next()
-                .unwrap()
-                .rule_id(),
-            "span_id.missing"
-        );
-    }
 }

relay-server/src/processing/utils/event.rs

@@ -301,7 +301,7 @@ pub fn normalize(
             performance_issues_spans: ctx
                 .project_info
                 .has_feature(Feature::PerformanceIssuesSpans),
-            derive_trace_id: project_info.has_feature(Feature::AddDefaultTraceID),
+            force_trace_context: true,
         };
 
         metric!(timer(RelayTimers::EventProcessingNormalization), {

tests/fixtures/extended-event-output.json

@@ -80,7 +80,13 @@
             "name": "Python Requests",
             "version": "2.26",
             "type": "browser"
-        }
+        },
+        "trace": {
+            "span_id": "69241adef5744ef1",
+            "status": "unknown",
+            "trace_id": "69241adef5744ef19bde5bbd06fe8177",
+            "type": "trace"
+         }
     },
     "exception": {
         "values": [