Skip to content

Commit

Permalink
ref(general): Split TransactionsProcessor into helper methods (#2243)
Browse files Browse the repository at this point in the history
getsentry/team-ingest#129 will require me to
change the order of processing steps for transactions. To make that
future change more readable, put processing steps into their own
functions.

This PR does not contain any functional changes.
  • Loading branch information
jjbayer authored Jun 23, 2023
1 parent b5d9b2f commit e13d231
Showing 1 changed file with 55 additions and 45 deletions.
100 changes: 55 additions & 45 deletions relay-general/src/store/transactions/processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,41 @@ impl<'r> TransactionsProcessor<'r> {

Ok(())
}

fn normalize_transaction_name(&self, event: &mut Event) -> ProcessingResult {
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)?;
}

if !self.name_config.rules.is_empty() {
self.apply_transaction_rename_rule(
&mut event.transaction,
event.transaction_info.value_mut(),
)?;
}

if matches!(event.get_transaction_source(), &TransactionSource::Url) {
// Always mark URL transactions as sanitized, even if no modification were made by
// clusterer rules or regex matchers. This has the consequence that the transaction name
// is always extracted as a tag on transaction metrics.
// Instead of changing the source to "sanitized", we could have changed metrics extraction
// to also extract the transaction name for URL transactions. But this is the safer way,
// because the product currently uses queries that assume that `source:url` is equivalent
// to `transaction:<< unparameterized >>`.
event
.transaction_info
.get_or_insert_with(Default::default)
.source
.set_value(Some(TransactionSource::Sanitized));
}

Ok(())
}
}

/// Get the value for a measurement, e.g. lcp -> event.measurements.lcp
Expand Down Expand Up @@ -457,6 +492,24 @@ fn is_sql_query_scrubbed(query: &Annotated<String>) -> bool {
.map_or(false, |q| SQL_ALREADY_NORMALIZED_REGEX.is_match(q))
}

fn end_all_spans(event: &mut Event) -> ProcessingResult {
let spans = event.spans.value_mut().get_or_insert_with(|| Vec::new());
for span in spans {
if let Some(span) = span.value_mut() {
if span.timestamp.value().is_none() {
// event timestamp guaranteed to be `Some` due to validate_transaction call
span.timestamp.set_value(event.timestamp.value().cloned());
span.status = Annotated::new(SpanStatus::DeadlineExceeded);
}
} else {
return Err(ProcessingAction::InvalidTransaction(
"spans must be valid in transaction event",
));
}
}
Ok(())
}

impl Processor for TransactionsProcessor<'_> {
fn process_event(
&mut self,
Expand All @@ -479,54 +532,11 @@ impl Processor for TransactionsProcessor<'_> {
.set_value(Some("<unlabeled transaction>".to_owned()))
}

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)?;
}

if !self.name_config.rules.is_empty() {
self.apply_transaction_rename_rule(
&mut event.transaction,
event.transaction_info.value_mut(),
)?;
}

if matches!(event.get_transaction_source(), &TransactionSource::Url) {
// Always mark URL transactions as sanitized, even if no modification were made by
// clusterer rules or regex matchers. This has the consequence that the transaction name
// is always extracted as a tag on transaction metrics.
// Instead of changing the source to "sanitized", we could have changed metrics extraction
// to also extract the transaction name for URL transactions. But this is the safer way,
// because the product currently uses queries that assume that `source:url` is equivalent
// to `transaction:<< unparameterized >>`.
event
.transaction_info
.get_or_insert_with(Default::default)
.source
.set_value(Some(TransactionSource::Sanitized));
}
self.normalize_transaction_name(event)?;

validate_transaction(event)?;

let spans = event.spans.value_mut().get_or_insert_with(|| Vec::new());

for span in spans {
if let Some(span) = span.value_mut() {
if span.timestamp.value().is_none() {
// event timestamp guaranteed to be `Some` due to validate_transaction call
span.timestamp.set_value(event.timestamp.value().cloned());
span.status = Annotated::new(SpanStatus::DeadlineExceeded);
}
} else {
return Err(ProcessingAction::InvalidTransaction(
"spans must be valid in transaction event",
));
}
}
end_all_spans(event)?;

set_default_transaction_source(event);

Expand Down

0 comments on commit e13d231

Please sign in to comment.