Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update rollouts to re-evaluate enrollment if they are disqualified for targeting or bucketing #5716

Merged
merged 4 commits into from Jul 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Expand Up @@ -21,7 +21,7 @@

### 🦊 What's Changed 🦊

- When a rollout audience size decreases, the enrollment is re-evaluated and the client is unenrolled if no longer in the bucket. ([#5687](https://github.com/mozilla/application-services/pull/5687)).
- When a rollout audience size changes, the enrollment is re-evaluated and the client is un-enrolled if no longer in the bucket or re-enrolled if they had previously been disqualified from bucketing. ([#5687](https://github.com/mozilla/application-services/pull/5687), [#5716](https://github.com/mozilla/application-services/pull/5716)).
- The record of enrollment will be available in the new `enrollments` targeting attribute.
- Add `enrollments` value to `TargetingAttributes` — it is a set of strings containing all enrollments, past and present ([#5685](https://github.com/mozilla/application-services/pull/5685)).
- _Note: This change only applies to stateful uses of the Nimbus SDK, e.g. mobile_
Expand Down
33 changes: 28 additions & 5 deletions components/nimbus/src/enrollment.rs
Expand Up @@ -71,6 +71,8 @@ pub enum DisqualifiedReason {
OptOut,
/// The targeting has changed for an experiment.
NotTargeted,
/// The bucketing has changed for an experiment.
NotSelected,
}

// Every experiment has an ExperimentEnrollment, even when we aren't enrolled.
Expand Down Expand Up @@ -167,7 +169,7 @@ impl ExperimentEnrollment {
targeting_helper: &NimbusTargetingHelper,
out_enrollment_events: &mut Vec<EnrollmentChangeEvent>,
) -> Result<Self> {
Ok(match self.status {
Ok(match &self.status {
EnrollmentStatus::NotEnrolled { .. } | EnrollmentStatus::Error { .. } => {
if !is_user_participating || updated_experiment.is_enrollment_paused {
self.clone()
Expand All @@ -190,6 +192,7 @@ impl ExperimentEnrollment {
updated_enrollment
}
}

EnrollmentStatus::Enrolled {
ref branch,
ref reason,
Expand Down Expand Up @@ -240,10 +243,12 @@ impl ExperimentEnrollment {
EnrollmentStatus::NotEnrolled {
reason: NotEnrolledReason::NotSelected,
} => {
// In the case of a rollout being scaled back, we should end with WasEnrolled.
// In the case of a rollout being scaled back, we should be disqualified with NotSelected.
//
self.on_experiment_ended(out_enrollment_events)
.ok_or_else(|| NimbusError::InternalError("An unexpected None happened while ending an experiment prematurely"))?
let updated_enrollment =
self.disqualify_from_enrolled(DisqualifiedReason::NotSelected);
out_enrollment_events.push(updated_enrollment.get_change_event());
updated_enrollment
}
EnrollmentStatus::NotEnrolled { .. }
| EnrollmentStatus::Enrolled { .. }
Expand All @@ -255,6 +260,7 @@ impl ExperimentEnrollment {
EnrollmentStatus::Disqualified {
ref branch,
enrollment_id,
reason,
..
} => {
if !is_user_participating {
Expand All @@ -266,10 +272,26 @@ impl ExperimentEnrollment {
slug: self.slug.clone(),
status: EnrollmentStatus::Disqualified {
reason: DisqualifiedReason::OptOut,
enrollment_id,
enrollment_id: *enrollment_id,
branch: branch.clone(),
},
}
} else if updated_experiment.is_rollout
jeddai marked this conversation as resolved.
Show resolved Hide resolved
&& matches!(
reason,
DisqualifiedReason::NotSelected | DisqualifiedReason::NotTargeted,
)
{
let evaluated_enrollment = evaluate_enrollment(
nimbus_id,
available_randomization_units,
updated_experiment,
targeting_helper,
)?;
match evaluated_enrollment.status {
EnrollmentStatus::Enrolled { .. } => evaluated_enrollment,
_ => self.clone(),
}
} else {
self.clone()
}
Expand Down Expand Up @@ -432,6 +454,7 @@ impl ExperimentEnrollment {
enrollment_id,
branch,
match reason {
DisqualifiedReason::NotSelected => Some("bucketing"),
DisqualifiedReason::NotTargeted => Some("targeting"),
DisqualifiedReason::OptOut => Some("optout"),
DisqualifiedReason::Error => Some("error"),
Expand Down
2 changes: 1 addition & 1 deletion components/nimbus/src/nimbus_client.rs
Expand Up @@ -325,7 +325,7 @@ impl NimbusClient {
is_enrolled_set.insert(ee.slug.clone());
all_enrolled_set.insert(ee.slug.clone());
}
EnrollmentStatus::WasEnrolled { .. } => {
EnrollmentStatus::WasEnrolled { .. } | EnrollmentStatus::Disqualified { .. } => {
all_enrolled_set.insert(ee.slug.clone());
}
_ => {}
Expand Down
49 changes: 49 additions & 0 deletions components/nimbus/src/tests/helpers.rs
Expand Up @@ -9,6 +9,7 @@ use crate::{
AppContext, EnrollmentStatus, Experiment, FeatureConfig, NimbusTargetingHelper,
TargetingAttributes,
};
use serde::Serialize;
use serde_json::{json, Value};
use std::collections::HashSet;
use uuid::Uuid;
Expand Down Expand Up @@ -385,3 +386,51 @@ pub fn get_multi_feature_experiment(
pub fn no_coenrolling_features() -> HashSet<&'static str> {
Default::default()
}

#[cfg_attr(not(feature = "stateful"), allow(unused))]
pub fn to_local_experiments_string<S>(experiments: &[S]) -> crate::Result<String>
where
S: Serialize,
{
Ok(serde_json::to_string(&json!({ "data": experiments }))?)
}

#[cfg_attr(not(feature = "stateful"), allow(unused))]
pub fn get_targeted_experiment(slug: &str, targeting: &str) -> serde_json::Value {
json!({
"schemaVersion": "1.0.0",
"slug": slug,
"endDate": null,
"featureIds": ["some-feature-1"],
"branches": [
{
"slug": "control",
"ratio": 1
},
{
"slug": "treatment",
"ratio": 1
}
],
"channel": "nightly",
"probeSets": [],
"startDate": null,
"appName": "fenix",
"appId": "org.mozilla.fenix",
"bucketConfig": {
"count": 10000,
"start": 0,
"total": 10000,
"namespace": "secure-gold",
"randomizationUnit": "nimbus_id"
},
"targeting": targeting,
"userFacingName": "test experiment",
"referenceBranch": "control",
"isEnrollmentPaused": false,
"proposedEnrollment": 7,
"userFacingDescription": "This is a test experiment for testing purposes.",
"id": "secure-copper",
"last_modified": 1_602_197_324_372i64,
})
}