Adding events to bubble up from Rust

components/nimbus/android/src/main/java/org/mozilla/experiments/nimbus/Nimbus.kt

@@ -596,6 +596,19 @@ open class Nimbus(
                         enrollmentId = event.enrollmentId
                     ))
                 }
+                EnrollmentChangeEventType.ENROLL_FAILED -> {
+                    NimbusEvents.enrollFailed.record(NimbusEvents.EnrollFailedExtra(
+                        experiment = event.experimentSlug,
+                        branch = event.branchSlug,
+                        reason = event.reason
+                    ))
+                }
+                EnrollmentChangeEventType.UNENROLL_FAILED -> {
+                    NimbusEvents.unenrollFailed.record(NimbusEvents.UnenrollFailedExtra(
+                        experiment = event.experimentSlug,
+                        reason = event.reason
+                    ))
+                }
             }
         }
     }
@@ -616,8 +629,7 @@ open class Nimbus(
         activeExperiments.find { it.featureIds.contains(featureId) }?.also { experiment ->
             NimbusEvents.exposure.record(NimbusEvents.ExposureExtra(
                 experiment = experiment.slug,
-                branch = experiment.branchSlug,
-                enrollmentId = experiment.enrollmentId
+                branch = experiment.branchSlug
             ))
         }
     }

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

@@ -216,19 +216,15 @@ class NimbusTests {
 
         // Use the Glean test API to check that the valid event is present
         assertNotNull("Event must have a value", NimbusEvents.exposure.testGetValue())
-        val enrollmentEvents = NimbusEvents.exposure.testGetValue()!!
-        assertEquals("Event count must match", enrollmentEvents.count(), 1)
-        val enrollmentEventExtras = enrollmentEvents.first().extra!!
+        val exposureEvents = NimbusEvents.exposure.testGetValue()!!
+        assertEquals("Event count must match", exposureEvents.count(), 1)
+        val exposureEventExtras = exposureEvents.first().extra!!
         assertEquals(
             "Experiment slug must match",
             "test-experiment",
-            enrollmentEventExtras["experiment"]
-        )
-        assertEquals("Experiment branch must match", "test-branch", enrollmentEventExtras["branch"])
-        assertNotNull(
-            "Experiment enrollment-id must not be null",
-            enrollmentEventExtras["enrollment_id"]
+            exposureEventExtras["experiment"]
         )
+        assertEquals("Experiment branch must match", "test-branch", exposureEventExtras["branch"])
 
         // Attempt to record an event for a non-existent or feature we are not enrolled in an
         // experiment in to ensure nothing is recorded.
@@ -237,22 +233,18 @@ class NimbusTests {
         // Verify the invalid event was ignored by checking again that the valid event is still the only
         // event, and that it hasn't changed any of its extra properties.
         assertNotNull("Event must have a value", NimbusEvents.exposure.testGetValue())
-        val enrollmentEventsTryTwo = NimbusEvents.exposure.testGetValue()!!
-        assertEquals("Event count must match", enrollmentEventsTryTwo.count(), 1)
-        val enrollmentEventExtrasTryTwo = enrollmentEventsTryTwo.first().extra!!
+        val exposureEventsTryTwo = NimbusEvents.exposure.testGetValue()!!
+        assertEquals("Event count must match", exposureEventsTryTwo.count(), 1)
+        val exposureEventExtrasTryTwo = exposureEventsTryTwo.first().extra!!
         assertEquals(
             "Experiment slug must match",
             "test-experiment",
-            enrollmentEventExtrasTryTwo["experiment"]
+            exposureEventExtrasTryTwo["experiment"]
         )
         assertEquals(
             "Experiment branch must match",
             "test-branch",
-            enrollmentEventExtrasTryTwo["branch"]
-        )
-        assertNotNull(
-            "Experiment enrollment-id must not be null",
-            enrollmentEventExtrasTryTwo["enrollment_id"]
+            exposureEventExtrasTryTwo["branch"]
         )
     }
 

components/nimbus/ios/Nimbus/Nimbus.swift

@@ -70,7 +70,6 @@ extension Nimbus: FeaturesInterface {
             // for an experiment without an active enrollment.
             GleanMetrics.NimbusEvents.exposure.record(GleanMetrics.NimbusEvents.ExposureExtra(
                 branch: experiment.branchSlug,
-                enrollmentId: experiment.enrollmentId,
                 experiment: experiment.slug
             ))
         }
@@ -122,6 +121,18 @@ extension Nimbus: FeaturesInterface {
                     enrollmentId: event.enrollmentId,
                     experiment: event.experimentSlug
                 ))
+
+            case .enrollFailed:
+                GleanMetrics.NimbusEvents.enrollFailed.record(GleanMetrics.NimbusEvents.EnrollFailedExtra(
+                    branch: event.branchSlug,
+                    experiment: event.experimentSlug,
+                    reason: event.reason
+                ))
+            case .unenrollFailed:
+                GleanMetrics.NimbusEvents.unenrollFailed.record(GleanMetrics.NimbusEvents.UnenrollFailedExtra(
+                    experiment: event.experimentSlug,
+                    reason: event.reason
+                ))
             }
         }
     }

