feat: Extract normalized dist as metric (#1158)

Apply normalization to event.dist and add as tag to every transaction
metric.

CHANGELOG.md

@@ -26,6 +26,7 @@
 - Extract session metrics from aggregate sessions. ([#1140](https://github.com/getsentry/relay/pull/1140))
 - Prefix names of extracted metrics by `sentry.sessions.` or `sentry.transactions.`. ([#1147](https://github.com/getsentry/relay/pull/1147))
 - Extract transaction duration as metric. ([#1148](https://github.com/getsentry/relay/pull/1148))
+- Extract normalized dist as metric. ([#1158](https://github.com/getsentry/relay/pull/1158))
 
 ## 21.11.0
 

relay-general/src/store/mod.rs

@@ -23,6 +23,7 @@ mod trimming;
 pub use self::clock_drift::ClockDriftProcessor;
 pub use self::geo::{GeoIpError, GeoIpLookup};
 pub use normalize::breakdowns::BreakdownsConfig;
+pub use normalize::normalize_dist;
 
 /// The config for store.
 #[derive(Serialize, Deserialize, Debug, Default)]

relay-general/src/store/normalize.rs

@@ -66,6 +66,22 @@ fn is_valid_platform(platform: &str) -> bool {
     VALID_PLATFORMS.contains(&platform)
 }
 
+pub fn normalize_dist(dist: &mut Option<String>) {
+    let mut erase = false;
+    if let Some(val) = dist {
+        if val.is_empty() {
+            erase = true;
+        }
+        let trimmed = val.trim();
+        if trimmed != val {
+            *val = trimmed.to_string()
+        }
+    }
+    if erase {
+        *dist = None;
+    }
+}
+
 /// The processor that normalizes events for store.
 pub struct NormalizeProcessor<'a> {
     config: Arc<StoreConfig>,
@@ -120,15 +136,7 @@ impl<'a> NormalizeProcessor<'a> {
 
     /// Ensures that the `release` and `dist` fields match up.
     fn normalize_release_dist(&self, event: &mut Event) {
-        if event.dist.value().is_some() && event.release.value().is_empty() {
-            event.dist.set_value(None);
-        }
-
-        if let Some(dist) = event.dist.value_mut() {
-            if dist.trim() != dist {
-                *dist = dist.trim().to_string();
-            }
-        }
+        normalize_dist(event.dist.value_mut());
     }
 
     /// Validates the timestamp range and sets a default value.
@@ -1600,3 +1608,31 @@ fn test_past_timestamp() {
     }
     "###);
 }
+
+#[test]
+fn test_normalize_dist_none() {
+    let mut dist = None;
+    normalize_dist(&mut dist);
+    assert_eq!(dist, None);
+}
+
+#[test]
+fn test_normalize_dist_empty() {
+    let mut dist = Some("".to_owned());
+    normalize_dist(&mut dist);
+    assert_eq!(dist, None);
+}
+
+#[test]
+fn test_normalize_dist_trim() {
+    let mut dist = Some(" foo  ".to_owned());
+    normalize_dist(&mut dist);
+    assert_eq!(dist.unwrap(), "foo");
+}
+
+#[test]
+fn test_normalize_dist_whitespace() {
+    let mut dist = Some(" ".to_owned());
+    normalize_dist(&mut dist);
+    assert_eq!(dist.unwrap(), ""); // Not sure if this is what we want
+}

relay-server/src/metrics_extraction/transactions.rs

@@ -5,6 +5,7 @@ use std::collections::BTreeSet;
 use {
     relay_common::UnixTimestamp,
     relay_general::protocol::{AsPair, Event, EventType},
+    relay_general::store::normalize_dist,
     relay_metrics::{Metric, MetricUnit, MetricValue},
     std::collections::BTreeMap,
     std::fmt::Write,
@@ -34,7 +35,7 @@ fn metric_name(parts: &[&str]) -> String {
 }
 
 #[cfg(feature = "processing")]
-fn transaction_status(transaction: &Event) -> Option<String> {
+fn extract_transaction_status(transaction: &Event) -> Option<String> {
     use relay_general::{
         protocol::{Context, ContextInner},
         types::Annotated,
@@ -49,6 +50,13 @@ fn transaction_status(transaction: &Event) -> Option<String> {
     Some(span_status.to_string())
 }
 
+#[cfg(feature = "processing")]
+fn extract_dist(transaction: &Event) -> Option<String> {
+    let mut dist = transaction.dist.0.clone();
+    normalize_dist(&mut dist);
+    dist
+}
+
 #[cfg(feature = "processing")]
 pub fn extract_transaction_metrics(
     config: &TransactionMetricsConfig,
@@ -90,6 +98,9 @@ pub fn extract_transaction_metrics(
     if let Some(release) = event.release.as_str() {
         tags.insert("release".to_owned(), release.to_owned());
     }
+    if let Some(dist) = extract_dist(event) {
+        tags.insert("dist".to_owned(), dist);
+    }
     if let Some(environment) = event.environment.as_str() {
         tags.insert("environment".to_owned(), environment.to_owned());
     }
@@ -182,7 +193,7 @@ pub fn extract_transaction_metrics(
         unit: MetricUnit::Duration(DurationPrecision::MilliSecond),
         value: MetricValue::Distribution(duration_millis as f64),
         timestamp,
-        tags: match transaction_status(event) {
+        tags: match extract_transaction_status(event) {
             Some(status) => with_tag(&tags, "transaction.status", status),
             None => tags.clone(),
         },
@@ -225,6 +236,7 @@ mod tests {
             "type": "transaction",
             "timestamp": "2021-04-26T08:00:00+0100",
             "release": "1.2.3",
+            "dist": "foo ",
             "environment": "fake_environment",
             "transaction": "mytransaction",
             "tags": {
@@ -311,6 +323,7 @@ mod tests {
         for metric in metrics {
             assert!(matches!(metric.value, MetricValue::Distribution(_)));
             assert_eq!(metric.tags["release"], "1.2.3");
+            assert_eq!(metric.tags["dist"], "foo");
             assert_eq!(metric.tags["environment"], "fake_environment");
             assert_eq!(metric.tags["transaction"], "mytransaction");
             assert_eq!(metric.tags["fOO"], "bar");
@@ -369,6 +382,7 @@ mod tests {
         }
 
         assert_eq!(duration_metric.tags.len(), 4);
+        assert_eq!(duration_metric.tags["release"], "1.2.3");
         assert_eq!(duration_metric.tags["transaction.status"], "ok");
         assert_eq!(duration_metric.tags["environment"], "fake_environment");
         assert_eq!(duration_metric.tags["transaction"], "mytransaction");