Skip to content

Commit

Permalink
ref(txnames): Remove temporary feature flag (#2280)
Browse files Browse the repository at this point in the history
#2250 introduced a temporary
feature flag because I wanted to do a gradual rollout. That flag can now
be removed.
  • Loading branch information
jjbayer committed Jul 5, 2023
1 parent 5e161d1 commit ee60bc8
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 98 deletions.
4 changes: 0 additions & 4 deletions relay-dynamic-config/src/feature.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,6 @@ pub enum Feature {
/// Enables metric extraction from spans.
#[serde(rename = "projects:span-metrics-extraction")]
SpanMetricsExtraction,
/// Apply transaction normalization rules to transactions from legacy SDKs.
#[serde(rename = "organizations:transaction-name-normalize-legacy")]
NormalizeLegacyTransactions,

/// Deprecated, still forwarded for older downstream Relays.
#[serde(rename = "organizations:transaction-name-mark-scrubbed-as-sanitized")]
Deprecated1,
Expand Down
95 changes: 5 additions & 90 deletions relay-general/src/store/transactions/processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,6 @@ use crate::types::{
pub struct TransactionNameConfig<'r> {
/// Rules for identifier replacement that were discovered by Sentry's transaction clusterer.
pub rules: &'r [TransactionNameRule],
/// Apply normalization rules to transactions without source that we expect to be high-cardinality
/// URLs.
pub normalize_legacy: bool,
}

/// Rejects transactions based on required fields.
Expand Down Expand Up @@ -199,8 +196,7 @@ impl<'r> TransactionsProcessor<'r> {
matches!(
source,
Some(&TransactionSource::Url | &TransactionSource::Sanitized)
) || (self.name_config.normalize_legacy
&& matches!(source, None)
) || (matches!(source, None)
&& event.transaction.value().map_or(false, |t| t.contains('/')))
}

Expand Down Expand Up @@ -1399,44 +1395,6 @@ mod tests {
assert_eq!(source, &TransactionSource::Unknown);
}

#[test]
fn test_default_transaction_source_none() {
let mut event = Annotated::<Event>::from_json(
r###"
{
"type": "transaction",
"transaction": "/",
"timestamp": 946684810.0,
"start_timestamp": 946684800.0,
"contexts": {
"trace": {
"trace_id": "4c79f60c11214eb38604f4ae0781bfb2",
"span_id": "fa90fdead5f74053",
"op": "http.server",
"type": "trace"
}
},
"sdk": {
"name": "sentry.javascript.browser"
},
"spans": []
}
"###,
)
.unwrap();

process_value(
&mut event,
&mut TransactionsProcessor::default(),
ProcessingState::root(),
)
.unwrap();

let transaction_info = &event.value().unwrap().transaction_info;

assert!(transaction_info.value().is_none());
}

#[test]
fn test_allows_valid_transaction_event_with_spans() {
let mut event = new_test_event();
Expand Down Expand Up @@ -1923,7 +1881,6 @@ mod tests {
&mut TransactionsProcessor::new(
TransactionNameConfig {
rules: rules.as_ref(),
..Default::default()
},
false,
None,
Expand Down Expand Up @@ -1989,7 +1946,6 @@ mod tests {
&mut TransactionsProcessor::new(
TransactionNameConfig {
rules: rules.as_ref(),
..Default::default()
},
false,
None,
Expand Down Expand Up @@ -2079,7 +2035,6 @@ mod tests {
let mut processor = TransactionsProcessor::new(
TransactionNameConfig {
rules: rules.as_ref(),
..Default::default()
},
false,
None,
Expand Down Expand Up @@ -2212,7 +2167,6 @@ mod tests {
&mut TransactionsProcessor::new(
TransactionNameConfig {
rules: rules.as_ref(),
..Default::default()
},
false,
None,
Expand Down Expand Up @@ -2244,7 +2198,7 @@ mod tests {
"###);
}

fn run_with_unknown_source(feature: bool, sdk: &str) -> Annotated<Event> {
fn run_with_unknown_source(sdk: &str) -> Annotated<Event> {
let json = r#"
{
"type": "transaction",
Expand Down Expand Up @@ -2281,7 +2235,6 @@ mod tests {
&mut TransactionsProcessor::new(
TransactionNameConfig {
rules: rules.as_ref(),
normalize_legacy: feature,
},
false,
None,
Expand All @@ -2295,7 +2248,7 @@ mod tests {
#[test]
fn test_normalize_legacy_javascript() {
// Javascript without source annotation gets sanitized.
let event = run_with_unknown_source(true, "sentry.javascript.browser");
let event = run_with_unknown_source("sentry.javascript.browser");
assert_annotated_snapshot!(event, @r###"
{
"type": "transaction",
Expand Down Expand Up @@ -2339,7 +2292,7 @@ mod tests {
fn test_normalize_legacy_python() {
// Python without source annotation does not get sanitized, because we assume it to be
// low cardinality.
let event = run_with_unknown_source(true, "sentry.python");
let event = run_with_unknown_source("sentry.python");
assert_annotated_snapshot!(event, @r###"
{
"type": "transaction",
Expand All @@ -2366,33 +2319,6 @@ mod tests {
"###);
}

#[test]
fn test_normalize_legacy_javascript_disabled() {
// Sanitation is skipped when the config flag is disabled.
let event = run_with_unknown_source(false, "sentry.javascript.browser");
assert_annotated_snapshot!(event, @r###"
{
"type": "transaction",
"transaction": "/user/jane/blog/",
"timestamp": 1619420400.0,
"start_timestamp": 1619420341.0,
"contexts": {
"trace": {
"trace_id": "4c79f60c11214eb38604f4ae0781bfb2",
"span_id": "fa90fdead5f74053",
"op": "rails.request",
"status": "ok",
"type": "trace"
}
},
"sdk": {
"name": "sentry.javascript.browser"
},
"spans": []
}
"###);
}

#[test]
fn test_transaction_name_rename_end_slash() {
let json = r#"
Expand Down Expand Up @@ -2428,14 +2354,7 @@ mod tests {

process_value(
&mut event,
&mut TransactionsProcessor::new(
TransactionNameConfig {
rules: &[rule],
..Default::default()
},
false,
None,
),
&mut TransactionsProcessor::new(TransactionNameConfig { rules: &[rule] }, false, None),
ProcessingState::root(),
)
.unwrap();
Expand Down Expand Up @@ -2661,7 +2580,6 @@ mod tests {
expiry: Utc.with_ymd_and_hms(3000, 1, 1, 1, 1, 1).unwrap(),
redaction: RedactionRule::default(),
}],
..Default::default()
},
false,
None,
Expand Down Expand Up @@ -2708,7 +2626,6 @@ mod tests {
expiry: Utc.with_ymd_and_hms(3000, 1, 1, 1, 1, 1).unwrap(),
redaction: RedactionRule::default(),
}],
..Default::default()
},
false,
None,
Expand Down Expand Up @@ -3194,7 +3111,6 @@ mod tests {
expiry: Utc.with_ymd_and_hms(3000, 1, 1, 1, 1, 1).unwrap(),
redaction: RedactionRule::default(),
}],
..Default::default()
},
true,
None,
Expand Down Expand Up @@ -3294,7 +3210,6 @@ mod tests {
expiry: Utc.with_ymd_and_hms(3000, 1, 1, 1, 1, 1).unwrap(),
redaction: RedactionRule::default(),
}],
..Default::default()
},
true,
Some(&Vec::from([SpanDescriptionRule {
Expand Down
4 changes: 0 additions & 4 deletions relay-server/src/actors/processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2393,9 +2393,6 @@ impl EnvelopeProcessorService {
normalize_user_agent: Some(true),
transaction_name_config: TransactionNameConfig {
rules: &state.project_state.config.tx_name_rules,
normalize_legacy: state
.project_state
.has_feature(Feature::NormalizeLegacyTransactions),
},
device_class_synthesis_config: state
.project_state
Expand Down Expand Up @@ -3752,7 +3749,6 @@ mod tests {
substitution: "*".to_owned(),
},
}],
..Default::default()
},
..Default::default()
};
Expand Down

0 comments on commit ee60bc8

Please sign in to comment.