diff --git a/CHANGELOG.md b/CHANGELOG.md index b532433a820..4e2ff9b914b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ - Generate `sentry.name` attributes for spans without names. ([#5143](https://github.com/getsentry/relay/pull/5143)) - Add integration endpoints for OTLP. ([#5176](https://github.com/getsentry/relay/pull/5176)) - Emit a metric to record keep/drop decisions in Dynamic Sampling. ([#5164](https://github.com/getsentry/relay/pull/5164)) +- Trim event tag keys & values to 200 chars instead of dropping them. ([#5198](https://github.com/getsentry/relay/pull/5198)) **Bug Fixes**: diff --git a/py/CHANGELOG.md b/py/CHANGELOG.md index c48b02b96a1..3329b9fa556 100644 --- a/py/CHANGELOG.md +++ b/py/CHANGELOG.md @@ -5,6 +5,7 @@ - Introduces a project scope sampling rule type. ([#5077](https://github.com/getsentry/relay/pull/5077)) - Add InstallableBuild and SizeAnalysis data categories. ([#5084](https://github.com/getsentry/relay/pull/5084)) - Add `retentions` to the project configuration. ([#5135](https://github.com/getsentry/relay/pull/5135)) +- Normalization: Trim event tag keys & values to 200 chars instead of dropping them. ([#5198](https://github.com/getsentry/relay/pull/5198)) ## 0.9.16 diff --git a/relay-event-normalization/src/event.rs b/relay-event-normalization/src/event.rs index f24bb7adce0..e681dc87ab3 100644 --- a/relay-event-normalization/src/event.rs +++ b/relay-event-normalization/src/event.rs @@ -668,20 +668,16 @@ fn normalize_event_tags(event: &mut Event) { for tag in tags.iter_mut() { let _ = processor::apply(tag, |tag, _| { - if let Some(key) = tag.key() { - if key.is_empty() { - tag.0 = Annotated::from_error(Error::nonempty(), None); - } else if bytecount::num_chars(key.as_bytes()) > MaxChars::TagKey.limit() { - tag.0 = Annotated::from_error(Error::new(ErrorKind::ValueTooLong), None); - } + if let Some(key) = tag.key() + && key.is_empty() + { + tag.0 = Annotated::from_error(Error::nonempty(), None); } - if let Some(value) = tag.value() { - if value.is_empty() { - tag.1 = Annotated::from_error(Error::nonempty(), None); - } else if bytecount::num_chars(value.as_bytes()) > MaxChars::TagValue.limit() { - tag.1 = Annotated::from_error(Error::new(ErrorKind::ValueTooLong), None); - } + if let Some(value) = tag.value() + && value.is_empty() + { + tag.1 = Annotated::from_error(Error::nonempty(), None); } Ok(()) @@ -5225,4 +5221,85 @@ mod tests { } "###); } + + #[test] + fn test_tags_are_trimmed() { + let json = r#" + { + "tags": { + "key": "too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_", + "too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_": "value" + } + } + "#; + + let mut event = Annotated::::from_json(json).unwrap(); + + normalize_event( + &mut event, + &NormalizationConfig { + enable_trimming: true, + ..NormalizationConfig::default() + }, + ); + + insta::assert_debug_snapshot!(get_value!(event.tags!), @r###" + Tags( + PairList( + [ + TagEntry( + "key", + Annotated( + "too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__lo...", + Meta { + remarks: [ + Remark { + ty: Substituted, + rule_id: "!limit", + range: Some( + ( + 197, + 200, + ), + ), + }, + ], + errors: [], + original_length: Some( + 210, + ), + original_value: None, + }, + ), + ), + TagEntry( + Annotated( + "too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__long_too__lo...", + Meta { + remarks: [ + Remark { + ty: Substituted, + rule_id: "!limit", + range: Some( + ( + 197, + 200, + ), + ), + }, + ], + errors: [], + original_length: Some( + 210, + ), + original_value: None, + }, + ), + "value", + ), + ], + ), + ) + "###); + } } diff --git a/relay-event-normalization/src/lib.rs b/relay-event-normalization/src/lib.rs index 30cbd494b53..44ce3b6e596 100644 --- a/relay-event-normalization/src/lib.rs +++ b/relay-event-normalization/src/lib.rs @@ -49,8 +49,6 @@ pub use sentry_release_parser::{validate_environment, validate_release}; /// Must be aligned with the `max_chars` field in the metastructure of the /// payload's attribute. enum MaxChars { - TagKey, - TagValue, Distribution, Logger, } @@ -58,8 +56,6 @@ enum MaxChars { impl MaxChars { pub fn limit(self) -> usize { match self { - Self::TagKey => 200, - Self::TagValue => 200, Self::Distribution => 64, Self::Logger => 64, } diff --git a/relay-event-normalization/src/normalize/mod.rs b/relay-event-normalization/src/normalize/mod.rs index d724f25ce8e..ff3699a9650 100644 --- a/relay-event-normalization/src/normalize/mod.rs +++ b/relay-event-normalization/src/normalize/mod.rs @@ -1283,38 +1283,6 @@ mod tests { ); } - #[test] - fn test_too_long_tags() { - let mut event = Annotated::new(Event { - tags: Annotated::new(Tags(PairList( - vec![Annotated::new(TagEntry( - Annotated::new("foobar".to_owned()), - Annotated::new("...........................................................................................................................................................................................................".to_owned()), - )), Annotated::new(TagEntry( - Annotated::new("foooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo".to_owned()), - Annotated::new("bar".to_owned()), - ))]), - )), - ..Event::default() - }); - - normalize_event(&mut event, &NormalizationConfig::default()); - - assert_eq!( - get_value!(event.tags!), - &Tags(PairList(vec![ - Annotated::new(TagEntry( - Annotated::new("foobar".to_owned()), - Annotated::from_error(Error::new(ErrorKind::ValueTooLong), None), - )), - Annotated::new(TagEntry( - Annotated::from_error(Error::new(ErrorKind::ValueTooLong), None), - Annotated::new("bar".to_owned()), - )), - ])) - ); - } - #[test] fn test_too_long_distribution() { let json = r#"{ diff --git a/relay-event-normalization/src/trimming.rs b/relay-event-normalization/src/trimming.rs index 25658604777..fa304133f76 100644 --- a/relay-event-normalization/src/trimming.rs +++ b/relay-event-normalization/src/trimming.rs @@ -439,8 +439,8 @@ mod tests { use crate::MaxChars; use chrono::DateTime; use relay_event_schema::protocol::{ - Breadcrumb, Context, Contexts, Event, Exception, ExtraValue, SentryTags, Span, SpanId, - TagEntry, Tags, Timestamp, TraceId, Values, + Breadcrumb, Context, Contexts, Event, Exception, ExtraValue, PairList, SentryTags, Span, + SpanId, TagEntry, Tags, Timestamp, TraceId, Values, }; use relay_protocol::{Map, Remark, SerializableAnnotated, get_value}; use similar_asserts::assert_eq; @@ -1118,4 +1118,82 @@ mod tests { assert!(get_value!(event.spans[1].start_timestamp).is_some()); assert!(get_value!(event.spans[1].timestamp).is_some()); } + + #[test] + fn test_too_long_tags() { + let mut event = Annotated::new(Event { + tags: Annotated::new(Tags(PairList( + vec![Annotated::new(TagEntry( + Annotated::new("foobar".to_owned()), + Annotated::new("xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx".to_owned()), + )), Annotated::new(TagEntry( + Annotated::new("foooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo".to_owned()), + Annotated::new("bar".to_owned()), + ))]), + )), + ..Event::default() + }); + + let mut processor = TrimmingProcessor::new(); + processor::process_value(&mut event, &mut processor, ProcessingState::root()).unwrap(); + + insta::assert_debug_snapshot!(get_value!(event.tags!), @r###" + Tags( + PairList( + [ + TagEntry( + "foobar", + Annotated( + "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx...", + Meta { + remarks: [ + Remark { + ty: Substituted, + rule_id: "!limit", + range: Some( + ( + 197, + 200, + ), + ), + }, + ], + errors: [], + original_length: Some( + 203, + ), + original_value: None, + }, + ), + ), + TagEntry( + Annotated( + "foooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo...", + Meta { + remarks: [ + Remark { + ty: Substituted, + rule_id: "!limit", + range: Some( + ( + 197, + 200, + ), + ), + }, + ], + errors: [], + original_length: Some( + 203, + ), + original_value: None, + }, + ), + "bar", + ), + ], + ), + ) + "###); + } }