-
Notifications
You must be signed in to change notification settings - Fork 112
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.
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
Merged
Changes from 9 commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
b00033b
WIP
loewenheim 6ad3848
Remove clones
loewenheim 9d4d6f6
DeleteValueFirm -> DeleteValueWithRemark
loewenheim 02601bc
Give `DeleteValueWithRemark` a string slice
loewenheim b84d4d4
Docs
loewenheim 47a255a
Fix split key tracking bug in process_object
loewenheim 2b4e627
Port deletion logic with remarks to process_array
loewenheim 47399dc
Fix snapshot
loewenheim 858bbb5
Introduce DeleteAction enum in eap trimming
loewenheim 772bef6
Add links in doc comments
loewenheim 69a9168
Move delete functions into processor
loewenheim File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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, | ||
| }; | ||
|
|
@@ -13,6 +15,24 @@ struct SizeState { | |
| size_remaining: Option<usize>, | ||
| } | ||
|
|
||
| /// The action to take when deleting a value. | ||
| #[derive(Debug, Clone, Copy)] | ||
| enum DeleteAction { | ||
| /// Delete the value without leaving a trace. | ||
| Hard, | ||
| /// Delete the value and leave a remark with the given rule ID. | ||
| WithRemark(&'static str), | ||
| } | ||
|
|
||
| impl From<DeleteAction> for ProcessingAction { | ||
| fn from(action: DeleteAction) -> Self { | ||
| match action { | ||
| DeleteAction::Hard => ProcessingAction::DeleteValueHard, | ||
| DeleteAction::WithRemark(rule_id) => ProcessingAction::DeleteValueWithRemark(rule_id), | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// Processor for trimming EAP items (logs, V2 spans). | ||
| /// | ||
| /// This primarily differs from the regular [`TrimmingProcessor`](crate::trimming::TrimmingProcessor) | ||
|
|
@@ -27,13 +47,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, | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -76,6 +98,21 @@ impl TrimmingProcessor { | |
| *remaining = remaining.saturating_sub(size); | ||
| } | ||
| } | ||
|
|
||
| /// Returns a `DeleteAction` for removing the given key. | ||
| /// | ||
| /// If there is enough `removed_key_byte_budget` left to accomodate the key, | ||
| /// this will be `DeleteAction::WithRemark` (which causes a remark to be left). | ||
| /// Otherwise, it will be `DeleteAction::Hard` (the key is removed without a trace). | ||
| fn delete_value(&mut self, key: Option<&str>) -> DeleteAction { | ||
| let len = key.map_or(0, |key| key.len()); | ||
| if len <= self.removed_key_byte_budget { | ||
| self.removed_key_byte_budget -= len; | ||
| DeleteAction::WithRemark("trimmed") | ||
| } else { | ||
| DeleteAction::Hard | ||
| } | ||
| } | ||
| } | ||
|
|
||
| impl Processor for TrimmingProcessor { | ||
|
|
@@ -96,11 +133,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).into()); | ||
| } | ||
| if self.remaining_depth(state) == Some(0) { | ||
| return Err(ProcessingAction::DeleteValueHard); | ||
| return Err(self.delete_value(key).into()); | ||
| } | ||
| } | ||
| Ok(()) | ||
|
|
@@ -220,7 +258,18 @@ impl Processor for TrimmingProcessor { | |
| } | ||
|
|
||
| if let Some(split_index) = split_index { | ||
| let _ = value.split_off(split_index); | ||
| let mut i = split_index; | ||
|
|
||
| for item in &mut value[split_index..] { | ||
| match self.delete_value(None) { | ||
| DeleteAction::Hard => break, | ||
| DeleteAction::WithRemark(rule_id) => item.delete_with_remark(rule_id), | ||
| } | ||
|
|
||
| i += 1; | ||
| } | ||
|
|
||
| let _ = value.split_off(i); | ||
| } | ||
|
|
||
| if value.len() != original_length { | ||
|
|
@@ -266,6 +315,22 @@ 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::Included(split_key.as_str()), Bound::Unbounded)) | ||
| { | ||
| i = key.as_str(); | ||
|
|
||
| match self.delete_value(Some(key.as_ref())) { | ||
| DeleteAction::Hard => break, | ||
| DeleteAction::WithRemark(rule_id) => value.delete_with_remark(rule_id), | ||
| } | ||
| } | ||
|
|
||
| let split_key = i.to_owned(); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think there's no way around this
loewenheim marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| let _ = value.split_off(&split_key); | ||
| } | ||
|
|
||
|
|
@@ -312,7 +377,18 @@ 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())) { | ||
| DeleteAction::Hard => break, | ||
| DeleteAction::WithRemark(rule_id) => value.delete_with_remark(rule_id), | ||
| } | ||
|
|
||
| i += 1; | ||
| } | ||
|
|
||
| let _ = sorted.split_off(i); | ||
| } | ||
|
|
||
| attributes.0 = sorted.into_iter().collect(); | ||
|
|
@@ -370,7 +446,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(); | ||
|
|
@@ -379,6 +455,7 @@ mod tests { | |
| { | ||
| "body": "This is...", | ||
| "attributes": { | ||
| "attribute is very large and should be removed": null, | ||
| "medium string": { | ||
| "type": "string", | ||
| "value": "This string..." | ||
|
|
@@ -393,6 +470,16 @@ mod tests { | |
| "": { | ||
| "len": 101 | ||
| }, | ||
| "attribute is very large and should be removed": { | ||
| "": { | ||
| "rem": [ | ||
| [ | ||
| "trimmed", | ||
| "x" | ||
| ] | ||
| ] | ||
| } | ||
| }, | ||
| "medium string": { | ||
| "value": { | ||
| "": { | ||
|
|
@@ -444,7 +531,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(); | ||
|
|
@@ -519,7 +606,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(); | ||
|
|
@@ -528,6 +615,7 @@ mod tests { | |
| { | ||
| "body": "This is...", | ||
| "attributes": { | ||
| "attribute is very large and should be removed": null, | ||
| "attribute with long name": { | ||
| "type": "integer", | ||
| "value": 71 | ||
|
|
@@ -541,6 +629,16 @@ mod tests { | |
| "attributes": { | ||
| "": { | ||
| "len": 91 | ||
| }, | ||
| "attribute is very large and should be removed": { | ||
| "": { | ||
| "rem": [ | ||
| [ | ||
| "trimmed", | ||
| "x" | ||
| ] | ||
| ] | ||
| } | ||
| } | ||
| }, | ||
| "body": { | ||
|
|
@@ -577,7 +675,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. | ||
|
|
@@ -593,6 +691,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..." | ||
|
|
@@ -602,11 +701,22 @@ mod tests { | |
| "value": 17 | ||
| } | ||
| }, | ||
| "footer": null, | ||
| "_meta": { | ||
| "attributes": { | ||
| "": { | ||
| "len": 101 | ||
| }, | ||
| "attribute is very large and should be removed": { | ||
| "": { | ||
| "rem": [ | ||
| [ | ||
| "trimmed", | ||
| "x" | ||
| ] | ||
| ] | ||
| } | ||
| }, | ||
| "medium string": { | ||
| "value": { | ||
| "": { | ||
|
|
@@ -622,6 +732,16 @@ mod tests { | |
| } | ||
| } | ||
| } | ||
| }, | ||
| "footer": { | ||
| "": { | ||
| "rem": [ | ||
| [ | ||
| "trimmed", | ||
| "x" | ||
| ] | ||
| ] | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -655,7 +775,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(); | ||
|
|
||
|
|
@@ -670,7 +790,8 @@ mod tests { | |
| "value": [ | ||
| "first string", | ||
| "second string", | ||
| "another..." | ||
| "another...", | ||
| null | ||
| ] | ||
| } | ||
| }, | ||
|
|
@@ -681,9 +802,6 @@ mod tests { | |
| }, | ||
| "array": { | ||
| "value": { | ||
| "": { | ||
| "len": 4 | ||
| }, | ||
| "2": { | ||
| "": { | ||
| "rem": [ | ||
|
|
@@ -696,6 +814,16 @@ mod tests { | |
| ], | ||
| "len": 14 | ||
| } | ||
| }, | ||
| "3": { | ||
| "": { | ||
| "rem": [ | ||
| [ | ||
| "trimmed", | ||
| "x" | ||
| ] | ||
| ] | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -719,7 +847,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(); | ||
|
|
@@ -732,13 +860,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": [ | ||
| [ | ||
| "trimmed", | ||
| "x" | ||
| ] | ||
| ] | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.