ref(logs): Change logic for scrubbing attributes

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

@@ -114,7 +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);
+    process_method!(process_attribute, crate::protocol::Attribute);
 
     fn process_other(
         &mut self,

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

@@ -6,7 +6,7 @@ use std::{borrow::Borrow, fmt};
 
 use crate::processor::{ProcessValue, ProcessingResult, ProcessingState, Processor, ValueType};
 
-#[derive(Clone, PartialEq, Empty, FromValue, IntoValue, ProcessValue)]
+#[derive(Clone, PartialEq, Empty, FromValue, IntoValue)]
 pub struct Attribute {
     #[metastructure(flatten)]
     pub value: AttributeValue,
@@ -38,6 +38,36 @@ impl fmt::Debug for Attribute {
     }
 }
 
+impl ProcessValue for Attribute {
+    fn value_type(&self) -> EnumSet<ValueType> {
+        ValueType::for_field(&self.value.value)
+    }
+
+    fn process_value<P>(
+        &mut self,
+        meta: &mut Meta,
+        processor: &mut P,
+        state: &ProcessingState<'_>,
+    ) -> ProcessingResult
+    where
+        P: Processor,
+    {
+        processor.process_attribute(self, meta, state)
+    }
+
+    fn process_child_values<P>(
+        &mut self,
+        processor: &mut P,
+        state: &ProcessingState<'_>,
+    ) -> ProcessingResult
+    where
+        P: Processor,
+    {
+        self.value.process_child_values(processor, state)?;
+        self.other.process_child_values(processor, state)
+    }
+}
+
 #[derive(Debug, Clone, PartialEq, Empty, FromValue, IntoValue, ProcessValue)]
 pub struct AttributeValue {
     #[metastructure(field = "type", required = true, trim = false)]
@@ -152,7 +182,7 @@ impl IntoValue for AttributeType {
 }
 
 /// Wrapper struct around a collection of attributes with some convenience methods.
-#[derive(Debug, Clone, Default, PartialEq, Empty, FromValue, IntoValue)]
+#[derive(Debug, Clone, Default, PartialEq, Empty, FromValue, IntoValue, ProcessValue)]
 pub struct Attributes(pub Object<Attribute>);
 
 impl Attributes {
@@ -242,34 +272,3 @@ 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-pii/src/processor.rs

@@ -5,8 +5,8 @@ use std::sync::OnceLock;
 
 use regex::Regex;
 use relay_event_schema::processor::{
-    self, Chunk, Pii, ProcessValue, ProcessingAction, ProcessingResult, ProcessingState, Processor,
-    ValueType, enum_set, process_value,
+    self, Chunk, FieldAttrs, Pii, ProcessValue, ProcessingAction, ProcessingResult,
+    ProcessingState, Processor, ValueType, enum_set, process_value,
 };
 use relay_event_schema::protocol::{
     AsPair, Event, IpAddr, NativeImagePath, PairList, Replay, ResponseContext, User,
@@ -19,8 +19,21 @@ use crate::redactions::Redaction;
 use crate::regexes::{self, ANYTHING_REGEX, PatternType, ReplaceBehavior};
 use crate::utils;
 
+/// Controls how scrubbing rules are applied to attributes.
+#[derive(Debug, Clone, Copy)]
+pub enum AttributeMode {
+    /// Treat the attribute as an object and allow referring
+    /// to individual fields.
+    Object,
+    /// Identify the attribute with its value and apply all
+    /// rules there directly.
+    ValueOnly,
+}
+
 /// A processor that performs PII stripping.
 pub struct PiiProcessor<'a> {
+    /// Controls how rules are applied to attributes.
+    attribute_mode: AttributeMode,
     compiled_config: &'a CompiledPiiConfig,
 }
 
@@ -29,7 +42,16 @@ impl<'a> PiiProcessor<'a> {
     pub fn new(compiled_config: &'a CompiledPiiConfig) -> PiiProcessor<'a> {
         // this constructor needs to be cheap... a new PiiProcessor is created for each event. Move
         // any init logic into CompiledPiiConfig::new.
-        PiiProcessor { compiled_config }
+        PiiProcessor {
+            compiled_config,
+            attribute_mode: AttributeMode::Object,
+        }
+    }
+
+    /// Sets an `AttributeMode` on this processor.
+    pub fn attribute_mode(mut self, attribute_mode: AttributeMode) -> Self {
+        self.attribute_mode = attribute_mode;
+        self
     }
 
     fn apply_all_rules(
@@ -45,10 +67,8 @@ impl<'a> PiiProcessor<'a> {
 
         for (selector, rules) in self.compiled_config.applications.iter() {
             if selector.matches_path(&state.path()) {
-                #[allow(clippy::needless_option_as_deref)]
                 for rule in rules {
-                    let reborrowed_value = value.as_deref_mut();
-                    apply_rule_to_value(meta, rule, state.path().key(), reborrowed_value)?;
+                    apply_rule_to_value(meta, rule, state.path().key(), value.as_deref_mut())?;
                 }
             }
         }
@@ -199,13 +219,24 @@ impl Processor for PiiProcessor<'_> {
         utils::process_pairlist(self, value, state)
     }
 
-    fn process_attributes(
+    fn process_attribute(
         &mut self,
-        value: &mut relay_event_schema::protocol::Attributes,
+        value: &mut relay_event_schema::protocol::Attribute,
         _meta: &mut Meta,
         state: &ProcessingState,
     ) -> ProcessingResult {
-        utils::process_attributes(value, self, state)
+        match self.attribute_mode {
+            AttributeMode::Object => {
+                // Treat the attribute as an object and recurse into its children.
+                value.process_child_values(self, state)
+            }
+            AttributeMode::ValueOnly => {
+                // Identify the attribute with its value and apply rules there directly.
+                let value_state =
+                    state.enter_nothing(Some(Cow::Owned(FieldAttrs::new().pii(Pii::True))));
+                processor::process_value(&mut value.value.value, self, &value_state)
+            }
+        }
     }
 
     fn process_user(

relay-pii/src/utils.rs

@@ -1,12 +1,9 @@
-use std::borrow::Cow;
-
 use hmac::{Hmac, Mac};
 
 use relay_event_schema::processor::{
-    self, FieldAttrs, Pii, ProcessValue, ProcessingResult, ProcessingState, Processor, ValueType,
-    enum_set,
+    self, ProcessValue, ProcessingResult, ProcessingState, Processor, ValueType,
 };
-use relay_event_schema::protocol::{AsPair, Attributes, PairList};
+use relay_event_schema::protocol::{AsPair, PairList};
 use sha1::Sha1;
 
 pub fn process_pairlist<P: Processor, T: ProcessValue + AsPair>(
@@ -41,46 +38,6 @@ 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);

relay-server/src/processing/logs/process.rs

@@ -1,8 +1,8 @@
 use relay_event_normalization::{SchemaProcessor, eap};
-use relay_event_schema::processor::{ProcessingState, ValueType, process_value};
+use relay_event_schema::processor::{ProcessingState, process_value};
 use relay_event_schema::protocol::{OurLog, OurLogHeader};
 use relay_ourlogs::OtelLog;
-use relay_pii::PiiProcessor;
+use relay_pii::{AttributeMode, PiiProcessor};
 use relay_protocol::Annotated;
 use relay_quotas::DataCategory;
 
@@ -139,17 +139,17 @@ fn scrub_log(log: &mut Annotated<OurLog>, ctx: Context<'_>) -> Result<()> {
         .map_err(|e| Error::PiiConfig(e.clone()))?;
 
     if let Some(ref config) = ctx.project_info.config.pii_config {
-        let mut processor = PiiProcessor::new(config.compiled());
-        let root_state = ProcessingState::root().enter_static("", None, Some(ValueType::OurLog));
-        process_value(log, &mut processor, &root_state)?;
+        let mut processor = PiiProcessor::new(config.compiled())
+            // For advanced rules we want to treat attributes as objects.
+            .attribute_mode(AttributeMode::Object);
+        process_value(log, &mut processor, ProcessingState::root())?;
     }
 
     if let Some(config) = pii_config_from_scrubbing {
-        let mut processor = PiiProcessor::new(config.compiled());
-        // Use empty root (assumed to be Event) for legacy/default scrubbing rules.
-        // process_attributes will collapse Attribute into it's value for the default rules.
-        let root_state = ProcessingState::root();
-        process_value(log, &mut processor, root_state)?;
+        let mut processor = PiiProcessor::new(config.compiled())
+            // For "legacy" rules we want to identify attributes with their values.
+            .attribute_mode(AttributeMode::ValueOnly);
+        process_value(log, &mut processor, ProcessingState::root())?;
     }
 
     Ok(())
@@ -309,10 +309,7 @@ mod tests {
         scrubbing_config.scrub_data = true;
         scrubbing_config.scrub_defaults = true;
         scrubbing_config.scrub_ip_addresses = true;
-        scrubbing_config.sensitive_fields = vec![
-            "value".to_owned(), // Make sure the inner 'value' of the attribute object isn't scrubbed.
-            "very_sensitive_data".to_owned(),
-        ];
+        scrubbing_config.sensitive_fields = vec!["very_sensitive_data".to_owned()];
         scrubbing_config.exclude_fields = vec!["public_data".to_owned()];
 
         let ctx = make_context(scrubbing_config, None);
@@ -1098,7 +1095,7 @@ mod tests {
                 },
             },
             "applications": {
-                "$log.attributes.remove_this_string_abc123.value": ["remove_abc123"],
+                "attributes.remove_this_string_abc123.value": ["remove_abc123"],
             }
         }))
         .unwrap();