diff --git a/components/nimbus/src/cirrus.udl b/components/nimbus/src/cirrus.udl index 05bc24e310..49a984b990 100644 --- a/components/nimbus/src/cirrus.udl +++ b/components/nimbus/src/cirrus.udl @@ -12,7 +12,7 @@ enum NimbusError { }; callback interface MetricsHandler { - void record_enrollment_statuses(sequence enrollment_status_extras); + void record_enrollment_statuses_v2(sequence enrollment_status_extras, string? nimbus_user_id); }; dictionary EnrollmentStatusExtraDef { diff --git a/components/nimbus/src/metrics.rs b/components/nimbus/src/metrics.rs index 6a21dc1f04..39fff19036 100644 --- a/components/nimbus/src/metrics.rs +++ b/components/nimbus/src/metrics.rs @@ -9,8 +9,16 @@ use crate::{enrollment::ExperimentEnrollment, EnrolledFeature, EnrollmentStatus} use serde_derive::{Deserialize, Serialize}; pub trait MetricsHandler: Send + Sync { + #[cfg(feature = "stateful")] fn record_enrollment_statuses(&self, enrollment_status_extras: Vec); + #[cfg(not(feature = "stateful"))] + fn record_enrollment_statuses_v2( + &self, + enrollment_status_extras: Vec, + nimbus_user_id: Option, + ); + #[cfg(feature = "stateful")] fn record_feature_activation(&self, event: FeatureExposureExtraDef); diff --git a/components/nimbus/src/stateless/cirrus_client.rs b/components/nimbus/src/stateless/cirrus_client.rs index 33e175f43d..9c1dcbf0fa 100644 --- a/components/nimbus/src/stateless/cirrus_client.rs +++ b/components/nimbus/src/stateless/cirrus_client.rs @@ -149,7 +149,7 @@ impl CirrusClient { prev_enrollments, )?; - self.metrics_handler.record_enrollment_statuses( + self.metrics_handler.record_enrollment_statuses_v2( enrollments .iter() .cloned() @@ -159,6 +159,7 @@ impl CirrusClient { extra }) .collect(), + Some(user_id.clone()), ); let enrolled_feature_config_map = diff --git a/components/nimbus/src/tests/helpers.rs b/components/nimbus/src/tests/helpers.rs index e31a5d689b..57f7d8a46f 100644 --- a/components/nimbus/src/tests/helpers.rs +++ b/components/nimbus/src/tests/helpers.rs @@ -154,6 +154,8 @@ struct MetricState { exposures: Vec, #[cfg(feature = "stateful")] malformeds: Vec, + #[cfg(not(feature = "stateful"))] + nimbus_user_id: Option, } /// A Rust implementation of the MetricsHandler trait @@ -175,6 +177,11 @@ impl TestMetrics { pub fn get_enrollment_statuses(&self) -> Vec { self.state.lock().unwrap().enrollment_statuses.clone() } + + #[cfg(not(feature = "stateful"))] + pub fn get_nimbus_user_id(&self) -> Option { + self.state.lock().unwrap().nimbus_user_id.clone() + } } #[cfg(feature = "stateful")] @@ -196,11 +203,23 @@ impl TestMetrics { } impl MetricsHandler for TestMetrics { + #[cfg(feature = "stateful")] fn record_enrollment_statuses(&self, enrollment_status_extras: Vec) { let mut state = self.state.lock().unwrap(); state.enrollment_statuses.extend(enrollment_status_extras); } + #[cfg(not(feature = "stateful"))] + fn record_enrollment_statuses_v2( + &self, + enrollment_status_extras: Vec, + nimbus_user_id: Option, + ) { + let mut state = self.state.lock().unwrap(); + state.enrollment_statuses.extend(enrollment_status_extras); + state.nimbus_user_id = nimbus_user_id; + } + #[cfg(feature = "stateful")] fn record_feature_activation(&self, event: FeatureExposureExtraDef) { let mut state = self.state.lock().unwrap(); diff --git a/components/nimbus/src/tests/stateless/test_cirrus_client.rs b/components/nimbus/src/tests/stateless/test_cirrus_client.rs index 2652e7fee1..cae5537cff 100644 --- a/components/nimbus/src/tests/stateless/test_cirrus_client.rs +++ b/components/nimbus/src/tests/stateless/test_cirrus_client.rs @@ -205,6 +205,9 @@ fn test_sends_metrics_on_enrollment() -> Result<()> { assert_eq!(metric_records[0].branch(), "treatment"); assert_eq!(metric_records[0].user_id(), "test"); + let nimbus_user_id: Option = metrics_handler.get_nimbus_user_id(); + assert_eq!(nimbus_user_id, Some("test".into())); + Ok(()) } diff --git a/components/nimbus/tests/common/mod.rs b/components/nimbus/tests/common/mod.rs index c0db8282eb..2fb39314d8 100644 --- a/components/nimbus/tests/common/mod.rs +++ b/components/nimbus/tests/common/mod.rs @@ -17,10 +17,16 @@ use nimbus::{ pub struct NoopMetricsHandler; impl MetricsHandler for NoopMetricsHandler { + #[cfg(feature = "stateful")] fn record_enrollment_statuses(&self, _: Vec) { // do nothing } + #[cfg(not(feature = "stateful"))] + fn record_enrollment_statuses_v2(&self, _: Vec, _: Option) { + // do nothing + } + #[cfg(feature = "stateful")] fn record_feature_activation(&self, _activation_event: FeatureExposureExtraDef) { // do nothing diff --git a/megazords/cirrus/tests/python-tests/conftest.py b/megazords/cirrus/tests/python-tests/conftest.py index ba66c821d8..67ffe6cdb1 100644 --- a/megazords/cirrus/tests/python-tests/conftest.py +++ b/megazords/cirrus/tests/python-tests/conftest.py @@ -10,12 +10,14 @@ class TestMetricsHandler(MetricsHandler): recordings = [] + nimbus_user_id = None - def record_enrollment_statuses( - self, enrollment_status_extras: [EnrollmentStatusExtraDef] + def record_enrollment_statuses_v2( + self, enrollment_status_extras: [EnrollmentStatusExtraDef], nimbus_user_id: str ): self.recordings.clear() self.recordings.extend(enrollment_status_extras) + self.nimbus_user_id = nimbus_user_id @pytest.fixture diff --git a/megazords/cirrus/tests/python-tests/test_cirrus.py b/megazords/cirrus/tests/python-tests/test_cirrus.py index 14946a417a..afe9eb9935 100644 --- a/megazords/cirrus/tests/python-tests/test_cirrus.py +++ b/megazords/cirrus/tests/python-tests/test_cirrus.py @@ -60,3 +60,4 @@ def test_metrics_handler(app_context, experiment, req): json.loads(client.handle_enrollment(req())) assert len(test_metrics.recordings) == 1 + assert test_metrics.nimbus_user_id