Skip to content

Commit 9335926

Browse files
Bug 2044397 - Add feature IDs to enrollment change events (#7391)
Enrollment change events now include the feature IDs of the changed enrollments when possible. This will allow us to trigger update handlers in client applications. Additionally, a bug was fixed where change events were not emitted when rollouts re-enrolled after previously unenrolling. Some of these tests have been made more deterministic by using a fixed UUID for the Nimbus ID instead of a random UUID, resulting in enrolling in the same branches every test run.
1 parent f7ffaa7 commit 9335926

11 files changed

Lines changed: 1219 additions & 271 deletions

File tree

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
# v153.0 (In progress)
22

3+
### Nimbus
4+
5+
- Enrollment change events (visible to the UDL) now include the feature IDs of the features involved when possible. ([#7391](https://github.com/mozilla/application-services/pull/7391/))
6+
- Fixed a bug where enrollment change events were not emitted for rollouts that re-enrolled after previously unenrolling. ([#7391](https://github.com/mozilla/application-services/pull/7391/))
7+
38
[Full Changelog](In progress)
49

510
# v152.0 (_2026-05-18_)

components/nimbus/android/src/test/java/org/mozilla/experiments/nimbus/NimbusTests.kt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,18 +155,21 @@ class NimbusTests {
155155
branchSlug = "test-branch",
156156
reason = "test-reason",
157157
change = EnrollmentChangeEventType.ENROLLMENT,
158+
featureIds = listOf(),
158159
),
159160
EnrollmentChangeEvent(
160161
experimentSlug = "test-experiment",
161162
branchSlug = "test-branch",
162163
reason = "test-reason",
163164
change = EnrollmentChangeEventType.UNENROLLMENT,
165+
featureIds = listOf(),
164166
),
165167
EnrollmentChangeEvent(
166168
experimentSlug = "test-experiment",
167169
branchSlug = "test-branch",
168170
reason = "test-reason",
169171
change = EnrollmentChangeEventType.DISQUALIFICATION,
172+
featureIds = listOf(),
170173
),
171174
)
172175

components/nimbus/src/enrollment.rs

Lines changed: 45 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,7 @@ impl ExperimentEnrollment {
227227
&enrollment.slug, &enrollment
228228
);
229229
if matches!(enrollment.status, EnrollmentStatus::Enrolled { .. }) {
230-
out_enrollment_events.push(enrollment.get_change_event())
230+
out_enrollment_events.push(enrollment.get_change_event(Some(experiment)))
231231
}
232232
enrollment
233233
})
@@ -246,6 +246,7 @@ impl ExperimentEnrollment {
246246
branch_slug: branch_slug.to_string(),
247247
reason: Some("does-not-exist".to_string()),
248248
change: EnrollmentChangeEventType::EnrollFailed,
249+
feature_ids: experiment.get_feature_ids(),
249250
});
250251

251252
return Err(NimbusError::NoSuchBranch(
@@ -257,7 +258,7 @@ impl ExperimentEnrollment {
257258
slug: experiment.slug.clone(),
258259
status: EnrollmentStatus::new_enrolled(EnrolledReason::OptIn, branch_slug),
259260
};
260-
out_enrollment_events.push(enrollment.get_change_event());
261+
out_enrollment_events.push(enrollment.get_change_event(Some(experiment)));
261262
Ok(enrollment)
262263
}
263264

@@ -287,7 +288,8 @@ impl ExperimentEnrollment {
287288
&self.slug, &self, updated_enrollment
288289
);
289290
if matches!(updated_enrollment.status, EnrollmentStatus::Enrolled { .. }) {
290-
out_enrollment_events.push(updated_enrollment.get_change_event());
291+
out_enrollment_events
292+
.push(updated_enrollment.get_change_event(Some(updated_experiment)));
291293
}
292294
updated_enrollment
293295
}
@@ -303,15 +305,17 @@ impl ExperimentEnrollment {
303305
self.maybe_revert_all_gecko_pref_states(gecko_pref_store);
304306
let updated_enrollment =
305307
self.disqualify_from_enrolled(DisqualifiedReason::OptOut);
306-
out_enrollment_events.push(updated_enrollment.get_change_event());
308+
out_enrollment_events
309+
.push(updated_enrollment.get_change_event(Some(updated_experiment)));
307310
updated_enrollment
308311
} else if !updated_experiment.has_branch(branch) {
309312
// The branch we were in disappeared!
310313
#[cfg(feature = "stateful")]
311314
self.maybe_revert_all_gecko_pref_states(gecko_pref_store);
312315
let updated_enrollment =
313316
self.disqualify_from_enrolled(DisqualifiedReason::Error);
314-
out_enrollment_events.push(updated_enrollment.get_change_event());
317+
out_enrollment_events
318+
.push(updated_enrollment.get_change_event(Some(updated_experiment)));
315319
updated_enrollment
316320
} else if matches!(reason, EnrolledReason::OptIn) {
317321
// we check if we opted-in an experiment, if so
@@ -341,7 +345,9 @@ impl ExperimentEnrollment {
341345
EnrollmentStatus::Error { .. } => {
342346
let updated_enrollment =
343347
self.disqualify_from_enrolled(DisqualifiedReason::Error);
344-
out_enrollment_events.push(updated_enrollment.get_change_event());
348+
out_enrollment_events.push(
349+
updated_enrollment.get_change_event(Some(updated_experiment)),
350+
);
345351
updated_enrollment
346352
}
347353
EnrollmentStatus::NotEnrolled {
@@ -359,7 +365,9 @@ impl ExperimentEnrollment {
359365
);
360366
let updated_enrollment =
361367
self.disqualify_from_enrolled(DisqualifiedReason::NotTargeted);
362-
out_enrollment_events.push(updated_enrollment.get_change_event());
368+
out_enrollment_events.push(
369+
updated_enrollment.get_change_event(Some(updated_experiment)),
370+
);
363371
updated_enrollment
364372
}
365373
EnrollmentStatus::NotEnrolled {
@@ -369,7 +377,9 @@ impl ExperimentEnrollment {
369377
//
370378
let updated_enrollment =
371379
self.disqualify_from_enrolled(DisqualifiedReason::NotSelected);
372-
out_enrollment_events.push(updated_enrollment.get_change_event());
380+
out_enrollment_events.push(
381+
updated_enrollment.get_change_event(Some(updated_experiment)),
382+
);
373383
updated_enrollment
374384
}
375385
EnrollmentStatus::NotEnrolled { .. }
@@ -398,13 +408,18 @@ impl ExperimentEnrollment {
398408
DisqualifiedReason::NotSelected | DisqualifiedReason::NotTargeted,
399409
)
400410
{
401-
let evaluated_enrollment = evaluate_enrollment(
411+
let updated_enrollment = evaluate_enrollment(
402412
available_randomization_units,
403413
updated_experiment,
404414
targeting_helper,
405415
)?;
406-
match evaluated_enrollment.status {
407-
EnrollmentStatus::Enrolled { .. } => evaluated_enrollment,
416+
match updated_enrollment.status {
417+
EnrollmentStatus::Enrolled { .. } => {
418+
out_enrollment_events.push(
419+
updated_enrollment.get_change_event(Some(updated_experiment)),
420+
);
421+
updated_enrollment
422+
}
408423
_ => self.clone(),
409424
}
410425
} else {
@@ -422,6 +437,7 @@ impl ExperimentEnrollment {
422437
/// from the database after `PREVIOUS_ENROLLMENTS_GC_TIME`.
423438
fn on_experiment_ended(
424439
&self,
440+
experiment: &Experiment,
425441
#[cfg(feature = "stateful")] gecko_pref_store: Option<&GeckoPrefStore>,
426442
out_enrollment_events: &mut Vec<EnrollmentChangeEvent>,
427443
) -> Option<Self> {
@@ -446,7 +462,7 @@ impl ExperimentEnrollment {
446462
experiment_ended_at: now_secs(),
447463
},
448464
};
449-
out_enrollment_events.push(enrollment.get_change_event());
465+
out_enrollment_events.push(enrollment.get_change_event(Some(experiment)));
450466
Some(enrollment)
451467
}
452468

@@ -455,6 +471,7 @@ impl ExperimentEnrollment {
455471
#[cfg_attr(not(feature = "stateful"), allow(unused))]
456472
pub(crate) fn on_explicit_opt_out(
457473
&self,
474+
experiment: Option<&Experiment>,
458475
out_enrollment_events: &mut Vec<EnrollmentChangeEvent>,
459476
#[cfg(feature = "stateful")] gecko_pref_store: Option<&GeckoPrefStore>,
460477
) -> ExperimentEnrollment {
@@ -464,7 +481,7 @@ impl ExperimentEnrollment {
464481
self.maybe_revert_all_gecko_pref_states(gecko_pref_store);
465482

466483
let enrollment = self.disqualify_from_enrolled(DisqualifiedReason::OptOut);
467-
out_enrollment_events.push(enrollment.get_change_event());
484+
out_enrollment_events.push(enrollment.get_change_event(experiment));
468485
enrollment
469486
}
470487
EnrollmentStatus::NotEnrolled { .. } => Self {
@@ -486,6 +503,7 @@ impl ExperimentEnrollment {
486503
pub(crate) fn on_pref_unenroll(
487504
&self,
488505
pref_unenroll_reason: PrefUnenrollReason,
506+
experiment: Option<&Experiment>,
489507
out_enrollment_events: &mut Vec<EnrollmentChangeEvent>,
490508
) -> ExperimentEnrollment {
491509
match self.status {
@@ -494,7 +512,7 @@ impl ExperimentEnrollment {
494512
self.disqualify_from_enrolled(DisqualifiedReason::PrefUnenrollReason {
495513
reason: pref_unenroll_reason,
496514
});
497-
out_enrollment_events.push(enrollment.get_change_event());
515+
out_enrollment_events.push(enrollment.get_change_event(experiment));
498516
enrollment
499517
}
500518
_ => self.clone(),
@@ -564,12 +582,13 @@ impl ExperimentEnrollment {
564582
#[cfg_attr(not(feature = "stateful"), allow(unused))]
565583
pub fn reset_telemetry_identifiers(
566584
&self,
585+
experiment: Option<&Experiment>,
567586
out_enrollment_events: &mut Vec<EnrollmentChangeEvent>,
568587
) -> Self {
569588
let updated = match self.status {
570589
EnrollmentStatus::Enrolled { .. } => {
571590
let disqualified = self.disqualify_from_enrolled(DisqualifiedReason::OptOut);
572-
out_enrollment_events.push(disqualified.get_change_event());
591+
out_enrollment_events.push(disqualified.get_change_event(experiment));
573592
disqualified
574593
}
575594
EnrollmentStatus::NotEnrolled { .. }
@@ -602,19 +621,21 @@ impl ExperimentEnrollment {
602621

603622
// Create a telemetry event describing the transition
604623
// to the current enrollment state.
605-
fn get_change_event(&self) -> EnrollmentChangeEvent {
624+
fn get_change_event(&self, experiment: Option<&Experiment>) -> EnrollmentChangeEvent {
606625
match &self.status {
607626
EnrollmentStatus::Enrolled { branch, .. } => EnrollmentChangeEvent::new(
608627
&self.slug,
609628
branch,
610629
None,
611630
EnrollmentChangeEventType::Enrollment,
631+
experiment,
612632
),
613633
EnrollmentStatus::WasEnrolled { branch, .. } => EnrollmentChangeEvent::new(
614634
&self.slug,
615635
branch,
616636
None,
617637
EnrollmentChangeEventType::Unenrollment,
638+
experiment,
618639
),
619640
EnrollmentStatus::Disqualified { branch, reason, .. } => EnrollmentChangeEvent::new(
620641
&self.slug,
@@ -631,6 +652,7 @@ impl ExperimentEnrollment {
631652
},
632653
},
633654
EnrollmentChangeEventType::Disqualification,
655+
experiment,
634656
),
635657
EnrollmentStatus::NotEnrolled { .. } | EnrollmentStatus::Error { .. } => {
636658
unreachable!()
@@ -988,6 +1010,7 @@ impl<'a> EnrollmentsEvolver<'a> {
9881010
branch_slug: "N/A".to_string(),
9891011
reason: Some("feature-conflict".to_string()),
9901012
change: EnrollmentChangeEventType::EnrollFailed,
1013+
feature_ids: next_experiment.get_feature_ids(),
9911014
})
9921015
}
9931016
// Whether it's our experiment or not that is using these features, no further enrollment can
@@ -1142,7 +1165,8 @@ impl<'a> EnrollmentsEvolver<'a> {
11421165
out_enrollment_events,
11431166
)?),
11441167
// Experiment deleted remotely.
1145-
(Some(_), None, Some(enrollment)) => enrollment.on_experiment_ended(
1168+
(Some(prev_experiment), None, Some(enrollment)) => enrollment.on_experiment_ended(
1169+
prev_experiment,
11461170
#[cfg(feature = "stateful")]
11471171
gecko_pref_store,
11481172
out_enrollment_events,
@@ -1452,11 +1476,13 @@ impl From<&EnrolledFeatureConfig> for EnrolledFeature {
14521476
}
14531477

14541478
#[derive(Serialize, Deserialize, Debug, Clone)]
1479+
#[cfg_attr(test, derive(Eq, PartialEq))]
14551480
pub struct EnrollmentChangeEvent {
14561481
pub experiment_slug: String,
14571482
pub branch_slug: String,
14581483
pub reason: Option<String>,
14591484
pub change: EnrollmentChangeEventType,
1485+
pub feature_ids: Vec<String>,
14601486
}
14611487

14621488
impl EnrollmentChangeEvent {
@@ -1465,12 +1491,14 @@ impl EnrollmentChangeEvent {
14651491
branch: &str,
14661492
reason: Option<&str>,
14671493
change: EnrollmentChangeEventType,
1494+
experiment: Option<&Experiment>,
14681495
) -> Self {
14691496
Self {
14701497
experiment_slug: slug.to_owned(),
14711498
branch_slug: branch.to_owned(),
14721499
reason: reason.map(|s| s.to_owned()),
14731500
change,
1501+
feature_ids: experiment.map(|e| e.get_feature_ids()).unwrap_or_default(),
14741502
}
14751503
}
14761504
}

components/nimbus/src/metrics.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ use crate::enrollment::PreviousGeckoPrefState;
1111
pub use crate::metrics::detail::*;
1212

1313
#[derive(Serialize, Deserialize, Clone)]
14+
#[cfg_attr(test, derive(Debug))]
1415
pub struct EnrollmentStatusExtraDef {
1516
pub branch: Option<String>,
1617
pub conflict_slug: Option<String>,

components/nimbus/src/nimbus.udl

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ dictionary EnrollmentChangeEvent {
7373
string branch_slug;
7474
string? reason;
7575
EnrollmentChangeEventType change;
76+
sequence<string> feature_ids;
7677
};
7778

7879
enum EnrollmentChangeEventType {

components/nimbus/src/schema.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// License, v. 2.0. If a copy of the MPL was not distributed with this
33
// file, You can obtain one at https://mozilla.org/MPL/2.0/.
44

5-
use std::collections::HashSet;
5+
use std::collections::BTreeSet;
66

77
use serde_derive::{Deserialize, Serialize};
88
use serde_json::{Map, Value};
@@ -75,11 +75,13 @@ impl Experiment {
7575
.flat_map(|b| {
7676
b.get_feature_configs()
7777
.iter()
78-
.map(|f| f.to_owned().feature_id)
78+
.map(|f| f.feature_id.clone())
7979
.collect::<Vec<_>>()
8080
})
81-
.collect::<HashSet<_>>();
81+
.collect::<BTreeSet<_>>();
8282

83+
// Using a BTreeSet generates the feature IDs in a sorted order, which helps
84+
// make testing easier.
8385
feature_ids.into_iter().collect()
8486
}
8587

0 commit comments

Comments
 (0)