Skip to content

Commit

Permalink
ref(server): Refactor get_transaction_name in metric extraction (#2405)
Browse files Browse the repository at this point in the history
Internal refactor that separates tracking a statsd metric for
unparameterized transaction names from the logic of extracting a
transaction name from the event. This will help with moving this
functionality to `FieldValueProvider`.
  • Loading branch information
jan-auer committed Aug 16, 2023
1 parent dc3306e commit def4373
Showing 1 changed file with 20 additions and 33 deletions.
53 changes: 20 additions & 33 deletions relay-server/src/metrics_extraction/transactions/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ use relay_general::store::utils::{

pub mod types;

/// Placeholder for transaction names in metrics that is used when SDKs are likely sending high
/// cardinality data such as raw URLs.
const PLACEHOLDER_UNPARAMETERIZED: &str = "<< unparameterized >>";

/// Extract transaction status, defaulting to [`SpanStatus::Unknown`].
/// Must be consistent with `process_trace_context` in [`relay_general::store`].
fn extract_transaction_status(trace_context: &TraceContext) -> SpanStatus {
Expand Down Expand Up @@ -55,11 +59,7 @@ fn extract_geo_country_code(event: &Event) -> Option<String> {
geo.country_code.value().cloned()
}

fn is_low_cardinality(source: Option<&TransactionSource>) -> bool {
// `None` is used to mark a legacy SDK that does not send the transaction name,
// and we assume sends high-cardinality data. See `is_high_cardinality_transaction`.
let Some(source) = source else { return false };

fn is_low_cardinality(source: &TransactionSource) -> bool {
match source {
// For now, we hope that custom transaction names set by users are low-cardinality.
TransactionSource::Custom => true,
Expand Down Expand Up @@ -91,36 +91,25 @@ fn is_low_cardinality(source: Option<&TransactionSource>) -> bool {
/// High-cardinality sources are excluded to protect our metrics infrastructure.
/// Note that this will produce a discrepancy between metrics and raw transaction data.
fn get_transaction_name(event: &Event) -> Option<String> {
let original_transaction_name = match event.transaction.value() {
Some(name) => name,
None => {
return None;
}
};
let original = event.transaction.value()?;

let source = event
.transaction_info
.value()
.and_then(|info| info.source.value());

let use_original_name = is_low_cardinality(source);

let name_used;
let name = if use_original_name {
name_used = "original";
Some(original_transaction_name.clone())
} else {
// Pick a sentinel based on the transaction source:
match source {
None | Some(TransactionSource::Other(_)) => {
name_used = "none";
None
}
_ => {
name_used = "placeholder";
Some("<< unparameterized >>".to_owned())
}
}
match source {
Some(source) if is_low_cardinality(source) => Some(original.clone()),
Some(TransactionSource::Other(_)) | None => None,
Some(_) => Some(PLACEHOLDER_UNPARAMETERIZED.to_owned()),
}
}

fn track_transaction_name_stats(event: &Event) {
let name_used = match get_transaction_name(event).as_deref() {
Some(self::PLACEHOLDER_UNPARAMETERIZED) => "placeholder",
Some(_) => "original",
None => "none",
};

relay_statsd::metric!(
Expand All @@ -129,13 +118,10 @@ fn get_transaction_name(event: &Event) -> Option<String> {
sdk_name = event
.client_sdk
.value()
.and_then(|c| c.name.value())
.map(std::string::String::as_str)
.and_then(|sdk| sdk.name.as_str())
.unwrap_or_default(),
name_used = name_used,
);

name
}

/// These are the tags that are added to all extracted metrics.
Expand Down Expand Up @@ -274,6 +260,7 @@ impl TransactionExtractor<'_> {
return Err(ExtractMetricsError::InvalidTimestamp);
}

track_transaction_name_stats(event);
let tags = extract_universal_tags(event, self.config);

// Measurements
Expand Down

0 comments on commit def4373

Please sign in to comment.