fix(metrics): Move TrimmingProcessor to light normalization (#2342)

Because the `TrimmingProcessor` is not part of light normalization, we
extract the untrimmed transaction name from the payload, which then gets
dropped by the aggregator.

Solution: Call `TrimmingProcessor` in `light_normalize_event`.

After deploying this, we have to keep an eye on the
`event.processing_time` metric.

Fixes getsentry/team-ingest#187

CHANGELOG.md

@@ -2,7 +2,13 @@
 
 ## Unreleased
 
+**Bug Fixes**:
+
+- Trim fields (e.g. `transaction`) before metrics extraction. ([#2342](https://github.com/getsentry/relay/pull/2342))
 - Interpret `aggregator.max_tag_value_length` as characters instead of bytes. ([#2343](https://github.com/getsentry/relay/pull/2343))
+
+**Internal**:
+
 - Add capability to configure metrics aggregators per use case. ([#2341](https://github.com/getsentry/relay/pull/2341))
 
 ## 23.7.0

relay-cabi/src/processing.rs

@@ -137,6 +137,7 @@ pub unsafe extern "C" fn relay_store_normalizer_normalize_event(
         max_tag_value_length: usize::MAX,
         span_description_rules: None,
         geoip_lookup: None, // only supported in relay
+        enable_trimming: config.enable_trimming.unwrap_or_default(),
     };
     light_normalize_event(&mut event, light_normalization_config)?;
     process_value(&mut event, &mut *processor, ProcessingState::root())?;

relay-general/src/store/mod.rs

@@ -102,7 +102,6 @@ impl<'a> Processor for StoreProcessor<'a> {
     ) -> ProcessingResult {
         let is_renormalize = self.config.is_renormalize.unwrap_or(false);
         let remove_other = self.config.remove_other.unwrap_or(!is_renormalize);
-        let enable_trimming = self.config.enable_trimming.unwrap_or(true);
 
         // Convert legacy data structures to current format
         legacy::LegacyProcessor.process_event(event, meta, state)?;
@@ -122,11 +121,6 @@ impl<'a> Processor for StoreProcessor<'a> {
             event_error::EmitEventErrors::new().process_event(event, meta, state)?;
         }
 
-        if enable_trimming {
-            // Trim large strings and databags down
-            trimming::TrimmingProcessor::new().process_event(event, meta, state)?;
-        }
-
         Ok(())
     }
 }

relay-general/src/store/normalize.rs

@@ -21,7 +21,8 @@ use crate::protocol::{
     VALID_PLATFORMS,
 };
 use crate::store::{
-    ClockDriftProcessor, GeoIpLookup, SpanDescriptionRule, StoreConfig, TransactionNameConfig,
+    trimming, ClockDriftProcessor, GeoIpLookup, SpanDescriptionRule, StoreConfig,
+    TransactionNameConfig,
 };
 use crate::types::{
     Annotated, Empty, Error, ErrorKind, FromValue, Meta, Object, ProcessingAction,
@@ -751,6 +752,7 @@ pub struct LightNormalizationConfig<'a> {
     pub max_tag_value_length: usize, // TODO: move span related fields into separate config.
     pub span_description_rules: Option<&'a Vec<SpanDescriptionRule>>,
     pub geoip_lookup: Option<&'a GeoIpLookup>,
+    pub enable_trimming: bool,
 }
 
 impl Default for LightNormalizationConfig<'_> {
@@ -773,6 +775,7 @@ impl Default for LightNormalizationConfig<'_> {
             max_tag_value_length: usize::MAX,
             span_description_rules: Default::default(),
             geoip_lookup: Default::default(),
+            enable_trimming: false,
         }
     }
 }
@@ -874,6 +877,15 @@ pub fn light_normalize_event(
             );
         }
 
+        if config.enable_trimming {
+            // Trim large strings and databags down
+            trimming::TrimmingProcessor::new().process_event(
+                event,
+                meta,
+                ProcessingState::root(),
+            )?;
+        }
+
         Ok(())
     })
 }

relay-server/src/actors/processor.rs

@@ -2355,6 +2355,7 @@ impl EnvelopeProcessorService {
                 light_normalize_spans,
                 span_description_rules: state.project_state.config.span_description_rules.as_ref(),
                 geoip_lookup: self.geoip_lookup.as_ref(),
+                enable_trimming: true,
             };
 
             metric!(timer(RelayTimers::EventProcessingLightNormalization), {

tests/integration/test_metrics.py

@@ -969,6 +969,51 @@ def test_no_transaction_metrics_when_filtered(mini_sentry, relay):
     assert mini_sentry.captured_events.qsize() == 0
 
 
+def test_transaction_name_too_long(
+    transactions_consumer,
+    metrics_consumer,
+    mini_sentry,
+    relay_with_processing,
+):
+    """When a transaction name is truncated, the transaction metric should get the truncated value"""
+    project_id = 42
+    mini_sentry.add_full_project_config(project_id)
+    config = mini_sentry.project_configs[project_id]["config"]
+    config["transactionMetrics"] = {
+        "version": 1,
+    }
+
+    transaction = {
+        "event_id": "d2132d31b39445f1938d7e21b6bf0ec4",
+        "type": "transaction",
+        "transaction": 201 * "x",
+        "start_timestamp": 1597976392.6542819,
+        "contexts": {
+            "trace": {
+                "trace_id": "4C79F60C11214EB38604F4AE0781BFB2",
+                "span_id": "FA90FDEAD5F74052",
+            }
+        },
+    }
+    timestamp = datetime.now(tz=timezone.utc)
+    transaction["timestamp"] = timestamp.isoformat()
+
+    metrics_consumer = metrics_consumer()
+    tx_consumer = transactions_consumer()
+    processing = relay_with_processing(options=TEST_CONFIG)
+    processing.send_transaction(project_id, transaction)
+
+    expected_transaction_name = 197 * "x" + "..."
+
+    transaction, _ = tx_consumer.get_event()
+    assert transaction["transaction"] == expected_transaction_name
+
+    metrics = metrics_consumer.get_metrics()
+    for metric, _ in metrics:
+        if metric["name"] != "c:transactions/count_per_root_project@none":
+            assert metric["tags"].get("transaction") == expected_transaction_name
+
+
 @pytest.mark.skip(reason="flake")
 def test_graceful_shutdown(mini_sentry, relay):
     relay = relay(