Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion components/nimbus/src/cirrus.udl
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ enum NimbusError {
};

callback interface MetricsHandler {
void record_enrollment_statuses(sequence<EnrollmentStatusExtraDef> enrollment_status_extras);
void record_enrollment_statuses_v2(sequence<EnrollmentStatusExtraDef> enrollment_status_extras, string? nimbus_user_id);
};

dictionary EnrollmentStatusExtraDef {
Expand Down
8 changes: 8 additions & 0 deletions components/nimbus/src/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<EnrollmentStatusExtraDef>);

#[cfg(not(feature = "stateful"))]
fn record_enrollment_statuses_v2(
&self,
enrollment_status_extras: Vec<EnrollmentStatusExtraDef>,
nimbus_user_id: Option<String>,
);

#[cfg(feature = "stateful")]
fn record_feature_activation(&self, event: FeatureExposureExtraDef);

Expand Down
3 changes: 2 additions & 1 deletion components/nimbus/src/stateless/cirrus_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ impl CirrusClient {
prev_enrollments,
)?;

self.metrics_handler.record_enrollment_statuses(
self.metrics_handler.record_enrollment_statuses_v2(
enrollments
.iter()
.cloned()
Expand All @@ -159,6 +159,7 @@ impl CirrusClient {
extra
})
.collect(),
Some(user_id.clone()),
);

let enrolled_feature_config_map =
Expand Down
19 changes: 19 additions & 0 deletions components/nimbus/src/tests/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,8 @@ struct MetricState {
exposures: Vec<FeatureExposureExtraDef>,
#[cfg(feature = "stateful")]
malformeds: Vec<MalformedFeatureConfigExtraDef>,
#[cfg(not(feature = "stateful"))]
nimbus_user_id: Option<String>,
}

/// A Rust implementation of the MetricsHandler trait
Expand All @@ -175,6 +177,11 @@ impl TestMetrics {
pub fn get_enrollment_statuses(&self) -> Vec<EnrollmentStatusExtraDef> {
self.state.lock().unwrap().enrollment_statuses.clone()
}

#[cfg(not(feature = "stateful"))]
pub fn get_nimbus_user_id(&self) -> Option<String> {
self.state.lock().unwrap().nimbus_user_id.clone()
}
}

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

impl MetricsHandler for TestMetrics {
#[cfg(feature = "stateful")]
fn record_enrollment_statuses(&self, enrollment_status_extras: Vec<EnrollmentStatusExtraDef>) {
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<EnrollmentStatusExtraDef>,
nimbus_user_id: Option<String>,
) {
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();
Expand Down
3 changes: 3 additions & 0 deletions components/nimbus/src/tests/stateless/test_cirrus_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> = metrics_handler.get_nimbus_user_id();
assert_eq!(nimbus_user_id, Some("test".into()));

Ok(())
}

Expand Down
6 changes: 6 additions & 0 deletions components/nimbus/tests/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,16 @@ use nimbus::{
pub struct NoopMetricsHandler;

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

#[cfg(not(feature = "stateful"))]
fn record_enrollment_statuses_v2(&self, _: Vec<EnrollmentStatusExtraDef>, _: Option<String>) {
// do nothing
}

#[cfg(feature = "stateful")]
fn record_feature_activation(&self, _activation_event: FeatureExposureExtraDef) {
// do nothing
Expand Down
6 changes: 4 additions & 2 deletions megazords/cirrus/tests/python-tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions megazords/cirrus/tests/python-tests/test_cirrus.py
Original file line number Diff line number Diff line change
Expand Up @@ -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