diff --git a/CHANGELOG.md b/CHANGELOG.md index 82421279e34..3e0ff72be11 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,11 @@ - Added `list_credit_card_candidates()`, returning `CreditCardCandidate`: a card minus its encrypted fields, with `cc_number_last_4` and `has_cvv` in their place. Like `list_candidates()` in logins it never touches the key, so an application can list cards without a primary password prompt and call `get_credit_card()` for the single card whose number or CVV it needs. ([bug 2080209](https://bugzilla.mozilla.org/show_bug.cgi?id=2080209)) - Credit cards can store an optional CVV, encrypted like the number and handled transparently: `UpdatableCreditCardFields.cc_cvv` (cleartext in, defaults to none) and `CreditCard.cc_cvv` (decrypted out). A v6 schema upgrade adds the column; existing rows simply have no CVV. The CVV syncs as a new optional `cc-cvv` payload field; records written by older clients round-trip it through the mirror's unknown-fields handling, so their edits do not drop it. ([bug 2076154](https://bugzilla.mozilla.org/show_bug.cgi?id=2076154)) +### Nimbus +- Disabling experiments or rollouts will submit an event. [#7656](https://github.com/mozilla/application-services/pull/7656) +- If requesting experiment or rollout participation on Android throws, the getter will return true instead of throwing. This aligns the Android behaviour with iOS. [#7656](https://github.com/mozilla/application-services/pull/7656) +- Enabling experiments or rollouts while they are already enabled no longer performs an update. [#7656](https://github.com/mozilla/application-services/pull/7656) + ### Search - Added APIs to `SearchEngineSelector` for a new `search-config-v3` collection. 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 index 5a7aaab557d..b3515f486b1 100644 --- 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 @@ -164,12 +164,23 @@ open class Nimbus( override fun submitTargetingContext() { org.mozilla.experiments.nimbus.GleanMetrics.Pings.nimbusTargetingContext.submit() } + + override fun recordRolloutsOptOut() { + NimbusEvents.rolloutsOptOut.record() + } + + override fun recordStudiesOptOut() { + NimbusEvents.studiesOptOut.record() + } } private val nimbusClient: NimbusClientInterface override var experimentParticipation: Boolean - get() = nimbusClient.getExperimentParticipation() + get() = withCatchAll("getExperimentParticipation") { + nimbusClient.getExperimentParticipation() + } ?: true + set(active) { dbScope.launch { setExperimentParticipationOnThisThread(active) @@ -177,7 +188,10 @@ open class Nimbus( } override var rolloutParticipation: Boolean - get() = nimbusClient.getRolloutParticipation() + get() = withCatchAll("getExperimentParticipation") { + nimbusClient.getRolloutParticipation() + } ?: true + set(active) { dbScope.launch { setRolloutParticipationOnThisThread(active) 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 index 9ef624346fe..60227b07209 100644 --- 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 @@ -1118,6 +1118,14 @@ class NimbusTests { nimbus.getFeatureUpdateDispatcher()?.addCallback("foo", { calls++ }) nimbus.setExperimentParticipationOnThisThread(false) + assertEquals( + 0, + NimbusEvents.rolloutsOptOut.testGetValue("events")?.size ?: 0, + ) + assertEquals( + 1, + NimbusEvents.studiesOptOut.testGetValue("events")?.size ?: 0, + ) assertCalls(1) // The experiment won't re-enroll so there is no change. @@ -1125,6 +1133,14 @@ class NimbusTests { assertCalls(1) nimbus.setRolloutParticipationOnThisThread(false) + assertEquals( + 1, + NimbusEvents.rolloutsOptOut.testGetValue("events")?.size ?: 0, + ) + assertEquals( + 1, + NimbusEvents.studiesOptOut.testGetValue("events")?.size ?: 0, + ) assertCalls(2) // Rollout do re-enroll, so there is a change here. @@ -1132,6 +1148,57 @@ class NimbusTests { assertCalls(3) } + @Test + fun `setExperimentParticipation et al trigger telemetry`() { + fun getRolloutsOptOutCount() = NimbusEvents.rolloutsOptOut.testGetValue("events")?.size ?: 0 + fun getStudiesOptOutCount() = NimbusEvents.studiesOptOut.testGetValue("events")?.size ?: 0 + + assertTrue(nimbus.experimentParticipation) + assertTrue(nimbus.rolloutParticipation) + assertEquals(0, getRolloutsOptOutCount()) + assertEquals(0, getStudiesOptOutCount()) + + nimbus.setRolloutParticipationOnThisThread(true) + assertEquals(0, getRolloutsOptOutCount()) + assertEquals(0, getStudiesOptOutCount()) + + nimbus.setRolloutParticipationOnThisThread(false) + assertEquals(1, getRolloutsOptOutCount()) + assertEquals(0, getStudiesOptOutCount()) + + nimbus.setRolloutParticipationOnThisThread(false) + assertEquals(1, getRolloutsOptOutCount()) + assertEquals(0, getStudiesOptOutCount()) + + nimbus.setRolloutParticipationOnThisThread(true) + assertEquals(1, getRolloutsOptOutCount()) + assertEquals(0, getStudiesOptOutCount()) + + nimbus.setRolloutParticipationOnThisThread(false) + assertEquals(2, getRolloutsOptOutCount()) + assertEquals(0, getStudiesOptOutCount()) + + nimbus.setExperimentParticipationOnThisThread(true) + assertEquals(2, getRolloutsOptOutCount()) + assertEquals(0, getStudiesOptOutCount()) + + nimbus.setExperimentParticipationOnThisThread(false) + assertEquals(2, getRolloutsOptOutCount()) + assertEquals(1, getStudiesOptOutCount()) + + nimbus.setExperimentParticipationOnThisThread(false) + assertEquals(2, getRolloutsOptOutCount()) + assertEquals(1, getStudiesOptOutCount()) + + nimbus.setExperimentParticipationOnThisThread(true) + assertEquals(2, getRolloutsOptOutCount()) + assertEquals(1, getStudiesOptOutCount()) + + nimbus.setExperimentParticipationOnThisThread(false) + assertEquals(2, getRolloutsOptOutCount()) + assertEquals(2, getStudiesOptOutCount()) + } + @Test fun `resetTelemetryIdentifiers triggers callbacks`() { val nimbus = createNimbus() diff --git a/components/nimbus/metrics.yaml b/components/nimbus/metrics.yaml index e162cd4c69c..8c24e21ce2f 100644 --- a/components/nimbus/metrics.yaml +++ b/components/nimbus/metrics.yaml @@ -367,6 +367,41 @@ nimbus_events: - chumphreys@mozilla.com - project-nimbus@mozilla.com expires: never + + rollouts_opt_out: + type: event + description: > + Emitted when a user opts out of rollouts (remote improvements). + + bugs: + - https://bugzilla.mozilla.org/show_bug.cgi?id=2081293 + data_reviews: + - https://github.com/mozilla/application-services/pull/7656#pullrequestreview-5522772594 + + data_sensitivity: + - interaction + notification_emails: + - beth@mozilla.com + - project-nimbus@mozilla.com + expires: never + + studies_opt_out: + type: event + description: > + Emitted when a user opts out of studies (experiments). + + bugs: + - https://bugzilla.mozilla.org/show_bug.cgi?id=2081305 + data_reviews: + - https://github.com/mozilla/application-services/pull/7656#pullrequestreview-5522772594 + + data_sensitivity: + - interaction + notification_emails: + - beth@mozilla.com + - project-nimbus@mozilla.com + expires: never + nimbus_health: cache_not_ready_for_feature: type: event diff --git a/components/nimbus/src/metrics.rs b/components/nimbus/src/metrics.rs index ab97ddd3ec1..350b2c2ba82 100644 --- a/components/nimbus/src/metrics.rs +++ b/components/nimbus/src/metrics.rs @@ -92,6 +92,10 @@ mod detail { fn record_malformed_feature_config(&self, event: MalformedFeatureConfigExtraDef); fn submit_targeting_context(&self); + + fn record_rollouts_opt_out(&self); + + fn record_studies_opt_out(&self); } #[derive(Clone)] diff --git a/components/nimbus/src/nimbus.udl b/components/nimbus/src/nimbus.udl index 65900c3eac0..338e85c9fb9 100644 --- a/components/nimbus/src/nimbus.udl +++ b/components/nimbus/src/nimbus.udl @@ -113,6 +113,10 @@ interface MetricsHandler { void record_malformed_feature_config(MalformedFeatureConfigExtraDef event); void submit_targeting_context(); + + void record_rollouts_opt_out(); + + void record_studies_opt_out(); }; dictionary DatabaseLoadExtraDef { diff --git a/components/nimbus/src/stateful/enrollment.rs b/components/nimbus/src/stateful/enrollment.rs index b4cc3b6fe61..9f38eabfb27 100644 --- a/components/nimbus/src/stateful/enrollment.rs +++ b/components/nimbus/src/stateful/enrollment.rs @@ -439,14 +439,22 @@ pub fn set_experiment_participation( db: &Database, writer: &mut Writer, opt_in: bool, -) -> Result<()> { +) -> Result { + let old_value = + get_experiment_participation(db, writer).unwrap_or(DEFAULT_EXPERIMENT_PARTICIPATION); + let store = db.get_store(StoreId::Meta); - store.put(writer, DB_KEY_EXPERIMENT_PARTICIPATION, &opt_in) + store.put(writer, DB_KEY_EXPERIMENT_PARTICIPATION, &opt_in)?; + + Ok(old_value) } -pub fn set_rollout_participation(db: &Database, writer: &mut Writer, opt_in: bool) -> Result<()> { +pub fn set_rollout_participation(db: &Database, writer: &mut Writer, opt_in: bool) -> Result { + let old_value = get_rollout_participation(db, writer).unwrap_or(DEFAULT_ROLLOUT_PARTICIPATION); let store = db.get_store(StoreId::Meta); - store.put(writer, DB_KEY_ROLLOUT_PARTICIPATION, &opt_in) + store.put(writer, DB_KEY_ROLLOUT_PARTICIPATION, &opt_in)?; + + Ok(old_value) } /// Reset unique identifiers in response to application-level telemetry reset. diff --git a/components/nimbus/src/stateful/nimbus_client.rs b/components/nimbus/src/stateful/nimbus_client.rs index 4779dcdc7c0..ba71530fdf0 100644 --- a/components/nimbus/src/stateful/nimbus_client.rs +++ b/components/nimbus/src/stateful/nimbus_client.rs @@ -286,15 +286,24 @@ impl NimbusClient { let db = self.db()?; let mut writer = db.write()?; let mut state = self.mutable_state.lock().unwrap(); - set_experiment_participation(db, &mut writer, user_participating)?; - - let existing_experiments: Vec = - db.get_store(StoreId::Experiments).collect_all(&writer)?; - let events = self.evolve_experiments(db, &mut writer, &mut state, &existing_experiments)?; - let res = self.end_initialize(db, writer, &mut state); - self.record_enrollment_status_telemetry(&mut state)?; - res?; - Ok(events) + let was_participating = set_experiment_participation(db, &mut writer, user_participating)?; + + if was_participating != user_participating { + if !user_participating { + self.metrics_handler.record_studies_opt_out(); + } + + let existing_experiments: Vec = + db.get_store(StoreId::Experiments).collect_all(&writer)?; + let events = + self.evolve_experiments(db, &mut writer, &mut state, &existing_experiments)?; + let res = self.end_initialize(db, writer, &mut state); + self.record_enrollment_status_telemetry(&mut state)?; + res?; + Ok(events) + } else { + Ok(vec![]) + } } pub fn set_rollout_participation( @@ -304,15 +313,24 @@ impl NimbusClient { let db = self.db()?; let mut writer = db.write()?; let mut state = self.mutable_state.lock().unwrap(); - set_rollout_participation(db, &mut writer, user_participating)?; - - let existing_experiments: Vec = - db.get_store(StoreId::Experiments).collect_all(&writer)?; - let events = self.evolve_experiments(db, &mut writer, &mut state, &existing_experiments)?; - let res = self.end_initialize(db, writer, &mut state); - self.record_enrollment_status_telemetry(&mut state)?; - res?; - Ok(events) + let was_participating = set_rollout_participation(db, &mut writer, user_participating)?; + + if was_participating != user_participating { + if !user_participating { + self.metrics_handler.record_rollouts_opt_out(); + } + + let existing_experiments: Vec = + db.get_store(StoreId::Experiments).collect_all(&writer)?; + let events = + self.evolve_experiments(db, &mut writer, &mut state, &existing_experiments)?; + let res = self.end_initialize(db, writer, &mut state); + self.record_enrollment_status_telemetry(&mut state)?; + res?; + Ok(events) + } else { + Ok(vec![]) + } } pub fn get_active_experiments(&self) -> Result> { diff --git a/components/nimbus/src/tests/helpers.rs b/components/nimbus/src/tests/helpers.rs index 41db485398c..af4cdc594f1 100644 --- a/components/nimbus/src/tests/helpers.rs +++ b/components/nimbus/src/tests/helpers.rs @@ -801,6 +801,8 @@ mod detail { pub exposures: Vec, pub malformeds: Vec, pub submit_targeting_context_calls: u64, + pub rollouts_opt_out_events: u64, + pub studies_opt_out_events: u64, } impl TestMetrics { @@ -867,6 +869,16 @@ mod detail { let mut state = self.state.lock().unwrap(); state.submit_targeting_context_calls += 1; } + + fn record_rollouts_opt_out(&self) { + let mut state = self.state.lock().unwrap(); + state.rollouts_opt_out_events += 1 + } + + fn record_studies_opt_out(&self) { + let mut state = self.state.lock().unwrap(); + state.studies_opt_out_events += 1 + } } } diff --git a/components/nimbus/src/tests/stateful/test_nimbus.rs b/components/nimbus/src/tests/stateful/test_nimbus.rs index 5a36fd33e29..37d68326771 100644 --- a/components/nimbus/src/tests/stateful/test_nimbus.rs +++ b/components/nimbus/src/tests/stateful/test_nimbus.rs @@ -906,7 +906,7 @@ fn event_store_on_targeting_attributes_is_updated_after_an_event_is_recorded() - "last_modified": 1_602_197_324_372i64, } ]}))?; - client.set_experiments_locally(experiment_json)?; + client.set_experiments_locally(experiment_json.clone())?; client.apply_pending_experiments()?; let active_experiments = client.get_active_experiments()?; @@ -914,7 +914,8 @@ fn event_store_on_targeting_attributes_is_updated_after_an_event_is_recorded() - client.record_event("app.foregrounded".to_string(), 1)?; - client.set_experiment_participation(true)?; + client.set_experiments_locally(experiment_json)?; + client.apply_pending_experiments()?; // The number of non-zero days in our event store is 2, so the first experiment // should be applied, but the second experiment will not be. diff --git a/components/nimbus/tests/common.rs b/components/nimbus/tests/common.rs index 2b75ca58a55..da80a38f06d 100644 --- a/components/nimbus/tests/common.rs +++ b/components/nimbus/tests/common.rs @@ -45,10 +45,17 @@ impl MetricsHandler for NoopMetricsHandler { // do nothing } - #[cfg(feature = "stateful")] fn submit_targeting_context(&self) { // do nothing } + + fn record_rollouts_opt_out(&self) { + // do nothing + } + + fn record_studies_opt_out(&self) { + // do nothing + } } #[cfg(not(feature = "stateful"))] diff --git a/examples/nimbus/src/main.rs b/examples/nimbus/src/main.rs index 62600e9b3cc..b94509b41d0 100644 --- a/examples/nimbus/src/main.rs +++ b/examples/nimbus/src/main.rs @@ -140,6 +140,14 @@ fn main() -> Result<()> { fn submit_targeting_context(&self) { // do nothing } + + fn record_rollouts_opt_out(&self) { + // do nothing + } + + fn record_studies_opt_out(&self) { + // do nothing + } } // We set the logging level to be `warn` here, meaning that only