Skip to content

Commit

Permalink
update record_enrollment_status_telemetry to only send metrics for av…
Browse files Browse the repository at this point in the history
…ailable experiments
  • Loading branch information
jeddai authored and rvandermeulen committed Nov 29, 2023
1 parent 2554832 commit 0f82c29
Show file tree
Hide file tree
Showing 2 changed files with 109 additions and 3 deletions.
18 changes: 15 additions & 3 deletions components/nimbus/src/stateful/nimbus_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ impl NimbusClient {
.collect();
self.database_cache
.commit_and_update(db, writer, &coenrolling_ids)?;
self.record_enrollment_status_telemetry()?;
self.record_enrollment_status_telemetry(state)?;
Ok(())
}

Expand Down Expand Up @@ -691,12 +691,24 @@ impl NimbusClient {
Ok(())
}

fn record_enrollment_status_telemetry(&self) -> Result<()> {
fn record_enrollment_status_telemetry(
&self,
state: &mut MutexGuard<InternalMutableState>,
) -> Result<()> {
let targeting_helper = NimbusTargetingHelper::new(
state.targeting_attributes.clone(),
self.event_store.clone(),
);
let experiments = self
.database_cache
.get_experiments()?
.iter()
.map(|exp| exp.slug.clone())
.filter_map(
|exp| match is_experiment_available(&targeting_helper, exp, true) {
true => Some(exp.slug.clone()),
false => None,
},
)
.collect::<HashSet<String>>();
self.metrics_handler.record_enrollment_statuses(
self.database_cache
Expand Down
94 changes: 94 additions & 0 deletions components/nimbus/src/tests/stateful/test_nimbus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1423,3 +1423,97 @@ fn test_enrollment_status_metrics_recorded() -> Result<()> {

Ok(())
}

#[test]
fn test_enrollment_status_metrics_not_recorded_app_name_mismatch() -> Result<()> {
let metrics = TestMetrics::new();
let mock_client_id = "client-1".to_string();

let temp_dir = tempfile::tempdir()?;

let app_context = AppContext {
app_name: "not-fenix".to_string(),
app_id: "org.mozilla.fenix".to_string(),
channel: "nightly".to_string(),
..Default::default()
};

let mut client = NimbusClient::new(
app_context.clone(),
Default::default(),
temp_dir.path(),
None,
AvailableRandomizationUnits {
client_id: Some(mock_client_id),
..AvailableRandomizationUnits::default()
},
Box::new(metrics.clone()),
)?;
client.set_nimbus_id(&Uuid::from_str("53baafb3-b800-42ac-878c-c3451e250928")?)?;

let targeting_attributes = TargetingAttributes {
app_context,
..Default::default()
};
client.with_targeting_attributes(targeting_attributes);
client.initialize()?;

let slug_1 = "experiment-1";
let exp_1 = get_targeted_experiment(slug_1, "true");
client.set_experiments_locally(to_local_experiments_string(&[exp_1])?)?;

client.apply_pending_experiments()?;

let metric_records: Vec<EnrollmentStatusExtraDef> =
serde_json::from_value(metrics.assert_get_vec_value("enrollment_status"))?;
assert_eq!(metric_records.len(), 0);

Ok(())
}

#[test]
fn test_enrollment_status_metrics_not_recorded_channel_mismatch() -> Result<()> {
let metrics = TestMetrics::new();
let mock_client_id = "client-1".to_string();

let temp_dir = tempfile::tempdir()?;

let app_context = AppContext {
app_name: "fenix".to_string(),
app_id: "org.mozilla.fenix".to_string(),
channel: "random-channel".to_string(),
..Default::default()
};

let mut client = NimbusClient::new(
app_context.clone(),
Default::default(),
temp_dir.path(),
None,
AvailableRandomizationUnits {
client_id: Some(mock_client_id),
..AvailableRandomizationUnits::default()
},
Box::new(metrics.clone()),
)?;
client.set_nimbus_id(&Uuid::from_str("53baafb3-b800-42ac-878c-c3451e250928")?)?;

let targeting_attributes = TargetingAttributes {
app_context,
..Default::default()
};
client.with_targeting_attributes(targeting_attributes);
client.initialize()?;

let slug_1 = "experiment-1";
let exp_1 = get_targeted_experiment(slug_1, "true");
client.set_experiments_locally(to_local_experiments_string(&[exp_1])?)?;

client.apply_pending_experiments()?;

let metric_records: Vec<EnrollmentStatusExtraDef> =
serde_json::from_value(metrics.assert_get_vec_value("enrollment_status"))?;
assert_eq!(metric_records.len(), 0);

Ok(())
}

0 comments on commit 0f82c29

Please sign in to comment.