From 6ea5b461cddca06eabefdcb10dad2e21930acce0 Mon Sep 17 00:00:00 2001 From: Joris Bayer Date: Thu, 7 Dec 2023 13:57:44 +0100 Subject: [PATCH 1/3] feat(perf): Fewer tags on performance score metrics --- .../metrics_extraction/transactions/mod.rs | 111 +++++++++++++++++- 1 file changed, 107 insertions(+), 4 deletions(-) diff --git a/relay-server/src/metrics_extraction/transactions/mod.rs b/relay-server/src/metrics_extraction/transactions/mod.rs index 3f1d846555c..6484f1b046b 100644 --- a/relay-server/src/metrics_extraction/transactions/mod.rs +++ b/relay-server/src/metrics_extraction/transactions/mod.rs @@ -1,4 +1,4 @@ -use std::collections::BTreeMap; +use std::collections::{BTreeMap, BTreeSet}; use relay_base_schema::events::EventType; use relay_common::time::UnixTimestamp; @@ -24,6 +24,18 @@ pub mod types; /// cardinality data such as raw URLs. const PLACEHOLDER_UNPARAMETERIZED: &str = "<< unparameterized >>"; +/// Tags we set on metrics for performance score measurements (e.g. `score.lcp.weight`). +/// +/// These are a subset of "universal" tags. +const PERFORMANCE_SCORE_TAGS: [CommonTag; 6] = [ + CommonTag::Release, + CommonTag::Environment, + CommonTag::Transaction, + CommonTag::TransactionOp, + CommonTag::BrowserName, + CommonTag::TransactionOp, +]; + /// Extract HTTP method /// See . fn extract_http_method(transaction: &Event) -> Option { @@ -263,6 +275,14 @@ impl TransactionExtractor<'_> { let light_tags = extract_light_transaction_tags(event); // Measurements + let names: BTreeSet<_> = event + .measurements + .value() + .map(|x| x.keys()) + .into_iter() + .flatten() + .map(|x| x.as_str()) + .collect(); if let Some(measurements) = event.measurements.value() { for (name, annotated) in measurements.iter() { let measurement = match annotated.value() { @@ -275,9 +295,27 @@ impl TransactionExtractor<'_> { None => continue, }; + let is_performance_score = name == "score.total" + || name + .strip_prefix("score.weight.") + .or_else(|| name.strip_prefix("score.")) + .map_or(false, |suffix| names.contains(suffix)); let measurement_tags = TransactionMeasurementTags { measurement_rating: get_measurement_rating(name, value), - universal_tags: tags.clone(), + universal_tags: if is_performance_score { + CommonTags( + tags.0 + .iter() + .filter_map(|(key, value)| { + PERFORMANCE_SCORE_TAGS + .contains(key) + .then(|| (key.clone(), value.clone())) + }) + .collect::>(), + ) + } else { + tags.clone() + }, }; metrics.project_metrics.push( @@ -434,11 +472,12 @@ mod tests { use relay_dynamic_config::AcceptTransactionNames; use relay_event_normalization::{ normalize_event, set_default_transaction_source, BreakdownsConfig, - DynamicMeasurementsConfig, MeasurementsConfig, NormalizationConfig, + DynamicMeasurementsConfig, MeasurementsConfig, NormalizationConfig, PerformanceScoreConfig, + PerformanceScoreProfile, PerformanceScoreWeightedComponent, }; use relay_event_schema::protocol::User; use relay_metrics::BucketValue; - use relay_protocol::Annotated; + use relay_protocol::{Annotated, RuleCondition}; use super::*; @@ -520,6 +559,19 @@ mod tests { breakdowns_config: Some(&breakdowns_config), enrich_spans: false, normalize_spans: true, + performance_score: Some(&PerformanceScoreConfig { + profiles: vec![PerformanceScoreProfile { + name: Some("".into()), + score_components: vec![PerformanceScoreWeightedComponent { + measurement: "lcp".into(), + weight: 0.5, + p10: 2.0, + p50: 3.0, + optional: false, + }], + condition: Some(RuleCondition::all()), + }], + }), ..Default::default() }, ) @@ -629,6 +681,57 @@ mod tests { "transaction.status": "ok", }, }, + Bucket { + timestamp: UnixTimestamp(1619420400), + width: 0, + name: "d:transactions/measurements.score.lcp@ratio", + value: Distribution( + [ + 0.0, + ], + ), + tags: { + "browser.name": "Chrome", + "environment": "fake_environment", + "release": "1.2.3", + "transaction": "gEt /api/:version/users/", + "transaction.op": "mYOp", + }, + }, + Bucket { + timestamp: UnixTimestamp(1619420400), + width: 0, + name: "d:transactions/measurements.score.total@ratio", + value: Distribution( + [ + 0.0, + ], + ), + tags: { + "browser.name": "Chrome", + "environment": "fake_environment", + "release": "1.2.3", + "transaction": "gEt /api/:version/users/", + "transaction.op": "mYOp", + }, + }, + Bucket { + timestamp: UnixTimestamp(1619420400), + width: 0, + name: "d:transactions/measurements.score.weight.lcp@ratio", + value: Distribution( + [ + 1.0, + ], + ), + tags: { + "browser.name": "Chrome", + "environment": "fake_environment", + "release": "1.2.3", + "transaction": "gEt /api/:version/users/", + "transaction.op": "mYOp", + }, + }, Bucket { timestamp: UnixTimestamp(1619420400), width: 0, From ac5cea3b064ff1d69004476a25033cb4b60881db Mon Sep 17 00:00:00 2001 From: Joris Bayer Date: Thu, 7 Dec 2023 14:07:32 +0100 Subject: [PATCH 2/3] ref --- .../metrics_extraction/transactions/mod.rs | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/relay-server/src/metrics_extraction/transactions/mod.rs b/relay-server/src/metrics_extraction/transactions/mod.rs index 6484f1b046b..4a3d3fbd404 100644 --- a/relay-server/src/metrics_extraction/transactions/mod.rs +++ b/relay-server/src/metrics_extraction/transactions/mod.rs @@ -275,13 +275,12 @@ impl TransactionExtractor<'_> { let light_tags = extract_light_transaction_tags(event); // Measurements - let names: BTreeSet<_> = event + let measurement_names: BTreeSet<_> = event .measurements .value() - .map(|x| x.keys()) .into_iter() - .flatten() - .map(|x| x.as_str()) + .flat_map(|measurements| measurements.keys()) + .map(String::as_str) .collect(); if let Some(measurements) = event.measurements.value() { for (name, annotated) in measurements.iter() { @@ -295,22 +294,22 @@ impl TransactionExtractor<'_> { None => continue, }; + // We treat a measurement as "performance score" if its name is the name of another + // measurement prefixed by `score.`. let is_performance_score = name == "score.total" || name .strip_prefix("score.weight.") .or_else(|| name.strip_prefix("score.")) - .map_or(false, |suffix| names.contains(suffix)); + .map_or(false, |suffix| measurement_names.contains(suffix)); + let measurement_tags = TransactionMeasurementTags { measurement_rating: get_measurement_rating(name, value), universal_tags: if is_performance_score { CommonTags( tags.0 .iter() - .filter_map(|(key, value)| { - PERFORMANCE_SCORE_TAGS - .contains(key) - .then(|| (key.clone(), value.clone())) - }) + .filter(|&(key, _)| PERFORMANCE_SCORE_TAGS.contains(key)) + .map(|(key, value)| (key.clone(), value.clone())) .collect::>(), ) } else { From fe54d1eacc9abfb38eb3595e416b15886107aba2 Mon Sep 17 00:00:00 2001 From: Joris Bayer Date: Thu, 7 Dec 2023 16:18:36 +0100 Subject: [PATCH 3/3] fix --- relay-server/src/metrics_extraction/transactions/mod.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/relay-server/src/metrics_extraction/transactions/mod.rs b/relay-server/src/metrics_extraction/transactions/mod.rs index 4a3d3fbd404..6ec44cf320a 100644 --- a/relay-server/src/metrics_extraction/transactions/mod.rs +++ b/relay-server/src/metrics_extraction/transactions/mod.rs @@ -28,12 +28,12 @@ const PLACEHOLDER_UNPARAMETERIZED: &str = "<< unparameterized >>"; /// /// These are a subset of "universal" tags. const PERFORMANCE_SCORE_TAGS: [CommonTag; 6] = [ - CommonTag::Release, + CommonTag::BrowserName, CommonTag::Environment, + CommonTag::GeoCountryCode, + CommonTag::Release, CommonTag::Transaction, CommonTag::TransactionOp, - CommonTag::BrowserName, - CommonTag::TransactionOp, ]; /// Extract HTTP method @@ -692,6 +692,7 @@ mod tests { tags: { "browser.name": "Chrome", "environment": "fake_environment", + "geo.country_code": "US", "release": "1.2.3", "transaction": "gEt /api/:version/users/", "transaction.op": "mYOp", @@ -709,6 +710,7 @@ mod tests { tags: { "browser.name": "Chrome", "environment": "fake_environment", + "geo.country_code": "US", "release": "1.2.3", "transaction": "gEt /api/:version/users/", "transaction.op": "mYOp", @@ -726,6 +728,7 @@ mod tests { tags: { "browser.name": "Chrome", "environment": "fake_environment", + "geo.country_code": "US", "release": "1.2.3", "transaction": "gEt /api/:version/users/", "transaction.op": "mYOp",