fix(ourlogs): Improve PII Scrubbing for attributes

CHANGELOG.md

@@ -1,5 +1,9 @@
 # Changelog
 
+## Unreleased
+
+- Improved PII Scrubbing for attributes / logs ([#5061](https://github.com/getsentry/relay/pull/5061)))
+
 ## 25.8.0
 
 - Add data categories for Prevent. ([#5052](https://github.com/getsentry/relay/pull/5052))

py/CHANGELOG.md

@@ -5,6 +5,7 @@
 - Add `trusted_relay_settings` to the project configuration. ([#4772](https://github.com/getsentry/relay/pull/4772))
 - Add `downsampled_event_retention` to the project configuration. ([#5013](https://github.com/getsentry/relay/pull/5013))
 - Add data categories for Prevent. ([#5052](https://github.com/getsentry/relay/pull/5052))
+- Improved PII Scrubbing for attributes / logs ([#5061](https://github.com/getsentry/relay/pull/5061)))
 
 ## 0.9.12
 

relay-event-schema/src/processor/attrs.rs

@@ -85,7 +85,8 @@ relay_common::derive_fromstr_and_display!(ValueType, UnknownValueTypeError, {
     ValueType::Message => "message",
     ValueType::Thread => "thread",
     ValueType::Breadcrumb => "breadcrumb",
-    ValueType::OurLog => "ourlog",
+    ValueType::OurLog => "log",
+
     ValueType::Span => "span",
     ValueType::ClientSdkInfo => "sdk",
     ValueType::Minidump => "minidump",

relay-event-schema/src/processor/traits.rs

@@ -114,6 +114,7 @@ pub trait Processor: Sized {
     process_method!(process_trace_context, crate::protocol::TraceContext);
     process_method!(process_native_image_path, crate::protocol::NativeImagePath);
     process_method!(process_contexts, crate::protocol::Contexts);
+    process_method!(process_attributes, crate::protocol::Attributes);
 
     fn process_other(
         &mut self,

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

@@ -1,7 +1,10 @@
-use relay_protocol::{Annotated, Empty, FromValue, IntoValue, Object, SkipSerialization, Value};
+use enumset::EnumSet;
+use relay_protocol::{
+    Annotated, Empty, FromValue, IntoValue, Meta, Object, SkipSerialization, Value,
+};
 use std::{borrow::Borrow, fmt};
 
-use crate::processor::ProcessValue;
+use crate::processor::{ProcessValue, ProcessingResult, ProcessingState, Processor, ValueType};
 
 #[derive(Clone, PartialEq, Empty, FromValue, IntoValue, ProcessValue)]
 pub struct Attribute {
@@ -161,7 +164,7 @@ impl IntoValue for AttributeType {
 }
 
 /// Wrapper struct around a collection of attributes with some convenience methods.
-#[derive(Debug, Clone, Default, PartialEq, Empty, FromValue, IntoValue, ProcessValue)]
+#[derive(Debug, Clone, Default, PartialEq, Empty, FromValue, IntoValue)]
 pub struct Attributes(pub Object<Attribute>);
 
 impl Attributes {
@@ -249,3 +252,34 @@ impl FromIterator<(String, Annotated<Attribute>)> for Attributes {
         Self(Object::from_iter(iter))
     }
 }
+
+impl ProcessValue for Attributes {
+    #[inline]
+    fn value_type(&self) -> EnumSet<ValueType> {
+        EnumSet::only(ValueType::Object)
+    }
+
+    #[inline]
+    fn process_value<P>(
+        &mut self,
+        meta: &mut Meta,
+        processor: &mut P,
+        state: &ProcessingState<'_>,
+    ) -> ProcessingResult
+    where
+        P: Processor,
+    {
+        processor.process_attributes(self, meta, state)
+    }
+
+    fn process_child_values<P>(
+        &mut self,
+        _processor: &mut P,
+        _state: &ProcessingState<'_>,
+    ) -> ProcessingResult
+    where
+        P: Processor,
+    {
+        Ok(())
+    }
+}

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

@@ -33,7 +33,7 @@ pub struct OurLog {
     pub body: Annotated<String>,
 
     /// Arbitrary attributes on a log.
-    #[metastructure(pii = "true", trim = false)]
+    #[metastructure(pii = "maybe", trim = false)]
     pub attributes: Annotated<Attributes>,
 
     /// Additional arbitrary fields for forwards compatibility.

relay-pii/src/generate_selectors.rs

@@ -139,6 +139,14 @@ pub fn selector_suggestions_from_value<T: ProcessValue>(
         selectors: BTreeSet::new(),
     };
 
+    // OurLog (logs) should not appear in selector suggestions as that is used by the old modal.
+    // This should not be possible unless we are explicitly sending ourlog json to 'relay_pii_selector_suggestions_from_event', but guarding against it just in case.
+    if let Some(value) = value.value_mut()
+        && value.value_type().contains(ValueType::OurLog)
+    {
+        return BTreeSet::new();
+    }
+
     processor::process_value(value, &mut processor, ProcessingState::root())
         .expect("This processor is supposed to be infallible");
 
@@ -147,7 +155,7 @@ pub fn selector_suggestions_from_value<T: ProcessValue>(
 
 #[cfg(test)]
 mod tests {
-    use relay_event_schema::protocol::Event;
+    use relay_event_schema::protocol::{Event, OurLog};
 
     use super::*;
 
@@ -290,4 +298,28 @@ mod tests {
         ]
         "###);
     }
+
+    #[test]
+    fn test_attributes() {
+        let mut annotated_log = Annotated::<OurLog>::from_json(
+            r#"
+            {
+              "timestamp": 1544719860.0,
+              "trace_id": "5b8efff798038103d269b633813fc60c",
+              "span_id": "eee19b7ec3c1b174",
+              "level": "info",
+              "body": "Test",
+              "attributes": {
+                  "example_attribute": {
+                      "type": "string",
+                      "value": "abc123"
+                  }
+              }
+            }"#,
+        )
+        .unwrap();
+
+        let selectors = selector_suggestions_from_value(&mut annotated_log);
+        insta::assert_json_snapshot!(selectors, @"[]");
+    }
 }

relay-pii/src/processor.rs

@@ -199,6 +199,15 @@ impl Processor for PiiProcessor<'_> {
         utils::process_pairlist(self, value, state)
     }
 
+    fn process_attributes(
+        &mut self,
+        value: &mut relay_event_schema::protocol::Attributes,
+        _meta: &mut Meta,
+        state: &ProcessingState,
+    ) -> ProcessingResult {
+        utils::process_attributes(value, self, state)
+    }
+
     fn process_user(
         &mut self,
         user: &mut User,

relay-pii/src/selector.rs

@@ -659,4 +659,16 @@ mod tests {
         // WAT.  We have the full path to a field here.
         assert_matches_pii_true!(minidump_state_inner, "$attachments.$minidump.$binary",);
     }
+
+    #[test]
+    fn test_logs_matching() {
+        let event_state = ProcessingState::new_root(None, None);
+        let log_state = event_state.enter_static("", None, Some(ValueType::OurLog)); // .
+        let body_state = log_state.enter_static("body", None, Some(ValueType::String));
+        let attributes_state = log_state.enter_static("attributes", None, Some(ValueType::Object));
+
+        assert_matches_pii_maybe!(log_state, "$log",);
+        assert_matches_pii_true!(body_state, "$log.body",);
+        assert_matches_pii_true!(attributes_state, "$log.attributes",);
+    }
 }

relay-pii/src/utils.rs

@@ -1,8 +1,12 @@
+use std::borrow::Cow;
+
 use hmac::{Hmac, Mac};
+
 use relay_event_schema::processor::{
-    self, ProcessValue, ProcessingResult, ProcessingState, Processor, ValueType,
+    self, FieldAttrs, Pii, ProcessValue, ProcessingResult, ProcessingState, Processor, ValueType,
+    enum_set,
 };
-use relay_event_schema::protocol::{AsPair, PairList};
+use relay_event_schema::protocol::{AsPair, Attributes, PairList};
 use sha1::Sha1;
 
 pub fn process_pairlist<P: Processor, T: ProcessValue + AsPair>(
@@ -37,6 +41,46 @@ pub fn process_pairlist<P: Processor, T: ProcessValue + AsPair>(
     Ok(())
 }
 
+pub fn process_attributes<P: Processor>(
+    value: &mut Attributes,
+    slf: &mut P,
+    state: &ProcessingState,
+) -> ProcessingResult {
+    // Check if we're in default rules (no root ValueType)
+    // This is a workaround to support explicit selectors eg. $log.attributes.KEY.value to work but keep implicit selectors for default rules.
+    let is_advanced_rules = state
+        .iter()
+        .nth(1)
+        .is_some_and(|s| s.value_type().contains(ValueType::OurLog));
+
+    for (key, annotated_attribute) in value.iter_mut() {
+        if let Some(attribute) = annotated_attribute.value_mut() {
+            if is_advanced_rules {
+                // Process normally to allow explicit selectors like $log.attributes.KEY.value to work
+                let field_value_type = ValueType::for_field(annotated_attribute);
+                let key_state = state.enter_borrowed(key, state.inner_attrs(), field_value_type);
+                processor::process_value(annotated_attribute, slf, &key_state)?;
+            } else {
+                // For the default rules, we want Pii::True since we want them to always be scrubbed.
+                let attrs = FieldAttrs::new().pii(Pii::True);
+                let inner_value = &mut attribute.value.value;
+                let inner_value_type = ValueType::for_field(inner_value);
+                let entered =
+                    state.enter_borrowed(key, Some(Cow::Borrowed(&attrs)), inner_value_type);
+                processor::process_value(inner_value, slf, &entered)?;
+
+                let object_value_type = enum_set!(ValueType::Object);
+                for (key, value) in attribute.other.iter_mut() {
+                    let other_state =
+                        state.enter_borrowed(key, Some(Cow::Borrowed(&attrs)), object_value_type);
+                    processor::process_value(value, slf, &other_state)?;
+                }
+            }
+        }
+    }
+    Ok(())
+}
+
 pub fn hash_value(data: &[u8]) -> String {
     let mut mac = Hmac::<Sha1>::new_from_slice(&[]).unwrap();
     mac.update(data);