From 677a20fd4c0660eba17650d725b540fcd928c185 Mon Sep 17 00:00:00 2001 From: Joris Bayer Date: Mon, 22 May 2023 09:37:47 +0200 Subject: [PATCH 1/3] wip --- .../src/store/transactions/processor.rs | 28 ++++++++++++++----- relay-server/src/actors/processor.rs | 2 ++ 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/relay-general/src/store/transactions/processor.rs b/relay-general/src/store/transactions/processor.rs index c6f7770c0f..50bb7d6371 100644 --- a/relay-general/src/store/transactions/processor.rs +++ b/relay-general/src/store/transactions/processor.rs @@ -23,6 +23,8 @@ use crate::types::{ pub struct TransactionNameConfig<'r> { /// Rules for identifier replacement that were discovered by Sentry's transaction clusterer. pub rules: &'r [TransactionNameRule], + /// True if URL transactions should be marked as sanitized, even if there are no rules. + pub ready: bool, } /// Rejects transactions based on required fields. @@ -371,16 +373,17 @@ impl Processor for TransactionsProcessor<'_> { .set_value(Some("".to_owned())) } - // Normalize transaction names for URLs and Sanitized transaction sources. - // This in addition to renaming rules can catch some high cardinality parts. - let mut sanitized = false; + // If the project is marked as 'ready', always set the transaction source to sanitized. + let mut mark_as_sanitized = self.name_config.ready; if matches!( event.get_transaction_source(), &TransactionSource::Url | &TransactionSource::Sanitized ) { + // Normalize transaction names for URLs and Sanitized transaction sources. + // This in addition to renaming rules can catch some high cardinality parts. scrub_identifiers(&mut event.transaction)?.then(|| { - sanitized = true; + mark_as_sanitized = true; }); } @@ -390,10 +393,10 @@ impl Processor for TransactionsProcessor<'_> { event.transaction_info.value_mut(), )?; - sanitized = true; + mark_as_sanitized = true; } - if sanitized && matches!(event.get_transaction_source(), &TransactionSource::Url) { + if mark_as_sanitized && matches!(event.get_transaction_source(), &TransactionSource::Url) { event .transaction_info .get_or_insert_with(Default::default) @@ -1730,6 +1733,7 @@ mod tests { &mut TransactionsProcessor::new( TransactionNameConfig { rules: rules.as_ref(), + ready: false, }, false, ), @@ -1794,6 +1798,7 @@ mod tests { &mut TransactionsProcessor::new( TransactionNameConfig { rules: rules.as_ref(), + ready: false, }, false, ), @@ -1892,6 +1897,7 @@ mod tests { &mut TransactionsProcessor::new( TransactionNameConfig { rules: rules.as_ref(), + ready: false, }, false, ), @@ -1958,7 +1964,13 @@ mod tests { process_value( &mut event, - &mut TransactionsProcessor::new(TransactionNameConfig { rules: &[rule] }, false), + &mut TransactionsProcessor::new( + TransactionNameConfig { + rules: &[rule], + ready: false, + }, + false, + ), ProcessingState::root(), ) .unwrap(); @@ -2185,6 +2197,7 @@ mod tests { scope: RuleScope::default(), redaction: RedactionRule::default(), }], + ready: false, }, false, ), @@ -2231,6 +2244,7 @@ mod tests { scope: RuleScope::default(), redaction: RedactionRule::default(), }], + ready: false, }, false, ), diff --git a/relay-server/src/actors/processor.rs b/relay-server/src/actors/processor.rs index 5ab2504d2b..450cdefe9f 100644 --- a/relay-server/src/actors/processor.rs +++ b/relay-server/src/actors/processor.rs @@ -2294,6 +2294,7 @@ impl EnvelopeProcessorService { normalize_user_agent: Some(true), transaction_name_config: TransactionNameConfig { rules: &state.project_state.config.tx_name_rules, + ready: state.project_state.config.tx_name_ready, }, device_class_synthesis_config: state .project_state @@ -3468,6 +3469,7 @@ mod tests { substitution: "*".to_owned(), }, }], + ready: false, }, ..Default::default() }; From 7603c5f09818131c3426805e0ff1c67d128bd7c4 Mon Sep 17 00:00:00 2001 From: Joris Bayer Date: Mon, 22 May 2023 09:43:01 +0200 Subject: [PATCH 2/3] test --- .../src/store/transactions/processor.rs | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/relay-general/src/store/transactions/processor.rs b/relay-general/src/store/transactions/processor.rs index 50bb7d6371..e42a839fd7 100644 --- a/relay-general/src/store/transactions/processor.rs +++ b/relay-general/src/store/transactions/processor.rs @@ -1680,6 +1680,67 @@ mod tests { "###); } + #[test] + /// When the `ready` flag is set, mark a transaction as `sanitized` even if there are no rules. + fn test_transaction_name_normalize_mark_as_sanitized_when_ready() { + let json = r#" + { + "type": "transaction", + "transaction": "/foo/bar/user/john/0", + "transaction_info": { + "source": "url" + }, + "timestamp": "2021-04-26T08:00:00+0100", + "start_timestamp": "2021-04-26T07:59:01+0100", + "contexts": { + "trace": { + "trace_id": "4c79f60c11214eb38604f4ae0781bfb2", + "span_id": "fa90fdead5f74053", + "op": "rails.request", + "status": "ok" + } + } + + } + "#; + let mut event = Annotated::::from_json(json).unwrap(); + + process_value( + &mut event, + &mut TransactionsProcessor::new( + TransactionNameConfig { + rules: &[], + ready: true, + }, + false, + ), + ProcessingState::root(), + ) + .unwrap(); + + assert_annotated_snapshot!(event, @r###" + { + "type": "transaction", + "transaction": "/foo/bar/user/john/0", + "transaction_info": { + "source": "sanitized" + }, + "timestamp": 1619420400.0, + "start_timestamp": 1619420341.0, + "contexts": { + "trace": { + "trace_id": "4c79f60c11214eb38604f4ae0781bfb2", + "span_id": "fa90fdead5f74053", + "op": "rails.request", + "status": "ok", + "type": "trace" + } + }, + "spans": [] + } + "###); + } + #[test] fn test_transaction_name_rename_with_rules() { let json = r#" From d98d38de4ab165de1b291f75f602ba3a760d32f8 Mon Sep 17 00:00:00 2001 From: Joris Bayer Date: Mon, 22 May 2023 13:11:39 +0200 Subject: [PATCH 3/3] doc: changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fcc1367ebc..918cd3630b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ **Internal**: -- Add `txNameReady` flag to project config. ([#2128](https://github.com/getsentry/relay/pull/2128)) +- Mark all URL transactions as `sanitized` when `txNameReady` flag is set. ([#2128](https://github.com/getsentry/relay/pull/2128), [#2139](https://github.com/getsentry/relay/pull/2139)) ## 23.5.0