fix(metrics): Apply global tags to transaction metrics (#3615)

Prerequisite to getsentry/sentry#71004: Global tag mappings were not yet being applied to transaction metrics. Beside the bug fix, this PR also bumps the metrics extraction version(s), and attempts to simplify the extraction function for transactions.
getsentry · May 21, 2024 · 75fc8de · 75fc8de
1 parent 3aa4991
commit 75fc8de
Show file tree

Hide file tree

Showing 7 changed files with 977 additions and 104 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,17 +2,27 @@
 
 ## Unreleased
 
+**Bug fixes**:
+
+- Apply globally defined metric tags to legacy transaction metrics. ([#3615](https://github.com/getsentry/relay/pull/3615))
+
+**Features**:
+
+- Apply legacy inbound filters to standalone spans. ([#3552](https://github.com/getsentry/relay/pull/3552))
+
 **Internal**:
 
+- Send microsecond precision timestamps. ([#3613](https://github.com/getsentry/relay/pull/3613))
 - Map outcome reasons for dynamic sampling to reduced set of values. ([#3623](https://github.com/getsentry/relay/pull/3623))
+- Extract status for spans. ([#3606](https://github.com/getsentry/relay/pull/3606))
 
 ## 24.5.0
 
 **Breaking Changes**:
 
 - Remove the AWS lambda extension. ([#3568](https://github.com/getsentry/relay/pull/3568))
 
-**Bug fixes:**
+**Bug fixes**:
 
 - Properly handle AI metrics from the Python SDK's `@ai_track` decorator. ([#3539](https://github.com/getsentry/relay/pull/3539))
 - Mitigate occasional slowness and timeouts of the healthcheck endpoint. The endpoint will now respond promptly an unhealthy state. ([#3567](https://github.com/getsentry/relay/pull/3567))
@@ -22,10 +32,6 @@
 - Apple trace-based sampling rules to standalone spans. ([#3476](https://github.com/getsentry/relay/pull/3476))
 - Localhost inbound filter filters sudomains of localhost. ([#3608](https://github.com/getsentry/relay/pull/3608))
 
-**Features**:
-
-- Apply legacy inbound filters to standalone spans. ([#3552](https://github.com/getsentry/relay/pull/3552))
-
 **Internal**:
 
 - Add metrics extraction config to global config. ([#3490](https://github.com/getsentry/relay/pull/3490), [#3504](https://github.com/getsentry/relay/pull/3504))
@@ -43,18 +49,16 @@
 - Add a calculated measurement based on the AI model and the tokens used. ([#3554](https://github.com/getsentry/relay/pull/3554))
 - Restrict usage of OTel endpoint. ([#3597](github.com/getsentry/relay/pull/3597))
 - Support new cache span ops in metrics and tag extraction. ([#3598](https://github.com/getsentry/relay/pull/3598))
-- Send microsecond precision timestamps. ([#3613](https://github.com/getsentry/relay/pull/3613))
 - Extract additional user fields for spans. ([#3599](https://github.com/getsentry/relay/pull/3599))
 - Disable `db.redis` span metrics extraction. ([#3600](https://github.com/getsentry/relay/pull/3600))
-- Extract status for spans. ([#3606](https://github.com/getsentry/relay/pull/3606))
 
 ## 24.4.2
 
 **Breaking Changes**:
 
 - Stop supporting dynamic sampling mode `"total"`, which adjusted for the client sample rate. ([#3474](https://github.com/getsentry/relay/pull/3474))
 
-**Bug fixes:**
+**Bug fixes**:
 
 - Respect country code TLDs when scrubbing span tags. ([#3458](https://github.com/getsentry/relay/pull/3458))
 - Extract HTTP status code from span data when sent as integers. ([#3491](https://github.com/getsentry/relay/pull/3491))

diff --git a/relay-dynamic-config/src/metrics.rs b/relay-dynamic-config/src/metrics.rs
@@ -115,7 +115,8 @@ pub struct CustomMeasurementConfig {
 ///      - Delay metrics extraction for indexed transactions.
 ///  - 4: Adds support for `RuleConfigs` with string comparisons.
 ///  - 5: No change, bumped together with [`MetricExtractionConfig::MAX_SUPPORTED_VERSION`].
-const TRANSACTION_EXTRACT_MAX_SUPPORTED_VERSION: u16 = 5;
+///  - 6: Bugfix to make transaction metrics extraction apply globally defined tag mappings.
+const TRANSACTION_EXTRACT_MAX_SUPPORTED_VERSION: u16 = 6;
 
 /// Deprecated. Defines whether URL transactions should be considered low cardinality.
 #[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq)]
@@ -324,7 +325,7 @@ impl MetricExtractionConfig {
     /// The latest version for this config struct.
     ///
     /// This is the maximum version supported by this Relay instance.
-    pub const MAX_SUPPORTED_VERSION: u16 = 3;
+    pub const MAX_SUPPORTED_VERSION: u16 = 4;
 
     /// Returns an empty `MetricExtractionConfig` with the latest version.
     ///

diff --git a/relay-server/src/metrics_extraction/transactions/mod.rs b/relay-server/src/metrics_extraction/transactions/mod.rs
@@ -2,7 +2,7 @@ use std::collections::{BTreeMap, BTreeSet};
 
 use relay_base_schema::events::EventType;
 use relay_common::time::UnixTimestamp;
-use relay_dynamic_config::{TagMapping, TransactionMetricsConfig};
+use relay_dynamic_config::{CombinedMetricExtractionConfig, TransactionMetricsConfig};
 use relay_event_normalization::utils as normalize_utils;
 use relay_event_schema::protocol::{
     AsPair, BrowserContext, Event, OsContext, PerformanceScoreContext, TraceContext,
@@ -247,7 +247,7 @@ impl ExtractedMetrics {
 /// A utility that extracts metrics from transactions.
 pub struct TransactionExtractor<'a> {
     pub config: &'a TransactionMetricsConfig,
-    pub generic_tags: &'a [TagMapping],
+    pub generic_config: Option<&'a CombinedMetricExtractionConfig<'a>>,
     pub transaction_from_dsc: Option<&'a str>,
     pub sampling_result: &'a SamplingResult,
     pub has_profile: bool,
@@ -469,8 +469,10 @@ impl TransactionExtractor<'_> {
 
         // Apply shared tags from generic metric extraction. Transaction metrics will adopt generic
         // metric extraction, after which this is done automatically.
-        generic::tmp_apply_tags(&mut metrics.project_metrics, event, self.generic_tags);
-        generic::tmp_apply_tags(&mut metrics.sampling_metrics, event, self.generic_tags);
+        if let Some(generic_config) = self.generic_config {
+            generic::tmp_apply_tags(&mut metrics.project_metrics, event, generic_config.tags());
+            generic::tmp_apply_tags(&mut metrics.sampling_metrics, event, generic_config.tags());
+        }
 
         Ok(metrics)
     }
@@ -500,7 +502,9 @@ fn get_measurement_rating(name: &str, value: f64) -> Option<String> {
 
 #[cfg(test)]
 mod tests {
-    use relay_dynamic_config::AcceptTransactionNames;
+    use relay_dynamic_config::{
+        AcceptTransactionNames, CombinedMetricExtractionConfig, MetricExtractionConfig, TagMapping,
+    };
     use relay_event_normalization::{
         normalize_event, set_default_transaction_source, validate_event_timestamps,
         validate_transaction, BreakdownsConfig, CombinedMeasurementsConfig, EventValidationConfig,
@@ -620,7 +624,7 @@ mod tests {
 
         let extractor = TransactionExtractor {
             config: &config,
-            generic_tags: &[],
+            generic_config: None,
             transaction_from_dsc: Some("test_transaction"),
             sampling_result: &SamplingResult::Pending,
             has_profile: false,
@@ -981,7 +985,7 @@ mod tests {
         let config = TransactionMetricsConfig::default();
         let extractor = TransactionExtractor {
             config: &config,
-            generic_tags: &[],
+            generic_config: None,
             transaction_from_dsc: Some("test_transaction"),
             sampling_result: &SamplingResult::Pending,
             has_profile: false,
@@ -1150,7 +1154,7 @@ mod tests {
         let config: TransactionMetricsConfig = TransactionMetricsConfig::default();
         let extractor = TransactionExtractor {
             config: &config,
-            generic_tags: &[],
+            generic_config: None,
             transaction_from_dsc: Some("test_transaction"),
             sampling_result: &SamplingResult::Pending,
             has_profile: false,
@@ -1296,7 +1300,7 @@ mod tests {
         let config = TransactionMetricsConfig::default();
         let extractor = TransactionExtractor {
             config: &config,
-            generic_tags: &[],
+            generic_config: None,
             transaction_from_dsc: Some("test_transaction"),
             sampling_result: &SamplingResult::Pending,
             has_profile: false,
@@ -1371,7 +1375,7 @@ mod tests {
         let config = TransactionMetricsConfig::default();
         let extractor = TransactionExtractor {
             config: &config,
-            generic_tags: &[],
+            generic_config: None,
             transaction_from_dsc: Some("test_transaction"),
             sampling_result: &SamplingResult::Pending,
             has_profile: false,
@@ -1530,7 +1534,7 @@ mod tests {
         let config = TransactionMetricsConfig::default();
         let extractor = TransactionExtractor {
             config: &config,
-            generic_tags: &[],
+            generic_config: None,
             transaction_from_dsc: Some("test_transaction"),
             sampling_result: &SamplingResult::Pending,
             has_profile: false,
@@ -1569,7 +1573,7 @@ mod tests {
         let config = TransactionMetricsConfig::default();
         let extractor = TransactionExtractor {
             config: &config,
-            generic_tags: &[],
+            generic_config: None,
             transaction_from_dsc: Some("test_transaction"),
             sampling_result: &SamplingResult::Pending,
             has_profile: false,
@@ -1637,7 +1641,7 @@ mod tests {
         let config = TransactionMetricsConfig::default();
         let extractor = TransactionExtractor {
             config: &config,
-            generic_tags: &[],
+            generic_config: None,
             transaction_from_dsc: Some("test_transaction"),
             sampling_result: &SamplingResult::Pending,
             has_profile: false,
@@ -1740,7 +1744,7 @@ mod tests {
         let config = TransactionMetricsConfig::default();
         let extractor = TransactionExtractor {
             config: &config,
-            generic_tags: &[],
+            generic_config: None,
             transaction_from_dsc: Some("test_transaction"),
             sampling_result: &SamplingResult::Pending,
             has_profile: false,
@@ -1773,7 +1777,7 @@ mod tests {
         let config = TransactionMetricsConfig::default();
         let extractor = TransactionExtractor {
             config: &config,
-            generic_tags: &[],
+            generic_config: None,
             transaction_from_dsc: Some("test_transaction"),
             sampling_result: &SamplingResult::Pending,
             has_profile: false,
@@ -1810,7 +1814,7 @@ mod tests {
         let config = TransactionMetricsConfig::default();
         let extractor = TransactionExtractor {
             config: &config,
-            generic_tags: &[],
+            generic_config: None,
             transaction_from_dsc: Some("root_transaction"),
             sampling_result: &SamplingResult::Pending,
             has_profile: false,
@@ -2078,7 +2082,7 @@ mod tests {
         let config = TransactionMetricsConfig::default();
         let extractor = TransactionExtractor {
             config: &config,
-            generic_tags: &[],
+            generic_config: None,
             transaction_from_dsc: Some("test_transaction"),
             sampling_result: &SamplingResult::Pending,
             has_profile: false,
@@ -2170,10 +2174,16 @@ mod tests {
             ]"#,
         )
         .unwrap();
+        let generic_config = MetricExtractionConfig {
+            version: 1,
+            tags: generic_tags,
+            ..Default::default()
+        };
+        let combined_config = CombinedMetricExtractionConfig::from(&generic_config);
 
         let extractor = TransactionExtractor {
             config: &config,
-            generic_tags: &generic_tags,
+            generic_config: Some(&combined_config),
             transaction_from_dsc: Some("test_transaction"),
             sampling_result: &SamplingResult::Pending,
             has_profile: false,