Bug 2045490 - Set experiments inactive when unenrolling (#7427)

freshstrangemusic · web-flow · commit 1ee69ac3e0d9 · 2026-06-17T19:21:03.000Z
Now when experiments unenroll at runtime, Glean's experiment state will
be updated to mark the experiments as inactive. Post-(un)enrollment
logic has all been refactored into a single function to ensure that that
all enrollments and unenrollments are handled identically.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -24,6 +24,7 @@
 - Attempting to opt-out from an experiment that is not currently enrolled is now a no-op. ([#7399](https://github.com/mozilla/application-services/pull/7399))
 - All enrollment updates (opt-in, opt-out, reseting telemetry identifiers, unenrolling for pref conflicts) now trigger feature invalidation in Android clients. ([#7405](https://github.com/mozilla/application-services/pull/7405))
 - Changing experiment and/or rollout participation no longer triggers a double update. Enrollment change telemetry is now appropriately triggered when this occurs. ([#7401](https://github.com/mozilla/application-services/pull/7401))
+- Experiments will be marked as inactive in Glean when unenrolling. ([#7427](https://github.com/mozilla/application-services/pull/7427))
 
 ## ✨ What's New ✨
 
diff --git a/components/nimbus/android/src/main/java/org/mozilla/experiments/nimbus/Nimbus.kt b/components/nimbus/android/src/main/java/org/mozilla/experiments/nimbus/Nimbus.kt
@@ -286,7 +286,7 @@ open class Nimbus(
     @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
     internal fun initializeOnThisThread() = withCatchAll("initialize") {
         nimbusClient.initialize()
-        postEnrolmentCalculation(initial = true)
+        postEnrolmentCalculation(emptyList(), initial = true)
     }
 
     override fun fetchExperiments() {
@@ -333,33 +333,31 @@ open class Nimbus(
         }
     }
 
-    override fun applyPendingExperiments(): Job =
+    override fun applyPendingExperiments(initial: Boolean): Job =
         dbScope.launch {
             withContext(NonCancellable) {
-                applyPendingExperimentsOnThisThread()
+                applyPendingExperimentsOnThisThread(initial)
             }
         }
 
     @WorkerThread
     @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
-    internal fun applyPendingExperimentsOnThisThread() = withCatchAll("applyPendingExperiments") {
-        try {
-            var events: List<EnrollmentChangeEvent>?
-            val time = measureTimeMillis {
-                events = nimbusClient.applyPendingExperiments()
+    internal fun applyPendingExperimentsOnThisThread(initial: Boolean = false) {
+        withCatchAll("applyPendingExperiments") {
+            try {
+                var enrollmentChangeEvents: List<EnrollmentChangeEvent>?
+                val time = measureTimeMillis {
+                    enrollmentChangeEvents = nimbusClient.applyPendingExperiments()
+                }
+                NimbusHealth.applyPendingExperimentsTime.accumulateSingleSample(time)
+
+                // SAFETY: events is only null at declaration time and is
+                // immediately assigned a non-null value inside the
+                // measureTimeMillis lambda.
+                postEnrolmentCalculation(enrollmentChangeEvents!!, initial)
+            } catch (e: NimbusException.InvalidExperimentFormat) {
+                reportError("Invalid experiment format", e)
             }
-            NimbusHealth.applyPendingExperimentsTime.accumulateSingleSample(time)
-
-            // SAFETY: events is only null at declaration time and is
-            // immediately assigned a non-null value inside the
-            // measureTimeMillis lambda.
-            recordExperimentTelemetryEvents(events!!)
-
-            // Get the experiments to record in telemetry
-            postEnrolmentCalculation()
-            updateDispatcher.notifyChanged(events)
-        } catch (e: NimbusException.InvalidExperimentFormat) {
-            reportError("Invalid experiment format", e)
         }
     }
 
@@ -395,31 +393,52 @@ open class Nimbus(
         withContext(NonCancellable) {
             if (experimentsJson != null) {
                 setExperimentsLocallyOnThisThread(experimentsJson)
-                applyPendingExperimentsOnThisThread()
+                applyPendingExperimentsOnThisThread(initial = true)
             } else {
                 initializeOnThisThread()
             }
         }
     }
 
     @WorkerThread
-    private fun postEnrolmentCalculation(initial: Boolean = false) {
-        nimbusClient.getActiveExperiments().also { experiments ->
-            recordExperimentTelemetry(experiments)
+    private fun postEnrolmentCalculation(
+        enrollmentChangeEvents: List<EnrollmentChangeEvent>,
+        initial: Boolean = false,
+    ) {
+        val experiments = nimbusClient.getActiveExperiments()
+
+        if (initial) {
+            // During initialization we need to report the experiment status of
+            // all pre-existing experiments.
+            for (experiment in experiments) {
+                Glean.setExperimentActive(experiment.slug, experiment.branchSlug)
+            }
+        }
+
+        if (initial || enrollmentChangeEvents.isNotEmpty()) {
+            // During initialization we need to inform the application when we've
+            // finished applying updates, even if there is nothing enrolled.
+            recordExperimentTelemetryEvents(enrollmentChangeEvents)
             updateObserver { observer ->
                 observer.onUpdatesApplied(experiments)
             }
+        }
 
-            if (initial) {
-                val featureIds = mutableSetOf<String>()
-                for (experiment in experiments) {
-                    for (featureId in experiment.featureIds) {
-                        featureIds.add(featureId)
-                    }
+        if (initial) {
+            // Likewise, during initialization we need to include trigger
+            // updates for all pre-existing experiments.
+            val featureIds = mutableSetOf<String>()
+            for (experiment in experiments) {
+                for (featureId in experiment.featureIds) {
+                    featureIds.add(featureId)
                 }
-
-                updateDispatcher.notifyFeatures(featureIds)
             }
+
+            updateDispatcher.notifyFeatures(featureIds)
+        } else {
+            // However, during subsequent enrollment changes we only need to
+            // trigger updates to features that changed.
+            updateDispatcher.notifyChanged(enrollmentChangeEvents)
         }
     }
 
@@ -445,24 +464,16 @@ open class Nimbus(
     @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
     internal fun setExperimentParticipationOnThisThread(active: Boolean) =
         withCatchAll("setExperimentParticipation") {
-            val enrolmentChanges = nimbusClient.setExperimentParticipation(active)
-            if (enrolmentChanges.isNotEmpty()) {
-                recordExperimentTelemetryEvents(enrolmentChanges)
-                postEnrolmentCalculation()
-                updateDispatcher.notifyChanged(enrolmentChanges)
-            }
+            val enrollmentChangeEvents = nimbusClient.setExperimentParticipation(active)
+            postEnrolmentCalculation(enrollmentChangeEvents)
         }
 
     @WorkerThread
     @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
     internal fun setRolloutParticipationOnThisThread(active: Boolean) =
         withCatchAll("setRolloutParticipation") {
-            val enrolmentChanges = nimbusClient.setRolloutParticipation(active)
-            if (enrolmentChanges.isNotEmpty()) {
-                recordExperimentTelemetryEvents(enrolmentChanges)
-                postEnrolmentCalculation()
-                updateDispatcher.notifyChanged(enrolmentChanges)
-            }
+            val enrollmentChangeEvents = nimbusClient.setRolloutParticipation(active)
+            postEnrolmentCalculation(enrollmentChangeEvents)
         }
 
     override fun optOut(experimentId: String) {
@@ -489,9 +500,7 @@ open class Nimbus(
     ): List<EnrollmentChangeEvent>? {
         return withCatchAll("unenrollForGeckoPref") {
             val enrollmentChangeEvents = nimbusClient.unenrollForGeckoPref(geckoPrefState, prefUnenrollReason)
-            recordExperimentTelemetryEvents(enrollmentChangeEvents)
-            postEnrolmentCalculation()
-            updateDispatcher.notifyChanged(enrollmentChangeEvents)
+            postEnrolmentCalculation(enrollmentChangeEvents)
             enrollmentChangeEvents
         }
     }
@@ -521,9 +530,7 @@ open class Nimbus(
     internal fun optOutOnThisThread(experimentId: String) {
         withCatchAll("optOut") {
             val enrollmentChangeEvents = nimbusClient.optOut(experimentId)
-            recordExperimentTelemetryEvents(enrollmentChangeEvents)
-            postEnrolmentCalculation()
-            updateDispatcher.notifyChanged(enrollmentChangeEvents)
+            postEnrolmentCalculation(enrollmentChangeEvents)
         }
     }
 
@@ -539,9 +546,7 @@ open class Nimbus(
     internal fun resetTelemetryIdentifiersOnThisThread() {
         withCatchAll("resetTelemetryIdentifiers") {
             val enrollmentChangeEvents = nimbusClient.resetTelemetryIdentifiers()
-            recordExperimentTelemetryEvents(enrollmentChangeEvents)
-            postEnrolmentCalculation()
-            updateDispatcher.notifyChanged(enrollmentChangeEvents)
+            postEnrolmentCalculation(enrollmentChangeEvents)
         }
     }
 
@@ -557,9 +562,7 @@ open class Nimbus(
     internal fun optInWithBranchOnThisThread(experimentId: String, branch: String) {
         withCatchAll("optIn") {
             val enrollmentChangeEvents = nimbusClient.optInWithBranch(experimentId, branch)
-            recordExperimentTelemetryEvents(enrollmentChangeEvents)
-            postEnrolmentCalculation()
-            updateDispatcher.notifyChanged(enrollmentChangeEvents)
+            postEnrolmentCalculation(enrollmentChangeEvents)
         }
     }
 
@@ -637,6 +640,11 @@ open class Nimbus(
                             branch = event.branchSlug,
                         ),
                     )
+
+                    Glean.setExperimentActive(
+                        event.experimentSlug,
+                        event.branchSlug,
+                    )
                 }
 
                 EnrollmentChangeEventType.DISQUALIFICATION -> {
@@ -646,6 +654,10 @@ open class Nimbus(
                             branch = event.branchSlug,
                         ),
                     )
+
+                    Glean.setExperimentInactive(
+                        event.experimentSlug,
+                    )
                 }
 
                 EnrollmentChangeEventType.UNENROLLMENT -> {
@@ -655,6 +667,10 @@ open class Nimbus(
                             branch = event.branchSlug,
                         ),
                     )
+
+                    Glean.setExperimentInactive(
+                        event.experimentSlug,
+                    )
                 }
 
                 EnrollmentChangeEventType.ENROLL_FAILED -> {
@@ -717,11 +733,7 @@ open class Nimbus(
     internal fun enrollInFirefoxLabOnThisThread(slug: String): FirefoxLabsEnrollStatus {
         return withCatchAll("enrollInFirefoxLab") {
             val result = nimbusClient.enrollInFirefoxLab(slug)
-            if (result.enrollmentChangeEvents.isNotEmpty()) {
-                recordExperimentTelemetryEvents(result.enrollmentChangeEvents)
-                postEnrolmentCalculation()
-                updateDispatcher.notifyChanged(result.enrollmentChangeEvents)
-            }
+            postEnrolmentCalculation(result.enrollmentChangeEvents)
 
             result.status
         } ?: FirefoxLabsEnrollStatus.ERROR
@@ -735,11 +747,7 @@ open class Nimbus(
     internal fun unenrollFromFirefoxLabOnThisThread(slug: String): FirefoxLabsUnenrollStatus {
         return withCatchAll("unenrollFromFirefoxLab") {
             val result = nimbusClient.unenrollFromFirefoxLab(slug)
-            if (result.enrollmentChangeEvents.isNotEmpty()) {
-                recordExperimentTelemetryEvents(result.enrollmentChangeEvents)
-                postEnrolmentCalculation()
-                updateDispatcher.notifyChanged(result.enrollmentChangeEvents)
-            }
+            postEnrolmentCalculation(result.enrollmentChangeEvents)
 
             result.status
         } ?: FirefoxLabsUnenrollStatus.ERROR
@@ -753,11 +761,7 @@ open class Nimbus(
     internal fun unenrollFromAllFirefoxLabsOnThisThread() {
         withCatchAll("unenrollFromAllFirefoxLabs") {
             val enrollmentChangeEvents = nimbusClient.unenrollFromAllFirefoxLabs()
-            if (enrollmentChangeEvents.isNotEmpty()) {
-                recordExperimentTelemetryEvents(enrollmentChangeEvents)
-                postEnrolmentCalculation()
-                updateDispatcher.notifyChanged(enrollmentChangeEvents)
-            }
+            postEnrolmentCalculation(enrollmentChangeEvents)
         }
     }
 
diff --git a/components/nimbus/android/src/main/java/org/mozilla/experiments/nimbus/NimbusBuilder.kt b/components/nimbus/android/src/main/java/org/mozilla/experiments/nimbus/NimbusBuilder.kt
@@ -120,7 +120,7 @@ abstract class AbstractNimbusBuilder<T : NimbusInterface>(val context: Context)
                 val job = if (initialExperiments != null && (isFirstRun || isLocalBuild())) {
                     applyLocalExperiments(initialExperiments!!)
                 } else {
-                    applyPendingExperiments()
+                    applyPendingExperiments(initial = true)
                 }
 
                 // We always want initialize Nimbus to happen ASAP and before any features (engine/UI)
diff --git a/components/nimbus/android/src/main/java/org/mozilla/experiments/nimbus/NimbusInterface.kt b/components/nimbus/android/src/main/java/org/mozilla/experiments/nimbus/NimbusInterface.kt
@@ -129,8 +129,11 @@ interface NimbusInterface : FeaturesInterface, NimbusMessagingInterface, NimbusE
      * `setExperimentsLocally`, and then informs Glean of new experiment enrolment.
      *
      * Notifies `onUpdatesApplied` once enrolments are recalculated.
+     *
+     * @param initial Whether or not this is called during Nimbus initialization
+     * by the NimbusBuilder.
      */
-    fun applyPendingExperiments(): Job = Job()
+    fun applyPendingExperiments(initial: Boolean = false): Job = Job()
 
     /**
      * Apply the set of local experiments.
diff --git a/components/nimbus/android/src/test/java/org/mozilla/experiments/nimbus/NimbusTests.kt b/components/nimbus/android/src/test/java/org/mozilla/experiments/nimbus/NimbusTests.kt
@@ -433,9 +433,13 @@ class NimbusTests {
             NimbusEvents.disqualification.testGetValue(),
         )
 
+        assertTrue(Glean.testIsExperimentActive("test-experiment"))
+
         // Opt out of the specific experiment
         nimbus.optOutOnThisThread("test-experiment")
 
+        assertFalse(Glean.testIsExperimentActive("test-experiment"))
+
         // Use the Glean test API to check that the valid event is present
         assertNotNull("Event must have a value", NimbusEvents.disqualification.testGetValue())
         val disqualificationEvents = NimbusEvents.disqualification.testGetValue()!!
@@ -450,7 +454,7 @@ class NimbusTests {
     }
 
     @Test
-    fun `toggling the global opt out generates the correct Glean event`() {
+    fun `toggling the experiment opt out generates the correct Glean event`() {
         // Load the experiment in nimbus so and optIn so that it will be active. This is necessary
         // because recordExposure checks for active experiments before recording.
         nimbus.setUpTestExperiments(packageName, appInfo)
@@ -461,9 +465,13 @@ class NimbusTests {
             NimbusEvents.disqualification.testGetValue(),
         )
 
+        assertTrue(Glean.testIsExperimentActive("test-experiment"))
+
         // Opt out of experiments but keep rollouts
         nimbus.setExperimentParticipationOnThisThread(false)
 
+        assertFalse(Glean.testIsExperimentActive("test-experiment"))
+
         // Use the Glean test API to check that the valid event is present
         assertNotNull("Event must have a value", NimbusEvents.disqualification.testGetValue())
         val disqualificationEvents = NimbusEvents.disqualification.testGetValue()!!
@@ -915,15 +923,9 @@ class NimbusTests {
         val handler = TestGeckoPrefHandler()
 
         val nimbus = createNimbus(geckoPrefHandler = handler)
+        nimbus.setUpTestExperiments(packageName, appInfo)
 
-        suspend fun getString(): String {
-            return testExperimentsJsonString(packageName, appInfo, "true")
-        }
-
-        val job = nimbus.applyLocalExperiments(::getString)
-        runBlocking {
-            job.join()
-        }
+        assertTrue(Glean.testIsExperimentActive("test-experiment"))
 
         assertEquals(1, handler.setValues?.size)
         assertEquals("42", handler.setValues?.get(0)?.enrollmentValue?.prefValue)
@@ -933,6 +935,8 @@ class NimbusTests {
             PrefUnenrollReason.FAILED_TO_SET,
         )
 
+        assertFalse(Glean.testIsExperimentActive("test-experiment"))
+
         assertNotNull(events)
         assertEquals(1, events!!.size)
         assertEquals(EnrollmentChangeEventType.DISQUALIFICATION, events[0].change)

Original file line number	Diff line number	Diff line change
`@@ -120,7 +120,7 @@ abstract class AbstractNimbusBuilder<T : NimbusInterface>(val context: Context)`
`120`	`120`	`val job = if (initialExperiments != null && (isFirstRun \|\| isLocalBuild())) {`
`121`	`121`	`applyLocalExperiments(initialExperiments!!)`
`122`	`122`	`} else {`
`123`		`- applyPendingExperiments()`
	`123`	`+ applyPendingExperiments(initial = true)`
`124`	`124`	`}`
`125`	`125`
`126`	`126`	`// We always want initialize Nimbus to happen ASAP and before any features (engine/UI)`