Skip to content

Commit

Permalink
feat(normalize): Keep meta for removed custom measurements (#1815)
Browse files Browse the repository at this point in the history
This adds a `_meta` entry for custom measurements that have been removed
during normalization as a result of the custom measurements limit. The
entry contains a generic error message along with the payloads of all
removed measurements.
  • Loading branch information
jan-auer authored Feb 3, 2023
1 parent 35d2c74 commit da6fda6
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

- Add metric name as tag on Sentry errors from relay dropping metrics. ([#1797](https://github.com/getsentry/relay/pull/1797))
- Make sure to scrub all the fields with PII. If the fields contain an object, the entire object will be removed. ([#1789](https://github.com/getsentry/relay/pull/1789))
- Keep meta for removed custom measurements. ([#1815](https://github.com/getsentry/relay/pull/1815))

## 23.1.1

Expand Down
37 changes: 35 additions & 2 deletions relay-general/src/store/normalize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -252,9 +252,12 @@ fn normalize_breakdowns(event: &mut Event, breakdowns_config: Option<&Breakdowns
/// measurements is retained.
fn remove_invalid_measurements(
measurements: &mut Measurements,
meta: &mut Meta,
measurements_config: &MeasurementsConfig,
) {
let mut custom_measurements_count = 0;
let mut removed_measurements = Object::new();

measurements.retain(|name, value| {
let measurement = match value.value() {
Some(m) => m,
Expand All @@ -279,8 +282,18 @@ fn remove_invalid_measurements(
return true;
}

// Retain payloads in _meta just for excessive custom measurements.
if let Some(measurement) = value.value_mut().take() {
removed_measurements.insert(name.clone(), Annotated::new(measurement));
}

false
});

if !removed_measurements.is_empty() {
meta.add_error(Error::invalid("too many measurements"));
meta.set_original_value(Some(removed_measurements));
}
}

/// Returns the unit of the provided metric.
Expand Down Expand Up @@ -339,10 +352,10 @@ fn normalize_measurements(event: &mut Event, measurements_config: Option<&Measur
if event.ty.value() != Some(&EventType::Transaction) {
// Only transaction events may have a measurements interface
event.measurements = Annotated::empty();
} else if let Some(measurements) = event.measurements.value_mut() {
} else if let Annotated(Some(ref mut measurements), ref mut meta) = event.measurements {
normalize_units(measurements);
if let Some(measurements_config) = measurements_config {
remove_invalid_measurements(measurements, measurements_config);
remove_invalid_measurements(measurements, meta, measurements_config);
}

let duration_millis = match (event.start_timestamp.0, event.timestamp.0) {
Expand Down Expand Up @@ -2184,6 +2197,26 @@ mod tests {
"unit": "none",
},
},
"_meta": {
"measurements": {
"": Meta(Some(MetaInner(
err: [
[
"invalid_data",
{
"reason": "too many measurements",
},
],
],
val: Some({
"my_custom_measurement_3": {
"unit": "none",
"value": 456.0,
},
}),
))),
},
},
}
"###);
}
Expand Down

0 comments on commit da6fda6

Please sign in to comment.