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 spans. ([#6001](https://github.com/getsentry/relay/pull/6001), [#6004](https://github.com/getsentry/relay/pull/6004), [#6008](https://github.com/getsentry/relay/pull/6008))
 
+**Internal**:
+
+- Unconditionally create a trace context with a trace id for errors. ([#6009](https://github.com/getsentry/relay/pull/6009)) 
+
 ## 26.5.0
 
 **Features**:

py/tests/test_processing.py

@@ -1,3 +1,4 @@
+from unittest import mock
 import sentry_relay
 
 import pytest
@@ -127,9 +128,10 @@ def test_normalize_user_agent(must_normalize):
                 "type": "browser",
             },
             "client_os": {"name": "Ubuntu", "os": "Ubuntu", "type": "os"},
+            "trace": mock.ANY,
         }
     else:
-        assert "contexts" not in event
+        assert set(event["contexts"].keys()) == {"trace"}
 
 
 def test_validate_pii_selector():

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
         dsc: None,
     };
     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,
@@ -174,8 +174,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,
 
     /// Dynamic sampling context
     pub dsc: Option<&'a DynamicSamplingContext>,
@@ -213,7 +222,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(),
             dsc: None,
         }
     }
@@ -339,11 +348,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) {
         span::normalize_app_start_spans(event);
@@ -1364,34 +1374,31 @@ 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(|| SpanId::derive_from_trace_id(trace_id));
+}
 
+/// 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);
             }
@@ -4931,48 +4938,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()
             },
         );
@@ -4983,67 +4965,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);
+        }
         _ => {}
     }
 }