components/nimbus/src/enrollment.rs

@@ -123,6 +123,14 @@ impl ExperimentEnrollment {
         out_enrollment_events: &mut Vec<EnrollmentChangeEvent>,
     ) -> Result<Self> {
         if !experiment.has_branch(branch_slug) {
+            out_enrollment_events.push(EnrollmentChangeEvent {
+                experiment_slug: experiment.slug.to_string(),
+                branch_slug: branch_slug.to_string(),
+                enrollment_id: "N/A".to_string(),
+                reason: Some("does-not-exist".to_string()),
+                change: EnrollmentChangeEventType::EnrollFailed,
+            });
+
             return Err(NimbusError::NoSuchBranch(
                 branch_slug.to_owned(),
                 experiment.slug.clone(),
@@ -760,6 +768,14 @@ impl<'a> EnrollmentsEvolver<'a> {
                             reason: NotEnrolledReason::FeatureConflict,
                         },
                     });
+
+                    enrollment_events.push(EnrollmentChangeEvent {
+                        experiment_slug: slug.clone(),
+                        branch_slug: "N/A".to_string(),
+                        enrollment_id: "N/A".to_string(),
+                        reason: Some("feature-conflict".to_string()),
+                        change: EnrollmentChangeEventType::EnrollFailed,
+                    })
                 }
                 // Whether it's our experiment or not that is using these features, no further enrollment can
                 // happen.
@@ -1140,13 +1156,23 @@ pub fn opt_in_with_branch(
     branch: &str,
 ) -> Result<Vec<EnrollmentChangeEvent>> {
     let mut events = vec![];
-    let exp: Experiment = db
+    if let Ok(Some(exp)) = db
         .get_store(StoreId::Experiments)
-        .get(writer, experiment_slug)?
-        .ok_or_else(|| NimbusError::NoSuchExperiment(experiment_slug.to_owned()))?;
-    let enrollment = ExperimentEnrollment::from_explicit_opt_in(&exp, branch, &mut events)?;
-    db.get_store(StoreId::Enrollments)
-        .put(writer, experiment_slug, &enrollment)?;
+        .get::<Experiment, Writer>(writer, experiment_slug)
+    {
+        let enrollment = ExperimentEnrollment::from_explicit_opt_in(&exp, branch, &mut events);
+        db.get_store(StoreId::Enrollments)
+            .put(writer, experiment_slug, &enrollment.unwrap())?;
+    } else {
+        events.push(EnrollmentChangeEvent {
+            experiment_slug: experiment_slug.to_string(),
+            branch_slug: branch.to_string(),
+            enrollment_id: "N/A".to_string(),
+            reason: Some("does-not-exist".to_string()),
+            change: EnrollmentChangeEventType::EnrollFailed,
+        });
+    }
+
     Ok(events)
 }
 
@@ -1157,11 +1183,21 @@ pub fn opt_out(
 ) -> Result<Vec<EnrollmentChangeEvent>> {
     let mut events = vec![];
     let enr_store = db.get_store(StoreId::Enrollments);
-    let existing_enrollment: ExperimentEnrollment = enr_store
-        .get(writer, experiment_slug)?
-        .ok_or_else(|| NimbusError::NoSuchExperiment(experiment_slug.to_owned()))?;
-    let updated_enrollment = existing_enrollment.on_explicit_opt_out(&mut events);
-    enr_store.put(writer, experiment_slug, &updated_enrollment)?;
+    if let Ok(Some(existing_enrollment)) =
+        enr_store.get::<ExperimentEnrollment, Writer>(writer, experiment_slug)
+    {
+        let updated_enrollment = &existing_enrollment.on_explicit_opt_out(&mut events);
+        enr_store.put(writer, experiment_slug, updated_enrollment)?;
+    } else {
+        events.push(EnrollmentChangeEvent {
+            experiment_slug: experiment_slug.to_string(),
+            branch_slug: "N/A".to_string(),
+            enrollment_id: "N/A".to_string(),
+            reason: Some("does-not-exist".to_string()),
+            change: EnrollmentChangeEventType::UnenrollFailed,
+        });
+    }
+
     Ok(events)
 }
 

components/nimbus/src/tests/test_enrollment.rs

@@ -1237,9 +1237,9 @@ fn test_evolver_experiment_not_enrolled_feature_conflict() -> Result<()> {
     log::debug!("events: {:?}", events);
 
     assert_eq!(
-        2,
+        3,
         events.len(),
-        "There should be exactly 2 enrollment_change_events"
+        "There should be exactly 3 enrollment_change_events (Enroll/Enroll/EnrollFailed)"
     );
 
     let enrolled_events = events
@@ -1275,9 +1275,9 @@ fn test_multi_feature_per_branch_conflict() -> Result<()> {
     );
 
     assert_eq!(
-        2,
+        3,
         events.len(),
-        "There should be exactly 2 enrollment_change_events"
+        "There should be exactly 2 enrollment_change_events (Enroll/Enroll/EnrollFailed)"
     );
 
     let enrolled_events = events
@@ -1403,7 +1403,11 @@ fn test_evolver_multi_feature_experiments() -> Result<()> {
         &prev_enrollments,
     )?;
 
-    assert_eq!(events.len(), 0);
+    assert_eq!(
+        events.len(),
+        1,
+        "A single EnrollFailed recorded due to the feature-conflict"
+    );
 
     let feature_map = map_features_by_feature_id(&enrollments, &next_experiments);
     assert_eq!(feature_map.len(), 2);
@@ -1523,7 +1527,11 @@ fn test_evolver_multi_feature_experiments() -> Result<()> {
         &prev_enrollments,
     )?;
 
-    assert_eq!(events.len(), 0);
+    assert_eq!(
+        events.len(),
+        2,
+        "Exactly two EnrollFailed events should be recorded"
+    );
     let feature_map = map_features_by_feature_id(&enrollments, &next_experiments);
     assert_eq!(feature_map.len(), 2);
     assert_eq!(
@@ -1559,7 +1567,11 @@ fn test_evolver_multi_feature_experiments() -> Result<()> {
         &prev_enrollments,
     )?;
 
-    assert_eq!(events.len(), 0);
+    assert_eq!(
+        events.len(),
+        1,
+        "Exactly one EnrollFailed event should be recorded"
+    );
     let feature_map = map_features_by_feature_id(&enrollments, &next_experiments);
     assert_eq!(feature_map.len(), 2);
     assert_eq!(
@@ -2085,7 +2097,7 @@ fn test_evolver_rollouts_do_not_conflict_with_rollouts() -> Result<()> {
     let evolver = enrollment_evolver(&nimbus_id, &targeting_attributes, &aru);
     let (enrollments, events) = evolver.evolve_enrollments(true, &[], recipes, &[])?;
     assert_eq!(enrollments.len(), 3);
-    assert_eq!(events.len(), 2);
+    assert_eq!(events.len(), 3);
 
     let enrollments: Vec<ExperimentEnrollment> = enrollments
         .into_iter()

megazords/ios-rust/MozillaTestServices/MozillaTestServicesTests/NimbusTests.swift

@@ -290,31 +290,29 @@ class NimbusTests: XCTestCase {
 
         // Use the Glean test API to check that the valid event is present
         XCTAssertNotNil(GleanMetrics.NimbusEvents.exposure.testGetValue(), "Event must have a value")
-        let enrollmentEvents = GleanMetrics.NimbusEvents.exposure.testGetValue()!
-        XCTAssertEqual(1, enrollmentEvents.count, "Event count must match")
-        let enrollmentEventExtras = enrollmentEvents.first!.extra
-        XCTAssertEqual("secure-gold", enrollmentEventExtras!["experiment"], "Experiment slug must match")
+        let exposureEvents = GleanMetrics.NimbusEvents.exposure.testGetValue()!
+        XCTAssertEqual(1, exposureEvents.count, "Event count must match")
+        let exposureEventExtras = exposureEvents.first!.extra
+        XCTAssertEqual("secure-gold", exposureEventExtras!["experiment"], "Experiment slug must match")
         XCTAssertTrue(
-            enrollmentEventExtras!["branch"] == "control" || enrollmentEventExtras!["branch"] == "treatment",
+            exposureEventExtras!["branch"] == "control" || exposureEventExtras!["branch"] == "treatment",
             "Experiment branch must match"
         )
-        XCTAssertNotNil(enrollmentEventExtras!["enrollment_id"], "Experiment enrollment id must not be nil")
 
         // Attempt to record an event for a non-existent or feature we are not enrolled in an
         // experiment in to ensure nothing is recorded.
         nimbus.recordExposureEvent(featureId: "not-a-feature")
 
         // Verify the invalid event was ignored by checking again that the valid event is still the only
         // event, and that it hasn't changed any of its extra properties.
-        let enrollmentEventsTryTwo = GleanMetrics.NimbusEvents.exposure.testGetValue()!
-        XCTAssertEqual(1, enrollmentEventsTryTwo.count, "Event count must match")
-        let enrollmentEventExtrasTryTwo = enrollmentEventsTryTwo.first!.extra
-        XCTAssertEqual("secure-gold", enrollmentEventExtrasTryTwo!["experiment"], "Experiment slug must match")
+        let exposureEventsTryTwo = GleanMetrics.NimbusEvents.exposure.testGetValue()!
+        XCTAssertEqual(1, exposureEventsTryTwo.count, "Event count must match")
+        let exposureEventExtrasTryTwo = exposureEventsTryTwo.first!.extra
+        XCTAssertEqual("secure-gold", exposureEventExtrasTryTwo!["experiment"], "Experiment slug must match")
         XCTAssertTrue(
-            enrollmentEventExtrasTryTwo!["branch"] == "control" || enrollmentEventExtrasTryTwo!["branch"] == "treatment",
+            exposureEventExtrasTryTwo!["branch"] == "control" || exposureEventExtrasTryTwo!["branch"] == "treatment",
             "Experiment branch must match"
         )
-        XCTAssertNotNil(enrollmentEventExtrasTryTwo!["enrollment_id"], "Experiment enrollment id must not be nil")
     }
 
     func testRecordDisqualificationOnOptOut() throws {