From 65597d687b79d82260237d51c0d99a31d05ea5ba Mon Sep 17 00:00:00 2001 From: Iker Barriocanal <32816711+iker-barriocanal@users.noreply.github.com> Date: Mon, 8 Aug 2022 12:46:21 +0200 Subject: [PATCH] ref(metrics): Remove `light_normalize` in favor of `light_normalize_event` (#1385) --- relay-cabi/src/processing.rs | 4 +- relay-general/src/store/mod.rs | 14 ++----- relay-general/src/store/normalize.rs | 58 ++++++++++++++-------------- relay-general/tests/test_fixtures.rs | 4 +- relay-server/src/actors/envelopes.rs | 4 +- 5 files changed, 38 insertions(+), 46 deletions(-) diff --git a/relay-cabi/src/processing.rs b/relay-cabi/src/processing.rs index 3dd7c7b54d..734491e374 100644 --- a/relay-cabi/src/processing.rs +++ b/relay-cabi/src/processing.rs @@ -14,7 +14,7 @@ use relay_general::pii::{ use relay_general::processor::{process_value, split_chunks, ProcessingState}; use relay_general::protocol::{Event, VALID_PLATFORMS}; use relay_general::store::{ - light_normalize, GeoIpLookup, LightNormalizationConfig, StoreConfig, StoreProcessor, + light_normalize_event, GeoIpLookup, LightNormalizationConfig, StoreConfig, StoreProcessor, }; use relay_general::types::{Annotated, Remark}; use relay_sampling::{RuleCondition, SamplingConfig}; @@ -115,7 +115,7 @@ pub unsafe extern "C" fn relay_store_normalizer_normalize_event( max_secs_in_future: None, breakdowns_config: None, }; - light_normalize(&mut event, &config)?; + light_normalize_event(&mut event, &config)?; process_value(&mut event, &mut *processor, ProcessingState::root())?; RelayStr::from_string(event.to_json()?) } diff --git a/relay-general/src/store/mod.rs b/relay-general/src/store/mod.rs index fef2231d68..5ad027f8ad 100644 --- a/relay-general/src/store/mod.rs +++ b/relay-general/src/store/mod.rs @@ -8,7 +8,7 @@ use serde_json::Value; use crate::processor::{ProcessingState, Processor}; use crate::protocol::{Event, IpAddr}; -use crate::types::{Annotated, Meta, ProcessingResult, SpanAttribute}; +use crate::types::{Meta, ProcessingResult, SpanAttribute}; mod clock_drift; mod event_error; @@ -26,7 +26,8 @@ pub use normalize::breakdowns::{ get_breakdown_measurements, BreakdownConfig, BreakdownsConfig, SpanOperationsConfig, }; pub use normalize::{ - compute_measurements, is_valid_platform, normalize_dist, LightNormalizationConfig, + compute_measurements, is_valid_platform, light_normalize_event, normalize_dist, + LightNormalizationConfig, }; pub use transactions::{ get_measurement, get_transaction_op, is_high_cardinality_sdk, validate_timestamps, @@ -137,12 +138,3 @@ impl<'a> Processor for StoreProcessor<'a> { Ok(()) } } - -pub fn light_normalize( - event: &mut Annotated, - config: &LightNormalizationConfig, -) -> ProcessingResult { - transactions::validate_annotated_transaction(event)?; - normalize::light_normalize_event(event, config)?; - Ok(()) -} diff --git a/relay-general/src/store/normalize.rs b/relay-general/src/store/normalize.rs index c00a195fb7..0035994f43 100644 --- a/relay-general/src/store/normalize.rs +++ b/relay-general/src/store/normalize.rs @@ -10,7 +10,7 @@ use regex::Regex; use relay_common::{DurationUnit, FractionUnit, MetricUnit}; use smallvec::SmallVec; -use super::{schema, BreakdownsConfig}; +use super::{schema, transactions, BreakdownsConfig}; use crate::processor::{MaxChars, ProcessValue, ProcessingState, Processor}; use crate::protocol::{ self, AsPair, Breadcrumb, ClientSdkInfo, Context, Contexts, DebugImage, Event, EventId, @@ -513,6 +513,7 @@ pub fn light_normalize_event( event: &mut Annotated, config: &LightNormalizationConfig, ) -> ProcessingResult { + transactions::validate_annotated_transaction(event)?; event.apply(|event, meta| { // Check for required and non-empty values schema::SchemaProcessor.process_event(event, meta, ProcessingState::root())?; @@ -819,7 +820,6 @@ impl<'a> Processor for NormalizeProcessor<'a> { use crate::{ processor::process_value, protocol::{PairList, TagEntry}, - store::light_normalize, testutils::{assert_eq_dbg, get_path, get_value}, }; @@ -959,7 +959,7 @@ fn test_user_ip_from_remote_addr() { let config = StoreConfig::default(); let mut processor = NormalizeProcessor::new(Arc::new(config), None); let config = get_empty_light_normalization_config(); - light_normalize(&mut event, &config).unwrap(); + light_normalize_event(&mut event, &config).unwrap(); process_value(&mut event, &mut processor, ProcessingState::root()).unwrap(); let ip_addr = get_value!(event.user.ip_address!); @@ -987,7 +987,7 @@ fn test_user_ip_from_invalid_remote_addr() { let config = StoreConfig::default(); let mut processor = NormalizeProcessor::new(Arc::new(config), None); let config = get_empty_light_normalization_config(); - light_normalize(&mut event, &config).unwrap(); + light_normalize_event(&mut event, &config).unwrap(); process_value(&mut event, &mut processor, ProcessingState::root()).unwrap(); assert_eq_dbg!(Annotated::empty(), event.value().unwrap().user); @@ -1009,7 +1009,7 @@ fn test_user_ip_from_client_ip_without_auto() { let mut processor = NormalizeProcessor::new(Arc::new(config), None); let mut config = get_empty_light_normalization_config(); config.client_ip = Some(&ip_address); - light_normalize(&mut event, &config).unwrap(); + light_normalize_event(&mut event, &config).unwrap(); process_value(&mut event, &mut processor, ProcessingState::root()).unwrap(); let ip_addr = get_value!(event.user.ip_address!); @@ -1036,7 +1036,7 @@ fn test_user_ip_from_client_ip_with_auto() { let mut processor = NormalizeProcessor::new(Arc::new(config), Some(&geo)); let mut config = get_empty_light_normalization_config(); config.client_ip = Some(&ip_address); - light_normalize(&mut event, &config).unwrap(); + light_normalize_event(&mut event, &config).unwrap(); process_value(&mut event, &mut processor, ProcessingState::root()).unwrap(); let user = get_value!(event.user!); @@ -1060,7 +1060,7 @@ fn test_user_ip_from_client_ip_without_appropriate_platform() { let mut processor = NormalizeProcessor::new(Arc::new(config), Some(&geo)); let mut config = get_empty_light_normalization_config(); config.client_ip = Some(&ip_address); - light_normalize(&mut event, &config).unwrap(); + light_normalize_event(&mut event, &config).unwrap(); process_value(&mut event, &mut processor, ProcessingState::root()).unwrap(); let user = get_value!(event.user!); @@ -1073,7 +1073,7 @@ fn test_event_level_defaulted() { let processor = &mut NormalizeProcessor::default(); let mut event = Annotated::new(Event::default()); let config = get_empty_light_normalization_config(); - light_normalize(&mut event, &config).unwrap(); + light_normalize_event(&mut event, &config).unwrap(); process_value(&mut event, processor, ProcessingState::root()).unwrap(); assert_eq_dbg!(get_value!(event.level), Some(&Level::Error)); } @@ -1104,7 +1104,7 @@ fn test_transaction_level_untouched() { ..Event::default() }); let config = get_empty_light_normalization_config(); - light_normalize(&mut event, &config).unwrap(); + light_normalize_event(&mut event, &config).unwrap(); process_value(&mut event, processor, ProcessingState::root()).unwrap(); assert_eq_dbg!(get_value!(event.level), Some(&Level::Info)); } @@ -1121,7 +1121,7 @@ fn test_environment_tag_is_moved() { let mut processor = NormalizeProcessor::default(); let config = get_empty_light_normalization_config(); - light_normalize(&mut event, &config).unwrap(); + light_normalize_event(&mut event, &config).unwrap(); process_value(&mut event, &mut processor, ProcessingState::root()).unwrap(); let event = event.value().unwrap(); @@ -1143,7 +1143,7 @@ fn test_empty_environment_is_removed_and_overwritten_with_tag() { let mut processor = NormalizeProcessor::default(); let config = get_empty_light_normalization_config(); - light_normalize(&mut event, &config).unwrap(); + light_normalize_event(&mut event, &config).unwrap(); process_value(&mut event, &mut processor, ProcessingState::root()).unwrap(); let event = event.value().unwrap(); @@ -1161,7 +1161,7 @@ fn test_empty_environment_is_removed() { let mut processor = NormalizeProcessor::default(); let config = get_empty_light_normalization_config(); - light_normalize(&mut event, &config).unwrap(); + light_normalize_event(&mut event, &config).unwrap(); process_value(&mut event, &mut processor, ProcessingState::root()).unwrap(); assert_eq_dbg!(get_value!(event.environment), None); } @@ -1175,7 +1175,7 @@ fn test_none_environment_errors() { let mut processor = NormalizeProcessor::default(); let config = get_empty_light_normalization_config(); - light_normalize(&mut event, &config).unwrap(); + light_normalize_event(&mut event, &config).unwrap(); process_value(&mut event, &mut processor, ProcessingState::root()).unwrap(); let environment = get_path!(event.environment!); @@ -1203,7 +1203,7 @@ fn test_invalid_release_removed() { let mut processor = NormalizeProcessor::default(); let config = get_empty_light_normalization_config(); - light_normalize(&mut event, &config).unwrap(); + light_normalize_event(&mut event, &config).unwrap(); process_value(&mut event, &mut processor, ProcessingState::root()).unwrap(); let release = get_path!(event.release!); @@ -1237,7 +1237,7 @@ fn test_top_level_keys_moved_into_tags() { let mut processor = NormalizeProcessor::default(); let config = get_empty_light_normalization_config(); - light_normalize(&mut event, &config).unwrap(); + light_normalize_event(&mut event, &config).unwrap(); process_value(&mut event, &mut processor, ProcessingState::root()).unwrap(); assert_eq_dbg!(get_value!(event.site), None); @@ -1292,7 +1292,7 @@ fn test_internal_tags_removed() { let mut processor = NormalizeProcessor::default(); let config = get_empty_light_normalization_config(); - light_normalize(&mut event, &config).unwrap(); + light_normalize_event(&mut event, &config).unwrap(); process_value(&mut event, &mut processor, ProcessingState::root()).unwrap(); assert_eq!(get_value!(event.tags!).len(), 1); @@ -1320,7 +1320,7 @@ fn test_empty_tags_removed() { let mut processor = NormalizeProcessor::default(); let config = get_empty_light_normalization_config(); - light_normalize(&mut event, &config).unwrap(); + light_normalize_event(&mut event, &config).unwrap(); process_value(&mut event, &mut processor, ProcessingState::root()).unwrap(); assert_eq_dbg!( @@ -1372,7 +1372,7 @@ fn test_tags_deduplicated() { let mut processor = NormalizeProcessor::default(); let config = get_empty_light_normalization_config(); - light_normalize(&mut event, &config).unwrap(); + light_normalize_event(&mut event, &config).unwrap(); process_value(&mut event, &mut processor, ProcessingState::root()).unwrap(); // should keep the first occurrence of every tag @@ -1436,7 +1436,7 @@ fn test_unknown_debug_image() { let mut processor = NormalizeProcessor::default(); let config = get_empty_light_normalization_config(); - light_normalize(&mut event, &config).unwrap(); + light_normalize_event(&mut event, &config).unwrap(); process_value(&mut event, &mut processor, ProcessingState::root()).unwrap(); assert_eq_dbg!( @@ -1526,7 +1526,7 @@ fn test_too_long_tags() { let mut processor = NormalizeProcessor::default(); let config = get_empty_light_normalization_config(); - light_normalize(&mut event, &config).unwrap(); + light_normalize_event(&mut event, &config).unwrap(); process_value(&mut event, &mut processor, ProcessingState::root()).unwrap(); assert_eq_dbg!( @@ -1572,7 +1572,7 @@ fn test_regression_backfills_abs_path_even_when_moving_stacktrace() { let mut processor = NormalizeProcessor::default(); let config = get_empty_light_normalization_config(); - light_normalize(&mut event, &config).unwrap(); + light_normalize_event(&mut event, &config).unwrap(); process_value(&mut event, &mut processor, ProcessingState::root()).unwrap(); assert_eq_dbg!( @@ -1602,7 +1602,7 @@ fn test_parses_sdk_info_from_header() { ); let config = get_empty_light_normalization_config(); - light_normalize(&mut event, &config).unwrap(); + light_normalize_event(&mut event, &config).unwrap(); process_value(&mut event, &mut processor, ProcessingState::root()).unwrap(); assert_eq_dbg!( @@ -1626,7 +1626,7 @@ fn test_discards_received() { let mut processor = NormalizeProcessor::default(); let config = get_empty_light_normalization_config(); - light_normalize(&mut event, &config).unwrap(); + light_normalize_event(&mut event, &config).unwrap(); process_value(&mut event, &mut processor, ProcessingState::root()).unwrap(); assert_eq_dbg!(get_value!(event.received!), get_value!(event.timestamp!)); @@ -1658,7 +1658,7 @@ fn test_grouping_config() { ); let config = get_empty_light_normalization_config(); - light_normalize(&mut event, &config).unwrap(); + light_normalize_event(&mut event, &config).unwrap(); process_value(&mut event, &mut processor, ProcessingState::root()).unwrap(); assert_ron_snapshot!(SerializableAnnotated(&event), { @@ -1717,7 +1717,7 @@ fn test_future_timestamp() { max_secs_in_future, breakdowns_config: None, }; - light_normalize(&mut event, &config).unwrap(); + light_normalize_event(&mut event, &config).unwrap(); process_value(&mut event, &mut processor, ProcessingState::root()).unwrap(); assert_ron_snapshot!(SerializableAnnotated(&event), { @@ -1783,7 +1783,7 @@ fn test_past_timestamp() { max_secs_in_future, breakdowns_config: None, }; - light_normalize(&mut event, &config).unwrap(); + light_normalize_event(&mut event, &config).unwrap(); process_value(&mut event, &mut processor, ProcessingState::root()).unwrap(); assert_ron_snapshot!(SerializableAnnotated(&event), { @@ -1957,19 +1957,19 @@ fn test_light_normalization_is_idempotent() { event } - light_normalize(&mut event, &config).unwrap(); + light_normalize_event(&mut event, &config).unwrap(); let first = remove_received_from_event(&mut event.clone()) .to_json() .unwrap(); // Expected some fields (such as timestamps) exist after first light normalization. - light_normalize(&mut event, &config).unwrap(); + light_normalize_event(&mut event, &config).unwrap(); let second = remove_received_from_event(&mut event.clone()) .to_json() .unwrap(); assert_eq!(&first, &second, "idempotency check failed"); - light_normalize(&mut event, &config).unwrap(); + light_normalize_event(&mut event, &config).unwrap(); let third = remove_received_from_event(&mut event.clone()) .to_json() .unwrap(); diff --git a/relay-general/tests/test_fixtures.rs b/relay-general/tests/test_fixtures.rs index 96f4858b76..8a9003e389 100644 --- a/relay-general/tests/test_fixtures.rs +++ b/relay-general/tests/test_fixtures.rs @@ -4,7 +4,7 @@ use relay_general::pii::{PiiConfig, PiiProcessor}; use relay_general::processor::{process_value, ProcessingState}; use relay_general::protocol::Event; use relay_general::store::{ - light_normalize, LightNormalizationConfig, StoreConfig, StoreProcessor, + light_normalize_event, LightNormalizationConfig, StoreConfig, StoreProcessor, }; use relay_general::types::{Annotated, SerializableAnnotated}; @@ -83,7 +83,7 @@ macro_rules! event_snapshot { max_secs_in_future: None, breakdowns_config: None, }; - light_normalize(&mut event, &config).unwrap(); + light_normalize_event(&mut event, &config).unwrap(); process_value(&mut event, &mut processor, ProcessingState::root()).unwrap(); let compiled = PII_CONFIG.compiled(); diff --git a/relay-server/src/actors/envelopes.rs b/relay-server/src/actors/envelopes.rs index e867dcd73a..2e0f346501 100644 --- a/relay-server/src/actors/envelopes.rs +++ b/relay-server/src/actors/envelopes.rs @@ -31,7 +31,7 @@ use relay_general::protocol::{ IpAddr, LenientString, Metrics, RelayInfo, SecurityReportType, SessionAggregates, SessionAttributes, SessionUpdate, Timestamp, UserReport, Values, }; -use relay_general::store::{light_normalize, ClockDriftProcessor, LightNormalizationConfig}; +use relay_general::store::{ClockDriftProcessor, LightNormalizationConfig}; use relay_general::types::{Annotated, Array, FromValue, Object, ProcessingAction, Value}; use relay_log::LogError; use relay_metrics::{Bucket, Metric}; @@ -1865,7 +1865,7 @@ impl EnvelopeProcessor { }; metric!(timer(RelayTimers::EventProcessingLightNormalization), { - light_normalize(&mut state.event, &config) + relay_general::store::light_normalize_event(&mut state.event, &config) .map_err(|_| ProcessingError::InvalidTransaction)?; });