Skip to content

feat(eap): Track removed keys in EAP trimming processor #5570

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
loewenheim merged 11 commits into from
Jan 28, 2026
Merged

feat(eap): Track removed keys in EAP trimming processor #5570

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
b00033b
WIP
loewenheim Jan 22, 2026
6ad3848
Remove clones
loewenheim Jan 22, 2026
9d4d6f6
DeleteValueFirm -> DeleteValueWithRemark
loewenheim Jan 26, 2026
02601bc
Give `DeleteValueWithRemark` a string slice
loewenheim Jan 26, 2026
b84d4d4
Docs
loewenheim Jan 26, 2026
47a255a
Fix split key tracking bug in process_object
loewenheim Jan 27, 2026
2b4e627
Port deletion logic with remarks to process_array
loewenheim Jan 27, 2026
47399dc
Fix snapshot
loewenheim Jan 27, 2026
858bbb5
Introduce DeleteAction enum in eap trimming
loewenheim Jan 27, 2026
772bef6
Add links in doc comments
loewenheim Jan 28, 2026
69a9168
Move delete functions into processor
loewenheim Jan 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
127 changes: 116 additions & 11 deletions relay-event-normalization/src/eap/trimming.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::ops::Bound;

use relay_event_schema::processor::{
self, ProcessValue, ProcessingAction, ProcessingResult, ProcessingState, Processor, ValueType,
};
Expand Down Expand Up @@ -27,13 +29,15 @@ struct SizeState {
#[derive(Default)]
pub struct TrimmingProcessor {
size_state: Vec<SizeState>,
removed_key_byte_budget: usize,
}

impl TrimmingProcessor {
/// Creates a new trimming processor.
pub fn new() -> Self {
pub fn new(removed_key_byte_budget: usize) -> Self {
Self {
size_state: Default::default(),
removed_key_byte_budget,
}
}

Expand Down Expand Up @@ -76,6 +80,21 @@ impl TrimmingProcessor {
*remaining = remaining.saturating_sub(size);
}
}

/// Returns a `ProcessingAction` for removing the given key.
///
/// If there is enough `removed_key_byte_budget` left to accomodate the key,
/// this will be `ProcessingAction::DeleteFirm` (which causes a remark to be left).
/// Otherwise, it will be `ProcessingAction::DeleteHard` (the key is removed without a trace).
fn delete_value(&mut self, key: Option<&str>) -> ProcessingAction {
let len = key.map_or(0, |key| key.len());
if len <= self.removed_key_byte_budget {
self.removed_key_byte_budget -= len;
ProcessingAction::DeleteValueFirm
} else {
ProcessingAction::DeleteValueHard
}
}
}

impl Processor for TrimmingProcessor {
Expand All @@ -96,11 +115,12 @@ impl Processor for TrimmingProcessor {
}

if state.attrs().trim {
let key = state.keys().next();
if self.remaining_size() == Some(0) {
return Err(ProcessingAction::DeleteValueHard);
return Err(self.delete_value(key));
}
if self.remaining_depth(state) == Some(0) {
return Err(ProcessingAction::DeleteValueHard);
return Err(self.delete_value(key));
}
}
Ok(())
Expand Down Expand Up @@ -266,6 +286,24 @@ impl Processor for TrimmingProcessor {
}

if let Some(split_key) = split_key {
let mut i = split_key.as_str();

// Morally this is just `range_mut(split_key.as_str()..)`, but that doesn't work for type
// inference reasons.
for (key, value) in value.range_mut::<str, (Bound<&str>, Bound<&str>)>((
Bound::Included(split_key.as_str()),
Bound::Unbounded,
)) {
match self.delete_value(Some(key.as_ref())) {
ProcessingAction::DeleteValueHard => break,
ProcessingAction::DeleteValueFirm => value.delete_firm(),
_ => unreachable!(),
}

i = key.as_str();
}

let split_key = i.to_owned();
Copy link
Contributor Author

@loewenheim loewenheim Jan 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there's no way around this to_owned; we need the key to be detached from the map or we can't modify it.

let _ = value.split_off(&split_key);
}

Expand Down Expand Up @@ -312,7 +350,19 @@ impl Processor for TrimmingProcessor {
}

if let Some(split_idx) = split_idx {
let _ = sorted.split_off(split_idx);
let mut i = split_idx;

for (key, value) in &mut sorted[split_idx..] {
match self.delete_value(Some(key.as_ref())) {
ProcessingAction::DeleteValueHard => break,
ProcessingAction::DeleteValueFirm => value.delete_firm(),
_ => unreachable!(),
}

i += 1;
}

let _ = sorted.split_off(i);
}

attributes.0 = sorted.into_iter().collect();
Expand Down Expand Up @@ -370,7 +420,7 @@ mod tests {
footer: Annotated::empty(),
});

let mut processor = TrimmingProcessor::new();
let mut processor = TrimmingProcessor::new(100);

let state = ProcessingState::new_root(Default::default(), []);
processor::process_value(&mut value, &mut processor, &state).unwrap();
Expand All @@ -379,6 +429,7 @@ mod tests {
{
"body": "This is...",
"attributes": {
"attribute is very large and should be removed": null,
"medium string": {
"type": "string",
"value": "This string..."
Expand All @@ -393,6 +444,16 @@ mod tests {
"": {
"len": 101
},
"attribute is very large and should be removed": {
"": {
"rem": [
[
"delete_firm",
"x"
]
]
}
},
"medium string": {
"value": {
"": {
Expand Down Expand Up @@ -444,7 +505,7 @@ mod tests {
footer: Annotated::empty(),
});

let mut processor = TrimmingProcessor::new();
let mut processor = TrimmingProcessor::new(100);

let state = ProcessingState::new_root(Default::default(), []);
processor::process_value(&mut value, &mut processor, &state).unwrap();
Expand Down Expand Up @@ -519,7 +580,7 @@ mod tests {
footer: Annotated::empty(),
});

let mut processor = TrimmingProcessor::new();
let mut processor = TrimmingProcessor::new(100);

let state = ProcessingState::new_root(Default::default(), []);
processor::process_value(&mut value, &mut processor, &state).unwrap();
Expand All @@ -528,6 +589,7 @@ mod tests {
{
"body": "This is...",
"attributes": {
"attribute is very large and should be removed": null,
"attribute with long name": {
"type": "integer",
"value": 71
Expand All @@ -541,6 +603,16 @@ mod tests {
"attributes": {
"": {
"len": 91
},
"attribute is very large and should be removed": {
"": {
"rem": [
[
"delete_firm",
"x"
]
]
}
}
},
"body": {
Expand Down Expand Up @@ -577,7 +649,7 @@ mod tests {
footer: Annotated::new("Hello World".to_owned()),
});

let mut processor = TrimmingProcessor::new();
let mut processor = TrimmingProcessor::new(100);

// The `body` takes up 5B, `other_number` 10B, the `"small"` attribute 13B, and the key "medium string" another 13B.
// That leaves 9B for the string's value.
Expand All @@ -593,6 +665,7 @@ mod tests {
"number": 0,
"other_number": 0,
"attributes": {
"attribute is very large and should be removed": null,
"medium string": {
"type": "string",
"value": "This s..."
Expand All @@ -602,11 +675,22 @@ mod tests {
"value": 17
}
},
"footer": null,
"_meta": {
"attributes": {
"": {
"len": 101
},
"attribute is very large and should be removed": {
"": {
"rem": [
[
"delete_firm",
"x"
]
]
}
},
"medium string": {
"value": {
"": {
Expand All @@ -622,6 +706,16 @@ mod tests {
}
}
}
},
"footer": {
"": {
"rem": [
[
"delete_firm",
"x"
]
]
}
}
}
}
Expand Down Expand Up @@ -655,7 +749,7 @@ mod tests {
footer: Annotated::empty(),
});

let mut processor = TrimmingProcessor::new();
let mut processor = TrimmingProcessor::new(100);
let state = ProcessingState::new_root(Default::default(), []);
processor::process_value(&mut value, &mut processor, &state).unwrap();

Expand Down Expand Up @@ -719,7 +813,7 @@ mod tests {
footer: Annotated::new("Hello World".to_owned()), // 11B
});

let mut processor = TrimmingProcessor::new();
let mut processor = TrimmingProcessor::new(100);
let state =
ProcessingState::new_root(Some(Cow::Owned(FieldAttrs::default().max_bytes(30))), []);
processor::process_value(&mut value, &mut processor, &state).unwrap();
Expand All @@ -732,13 +826,24 @@ mod tests {
"a": {
"type": "integer",
"value": 1
}
},
"this_key_is_exactly_35_chars_long!!": null
},
"footer": "Hello World",
"_meta": {
"attributes": {
"": {
"len": 45
},
"this_key_is_exactly_35_chars_long!!": {
"": {
"rem": [
[
"delete_firm",
"x"
]
]
}
}
}
}
Expand Down
8 changes: 4 additions & 4 deletions relay-event-schema/src/processor/funcs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ where

match result {
Ok(()) => (),
Err(ProcessingAction::DeleteValueHard) => v.0 = None,
Err(ProcessingAction::DeleteValueSoft) => {
v.1.set_original_value(v.0.take());
}
Err(ProcessingAction::DeleteValueHard) => v.delete_hard(),
Err(ProcessingAction::DeleteValueFirm) => v.delete_firm(),
Err(ProcessingAction::DeleteValueSoft) => v.delete_soft(),

x @ Err(ProcessingAction::InvalidTransaction(_)) => return x,
}

Expand Down
4 changes: 4 additions & 0 deletions relay-event-schema/src/processor/traits.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ pub enum ProcessingAction {
#[error("value should be hard-deleted (unreachable, should not surface as error!)")]
DeleteValueHard,

/// Discards the value entirely, but leaves a remark.
#[error("value should be hard-deleted (unreachable, should not surface as error!)")]
DeleteValueFirm,
Copy link
Member

@jjbayer jjbayer Jan 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though I appreciate "firm", I would call this "hard with remark" or just "with remark" instead.

Suggested change
DeleteValueFirm,
DeleteValueWithRemark,

Copy link
Contributor Author

@loewenheim loewenheim Jan 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, "Firm" was fully intended as a placeholder name.


/// Discards the value and moves it into meta's `original_value`.
#[error("value should be hard-deleted (unreachable, should not surface as error!)")]
DeleteValueSoft,
Expand Down
6 changes: 5 additions & 1 deletion relay-pii/src/processor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,11 @@ impl Processor for PiiProcessor<'_> {
let basename = value.split_off(index);
match self.process_string(value, meta, state) {
Ok(()) => value.push_str(&basename),
Err(ProcessingAction::DeleteValueHard) | Err(ProcessingAction::DeleteValueSoft) => {
Err(
ProcessingAction::DeleteValueHard
| ProcessingAction::DeleteValueFirm
| ProcessingAction::DeleteValueSoft,
) => {
basename[1..].clone_into(value);
}
Err(ProcessingAction::InvalidTransaction(x)) => {
Expand Down
20 changes: 20 additions & 0 deletions relay-protocol/src/annotated.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use serde::{Deserialize, Deserializer, Serialize, Serializer};
use crate::meta::{Error, Meta};
use crate::traits::{Empty, FromValue, IntoValue, SkipSerialization};
use crate::value::{Map, Value};
use crate::{Remark, RemarkType};

/// Represents a tree of meta objects.
#[derive(Default, Debug, Serialize)]
Expand Down Expand Up @@ -236,6 +237,25 @@ impl<T> Annotated<T> {

other()
}

pub fn delete_hard(&mut self) {
self.0 = None;
}

pub fn delete_firm(&mut self) {
self.0 = None;
self.1.add_remark(Remark {
ty: RemarkType::Removed,
rule_id: "delete_firm".to_owned(),
Copy link
Member

@jjbayer jjbayer Jan 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I believe something like "trimmed" would provide more valuable info to the user here.

Suggested change
rule_id: "delete_firm".to_owned(),
rule_id: "trimmed".to_owned(),

range: None,
});
}
}

impl<T: IntoValue> Annotated<T> {
pub fn delete_soft(&mut self) {
self.1.set_original_value(self.0.take());
}
}

impl<T> Annotated<Option<T>> {
Expand Down
Loading