Skip to content

Commit d181bbb

Browse files
jjbayerjjbayer
authored
feat(normalization): Trim tag keys & values (#5198)
We've been dropping event tag values when they are too long: 7af05d4 SDKs are removing truncation on their side, so Relay should truncate instead of dropping to create the same end-to-end behavior for the product. Fixes INGEST-538. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Switch handling of overlong tag keys/values from dropping to trimming with remarks, and update tests accordingly. > > - **Tag Trimming Behavior**: > - Implement trimming (with remarks) for overlong tag keys/values in `TrimmingProcessor` tests, replacing previous behavior that dropped them. > - **Tests**: > - Remove `normalize::tests::test_too_long_tags` that expected drop-on-too-long in `relay-event-normalization/src/normalize/mod.rs`. > - Add `trimming::tests::test_too_long_tags` verifying trimmed `Tags` `PairList` snapshots in `relay-event-normalization/src/trimming.rs`. > - Adjust imports to include `PairList` where needed. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 9020285. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent f5b9355 commit d181bbb

6 files changed

Lines changed: 171 additions & 50 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
- Generate `sentry.name` attributes for spans without names. ([#5143](https://github.com/getsentry/relay/pull/5143))
1212
- Add integration endpoints for OTLP. ([#5176](https://github.com/getsentry/relay/pull/5176))
1313
- Emit a metric to record keep/drop decisions in Dynamic Sampling. ([#5164](https://github.com/getsentry/relay/pull/5164))
14+
- Trim event tag keys & values to 200 chars instead of dropping them. ([#5198](https://github.com/getsentry/relay/pull/5198))
1415

1516
**Bug Fixes**:
1617

‎py/CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
- Introduces a project scope sampling rule type. ([#5077](https://github.com/getsentry/relay/pull/5077))
66
- Add InstallableBuild and SizeAnalysis data categories. ([#5084](https://github.com/getsentry/relay/pull/5084))
77
- Add `retentions` to the project configuration. ([#5135](https://github.com/getsentry/relay/pull/5135))
8+
- Normalization: Trim event tag keys & values to 200 chars instead of dropping them. ([#5198](https://github.com/getsentry/relay/pull/5198))
89

910
## 0.9.16
1011

‎relay-event-normalization/src/event.rs‎

Lines changed: 89 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -668,20 +668,16 @@ fn normalize_event_tags(event: &mut Event) {
668668

669669
for tag in tags.iter_mut() {
670670
let _ = processor::apply(tag, |tag, _| {
671-
if let Some(key) = tag.key() {
672-
if key.is_empty() {
673-
tag.0 = Annotated::from_error(Error::nonempty(), None);
674-
} else if bytecount::num_chars(key.as_bytes()) > MaxChars::TagKey.limit() {
675-
tag.0 = Annotated::from_error(Error::new(ErrorKind::ValueTooLong), None);
676-
}
671+
if let Some(key) = tag.key()
672+
&& key.is_empty()
673+
{
674+
tag.0 = Annotated::from_error(Error::nonempty(), None);
677675
}
678676

679-
if let Some(value) = tag.value() {
680-
if value.is_empty() {
681-
tag.1 = Annotated::from_error(Error::nonempty(), None);
682-
} else if bytecount::num_chars(value.as_bytes()) > MaxChars::TagValue.limit() {
683-
tag.1 = Annotated::from_error(Error::new(ErrorKind::ValueTooLong), None);
684-
}
677+
if let Some(value) = tag.value()
678+
&& value.is_empty()
679+
{
680+
tag.1 = Annotated::from_error(Error::nonempty(), None);
685681
}
686682

687683
Ok(())
@@ -5225,4 +5221,85 @@ mod tests {
52255221
}
52265222
"###);
52275223
}
5224+
5225+
#[test]
5226+
fn test_tags_are_trimmed() {
5227+
let json = r#"
5228+
{
5229+
"tags": {
5230+
"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_",
5231+
"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"
5232+
}
5233+
}
5234+
"#;
5235+
5236+
let mut event = Annotated::<Event>::from_json(json).unwrap();
5237+
5238+
normalize_event(
5239+
&mut event,
5240+
&NormalizationConfig {
5241+
enable_trimming: true,
5242+
..NormalizationConfig::default()
5243+
},
5244+
);
5245+
5246+
insta::assert_debug_snapshot!(get_value!(event.tags!), @r###"
5247+
Tags(
5248+
PairList(
5249+
[
5250+
TagEntry(
5251+
"key",
5252+
Annotated(
5253+
"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...",
5254+
Meta {
5255+
remarks: [
5256+
Remark {
5257+
ty: Substituted,
5258+
rule_id: "!limit",
5259+
range: Some(
5260+
(
5261+
197,
5262+
200,
5263+
),
5264+
),
5265+
},
5266+
],
5267+
errors: [],
5268+
original_length: Some(
5269+
210,
5270+
),
5271+
original_value: None,
5272+
},
5273+
),
5274+
),
5275+
TagEntry(
5276+
Annotated(
5277+
"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...",
5278+
Meta {
5279+
remarks: [
5280+
Remark {
5281+
ty: Substituted,
5282+
rule_id: "!limit",
5283+
range: Some(
5284+
(
5285+
197,
5286+
200,
5287+
),
5288+
),
5289+
},
5290+
],
5291+
errors: [],
5292+
original_length: Some(
5293+
210,
5294+
),
5295+
original_value: None,
5296+
},
5297+
),
5298+
"value",
5299+
),
5300+
],
5301+
),
5302+
)
5303+
"###);
5304+
}
52285305
}

‎relay-event-normalization/src/lib.rs‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,17 +49,13 @@ pub use sentry_release_parser::{validate_environment, validate_release};
4949
/// Must be aligned with the `max_chars` field in the metastructure of the
5050
/// payload's attribute.
5151
enum MaxChars {
52-
TagKey,
53-
TagValue,
5452
Distribution,
5553
Logger,
5654
}
5755

5856
impl MaxChars {
5957
pub fn limit(self) -> usize {
6058
match self {
61-
Self::TagKey => 200,
62-
Self::TagValue => 200,
6359
Self::Distribution => 64,
6460
Self::Logger => 64,
6561
}

‎relay-event-normalization/src/normalize/mod.rs‎

Lines changed: 0 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1283,38 +1283,6 @@ mod tests {
12831283
);
12841284
}
12851285

1286-
#[test]
1287-
fn test_too_long_tags() {
1288-
let mut event = Annotated::new(Event {
1289-
tags: Annotated::new(Tags(PairList(
1290-
vec![Annotated::new(TagEntry(
1291-
Annotated::new("foobar".to_owned()),
1292-
Annotated::new("...........................................................................................................................................................................................................".to_owned()),
1293-
)), Annotated::new(TagEntry(
1294-
Annotated::new("foooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo".to_owned()),
1295-
Annotated::new("bar".to_owned()),
1296-
))]),
1297-
)),
1298-
..Event::default()
1299-
});
1300-
1301-
normalize_event(&mut event, &NormalizationConfig::default());
1302-
1303-
assert_eq!(
1304-
get_value!(event.tags!),
1305-
&Tags(PairList(vec![
1306-
Annotated::new(TagEntry(
1307-
Annotated::new("foobar".to_owned()),
1308-
Annotated::from_error(Error::new(ErrorKind::ValueTooLong), None),
1309-
)),
1310-
Annotated::new(TagEntry(
1311-
Annotated::from_error(Error::new(ErrorKind::ValueTooLong), None),
1312-
Annotated::new("bar".to_owned()),
1313-
)),
1314-
]))
1315-
);
1316-
}
1317-
13181286
#[test]
13191287
fn test_too_long_distribution() {
13201288
let json = r#"{

‎relay-event-normalization/src/trimming.rs‎

Lines changed: 80 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -439,8 +439,8 @@ mod tests {
439439
use crate::MaxChars;
440440
use chrono::DateTime;
441441
use relay_event_schema::protocol::{
442-
Breadcrumb, Context, Contexts, Event, Exception, ExtraValue, SentryTags, Span, SpanId,
443-
TagEntry, Tags, Timestamp, TraceId, Values,
442+
Breadcrumb, Context, Contexts, Event, Exception, ExtraValue, PairList, SentryTags, Span,
443+
SpanId, TagEntry, Tags, Timestamp, TraceId, Values,
444444
};
445445
use relay_protocol::{Map, Remark, SerializableAnnotated, get_value};
446446
use similar_asserts::assert_eq;
@@ -1118,4 +1118,82 @@ mod tests {
11181118
assert!(get_value!(event.spans[1].start_timestamp).is_some());
11191119
assert!(get_value!(event.spans[1].timestamp).is_some());
11201120
}
1121+
1122+
#[test]
1123+
fn test_too_long_tags() {
1124+
let mut event = Annotated::new(Event {
1125+
tags: Annotated::new(Tags(PairList(
1126+
vec![Annotated::new(TagEntry(
1127+
Annotated::new("foobar".to_owned()),
1128+
Annotated::new("xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx".to_owned()),
1129+
)), Annotated::new(TagEntry(
1130+
Annotated::new("foooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo".to_owned()),
1131+
Annotated::new("bar".to_owned()),
1132+
))]),
1133+
)),
1134+
..Event::default()
1135+
});
1136+
1137+
let mut processor = TrimmingProcessor::new();
1138+
processor::process_value(&mut event, &mut processor, ProcessingState::root()).unwrap();
1139+
1140+
insta::assert_debug_snapshot!(get_value!(event.tags!), @r###"
1141+
Tags(
1142+
PairList(
1143+
[
1144+
TagEntry(
1145+
"foobar",
1146+
Annotated(
1147+
"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx...",
1148+
Meta {
1149+
remarks: [
1150+
Remark {
1151+
ty: Substituted,
1152+
rule_id: "!limit",
1153+
range: Some(
1154+
(
1155+
197,
1156+
200,
1157+
),
1158+
),
1159+
},
1160+
],
1161+
errors: [],
1162+
original_length: Some(
1163+
203,
1164+
),
1165+
original_value: None,
1166+
},
1167+
),
1168+
),
1169+
TagEntry(
1170+
Annotated(
1171+
"foooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo...",
1172+
Meta {
1173+
remarks: [
1174+
Remark {
1175+
ty: Substituted,
1176+
rule_id: "!limit",
1177+
range: Some(
1178+
(
1179+
197,
1180+
200,
1181+
),
1182+
),
1183+
},
1184+
],
1185+
errors: [],
1186+
original_length: Some(
1187+
203,
1188+
),
1189+
original_value: None,
1190+
},
1191+
),
1192+
"bar",
1193+
),
1194+
],
1195+
),
1196+
)
1197+
"###);
1198+
}
11211199
}

0 commit comments

Comments
 (0)