feat(metrics): Always extract distinct_id (#1275)

The javascript SDK apparently does not always send a session.distinct_id
with the initial session update, but only on subsequent updates.

To make sure we count those users as well, extract them under the tag
session.status=ok. We will need to update queries in sentry to include
these users.

CHANGELOG.md

@@ -4,7 +4,9 @@
 
 **Compatibility:** This version of Relay requires Sentry server `22.6.0` or newer.
 
-**Features**:
+**Bug Fixes**:
+
+- Session metrics extraction: Count distinct_ids from all session updates to prevent undercounting users. ([#1275](https://github.com/getsentry/relay/pull/1275))
 
 - Session metrics extraction: Count crashed+abnormal towards errored_preaggr. ([#1274](https://github.com/getsentry/relay/pull/1274))
 

relay-server/src/metrics_extraction/sessions.rs

@@ -197,6 +197,25 @@ pub fn extract_session_metrics<T: SessionLike>(
             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).
+            target.push(Metric::new_mri(
+                METRIC_NAMESPACE,
+                "user",
+                MetricUnit::None,
+                MetricValue::set_from_str(distinct_id),
+                timestamp,
+                with_tag(&tags, "session.status", SessionStatus::Ok),
+            ));
+        }
+    }
 }
 
 #[cfg(test)]
@@ -292,8 +311,12 @@ mod tests {
 
         extract_session_metrics(&session.attributes, &session, None, &mut metrics);
 
-        // A none-initial update will not trigger any metric if it's not errored/crashed
-        assert_eq!(metrics.len(), 0);
+        // A none-initial update which is not errored/crashed/abnormal will only emit a user metric.
+        assert_eq!(metrics.len(), 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");
     }
 
     #[test]
@@ -406,14 +429,19 @@ mod tests {
 
         extract_session_metrics(&session.attributes, &session, None, &mut metrics);
 
-        assert_eq!(metrics.len(), 1);
+        assert_eq!(metrics.len(), 2); // duration and user ID
 
         let duration_metric = &metrics[0];
         assert_eq!(duration_metric.name, "d:sessions/duration@second");
         assert!(matches!(
             duration_metric.value,
             MetricValue::Distribution(_)
         ));
+
+        let user_metric = &metrics[1];
+        assert_eq!(user_metric.name, "s:sessions/user@none");
+        assert!(matches!(user_metric.value, MetricValue::Set(_)));
+        assert_eq!(user_metric.tags["session.status"], "ok");
     }
 
     #[test]