ref(metrics): Fewer metrics extracted for release health (#1316)

Since #1275, we extract user IDs into one or more of init, ok, errored,
crashed, abnormal buckets. The latter three are used by the product, but
init and ok can be consolidated into a single tag, because the product
does not query them explicitily as of getsentry/sentry#34858 and
getsentry/sentry#34957.

In order to reduce the number of buckets stored in clickhouse,
consolidate these tags.

Also, stop collecting session.duration for non-healthy sessions.

CHANGELOG.md

@@ -9,6 +9,7 @@
 **Internal**:
 
 - Fall back to version 2 project config if version 3 fails. ([#1314](https://github.com/getsentry/relay/pull/1314))
+- Reduce number of metrics extracted for release health. ([#1316](https://github.com/getsentry/relay/pull/1316))
 
 ## 22.6.0
 

relay-server/src/metrics_extraction/sessions.rs

@@ -92,17 +92,6 @@ pub fn extract_session_metrics<T: SessionLike>(
             timestamp,
             with_tag(&tags, "session.status", "init"),
         ));
-
-        if let Some(distinct_id) = nil_to_none(session.distinct_id()) {
-            target.push(Metric::new_mri(
-                METRIC_NAMESPACE,
-                "user",
-                MetricUnit::None,
-                MetricValue::set_from_str(distinct_id),
-                timestamp,
-                with_tag(&tags, "session.status", "init"),
-            ));
-        }
     }
 
     // Mark the session as errored, which includes fatal sessions.
@@ -136,6 +125,18 @@ pub fn extract_session_metrics<T: SessionLike>(
                 with_tag(&tags, "session.status", "errored"),
             ));
         }
+    } else if let Some(distinct_id) = nil_to_none(session.distinct_id()) {
+        // For session updates without errors, we collect the user without a session.status tag.
+        // To get the number of healthy users (i.e. users without a single errored session), query
+        // |users| - |users{session.status:errored}|
+        target.push(Metric::new_mri(
+            METRIC_NAMESPACE,
+            "user",
+            MetricUnit::None,
+            MetricValue::set_from_str(distinct_id),
+            timestamp,
+            tags.clone(),
+        ));
     }
 
     // Record fatal sessions for crash rate computation. This is a strict subset of errored
@@ -184,35 +185,16 @@ pub fn extract_session_metrics<T: SessionLike>(
         }
     }
 
-    // Count durations for all exited/crashed sessions. Note that right now, in the product we
-    // really only use durations from session.status=exited, but decided it may be worth ingesting
-    // this data in case we need it. If we need to cut cost, this is one place to start though.
+    // Count durations only for exited sessions, since Sentry doesn't use durations for other types of sessions.
     if let Some((duration, status)) = session.final_duration() {
-        target.push(Metric::new_mri(
-            METRIC_NAMESPACE,
-            "duration",
-            MetricUnit::Duration(DurationUnit::Second),
-            MetricValue::Distribution(duration),
-            timestamp,
-            with_tag(&tags, "session.status", status),
-        ));
-    }
-
-    // Some SDKs only provide user information on subsequent updates (not when init==true), so
-    // collect the user ID here as well.
-    // We could also add the user ID to the "init" tag, but collecting it under a separate "ok" tag
-    // might help us with validation / debugging.
-    if let Some(distinct_id) = nil_to_none(session.distinct_id()) {
-        if session.total_count() == 0 && session.all_errors().is_none() {
-            // Assuming that aggregate items always have a total_count > 0,
-            // this is a session update to a healthy, individual session (init == false).
+        if status == SessionStatus::Exited {
             target.push(Metric::new_mri(
                 METRIC_NAMESPACE,
-                "user",
-                MetricUnit::None,
-                MetricValue::set_from_str(distinct_id),
+                "duration",
+                MetricUnit::Duration(DurationUnit::Second),
+                MetricValue::Distribution(duration),
                 timestamp,
-                with_tag(&tags, "session.status", SessionStatus::Ok),
+                with_tag(&tags, "session.status", status),
             ));
         }
     }
@@ -287,7 +269,7 @@ mod tests {
         assert_eq!(user_metric.timestamp, started());
         assert_eq!(user_metric.name, "s:sessions/user@none");
         assert!(matches!(user_metric.value, MetricValue::Set(_)));
-        assert_eq!(user_metric.tags["session.status"], "init");
+        assert!(!user_metric.tags.contains_key("session.status"));
         assert_eq!(user_metric.tags["release"], "1.0.0");
         assert_eq!(user_metric.tags["sdk"], client);
     }
@@ -316,7 +298,7 @@ mod tests {
         let user_metric = &metrics[0];
         assert_eq!(user_metric.name, "s:sessions/user@none");
         assert!(matches!(user_metric.value, MetricValue::Set(_)));
-        assert_eq!(user_metric.tags["session.status"], "ok");
+        assert!(!user_metric.tags.contains_key("session.status"));
     }
 
     #[test]
@@ -343,7 +325,7 @@ mod tests {
         update3.errors = 123;
 
         for (update, expected_metrics) in vec![
-            (update1, 4), // init == true, so expect 4 metrics
+            (update1, 3), // init == true, so expect 3 metrics
             (update2, 2),
             (update3, 2),
         ] {
@@ -431,17 +413,17 @@ mod tests {
 
         assert_eq!(metrics.len(), 2); // duration and user ID
 
-        let duration_metric = &metrics[0];
+        let duration_metric = &metrics[1];
         assert_eq!(duration_metric.name, "d:sessions/duration@second");
         assert!(matches!(
             duration_metric.value,
             MetricValue::Distribution(_)
         ));
 
-        let user_metric = &metrics[1];
+        let user_metric = &metrics[0];
         assert_eq!(user_metric.name, "s:sessions/user@none");
         assert!(matches!(user_metric.value, MetricValue::Set(_)));
-        assert_eq!(user_metric.tags["session.status"], "ok");
+        assert!(!user_metric.tags.contains_key("session.status"));
     }
 
     #[test]
@@ -545,19 +527,6 @@ mod tests {
                     "session.status": "init",
                 },
             },
-            Metric {
-                name: "s:sessions/user@none",
-                value: Set(
-                    3097475539,
-                ),
-                timestamp: UnixTimestamp(1581084961),
-                tags: {
-                    "environment": "development",
-                    "release": "my-project-name@1.0.0",
-                    "sdk": "sentry-test/1.0",
-                    "session.status": "init",
-                },
-            },
             Metric {
                 name: "c:sessions/session@none",
                 value: Counter(

tests/integration/test_metrics.py

@@ -277,7 +277,6 @@ def test_session_metrics_non_processing(
                     "sdk": "raven-node/2.6.3",
                     "environment": "production",
                     "release": "sentry-test@1.0.0",
-                    "session.status": "init",
                 },
                 "timestamp": ts,
                 "width": 1,
@@ -403,7 +402,6 @@ def test_session_metrics_processing(
             "sdk": "raven-node/2.6.3",
             "environment": "production",
             "release": "sentry-test@1.0.0",
-            "session.status": "init",
         },
     }