fix(spans): Prevent trimming of sentry tags and span data (#4557)

Small regression when typing out `SentryTags`, the new structure does
not inherit the trim flag.
Now trim can be applied to the container and is inherited for all fields
automatically.

This fixes all cases of partial queryable tags in the span explorer.

Author: Dav1dde

CHANGELOG.md

@@ -12,6 +12,10 @@
 - Update Apple device model classes ([#4529](https://github.com/getsentry/relay/pull/4529))
 - Remove separate quota and rate limit counting for replay videos ([#4554](https://github.com/getsentry/relay/pull/4554))
 
+**Bug Fixes**:
+
+- Prevent partial trims in indexed and queryable span data. ([#4557](https://github.com/getsentry/relay/pull/4557))
+
 **Internal**:
 
 - Track an utilization metric for internal services. ([#4501](https://github.com/getsentry/relay/pull/4501))

relay-event-derive/src/lib.rs

@@ -44,7 +44,7 @@ fn derive_process_value(mut s: synstructure::Structure<'_>) -> syn::Result<Token
             let bi = &variant.bindings()[0];
             let ident = &bi.binding;
             let field_attrs = parse_field_attributes(0, bi.ast(), &mut true)?;
-            let field_attrs_tokens = field_attrs.as_tokens(Some(quote!(parent_attrs)));
+            let field_attrs_tokens = field_attrs.as_tokens(&type_attrs, Some(quote!(parent_attrs)));
 
             Ok(quote! {
                 let parent_attrs = __state.attrs();
@@ -99,7 +99,7 @@ fn derive_process_value(mut s: synstructure::Structure<'_>) -> syn::Result<Token
             let field_attrs_name = Ident::new(&format!("FIELD_ATTRS_{index}"), Span::call_site());
             let field_name = field_attrs.field_name.clone();
 
-            let field_attrs_tokens = field_attrs.as_tokens(None);
+            let field_attrs_tokens = field_attrs.as_tokens(&type_attrs, None);
 
             (quote! {
                 static #field_attrs_name: crate::processor::FieldAttrs = #field_attrs_tokens;
@@ -223,6 +223,11 @@ fn derive_process_value(mut s: synstructure::Structure<'_>) -> syn::Result<Token
 struct TypeAttrs {
     process_func: Option<String>,
     value_type: Vec<String>,
+    /// The default trim value for the container.
+    ///
+    /// If `trim` is specified on the container all fields of the container,
+    /// will default to this value for `trim`.
+    trim: Option<bool>,
 }
 
 impl TypeAttrs {
@@ -257,6 +262,9 @@ fn parse_type_attributes(s: &synstructure::Structure<'_>) -> syn::Result<TypeAtt
             } else if ident == "value_type" {
                 let s = meta.value()?.parse::<LitStr>()?;
                 rv.value_type.push(s.value());
+            } else if ident == "trim" {
+                let s = meta.value()?.parse::<LitBool>()?;
+                rv.trim = Some(s.value());
             } else {
                 // Ignore other attributes used by `relay-protocol-derive`.
                 if !meta.input.peek(syn::Token![,]) {
@@ -307,7 +315,11 @@ struct FieldAttrs {
 }
 
 impl FieldAttrs {
-    fn as_tokens(&self, inherit_from_field_attrs: Option<TokenStream>) -> TokenStream {
+    fn as_tokens(
+        &self,
+        type_attrs: &TypeAttrs,
+        inherit_from_field_attrs: Option<TokenStream>,
+    ) -> TokenStream {
         let field_name = &self.field_name;
 
         if self.required.is_none() && self.nonempty.is_some() {
@@ -347,7 +359,7 @@ impl FieldAttrs {
             quote!(crate::processor::Pii::False)
         };
 
-        let trim = if let Some(trim) = self.trim {
+        let trim = if let Some(trim) = self.trim.or(type_attrs.trim) {
             quote!(#trim)
         } else if let Some(ref parent_attrs) = inherit_from_field_attrs {
             quote!(#parent_attrs.trim)

relay-event-normalization/src/trimming.rs

@@ -438,8 +438,8 @@ mod tests {
 
     use chrono::DateTime;
     use relay_event_schema::protocol::{
-        Breadcrumb, Context, Contexts, Event, Exception, ExtraValue, Span, SpanId, TagEntry, Tags,
-        Timestamp, TraceId, Values,
+        Breadcrumb, Context, Contexts, Event, Exception, ExtraValue, SentryTags, Span, SpanId,
+        TagEntry, Tags, Timestamp, TraceId, Values,
     };
     use relay_protocol::{get_value, Map, Remark, SerializableAnnotated};
     use similar_asserts::assert_eq;
@@ -1004,7 +1004,11 @@ mod tests {
     #[test]
     fn test_too_many_spans_trimmed() {
         let span = Span {
-            platform: Annotated::new("a".repeat(1024 * 100)),
+            platform: Annotated::new("a".repeat(1024 * 90)),
+            sentry_tags: Annotated::new(SentryTags {
+                release: Annotated::new("b".repeat(1024 * 100)),
+                ..Default::default()
+            }),
             ..Default::default()
         };
         let spans: Vec<_> = std::iter::repeat_with(|| Annotated::new(span.clone()))
@@ -1020,10 +1024,10 @@ mod tests {
         processor::process_value(&mut event, &mut processor, ProcessingState::root()).unwrap();
 
         let trimmed_spans = event.0.unwrap().spans.0.unwrap();
-        assert_eq!(trimmed_spans.len(), 8);
+        assert_eq!(trimmed_spans.len(), 5);
 
         // The actual spans were not touched:
-        assert_eq!(trimmed_spans.as_slice(), &spans[0..8]);
+        assert_eq!(trimmed_spans.as_slice(), &spans[0..5]);
     }
 
     #[test]

relay-event-schema/src/protocol/span.rs

@@ -11,46 +11,42 @@ use crate::protocol::{
 };
 
 #[derive(Clone, Debug, Default, PartialEq, Empty, FromValue, IntoValue, ProcessValue)]
-#[metastructure(process_func = "process_span", value_type = "Span")]
+#[metastructure(process_func = "process_span", value_type = "Span", trim = false)]
 pub struct Span {
     /// Timestamp when the span was ended.
-    #[metastructure(required = true, trim = false)]
+    #[metastructure(required = true)]
     pub timestamp: Annotated<Timestamp>,
 
     /// Timestamp when the span started.
-    #[metastructure(required = true, trim = false)]
+    #[metastructure(required = true)]
     pub start_timestamp: Annotated<Timestamp>,
 
     /// The amount of time in milliseconds spent in this span,
     /// excluding its immediate child spans.
-    #[metastructure(trim = false)]
     pub exclusive_time: Annotated<f64>,
 
     /// Span type (see `OperationType` docs).
-    #[metastructure(max_chars = 128, trim = false)]
+    #[metastructure(max_chars = 128)]
     pub op: Annotated<OperationType>,
 
     /// The Span id.
-    #[metastructure(required = true, trim = false)]
+    #[metastructure(required = true)]
     pub span_id: Annotated<SpanId>,
 
     /// The ID of the span enclosing this span.
-    #[metastructure(trim = false)]
     pub parent_span_id: Annotated<SpanId>,
 
     /// The ID of the trace the span belongs to.
-    #[metastructure(required = true, trim = false)]
+    #[metastructure(required = true)]
     pub trace_id: Annotated<TraceId>,
 
     /// A unique identifier for a segment within a trace (8 byte hexadecimal string).
     ///
     /// For spans embedded in transactions, the `segment_id` is the `span_id` of the containing
     /// transaction.
-    #[metastructure(trim = false)]
     pub segment_id: Annotated<SpanId>,
 
     /// Whether or not the current span is the root of the segment.
-    #[metastructure(trim = false)]
     pub is_segment: Annotated<bool>,
 
     /// Indicates whether a span's parent is remote.
@@ -62,66 +58,61 @@ pub struct Span {
     ///  - `empty`: unknown
     ///  - `false`: is not remote
     ///  - `true`: is remote
-    #[metastructure(trim = false)]
     pub is_remote: Annotated<bool>,
 
     /// The status of a span.
-    #[metastructure(trim = false)]
     pub status: Annotated<SpanStatus>,
 
     /// Human readable description of a span (e.g. method URL).
-    #[metastructure(pii = "maybe", trim = false)]
+    #[metastructure(pii = "maybe")]
     pub description: Annotated<String>,
 
     /// Arbitrary tags on a span, like on the top-level event.
-    #[metastructure(pii = "maybe", trim = false)]
+    #[metastructure(pii = "maybe")]
     pub tags: Annotated<Object<JsonLenientString>>,
 
     /// The origin of the span indicates what created the span (see [OriginType] docs).
-    #[metastructure(max_chars = 128, allow_chars = "a-zA-Z0-9_.", trim = false)]
+    #[metastructure(max_chars = 128, allow_chars = "a-zA-Z0-9_.")]
     pub origin: Annotated<OriginType>,
 
     /// ID of a profile that can be associated with the span.
-    #[metastructure(trim = false)]
     pub profile_id: Annotated<EventId>,
 
     /// Arbitrary additional data on a span.
     ///
     /// Besides arbitrary user data, this object also contains SDK-provided fields used by the
     /// product (see <https://develop.sentry.dev/sdk/performance/span-data-conventions/>).
-    #[metastructure(pii = "true", trim = false)]
+    #[metastructure(pii = "true")]
     pub data: Annotated<SpanData>,
 
     /// Links from this span to other spans
-    #[metastructure(pii = "maybe", trim = false)]
+    #[metastructure(pii = "maybe")]
     pub links: Annotated<Array<SpanLink>>,
 
     /// Tags generated by Relay. These tags are a superset of the tags set on span metrics.
-    #[metastructure(trim = false)]
     pub sentry_tags: Annotated<SentryTags>,
 
     /// Timestamp when the span has been received by Sentry.
-    #[metastructure(trim = false)]
     pub received: Annotated<Timestamp>,
 
     /// Measurements which holds observed values such as web vitals.
-    #[metastructure(skip_serialization = "empty", trim = false)]
+    #[metastructure(skip_serialization = "empty")]
     #[metastructure(omit_from_schema)] // we only document error events for now
     pub measurements: Annotated<Measurements>,
 
     /// Platform identifier.
     ///
     /// See [`Event::platform`](`crate::protocol::Event::platform`).
-    #[metastructure(skip_serialization = "empty", trim = false)]
+    #[metastructure(skip_serialization = "empty")]
     pub platform: Annotated<String>,
 
     /// Whether the span is a segment span that was converted from a transaction.
-    #[metastructure(skip_serialization = "empty", trim = false)]
+    #[metastructure(skip_serialization = "empty")]
     pub was_transaction: Annotated<bool>,
 
     // TODO remove retain when the api stabilizes
     /// Additional arbitrary fields for forwards compatibility.
-    #[metastructure(additional_properties, retain = true, pii = "maybe", trim = false)]
+    #[metastructure(additional_properties, retain = true, pii = "maybe")]
     pub other: Object<Value>,
 }
 
@@ -197,6 +188,7 @@ impl Getter for Span {
 
 /// Indexable fields added by sentry (server-side).
 #[derive(Clone, Debug, Default, PartialEq, Empty, FromValue, IntoValue, ProcessValue)]
+#[metastructure(trim = false)]
 pub struct SentryTags {
     pub release: Annotated<String>,
     #[metastructure(pii = "true")]
@@ -351,6 +343,7 @@ impl Getter for SentryTags {
 /// Besides arbitrary user data, this type also contains SDK-provided fields used by the
 /// product (see <https://develop.sentry.dev/sdk/performance/span-data-conventions/>).
 #[derive(Clone, Debug, Default, PartialEq, Empty, FromValue, IntoValue, ProcessValue)]
+#[metastructure(trim = false)]
 pub struct SpanData {
     /// Mobile app start variant.
     ///
@@ -750,6 +743,7 @@ impl Getter for SpanData {
 
 /// A link from a span to another span.
 #[derive(Clone, Debug, Default, PartialEq, Empty, FromValue, IntoValue, ProcessValue)]
+#[metastructure(trim = false)]
 pub struct SpanLink {
     /// The trace id of the linked span
     #[metastructure(required = true, trim = false)]