diff --git a/CHANGELOG.md b/CHANGELOG.md index cba0d7361d..2b0da4287d 100644 --- a/CHANGELOG.md +++ b/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)) diff --git a/relay-server/src/metrics_extraction/sessions.rs b/relay-server/src/metrics_extraction/sessions.rs index 751563889b..6ff7aad944 100644 --- a/relay-server/src/metrics_extraction/sessions.rs +++ b/relay-server/src/metrics_extraction/sessions.rs @@ -197,6 +197,25 @@ pub fn extract_session_metrics( 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,7 +429,7 @@ 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"); @@ -414,6 +437,11 @@ mod tests { 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]