Bug 1997627 - Add NotEnrolledReason::DifferentAppName and NotEnrolledReason::DifferentChannel (#7050)

freshstrangemusic · web-flow · commit b3d8cc2b9bc4 · 2025-11-04T14:25:07.000Z
diff --git a/components/nimbus/src/enrollment.rs b/components/nimbus/src/enrollment.rs
@@ -48,27 +48,33 @@ impl Display for EnrolledReason {
 // ⚠️ in `mod test_schema_bw_compat` below, and may require a DB migration. ⚠️
 #[derive(Deserialize, Serialize, Debug, Clone, Hash, Eq, PartialEq)]
 pub enum NotEnrolledReason {
-    /// The user opted-out of experiments before we ever got enrolled to this one.
-    OptOut,
-    /// The evaluator bucketing did not choose us.
-    NotSelected,
-    /// We are not being targeted for this experiment.
-    NotTargeted,
+    /// The experiment targets a different application.
+    DifferentAppName,
+    /// The experiment targets a different channel.
+    DifferentChannel,
     /// The experiment enrollment is paused.
     EnrollmentsPaused,
     /// The experiment used a feature that was already under experiment.
     FeatureConflict,
+    /// The evaluator bucketing did not choose us.
+    NotSelected,
+    /// We are not being targeted for this experiment.
+    NotTargeted,
+    /// The user opted-out of experiments before we ever got enrolled to this one.
+    OptOut,
 }
 
 impl Display for NotEnrolledReason {
     fn fmt(&self, f: &mut Formatter<'_>) -> FmtResult {
         Display::fmt(
             match self {
-                NotEnrolledReason::OptOut => "OptOut",
-                NotEnrolledReason::NotSelected => "NotSelected",
-                NotEnrolledReason::NotTargeted => "NotTargeted",
+                NotEnrolledReason::DifferentAppName => "DifferentAppName",
+                NotEnrolledReason::DifferentChannel => "DifferentChannel",
                 NotEnrolledReason::EnrollmentsPaused => "EnrollmentsPaused",
                 NotEnrolledReason::FeatureConflict => "FeatureConflict",
+                NotEnrolledReason::NotSelected => "NotSelected",
+                NotEnrolledReason::NotTargeted => "NotTargeted",
+                NotEnrolledReason::OptOut => "OptOut",
             },
             f,
         )
@@ -273,6 +279,12 @@ impl ExperimentEnrollment {
                             updated_enrollment
                         }
                         EnrollmentStatus::NotEnrolled {
+                            reason: NotEnrolledReason::DifferentAppName,
+                        }
+                        | EnrollmentStatus::NotEnrolled {
+                            reason: NotEnrolledReason::DifferentChannel,
+                        }
+                        | EnrollmentStatus::NotEnrolled {
                             reason: NotEnrolledReason::NotTargeted,
                         } => {
                             debug!("Existing experiment enrollment '{}' is now disqualified (targeting change)", &self.slug);
diff --git a/components/nimbus/src/evaluator.rs b/components/nimbus/src/evaluator.rs
@@ -72,12 +72,10 @@ pub fn evaluate_enrollment(
     exp: &Experiment,
     th: &NimbusTargetingHelper,
 ) -> Result<ExperimentEnrollment> {
-    if !is_experiment_available(th, exp, true) {
+    if let ExperimentAvailable::Unavailable { reason } = is_experiment_available(th, exp, true) {
         return Ok(ExperimentEnrollment {
             slug: exp.slug.clone(),
-            status: EnrollmentStatus::NotEnrolled {
-                reason: NotEnrolledReason::NotTargeted,
-            },
+            status: EnrollmentStatus::NotEnrolled { reason },
         });
     }
 
@@ -129,6 +127,17 @@ pub fn evaluate_enrollment(
     })
 }
 
+/// Whether or not an experiment is available.
+#[derive(Debug, Eq, PartialEq)]
+pub enum ExperimentAvailable {
+    /// The experiment is available (i.e., it is for this application and channel).
+    Available,
+
+    /// The experiment is not available (i.e., it is either not for this
+    /// application or not for this channel).
+    Unavailable { reason: NotEnrolledReason },
+}
+
 /// Check if an experiment is available for this app defined by this `AppContext`.
 ///
 /// # Arguments:
@@ -144,20 +153,22 @@ pub fn is_experiment_available(
     th: &NimbusTargetingHelper,
     exp: &Experiment,
     is_release: bool,
-) -> bool {
+) -> ExperimentAvailable {
     // Verify the app_name matches the application being targeted
     // by the experiment.
     match (&exp.app_name, th.context.get("app_name".to_string())) {
         (Some(exp), Some(Value::String(mine))) => {
             if !exp.eq(mine) {
-                return false;
+                return ExperimentAvailable::Unavailable {
+                    reason: NotEnrolledReason::DifferentAppName,
+                };
             }
         }
         (_, _) => debug!("Experiment missing app_name, skipping it as a targeting parameter"),
     }
 
     if !is_release {
-        return true;
+        return ExperimentAvailable::Available;
     }
 
     // Verify the channel matches the application being targeted
@@ -166,12 +177,15 @@ pub fn is_experiment_available(
     match (&exp.channel, th.context.get("channel".to_string())) {
         (Some(exp), Some(Value::String(mine))) => {
             if !exp.to_lowercase().eq(&mine.to_lowercase()) {
-                return false;
+                return ExperimentAvailable::Unavailable {
+                    reason: NotEnrolledReason::DifferentChannel,
+                };
             }
         }
         (_, _) => debug!("Experiment missing channel, skipping it as a targeting parameter"),
     }
-    true
+
+    ExperimentAvailable::Available
 }
 
 /// Chooses a branch randomly from a set of branches
diff --git a/components/nimbus/src/stateful/nimbus_client.rs b/components/nimbus/src/stateful/nimbus_client.rs
@@ -13,7 +13,7 @@ use crate::{
     error::{info, warn, BehaviorError},
     evaluator::{
         get_calculated_attributes, is_experiment_available, CalculatedAttributes,
-        TargetingAttributes,
+        ExperimentAvailable, TargetingAttributes,
     },
     json::{JsonObject, PrefValue},
     metrics::{
@@ -322,7 +322,9 @@ impl NimbusClient {
         Ok(self
             .get_all_experiments()?
             .into_iter()
-            .filter(|exp| is_experiment_available(&th, exp, false))
+            .filter(|exp| {
+                is_experiment_available(&th, exp, false) == ExperimentAvailable::Available
+            })
             .map(|exp| exp.into())
             .collect())
     }
@@ -952,22 +954,20 @@ impl NimbusClient {
             self.event_store.clone(),
             self.gecko_prefs.clone(),
         );
-        let experiments = self
-            .database_cache
-            .get_experiments()?
+        let experiments = self.database_cache.get_experiments()?;
+        let experiments = experiments
             .iter()
-            .filter_map(
-                |exp| match is_experiment_available(&targeting_helper, exp, true) {
-                    true => Some(exp.slug.clone()),
-                    false => None,
-                },
-            )
-            .collect::<HashSet<String>>();
+            .filter(|exp| {
+                is_experiment_available(&targeting_helper, exp, true)
+                    == ExperimentAvailable::Available
+            })
+            .map(|exp| &*exp.slug)
+            .collect::<HashSet<&str>>();
         self.metrics_handler.record_enrollment_statuses(
             self.database_cache
                 .get_enrollments()?
                 .into_iter()
-                .filter_map(|e| match experiments.contains(&e.slug) {
+                .filter_map(|e| match experiments.contains(&*e.slug) {
                     true => Some(e.into()),
                     false => None,
                 })
diff --git a/components/nimbus/src/tests/test_enrollment.rs b/components/nimbus/src/tests/test_enrollment.rs
@@ -764,6 +764,7 @@ fn test_evolver_experiment_update_enrolled_then_targeting_changed() -> Result<()
             &mut events,
         )?
         .unwrap();
+
     if let EnrollmentStatus::Disqualified {
         reason: DisqualifiedReason::NotTargeted,
         branch,
diff --git a/components/nimbus/src/tests/test_evaluator.rs b/components/nimbus/src/tests/test_evaluator.rs
@@ -6,7 +6,7 @@
 use crate::{
     enrollment::{EnrolledReason, EnrollmentStatus, NotEnrolledReason},
     evaluate_enrollment,
-    evaluator::{choose_branch, is_experiment_available, targeting},
+    evaluator::{choose_branch, is_experiment_available, targeting, ExperimentAvailable},
     AppContext, AvailableRandomizationUnits, Branch, BucketConfig, Experiment, RandomizationUnit,
     Result, TargetingAttributes,
 };
@@ -411,26 +411,50 @@ fn test_is_experiment_available() {
     .into();
     // If is_release is true, we should match on the exact combination of
     // app_name, channel and app_id.
-    assert!(!is_experiment_available(&th, &experiment, true));
+    assert_eq!(
+        is_experiment_available(&th, &experiment, true),
+        ExperimentAvailable::Unavailable {
+            reason: NotEnrolledReason::DifferentChannel
+        }
+    );
 
     // If is_release is false, we only match on app_name.
     // As a nightly build, we want to be able to test production experiments
-    assert!(is_experiment_available(&th, &experiment, false));
+    assert_eq!(
+        is_experiment_available(&th, &experiment, false),
+        ExperimentAvailable::Available
+    );
 
     let experiment = Experiment {
         channel: Some("nightly".to_string()),
         ..experiment
     };
     // channels now match, so should be available for enrollment (true) and testing (false)
-    assert!(is_experiment_available(&th, &experiment, true));
-    assert!(is_experiment_available(&th, &experiment, false));
+    assert_eq!(
+        is_experiment_available(&th, &experiment, true),
+        ExperimentAvailable::Available
+    );
+    assert_eq!(
+        is_experiment_available(&th, &experiment, false),
+        ExperimentAvailable::Available
+    );
 
     let experiment = Experiment {
         app_name: Some("a_different_app".to_string()),
         ..experiment
     };
-    assert!(!is_experiment_available(&th, &experiment, false));
-    assert!(!is_experiment_available(&th, &experiment, false));
+    assert_eq!(
+        is_experiment_available(&th, &experiment, true),
+        ExperimentAvailable::Unavailable {
+            reason: NotEnrolledReason::DifferentAppName
+        }
+    );
+    assert_eq!(
+        is_experiment_available(&th, &experiment, false),
+        ExperimentAvailable::Unavailable {
+            reason: NotEnrolledReason::DifferentAppName
+        }
+    );
 }
 
 #[test]
@@ -635,31 +659,30 @@ fn test_not_targeted_for_enrollment() {
         &ctx.clone().into(),
     )
     .unwrap();
-    assert!(matches!(
+    assert_eq!(
         enrollment.status,
         EnrollmentStatus::NotEnrolled {
-            reason: NotEnrolledReason::NotTargeted
+            reason: NotEnrolledReason::DifferentAppName
         }
-    ));
+    );
 
     // Change the app_name back and change the channel to test when it doesn't match:
     ctx.app_name = "NimbusTest".to_string();
     ctx.channel = "Wrong".to_string();
 
-    // Now we won't be enrolled in the experiment because we don't have the right channel, but with the same
-    // `NotTargeted` reason
+    // Now we won't be enrolled in the experiment because we don't have the right channel
     let enrollment = evaluate_enrollment(
         &AvailableRandomizationUnits::with_nimbus_id(&id),
         &experiment,
         &ctx.into(),
     )
     .unwrap();
-    assert!(matches!(
+    assert_eq!(
         enrollment.status,
         EnrollmentStatus::NotEnrolled {
-            reason: NotEnrolledReason::NotTargeted
+            reason: NotEnrolledReason::DifferentChannel
         }
-    ));
+    );
 }
 
 #[test]
