Skip to content

Commit 4cab4de

Browse files
committed
feat(cirrus): switch to record_enrollment_statuses_v2
1 parent b665841 commit 4cab4de

File tree

8 files changed

+44
-4
lines changed

8 files changed

+44
-4
lines changed

components/nimbus/src/cirrus.udl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ enum NimbusError {
1212
};
1313

1414
callback interface MetricsHandler {
15-
void record_enrollment_statuses(sequence<EnrollmentStatusExtraDef> enrollment_status_extras);
15+
void record_enrollment_statuses_v2(sequence<EnrollmentStatusExtraDef> enrollment_status_extras, string? nimbus_user_id);
1616
};
1717

1818
dictionary EnrollmentStatusExtraDef {

components/nimbus/src/metrics.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,16 @@ use crate::{enrollment::ExperimentEnrollment, EnrolledFeature, EnrollmentStatus}
99
use serde_derive::{Deserialize, Serialize};
1010

1111
pub trait MetricsHandler: Send + Sync {
12+
#[cfg(feature = "stateful")]
1213
fn record_enrollment_statuses(&self, enrollment_status_extras: Vec<EnrollmentStatusExtraDef>);
1314

15+
#[cfg(not(feature = "stateful"))]
16+
fn record_enrollment_statuses_v2(
17+
&self,
18+
enrollment_status_extras: Vec<EnrollmentStatusExtraDef>,
19+
nimbus_user_id: Option<String>,
20+
);
21+
1422
#[cfg(feature = "stateful")]
1523
fn record_feature_activation(&self, event: FeatureExposureExtraDef);
1624

components/nimbus/src/stateless/cirrus_client.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ impl CirrusClient {
149149
prev_enrollments,
150150
)?;
151151

152-
self.metrics_handler.record_enrollment_statuses(
152+
self.metrics_handler.record_enrollment_statuses_v2(
153153
enrollments
154154
.iter()
155155
.cloned()
@@ -159,6 +159,7 @@ impl CirrusClient {
159159
extra
160160
})
161161
.collect(),
162+
Some(user_id.clone()),
162163
);
163164

164165
let enrolled_feature_config_map =

components/nimbus/src/tests/helpers.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,8 @@ struct MetricState {
154154
exposures: Vec<FeatureExposureExtraDef>,
155155
#[cfg(feature = "stateful")]
156156
malformeds: Vec<MalformedFeatureConfigExtraDef>,
157+
#[cfg(not(feature = "stateful"))]
158+
nimbus_user_id: Option<String>,
157159
}
158160

159161
/// A Rust implementation of the MetricsHandler trait
@@ -175,6 +177,11 @@ impl TestMetrics {
175177
pub fn get_enrollment_statuses(&self) -> Vec<EnrollmentStatusExtraDef> {
176178
self.state.lock().unwrap().enrollment_statuses.clone()
177179
}
180+
181+
#[cfg(not(feature = "stateful"))]
182+
pub fn get_nimbus_user_id(&self) -> Option<String> {
183+
self.state.lock().unwrap().nimbus_user_id.clone()
184+
}
178185
}
179186

180187
#[cfg(feature = "stateful")]
@@ -196,11 +203,23 @@ impl TestMetrics {
196203
}
197204

198205
impl MetricsHandler for TestMetrics {
206+
#[cfg(feature = "stateful")]
199207
fn record_enrollment_statuses(&self, enrollment_status_extras: Vec<EnrollmentStatusExtraDef>) {
200208
let mut state = self.state.lock().unwrap();
201209
state.enrollment_statuses.extend(enrollment_status_extras);
202210
}
203211

212+
#[cfg(not(feature = "stateful"))]
213+
fn record_enrollment_statuses_v2(
214+
&self,
215+
enrollment_status_extras: Vec<EnrollmentStatusExtraDef>,
216+
nimbus_user_id: Option<String>,
217+
) {
218+
let mut state = self.state.lock().unwrap();
219+
state.enrollment_statuses.extend(enrollment_status_extras);
220+
state.nimbus_user_id = nimbus_user_id;
221+
}
222+
204223
#[cfg(feature = "stateful")]
205224
fn record_feature_activation(&self, event: FeatureExposureExtraDef) {
206225
let mut state = self.state.lock().unwrap();

components/nimbus/src/tests/stateless/test_cirrus_client.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,9 @@ fn test_sends_metrics_on_enrollment() -> Result<()> {
205205
assert_eq!(metric_records[0].branch(), "treatment");
206206
assert_eq!(metric_records[0].user_id(), "test");
207207

208+
let nimbus_user_id: Option<String> = metrics_handler.get_nimbus_user_id();
209+
assert_eq!(nimbus_user_id, "test");
210+
208211
Ok(())
209212
}
210213

components/nimbus/tests/common/mod.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,16 @@ use nimbus::{
1717
pub struct NoopMetricsHandler;
1818

1919
impl MetricsHandler for NoopMetricsHandler {
20+
#[cfg(feature = "stateful")]
2021
fn record_enrollment_statuses(&self, _: Vec<EnrollmentStatusExtraDef>) {
2122
// do nothing
2223
}
2324

25+
#[cfg(not(feature = "stateful"))]
26+
fn record_enrollment_statuses_v2(&self, _: Vec<EnrollmentStatusExtraDef>, _: Option<String>) {
27+
// do nothing
28+
}
29+
2430
#[cfg(feature = "stateful")]
2531
fn record_feature_activation(&self, _activation_event: FeatureExposureExtraDef) {
2632
// do nothing

megazords/cirrus/tests/python-tests/conftest.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,14 @@
1010

1111
class TestMetricsHandler(MetricsHandler):
1212
recordings = []
13+
nimbus_user_id = None
1314

14-
def record_enrollment_statuses(
15-
self, enrollment_status_extras: [EnrollmentStatusExtraDef]
15+
def record_enrollment_statuses_v2(
16+
self, enrollment_status_extras: [EnrollmentStatusExtraDef], nimbus_user_id: str | None
1617
):
1718
self.recordings.clear()
1819
self.recordings.extend(enrollment_status_extras)
20+
self.nimbus_user_id = nimbus_user_id
1921

2022

2123
@pytest.fixture

megazords/cirrus/tests/python-tests/test_cirrus.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,3 +60,4 @@ def test_metrics_handler(app_context, experiment, req):
6060
json.loads(client.handle_enrollment(req()))
6161

6262
assert len(test_metrics.recordings) == 1
63+
assert test_metrics.nimbus_user_id

0 commit comments

Comments
 (0)