Backed out 5 changesets (bug 1955169) for causing failures at test_nimbusTelemetry.js. CLOSED TREE

Backed out changeset c4164da0418a (bug 1955169)
Backed out changeset aec590ec0dcc (bug 1955169)
Backed out changeset 527cbe48536b (bug 1955169)
Backed out changeset dec22291ce40 (bug 1955169)
Backed out changeset 7523c66cf741 (bug 1955169)
This commit is contained in:
Butkovits Atila
2025-04-02 23:07:11 +03:00
parent 8c4df24840
commit aeba1d05c8
26 changed files with 332 additions and 878 deletions

View File

@@ -297,7 +297,7 @@ add_task(async function test_forceEnrollUpdatesMessages() {
await assertMessageInState("xman_test_message");
await ExperimentManager.unenroll(`optin-${experiment.slug}`);
await ExperimentManager.unenroll(`optin-${experiment.slug}`, "cleanup");
await SpecialPowers.popPrefEnv();
await cleanup();
});

View File

@@ -67,7 +67,7 @@ add_setup(async () => {
await manager.onStartup();
await manager.store.addEnrollment(ExperimentFakes.experiment("foo"));
manager.unenroll("foo");
manager.unenroll("foo", "some-reason");
await manager.store.addEnrollment(
ExperimentFakes.experiment("bar", { active: false })
);
@@ -76,7 +76,7 @@ add_setup(async () => {
);
manager.store.addEnrollment(ExperimentFakes.rollout("rol1"));
manager.unenroll("rol1");
manager.unenroll("rol1", "some-reason");
manager.store.addEnrollment(ExperimentFakes.rollout("rol2"));
});

View File

@@ -18,7 +18,7 @@ add_task(async function test_SUBMIT_ONBOARDING_OPT_OUT_PING() {
await manager.onStartup();
await manager.store.addEnrollment(ExperimentFakes.experiment("foo"));
manager.unenroll("foo");
manager.unenroll("foo", "some-reason");
await manager.store.addEnrollment(
ExperimentFakes.experiment("bar", { active: false })
);
@@ -27,7 +27,7 @@ add_task(async function test_SUBMIT_ONBOARDING_OPT_OUT_PING() {
);
manager.store.addEnrollment(ExperimentFakes.rollout("rol1"));
manager.unenroll("rol1");
manager.unenroll("rol1", "some-reason");
manager.store.addEnrollment(ExperimentFakes.rollout("rol2"));
let { promise, resolve } = Promise.withResolvers();

View File

@@ -16,7 +16,6 @@ ChromeUtils.defineESModuleGetters(lazy, {
RemoteSettings: "resource://services-settings/remote-settings.sys.mjs",
RemoteSettingsExperimentLoader:
"resource://nimbus/lib/RemoteSettingsExperimentLoader.sys.mjs",
UnenrollmentCause: "resource://nimbus/lib/ExperimentManager.sys.mjs",
});
ChromeUtils.defineLazyGetter(lazy, "log", () => {
@@ -593,9 +592,7 @@ export class _ExperimentFeature {
);
if (missingIds?.size) {
throw new ExperimentLocalizationError(
lazy.NimbusTelemetry.ValidationFailureReason.L10N_MISSING_ENTRY
);
throw new ExperimentLocalizationError("l10n-missing-entry");
}
return result;
@@ -648,9 +645,7 @@ export class _ExperimentFeature {
missingIds.add(value.id);
break;
} else {
throw new ExperimentLocalizationError(
lazy.NimbusTelemetry.ValidationFailureReason.L10N_MISSING_ENTRY
);
throw new ExperimentLocalizationError("l10n-missing-entry");
}
}
@@ -686,12 +681,7 @@ export class _ExperimentFeature {
(typeof enrollment.localizations[locale] !== "object" ||
enrollment.localizations[locale] === null)
) {
ExperimentAPI._manager._unenroll(
enrollment,
lazy.UnenrollmentCause.fromReason(
lazy.NimbusTelemetry.UnenrollReason.L10N_MISSING_LOCALE
)
);
ExperimentAPI._manager.unenroll(enrollment.slug, "l10n-missing-locale");
return undefined;
}
@@ -708,10 +698,7 @@ export class _ExperimentFeature {
} catch (e) {
// This should never happen.
if (e instanceof ExperimentLocalizationError) {
ExperimentAPI._manager._unenroll(
enrollment,
lazy.UnenrollmentCause.fromReason(e.reason)
);
ExperimentAPI._manager.unenroll(enrollment.slug, e.reason);
} else {
throw e;
}

View File

@@ -6,8 +6,6 @@ const lazy = {};
ChromeUtils.defineESModuleGetters(lazy, {
ExperimentAPI: "resource://nimbus/ExperimentAPI.sys.mjs",
NimbusTelemetry: "resource://nimbus/lib/Telemetry.sys.mjs",
UnenrollmentCause: "resource://nimbus/lib/ExperimentManager.sys.mjs",
});
ChromeUtils.defineLazyGetter(lazy, "log", () => {
@@ -97,12 +95,7 @@ export class FirefoxLabs {
}
try {
lazy.ExperimentAPI._manager.unenroll(
slug,
lazy.UnenrollmentCause.fromReason(
lazy.NimbusTelemetry.UnenrollReason.LABS_OPT_OUT
)
);
lazy.ExperimentAPI._manager.unenroll(slug, "labs-opt-out");
} catch (e) {
lazy.log.error(`unenroll: failed to unenroll from ${slug}`, e);
}

View File

@@ -36,8 +36,6 @@ const STUDIES_OPT_OUT_PREF = "app.shield.optoutstudies.enabled";
const STUDIES_ENABLED_CHANGED = "nimbus:studies-enabled-changed";
const FORCE_ENROLLMENT_SOURCE = "force-enrollment";
function featuresCompat(branch) {
if (!branch || (!branch.feature && !branch.features)) {
return [];
@@ -57,68 +55,6 @@ function getFeatureFromBranch(branch, featureId) {
);
}
export const UnenrollmentCause = {
fromCheckRecipeResult(result) {
const { UnenrollReason } = lazy.NimbusTelemetry;
let reason;
if (result.ok) {
switch (result.status) {
case lazy.MatchStatus.NOT_SEEN:
reason = UnenrollReason.RECIPE_NOT_SEEN;
break;
case lazy.MatchStatus.NO_MATCH:
reason = UnenrollReason.TARGETING_MISMATCH;
break;
case lazy.MatchStatus.TARGETING_ONLY:
reason = UnenrollReason.BUCKETING;
break;
// TARGETING_AND_BUCKETING cannot cause unenrollment.
}
} else {
reason = result.reason;
}
return { reason };
},
fromReason(reason) {
return { reason };
},
ChangedPref(pref) {
return {
reason: lazy.NimbusTelemetry.UnenrollReason.CHANGED_PREF,
changedPref: pref,
};
},
PrefFlipsConflict(conflictingSlug) {
return {
reason: lazy.NimbusTelemetry.UnenrollReason.PREF_FLIPS_CONFLICT,
conflictingSlug,
};
},
PrefFlipsFailed(prefName, prefType) {
return {
reason: lazy.NimbusTelemetry.UnenrollReason.PREF_FLIPS_FAILED,
prefName,
prefType,
};
},
Unknown() {
return {
reason: lazy.NimbusTelemetry.UnenrollReason.UNKNOWN,
};
},
};
/**
* A module for processes Experiment recipes, choosing and storing enrollment state,
* and sending experiment-related Telemetry.
@@ -324,39 +260,19 @@ export class _ExperimentManager {
return;
}
switch (result.status) {
case lazy.MatchStatus.ENROLLMENT_PAUSED:
if (result.status === lazy.MatchStatus.TARGETING_AND_BUCKETING) {
const enrollment = await this.enroll(recipe, source);
if (enrollment) {
lazy.NimbusTelemetry.recordEnrollmentStatus({
slug: recipe.slug,
status: EnrollmentStatus.NOT_ENROLLED,
reason: EnrollmentStatusReason.ENROLLMENTS_PAUSED,
slug: enrollment.slug,
branch: enrollment.branch.slug,
status: EnrollmentStatus.ENROLLED,
reason: EnrollmentStatusReason.QUALIFIED,
});
break;
case lazy.MatchStatus.NO_MATCH:
lazy.NimbusTelemetry.recordEnrollmentStatus({
slug: recipe.slug,
status: EnrollmentStatus.NOT_ENROLLED,
reason: EnrollmentStatusReason.NOT_TARGETED,
});
break;
case lazy.MatchStatus.TARGETING_ONLY:
lazy.NimbusTelemetry.recordEnrollmentStatus({
slug: recipe.slug,
status: EnrollmentStatus.NOT_ENROLLED,
reason: EnrollmentStatusReason.NOT_SELECTED,
});
break;
case lazy.MatchStatus.TARGETING_AND_BUCKETING:
await this.enroll(recipe, source);
break;
// This function will not be called with MatchStatus.NOT_SEEN --
// RemoteSettingsExperimentLoader will call updateEnrollment directly
// instead.
}
}
// TODO(bug 1955169): Record NotEnrolled enrollment status telemetry.
}
/**
@@ -511,18 +427,12 @@ export class _ExperimentManager {
slug,
lazy.NimbusTelemetry.EnrollmentFailureReason.NAME_CONFLICT
);
lazy.NimbusTelemetry.recordEnrollmentStatus({
slug,
status: lazy.NimbusTelemetry.EnrollmentStatus.NOT_ENROLLED,
reason: lazy.NimbusTelemetry.EnrollmentStatusReason.NAME_CONFLICT,
});
throw new Error(`An experiment with the slug "${slug}" already exists.`);
}
let storeLookupByFeature = recipe.isRollout
? this.store.getRolloutForFeature.bind(this.store)
: this.store.getExperimentForFeature.bind(this.store);
: this.store.hasExperimentForFeature.bind(this.store);
const userId = await this.getUserId(bucketConfig);
let branch;
@@ -551,9 +461,8 @@ export class _ExperimentManager {
}
const features = featuresCompat(branch);
for (const feature of features) {
const existingEnrollment = storeLookupByFeature(feature?.featureId);
if (existingEnrollment) {
for (let feature of features) {
if (storeLookupByFeature(feature?.featureId)) {
lazy.log.debug(
`Skipping enrollment for "${slug}" because there is an existing ${
recipe.isRollout ? "rollout" : "experiment"
@@ -563,12 +472,7 @@ export class _ExperimentManager {
slug,
lazy.NimbusTelemetry.EnrollmentFailureReason.FEATURE_CONFLICT
);
lazy.NimbusTelemetry.recordEnrollmentStatus({
slug,
status: lazy.NimbusTelemetry.EnrollmentStatus.NOT_ENROLLED,
reason: lazy.NimbusTelemetry.EnrollmentStatusReason.FEATURE_CONFLICT,
conflict_slug: existingEnrollment.slug,
});
// TODO (bug 1955170) Add enrollment status telemetry
return null;
}
}
@@ -615,7 +519,10 @@ export class _ExperimentManager {
for (const prefName of Object.keys(featureValue.prefs)) {
if (prefNames.has(prefName)) {
this._unenroll(enrollment, UnenrollmentCause.PrefFlipsConflict(slug));
this._unenroll(enrollment, {
reason: lazy.NimbusTelemetry.UnenrollReason.PREF_FLIPS_CONFLICT,
conflictingSlug: slug,
});
break;
}
}
@@ -681,7 +588,7 @@ export class _ExperimentManager {
return enrollment;
}
forceEnroll(recipe, branch) {
forceEnroll(recipe, branch, source = "force-enrollment") {
/**
* If we happen to be enrolled in an experiment for the same feature
* we need to unenroll from that experiment.
@@ -701,12 +608,7 @@ export class _ExperimentManager {
} found for the same feature ${feature.featureId}, unenrolling.`
);
this._unenroll(
enrollment,
UnenrollmentCause.fromReason(
lazy.NimbusTelemetry.UnenrollReason.FORCE_ENROLLMENT
)
);
this.unenroll(enrollment.slug, source);
}
}
@@ -719,7 +621,7 @@ export class _ExperimentManager {
slug,
},
branch,
FORCE_ENROLLMENT_SOURCE,
source,
{ force: true }
);
@@ -759,19 +661,26 @@ export class _ExperimentManager {
if (enrollment.active) {
if (!result.ok) {
// If the recipe failed validation then we must unenroll.
this._unenroll(
enrollment,
UnenrollmentCause.fromCheckRecipeResult(result)
);
this._unenroll(enrollment, { reason: result.reason });
lazy.NimbusTelemetry.recordEnrollmentStatus({
slug: enrollment.slug,
branch: enrollment.branch.slug,
status: EnrollmentStatus.DISQUALIFIED,
reason: EnrollmentStatusReason.ERROR,
error_string: result.reason,
});
return false;
}
if (result.status === lazy.MatchStatus.NOT_SEEN) {
// If the recipe was not present in the source we must unenroll.
this._unenroll(
enrollment,
UnenrollmentCause.fromCheckRecipeResult(result)
);
this._unenroll(enrollment, { reason: UnenrollReason.RECIPE_NOT_SEEN });
lazy.NimbusTelemetry.recordEnrollmentStatus({
slug: enrollment.slug,
branch: enrollment.branch.slug,
status: EnrollmentStatus.WAS_ENROLLED,
});
return false;
}
@@ -779,20 +688,30 @@ export class _ExperimentManager {
// Our branch has been removed so we must unenroll.
//
// This should not happen in practice.
this._unenroll(
enrollment,
UnenrollmentCause.fromReason(UnenrollReason.BRANCH_REMOVED)
);
this._unenroll(enrollment, { reason: UnenrollReason.BRANCH_REMOVED });
lazy.NimbusTelemetry.recordEnrollmentStatus({
slug: enrollment.slug,
branch: enrollment.branch.slug,
status: EnrollmentStatus.DISQUALIFIED,
reason: EnrollmentStatus.ERROR,
error_string: UnenrollReason.BRANCH_REMOVED,
});
return false;
}
if (result.status === lazy.MatchStatus.NO_MATCH) {
// If we have an active enrollment and we no longer match targeting we
// must unenroll.
this._unenroll(
enrollment,
UnenrollmentCause.fromCheckRecipeResult(result)
);
this._unenroll(enrollment, {
reason: UnenrollReason.TARGETING_MISMATCH,
});
lazy.NimbusTelemetry.recordEnrollmentStatus({
slug: enrollment.slug,
branch: enrollment.branch.slug,
status: EnrollmentStatus.DISQUALIFIED,
reason: EnrollmentStatusReason.NOT_TARGETED,
});
return false;
}
@@ -802,26 +721,20 @@ export class _ExperimentManager {
) {
// If we no longer fall in the bucketing allocation for this rollout we
// must unenroll.
this._unenroll(
enrollment,
UnenrollmentCause.fromCheckRecipeResult(result)
);
this._unenroll(enrollment, { reason: UnenrollReason.BUCKETING });
return false;
}
if (result.status === lazy.MatchStatus.TARGETING_AND_BUCKETING) {
lazy.NimbusTelemetry.recordEnrollmentStatus({
slug: enrollment.slug,
branch: enrollment.branch.slug,
status: EnrollmentStatus.ENROLLED,
reason: EnrollmentStatusReason.QUALIFIED,
});
}
// Either this recipe is not a rollout or both targeting matches and we
// are in the bucket allocation. For the former, we do not re-evaluate
// bucketing for experiments because the bucketing cannot change. For the
// latter, we are already active so we don't need to enroll.
lazy.NimbusTelemetry.recordEnrollmentStatus({
slug: enrollment.slug,
branch: enrollment.branch.slug,
status: EnrollmentStatus.ENROLLED,
reason: EnrollmentStatusReason.QUALIFIED,
});
return true;
}
@@ -839,7 +752,16 @@ export class _ExperimentManager {
// We only re-enroll if we match targeting and bucketing and the user did
// not purposefully opt out via about:studies.
lazy.log.debug(`Re-enrolling in rollout "${recipe.slug}`);
return !!(await this.enroll(recipe, source, { reenroll: true }));
const enrollment = await this.enroll(recipe, source, { reenroll: true });
if (enrollment) {
lazy.NimbusTelemetry.recordEnrollmentStatus({
slug: enrollment.slug,
branch: enrollment.branch.slug,
status: EnrollmentStatus.ENROLLED,
reason: EnrollmentStatusReason.QUALIFIED,
});
return true;
}
}
return false;
@@ -850,13 +772,13 @@ export class _ExperimentManager {
*
* @param {string} slug
* The slug of the enrollment to stop.
* @param {object?} cause
* The cause of this unenrollment. If not provided, "unknown" will be
* used for the unenrollment reason.
* @param {string} reason
* An optional reason for the unenrollment. If not provided, "unknown"
* will be used.
*
* See `UnenrollCause` for details.
* This will be reported in telemetry.
*/
unenroll(slug, cause) {
unenroll(slug, reason) {
const enrollment = this.store.get(slug);
if (!enrollment) {
lazy.NimbusTelemetry.recordUnenrollmentFailure(
@@ -867,7 +789,9 @@ export class _ExperimentManager {
return;
}
this._unenroll(enrollment, cause ?? UnenrollmentCause.Unknown());
this._unenroll(enrollment, {
reason: reason ?? lazy.NimbusTelemetry.UnenrollReason.UNKNOWN,
});
}
/**
@@ -876,18 +800,33 @@ export class _ExperimentManager {
* @param {Enrollment} enrollment
* The enrollment to end.
*
* @param {object} cause
* The cause of this unenrollment.
* @param {object} options
* @param {string} options.reason
* An optional reason for the unenrollment.
*
* See `UnenrollmentCause` for details.
* This will be reported in telemetry.
*
* @param {object?} options
* @param {object?} options.changedPref
* If the unenrollment was due to pref change, this will contain the
* information about the pref that changed.
*
* @param {boolean} options.duringRestore
* If true, this indicates that this was during the call to
* `_restoreEnrollmentPrefs`.
* @param {string} options.changedPref.name
* The name of the pref that caused the unenrollment.
*
* @param {string} options.changedPref.branch
* The branch that was changed ("user" or "default").
*/
_unenroll(enrollment, cause, { duringRestore = false } = {}) {
_unenroll(
enrollment,
{
reason = "unknown",
changedPref = undefined,
duringRestore = false,
conflictingSlug = undefined,
prefName = undefined,
prefType = undefined,
} = {}
) {
const { slug } = enrollment;
if (!enrollment.active) {
@@ -902,12 +841,22 @@ export class _ExperimentManager {
this.store.updateExperiment(slug, {
active: false,
unenrollReason: cause.reason,
unenrollReason: reason,
});
lazy.NimbusTelemetry.recordUnenrollment(enrollment, cause);
lazy.NimbusTelemetry.recordUnenrollment(
slug,
reason,
enrollment.branch.slug,
{
changedPref,
conflictingSlug,
prefType,
prefName,
}
);
this._unsetEnrollmentPrefs(enrollment, cause, { duringRestore });
this._unsetEnrollmentPrefs(enrollment, { changedPref, duringRestore });
lazy.log.debug(`Recipe unenrolled: ${slug}`);
}
@@ -924,21 +873,11 @@ export class _ExperimentManager {
* Unenroll from all active studies if user opts out.
*/
_handleStudiesOptOut() {
for (const enrollment of this.store.getAllActiveExperiments()) {
this._unenroll(
enrollment,
UnenrollmentCause.fromReason(
lazy.NimbusTelemetry.UnenrollReason.STUDIES_OPT_OUT
)
);
for (const { slug } of this.store.getAllActiveExperiments()) {
this.unenroll(slug, lazy.NimbusTelemetry.UnenrollReason.STUDIES_OPT_OUT);
}
for (const enrollment of this.store.getAllActiveRollouts()) {
this._unenroll(
enrollment,
UnenrollmentCause.fromReason(
lazy.NimbusTelemetry.UnenrollReason.STUDIES_OPT_OUT
)
);
for (const { slug } of this.store.getAllActiveRollouts()) {
this.unenroll(slug, lazy.NimbusTelemetry.UnenrollReason.STUDIES_OPT_OUT);
}
this.optInRecipes = [];
@@ -1182,20 +1121,26 @@ export class _ExperimentManager {
* Otherwise, it will be set to the original value from before the enrollment
* began.
*
* @param {object} enrollment
* @param {Enrollment} enrollment
* The enrollment that has ended.
*
* @param {object} cause
* The cause of the unenrollment.
*
* See `UnenrollmentCause` for details.
*
* @param {object} options
*
* @param {object?} options.changedPref
* If provided, a changed pref that caused the unenrollment that
* triggered unsetting these prefs. This is provided as to not
* overwrite a changed pref with an original value.
*
* @param {string} options.changedPref.name
* The name of the changed pref.
*
* @param {string} options.changedPref.branch
* The branch that was changed ("user" or "default").
*
* @param {boolean} options.duringRestore
* The unenrollment was caused during restore.
*/
_unsetEnrollmentPrefs(enrollment, cause, { duringRestore } = {}) {
_unsetEnrollmentPrefs(enrollment, { changedPref, duringRestore } = {}) {
if (!enrollment.prefs?.length) {
return;
}
@@ -1208,9 +1153,8 @@ export class _ExperimentManager {
this._removePrefObserver(pref.name, enrollment.slug);
if (
cause.reason === lazy.NimbusTelemetry.UnenrollReason.CHANGED_PREF &&
cause.changedPref.name === pref.name &&
cause.changedPref.branch === pref.branch
changedPref?.name == pref.name &&
changedPref.branch === pref.branch
) {
// Resetting the original value would overwite the pref the user just
// set. Skip it.
@@ -1280,8 +1224,6 @@ export class _ExperimentManager {
* enrollment has ended.
*/
_restoreEnrollmentPrefs(enrollment) {
const { UnenrollReason } = lazy.NimbusTelemetry;
const { branch, prefs = [], isRollout } = enrollment;
if (!prefs?.length) {
@@ -1295,11 +1237,10 @@ export class _ExperimentManager {
for (const { name, featureId, variable } of prefs) {
// If the feature no longer exists, unenroll.
if (!Object.hasOwn(lazy.NimbusFeatures, featureId)) {
this._unenroll(
enrollment,
UnenrollmentCause.fromReason(UnenrollReason.INVALID_FEATURE),
{ duringRestore: true }
);
this._unenroll(enrollment, {
reason: lazy.NimbusTelemetry.UnenrollReason.INVALID_FEATURE,
duringRestore: true,
});
return false;
}
@@ -1307,11 +1248,10 @@ export class _ExperimentManager {
// If the feature is missing a variable that set a pref, unenroll.
if (!Object.hasOwn(variables, variable)) {
this._unenroll(
enrollment,
UnenrollmentCause.fromReason(UnenrollReason.PREF_VARIABLE_MISSING),
{ duringRestore: true }
);
this._unenroll(enrollment, {
reason: lazy.NimbusTelemetry.UnenrollReason.PREF_VARIABLE_MISSING,
duringRestore: true,
});
return false;
}
@@ -1319,11 +1259,10 @@ export class _ExperimentManager {
// If the variable is no longer a pref-setting variable, unenroll.
if (!Object.hasOwn(variableDef, "setPref")) {
this._unenroll(
enrollment,
UnenrollmentCause.fromReason(UnenrollReason.PREF_VARIABLE_NO_LONGER),
{ duringRestore: true }
);
this._unenroll(enrollment, {
reason: lazy.NimbusTelemetry.UnenrollReason.PREF_VARIABLE_NO_LONGER,
duringRestore: true,
});
return false;
}
@@ -1334,11 +1273,10 @@ export class _ExperimentManager {
: variableDef.setPref;
if (prefName !== name) {
this._unenroll(
enrollment,
UnenrollmentCause.fromReason(UnenrollReason.PREF_VARIABLE_CHANGED),
{ duringRestore: true }
);
this._unenroll(enrollment, {
reason: lazy.NimbusTelemetry.UnenrollReason.PREF_VARIABLE_CHANGED,
duringRestore: true,
});
return false;
}
}
@@ -1559,7 +1497,10 @@ export class _ExperimentManager {
};
for (const enrollment of enrollments) {
this._unenroll(enrollment, UnenrollmentCause.ChangedPref(changedPref));
this._unenroll(enrollment, {
reason: lazy.NimbusTelemetry.UnenrollReason.CHANGED_PREF,
changedPref,
});
}
}
}

View File

@@ -6,8 +6,8 @@ const lazy = {};
ChromeUtils.defineESModuleGetters(lazy, {
NimbusFeatures: "resource://nimbus/ExperimentAPI.sys.mjs",
NimbusTelemetry: "resource://nimbus/lib/Telemetry.sys.mjs",
PrefUtils: "resource://normandy/lib/PrefUtils.sys.mjs",
UnenrollmentCause: "resource://nimbus/lib/ExperimentManager.sys.mjs",
});
const FEATURE_ID = "prefFlips";
@@ -91,10 +91,10 @@ export class PrefFlipsFeature {
}
for (const enrollment of toUnenroll) {
this.manager._unenroll(
enrollment,
lazy.UnenrollmentCause.PrefFlipsConflict(activeEnrollment.slug)
);
this.manager._unenroll(enrollment, {
reason: lazy.NimbusTelemetry.UnenrollReason.PREF_FLIPS_CONFLICT,
conflictingSlug: activeEnrollment.slug,
});
}
}
@@ -388,10 +388,11 @@ export class PrefFlipsFeature {
break;
}
this.manager._unenroll(
enrollment,
lazy.UnenrollmentCause.PrefFlipsFailed(pref, prefType)
);
this.manager._unenroll(enrollment, {
reason: lazy.NimbusTelemetry.UnenrollReason.PREF_FLIPS_FAILED,
prefName: pref,
prefType,
});
}
}

View File

@@ -99,7 +99,6 @@ const SCHEMAS = {
};
export const MatchStatus = Object.freeze({
ENROLLMENT_PAUSED: "ENROLLMENT_PAUSED",
NOT_SEEN: "NOT_SEEN",
NO_MATCH: "NO_MATCH",
TARGETING_ONLY: "TARGETING_ONLY",
@@ -806,11 +805,6 @@ export class EnrollmentsContext {
return CheckRecipeResult.UnsupportedFeatures(unsupportedFeatureIds);
}
if (recipe.isEnrollmentPaused) {
lazy.log.debug(`${recipe.slug}: enrollment paused`);
return CheckRecipeResult.Ok(MatchStatus.ENROLLMENT_PAUSED);
}
if (this.shouldCheckTargeting) {
const match = await this.checkTargeting(recipe);
@@ -867,6 +861,11 @@ export class EnrollmentsContext {
return result;
}
if (recipe.isEnrollmentPaused) {
lazy.log.debug(`${recipe.slug}: enrollment paused`);
return CheckRecipeResult.Ok(MatchStatus.TARGETING_ONLY);
}
if (!(await this.manager.isInBucketAllocation(recipe.bucketConfig))) {
lazy.log.debug(`${recipe.slug} did not match bucket sampling`);
return CheckRecipeResult.Ok(MatchStatus.TARGETING_ONLY);

View File

@@ -29,7 +29,6 @@ const EnrollmentStatus = Object.freeze({
});
const EnrollmentStatusReason = Object.freeze({
CHANGED_PREF: "ChangedPref",
QUALIFIED: "Qualified",
OPT_IN: "OptIn",
OPT_OUT: "OptOut",
@@ -37,9 +36,6 @@ const EnrollmentStatusReason = Object.freeze({
NOT_TARGETED: "NotTargeted",
ENROLLMENTS_PAUSED: "EnrollmentsPaused",
FEATURE_CONFLICT: "FeatureConflict",
FORCE_ENROLLMENT: "ForceEnrollment",
NAME_CONFLICT: "NameConflict",
PREF_FLIPS_CONFLICT: "PrefFlipsConflict",
ERROR: "Error",
});
@@ -66,9 +62,7 @@ const UnenrollReason = Object.freeze({
BRANCH_REMOVED: "branch-removed",
BUCKETING: "bucketing",
CHANGED_PREF: "changed-pref",
FORCE_ENROLLMENT: "force-enrollment",
INDIVIDUAL_OPT_OUT: "individual-opt-out",
LABS_OPT_OUT: "labs-opt-out",
PREF_FLIPS_CONFLICT: "prefFlips-conflict",
PREF_FLIPS_FAILED: "prefFlips-failed",
PREF_VARIABLE_CHANGED: "pref-variable-changed",
@@ -107,16 +101,6 @@ export const NimbusTelemetry = {
branch: enrollment.branch.slug,
experiment_type: enrollment.experimentType,
});
this.recordEnrollmentStatus({
slug: enrollment.slug,
branch: enrollment.branch.slug,
status: EnrollmentStatus.ENROLLED,
reason:
enrollment.force || enrollment.isFirefoxLabsOptIn
? EnrollmentStatusReason.OPT_IN
: EnrollmentStatusReason.QUALIFIED,
});
},
recordEnrollmentFailure(slug, reason) {
@@ -177,99 +161,57 @@ export const NimbusTelemetry = {
);
},
recordUnenrollment(enrollment, cause) {
lazy.TelemetryEnvironment.setExperimentInactive(enrollment.slug);
Services.fog.setExperimentInactive(enrollment.slug);
const legacyEventExtra = {
branch: enrollment.branch.slug,
reason: cause.reason,
};
const gleanEvent = {
experiment: enrollment.slug,
branch: enrollment.branch.slug,
reason: cause.reason,
};
switch (cause.reason) {
case UnenrollReason.CHANGED_PREF:
legacyEventExtra.changedPref = cause.changedPref.name;
gleanEvent.changed_pref = cause.changedPref.name;
break;
case UnenrollReason.PREF_FLIPS_CONFLICT:
legacyEventExtra.conflictingSlug = cause.conflictingSlug;
gleanEvent.conflicting_slug = cause.conflictingSlug;
break;
case UnenrollReason.PREF_FLIPS_FAILED:
legacyEventExtra.prefType = cause.prefType;
gleanEvent.pref_type = cause.prefType;
legacyEventExtra.prefName = cause.prefName;
gleanEvent.pref_name = cause.prefName;
break;
}
recordUnenrollment(
slug,
reason,
branchSlug,
{ changedPref, conflictingSlug, prefType, prefName } = {}
) {
lazy.TelemetryEnvironment.setExperimentInactive(slug);
Services.fog.setExperimentInactive(slug);
lazy.TelemetryEvents.sendEvent(
LegacyTelemetryEvents.UNENROLL,
LEGACY_TELEMETRY_EVENT_OBJECT,
enrollment.slug,
legacyEventExtra
slug,
Object.assign(
{
reason,
branch: branchSlug,
},
reason === UnenrollReason.CHANGED_PREF
? { changedPref: changedPref.name }
: {},
reason === UnenrollReason.PREF_FLIPS_CONFLICT
? { conflictingSlug }
: {},
reason === UnenrollReason.PREF_FLIPS_FAILED
? { prefType, prefName }
: {}
)
);
Glean.nimbusEvents.unenrollment.record(gleanEvent);
const enrollmentStatus = {
slug: enrollment.slug,
branch: enrollment.branch.slug,
};
switch (cause.reason) {
case UnenrollReason.BUCKETING:
enrollmentStatus.status = EnrollmentStatus.DISQUALIFIED;
enrollmentStatus.reason = EnrollmentStatusReason.NOT_SELECTED;
break;
case UnenrollReason.RECIPE_NOT_SEEN:
enrollmentStatus.status = EnrollmentStatus.WAS_ENROLLED;
break;
case UnenrollReason.TARGETING_MISMATCH:
enrollmentStatus.status = EnrollmentStatus.DISQUALIFIED;
enrollmentStatus.reason = EnrollmentStatusReason.NOT_TARGETED;
break;
case UnenrollReason.INDIVIDUAL_OPT_OUT:
case UnenrollReason.LABS_OPT_OUT:
case UnenrollReason.STUDIES_OPT_OUT:
enrollmentStatus.status = EnrollmentStatus.DISQUALIFIED;
enrollmentStatus.reason = EnrollmentStatusReason.OPT_OUT;
break;
case UnenrollReason.CHANGED_PREF:
enrollmentStatus.status = EnrollmentStatus.DISQUALIFIED;
enrollmentStatus.reason = EnrollmentStatusReason.CHANGED_PREF;
break;
case UnenrollReason.FORCE_ENROLLMENT:
enrollmentStatus.status = EnrollmentStatus.DISQUALIFIED;
enrollmentStatus.reason = EnrollmentStatusReason.FORCE_ENROLLMENT;
break;
case UnenrollReason.PREF_FLIPS_CONFLICT:
enrollmentStatus.status = EnrollmentStatus.DISQUALIFIED;
enrollmentStatus.reason = EnrollmentStatusReason.PREF_FLIPS_CONFLICT;
enrollmentStatus.conflict_slug = cause.conflictingSlug;
break;
default:
enrollmentStatus.status = EnrollmentStatus.DISQUALIFIED;
enrollmentStatus.reason = EnrollmentStatusReason.ERROR;
enrollmentStatus.error_string = cause.reason;
}
this.recordEnrollmentStatus(enrollmentStatus);
Glean.nimbusEvents.unenrollment.record(
Object.assign(
{
experiment: slug,
branch: branchSlug,
reason,
},
reason === UnenrollReason.CHANGED_PREF
? { changed_pref: changedPref.name }
: {},
reason === UnenrollReason.PREF_FLIPS_CONFLICT
? { conflicting_slug: conflictingSlug }
: {},
reason === UnenrollReason.PREF_FLIPS_FAILED
? {
pref_type: prefType,
pref_name: prefName,
}
: {}
)
);
},
recordUnenrollmentFailure(slug, reason) {

View File

@@ -963,10 +963,8 @@ nimbus_events:
description: If the enrollment hit a feature conflict, the slug of the conflicting experiment/rollout
bugs:
- https://bugzilla.mozilla.org/show_bug.cgi?id=1817481
- https://bugzilla.mozilla.org/show_bug.cgi?id=1955169
data_reviews:
- https://bugzilla.mozilla.org/show_bug.cgi?id=1817481
- https://bugzilla.mozilla.org/show_bug.cgi?id=1955169
data_sensitivity:
- technical
notification_emails:

View File

@@ -255,13 +255,13 @@ export const ExperimentFakes = {
}
return function doEnrollmentCleanup() {
manager.unenroll(enrollment.slug);
manager.unenroll(enrollment.slug, "cleanup");
manager.store._deleteForTests(enrollment.slug);
};
},
async cleanupAll(slugs, { manager = lazy.ExperimentAPI._manager } = {}) {
for (const slug of slugs) {
manager.unenroll(slug);
manager.unenroll(slug, "cleanup");
}
if (manager.store.getAllActiveExperiments().length) {

View File

@@ -63,7 +63,7 @@ add_task(async function test_experiment_enroll_unenroll_Telemetry() {
object: TELEMETRY_OBJECT,
value: experiment.slug,
extra: {
reason: "unknown",
reason: "cleanup",
branch: experiment.branch.slug,
},
},

View File

@@ -55,7 +55,7 @@ add_task(async function test_experimentEnrollment() {
Assert.ok(experiment.active, "Should be enrolled in the experiment");
ExperimentManager.unenroll(recipe.slug);
ExperimentManager.unenroll(recipe.slug, "mochitest-cleanup");
experiment = ExperimentAPI.getExperiment({
slug: recipe.slug,

View File

@@ -95,7 +95,7 @@ add_task(async function readyCallAfterStore_with_remote_value() {
Assert.ok(!feature.getVariable("enabled"), "Loads value from store");
manager.unenroll(MATCHING_ROLLOUT.slug);
manager.unenroll(MATCHING_ROLLOUT.slug, "test-cleanup");
sandbox.restore();
assertEmptyStore(manager.store);
@@ -146,7 +146,7 @@ add_task(async function update_remote_defaults_onUpdate() {
Assert.equal(stub.callCount, 1, "Called once for remote configs");
Assert.equal(stub.firstCall.args[1], "rollout-updated", "Correct reason");
manager.unenroll(MATCHING_ROLLOUT.slug);
manager.unenroll(MATCHING_ROLLOUT.slug, "test-cleanup");
sandbox.restore();
assertEmptyStore(manager.store);
@@ -224,7 +224,7 @@ add_task(async function update_remote_defaults_readyPromise() {
"Update called after enrollment processed."
);
manager.unenroll(MATCHING_ROLLOUT.slug);
manager.unenroll(MATCHING_ROLLOUT.slug, "test-cleanup");
assertEmptyStore(manager.store);
sandbox.restore();
});
@@ -246,7 +246,7 @@ add_task(async function update_remote_defaults_enabled() {
"Feature is disabled by remote configuration"
);
manager.unenroll(NON_MATCHING_ROLLOUT.slug);
manager.unenroll(NON_MATCHING_ROLLOUT.slug, "test-cleanup");
assertEmptyStore(manager.store);
sandbox.restore();
});
@@ -305,6 +305,6 @@ add_task(async function remote_isEarlyStartup_config() {
"nimbus.syncdefaultsstore.password-autocomplete"
);
manager.unenroll(rollout.slug);
manager.unenroll(rollout.slug, "test-cleanup");
assertEmptyStore(manager.store);
});

View File

@@ -59,7 +59,7 @@ add_task(async function test_add_to_store() {
);
Assert.equal(experiment.active, true, "should set .active = true");
manager.unenroll("foo");
manager.unenroll("foo", "test-cleanup");
assertEmptyStore(manager.store);
});
@@ -92,7 +92,7 @@ add_task(async function test_add_rollout_to_store() {
);
Assert.equal(experiment.isRollout, true, "should have .isRollout");
manager.unenroll("rollout-slug");
manager.unenroll("rollout-slug", "test-cleanup");
assertEmptyStore(manager.store);
});
@@ -219,7 +219,7 @@ add_task(async function test_setExperimentActive_recordEnrollment_called() {
"Glean.nimbusEvents.enrollment recorded with correct experiment type"
);
manager.unenroll("foo");
manager.unenroll("foo", "test-cleanup");
assertEmptyStore(manager.store);
sandbox.restore();
@@ -327,7 +327,7 @@ add_task(async function test_setRolloutActive_recordEnrollment_called() {
"Glean.nimbusEvents.enrollment recorded with correct experiment type"
);
manager.unenroll("rollout");
manager.unenroll("rollout", "test-cleanup");
assertEmptyStore(manager.store);
sandbox.restore();
@@ -346,20 +346,14 @@ add_task(async function test_failure_name_conflict() {
// Clear any pre-existing data in Glean
Services.fog.testResetFOG();
Services.fog.applyServerKnobsConfig(
JSON.stringify({
metrics_enabled: {
"nimbus_events.enrollment_status": true,
},
})
);
await manager.onStartup();
// Check that there aren't any Glean enroll_failed events yet
var failureEvents = Glean.nimbusEvents.enrollFailed.testGetValue("events");
Assert.equal(
Glean.nimbusEvents.enrollFailed.testGetValue("events"),
null,
undefined,
failureEvents,
"no Glean enroll_failed events before failure"
);
@@ -372,35 +366,29 @@ add_task(async function test_failure_name_conflict() {
"should throw if a conflicting experiment exists"
);
// Check that the Glean events were recorded.
Assert.deepEqual(
Glean.nimbusEvents.enrollFailed.testGetValue("events").map(ev => ev.extra),
[
{
experiment: "foo",
reason: "name-conflict",
},
],
"enrollFailed telemetry recorded correctly"
Assert.equal(
NimbusTelemetry.recordEnrollmentFailure.calledWith("foo", "name-conflict"),
true,
"should send failure telemetry if a conflicting experiment exists"
);
Assert.deepEqual(
Glean.nimbusEvents.enrollmentStatus
.testGetValue("events")
.map(ev => ev.extra),
[
{
slug: "foo",
status: "NotEnrolled",
reason: "NameConflict",
},
],
"enrollmentStatus telemetry recorded correctly"
// Check that the Glean enrollment event was recorded.
failureEvents = Glean.nimbusEvents.enrollFailed.testGetValue("events");
// We expect only one event
Assert.equal(1, failureEvents.length);
// And that one event matches the expected enrolled experiment
Assert.equal(
"foo",
failureEvents[0].extra.experiment,
"Glean.nimbusEvents.enroll_failed recorded with correct experiment slug"
);
Assert.equal(
"name-conflict",
failureEvents[0].extra.reason,
"Glean.nimbusEvents.enroll_failed recorded with correct reason"
);
Services.fog.testResetFOG();
manager.unenroll("foo");
manager.unenroll("foo", "test-cleanup");
assertEmptyStore(manager.store);
sandbox.restore();
@@ -480,7 +468,7 @@ add_task(async function test_failure_group_conflict() {
"Glean.nimbusEvents.enroll_failed recorded with correct reason"
);
manager.unenroll("foo");
manager.unenroll("foo", "test-cleanup");
assertEmptyStore(manager.store);
sandbox.restore();
@@ -547,7 +535,7 @@ add_task(async function test_rollout_failure_group_conflict() {
"Glean.nimbusEvents.enroll_failed recorded with correct reason"
);
manager.unenroll("rollout-recipe");
manager.unenroll("rollout-recipe", "test-cleanup");
assertEmptyStore(manager.store);
sandbox.restore();
@@ -708,17 +696,16 @@ add_task(async function enroll_in_reference_aw_experiment() {
// in prefs.
Assert.ok(prefValue.length < 3498, "Make sure we don't bloat the prefs");
manager.unenroll(recipe.slug);
manager.unenroll(recipe.slug, "enroll_in_reference_aw_experiment:cleanup");
assertEmptyStore(manager.store);
});
add_task(async function test_forceEnroll_cleanup() {
Services.fog.testResetFOG();
const manager = ExperimentFakes.manager();
const sandbox = sinon.createSandbox();
const existingRecipe = ExperimentFakes.recipe("foo", {
let unenrollStub = sandbox.spy(manager, "unenroll");
let existingRecipe = ExperimentFakes.recipe("foo", {
branches: [
{
slug: "treatment",
@@ -727,7 +714,7 @@ add_task(async function test_forceEnroll_cleanup() {
},
],
});
const forcedRecipe = ExperimentFakes.recipe("bar", {
let forcedRecipe = ExperimentFakes.recipe("bar", {
branches: [
{
slug: "treatment",
@@ -737,61 +724,34 @@ add_task(async function test_forceEnroll_cleanup() {
],
});
sandbox.spy(manager, "_unenroll");
await manager.onStartup();
await manager.enroll(existingRecipe, "test_forceEnroll_cleanup");
Services.fog.applyServerKnobsConfig(
JSON.stringify({
metrics_enabled: {
"nimbus_events.enrollment_status": true,
},
})
);
sandbox.spy(NimbusTelemetry, "setExperimentActive");
manager.forceEnroll(forcedRecipe, forcedRecipe.branches[0]);
Assert.deepEqual(
Glean.nimbusEvents.enrollmentStatus
.testGetValue("events")
?.map(ev => ev.extra),
[
{
slug: "foo",
branch: "treatment",
status: "Disqualified",
reason: "ForceEnrollment",
},
{
slug: "optin-bar",
branch: "treatment",
status: "Enrolled",
reason: "OptIn",
},
]
);
Assert.ok(
manager._unenroll.calledOnceWith(
sinon.match({ slug: existingRecipe.slug }),
{ reason: "force-enrollment" }
),
"Unenrolled from existing experiment"
Assert.ok(unenrollStub.called, "Unenrolled from existing experiment");
Assert.equal(
unenrollStub.firstCall.args[0],
existingRecipe.slug,
"Called with existing recipe slug"
);
Assert.ok(
NimbusTelemetry.setExperimentActive.calledOnceWith(
sinon.match({ slug: "optin-bar" })
),
NimbusTelemetry.setExperimentActive.calledOnce,
"Activated forced experiment"
);
Assert.ok(
manager.store.get("optin-bar")?.active,
Assert.equal(
NimbusTelemetry.setExperimentActive.firstCall.args[0].slug,
`optin-${forcedRecipe.slug}`,
"Called with forced experiment slug"
);
Assert.equal(
manager.store.getExperimentForFeature("force-enrollment").slug,
`optin-${forcedRecipe.slug}`,
"Enrolled in forced experiment"
);
manager.unenroll(`optin-bar`);
manager.unenroll(`optin-${forcedRecipe.slug}`, "test-cleanup");
assertEmptyStore(manager.store);
@@ -801,44 +761,23 @@ add_task(async function test_forceEnroll_cleanup() {
add_task(async function test_rollout_unenroll_conflict() {
const manager = ExperimentFakes.manager();
const sandbox = sinon.createSandbox();
const conflictingRollout = ExperimentFakes.recipe("conflicting-rollout", {
bucketConfig: {
...ExperimentFakes.recipe.bucketConfig,
count: 1000,
},
isRollout: true,
});
const rollout = ExperimentFakes.recipe("rollout", { isRollout: true });
await manager.onStartup();
let unenrollStub = sandbox.stub(manager, "unenroll").returns(true);
let enrollStub = sandbox.stub(manager, "_enroll").returns(true);
let rollout = ExperimentFakes.rollout("rollout_conflict");
// We want to force a conflict
await manager.enroll(conflictingRollout, "rs-loader");
sandbox.stub(manager.store, "getRolloutForFeature").returns(rollout);
sandbox.spy(manager, "_unenroll");
manager.forceEnroll(rollout, rollout.branches[0]);
manager.forceEnroll(rollout, rollout.branch);
Assert.ok(unenrollStub.calledOnce, "Should unenroll the conflicting rollout");
Assert.ok(
manager._unenroll.calledOnceWith(
sinon.match({ slug: conflictingRollout.slug }),
{ reason: "force-enrollment" }
),
"Should unenroll the conflicting rollout"
unenrollStub.calledWith(rollout.slug, "force-enrollment"),
"Should call with expected slug"
);
Assert.ok(enrollStub.calledOnce, "Should call enroll as expected");
Assert.ok(
!manager.store.get(conflictingRollout.slug)?.active,
"Conflicting rollout should be inactive"
);
Assert.ok(
manager.store.get(`optin-${rollout.slug}`)?.active,
"Rollout should be active"
);
manager.unenroll(`optin-${rollout.slug}`);
manager.unenroll(rollout.slug, "test-cleanup");
assertEmptyStore(manager.store);
sandbox.restore();
@@ -1176,10 +1115,10 @@ add_task(async function test_group_enrollment() {
);
// Cleanup
manager1.unenroll("group_enroll");
manager1.unenroll("group_enroll", "test-cleanup");
assertEmptyStore(manager1.store);
manager2.unenroll("group_enroll");
manager2.unenroll("group_enroll", "test-cleanup");
assertEmptyStore(manager2.store);
});

View File

@@ -11,9 +11,6 @@ const { MatchStatus } = ChromeUtils.importESModule(
const { NimbusTelemetry } = ChromeUtils.importESModule(
"resource://nimbus/lib/Telemetry.sys.mjs"
);
const { UnenrollmentCause } = ChromeUtils.importESModule(
"resource://nimbus/lib/ExperimentManager.sys.mjs"
);
async function cleanupStore(store) {
Assert.deepEqual(
@@ -122,17 +119,16 @@ add_task(async function test_startup_unenroll() {
store.addEnrollment(recipe);
const manager = ExperimentFakes.manager(store);
sandbox.spy(manager, "_unenroll");
const unenrollSpy = sandbox.spy(manager, "unenroll");
await manager.onStartup();
Assert.ok(
manager._unenroll.calledOnceWith(
sinon.match({ slug: "startup_unenroll" }),
{
reason: "studies-opt-out",
}
),
unenrollSpy.calledOnce,
"Unenrolled from active experiment if user opt out is true"
);
Assert.ok(
unenrollSpy.calledWith("startup_unenroll", "studies-opt-out"),
"Called unenroll for expected recipe"
);
@@ -174,7 +170,7 @@ add_task(async function test_onRecipe_enroll() {
"should add recipe to the store"
);
manager.unenroll(fooRecipe.slug);
manager.unenroll(fooRecipe.slug, "test-cleanup");
await cleanupStore(manager.store);
});
@@ -208,7 +204,7 @@ add_task(async function test_onRecipe_update() {
"should call .updateEnrollment() if the recipe has already been enrolled"
);
manager.unenroll(fooRecipe.slug);
manager.unenroll(fooRecipe.slug, "test-cleanup");
await cleanupStore(manager.store);
});
@@ -427,12 +423,7 @@ add_task(async function test_experimentStore_updateEvent() {
);
stub.resetHistory();
manager.unenroll(
"experiment",
UnenrollmentCause.fromReason(
NimbusTelemetry.UnenrollReason.INDIVIDUAL_OPT_OUT
)
);
manager.unenroll("experiment", "individual-opt-out");
Assert.ok(
stub.calledOnceWith("update", {
slug: "experiment",

View File

@@ -4,9 +4,6 @@
const { _ExperimentFeature: ExperimentFeature, NimbusFeatures } =
ChromeUtils.importESModule("resource://nimbus/ExperimentAPI.sys.mjs");
const { ObjectUtils } = ChromeUtils.importESModule(
"resource://gre/modules/ObjectUtils.sys.mjs"
);
const { PrefUtils } = ChromeUtils.importESModule(
"resource://normandy/lib/PrefUtils.sys.mjs"
);
@@ -15,18 +12,6 @@ const { TelemetryTestUtils } = ChromeUtils.importESModule(
"resource://testing-common/TelemetryTestUtils.sys.mjs"
);
function assertIncludes(array, obj, msg) {
let found = false;
for (const el of array) {
if (ObjectUtils.deepEqual(el, obj)) {
found = true;
break;
}
}
Assert.ok(found, msg);
}
/**
* Pick a single entry from an object and return a new object containing only
* that entry.
@@ -1736,13 +1721,6 @@ add_task(async function test_prefChange() {
expectedUser = null,
}) {
Services.fog.testResetFOG();
Services.fog.applyServerKnobsConfig(
JSON.stringify({
metrics_enabled: {
"nimbus_events.enrollment_status": true,
},
})
);
Services.telemetry.snapshotEvents(
Ci.nsITelemetry.DATASET_PRERELEASE_CHANNELS,
/* clear = */ true
@@ -1883,42 +1861,6 @@ add_task(async function test_prefChange() {
);
}
const expectedEnrollmentStatusEvents = [];
for (const enrollmentKind of Object.keys(configs)) {
expectedEnrollmentStatusEvents.push({
slug: slugs[enrollmentKind],
branch: "control",
status: "Enrolled",
reason: "Qualified",
});
}
for (const ev of expectedLegacyEvents) {
expectedEnrollmentStatusEvents.push({
slug: ev.value,
branch: "control",
status: "Disqualified",
reason: "ChangedPref",
});
}
const enrollmentStatusEvents = (
Glean.nimbusEvents.enrollmentStatus.testGetValue("events") ?? []
).map(ev => ev.extra);
for (const expectedEvent of expectedEnrollmentStatusEvents) {
assertIncludes(
enrollmentStatusEvents,
expectedEvent,
"Event should appear in the enrollment status telemetry"
);
}
Assert.equal(
enrollmentStatusEvents.length,
expectedEnrollmentStatusEvents.length,
"We should see the expected number of enrollment status events"
);
for (const enrollmentKind of expectedEnrollments) {
await cleanup[enrollmentKind]();
}

View File

@@ -32,15 +32,13 @@ add_task(async function test_set_inactive() {
await manager.onStartup();
await manager.store.addEnrollment(ExperimentFakes.experiment("foo"));
manager.unenroll("foo");
manager.unenroll("foo", "some-reason");
Assert.equal(
manager.store.get("foo").active,
false,
"should set .active to false"
);
assertEmptyStore(manager.store);
});
add_task(async function test_unenroll_opt_out() {
@@ -109,8 +107,6 @@ add_task(async function test_unenroll_opt_out() {
"Glean.nimbusEvents.unenrollment recorded with correct reason"
);
assertEmptyStore(manager.store);
// reset pref
Services.prefs.clearUserPref(STUDIES_OPT_OUT_PREF);
sandbox.restore();
@@ -182,8 +178,6 @@ add_task(async function test_unenroll_rollout_opt_out() {
"Glean.nimbusEvents.unenrollment recorded with correct reason"
);
assertEmptyStore(manager.store);
// reset pref
Services.prefs.clearUserPref(STUDIES_OPT_OUT_PREF);
sandbox.restore();
@@ -209,9 +203,6 @@ add_task(async function test_unenroll_uploadPref() {
false,
"Should set .active to false"
);
assertEmptyStore(manager.store);
Services.prefs.clearUserPref(UPLOAD_ENABLED_PREF);
});
@@ -240,7 +231,7 @@ add_task(async function test_setExperimentInactive_called() {
"experiment should be active before unenroll"
);
manager.unenroll("foo");
manager.unenroll("foo", "some-reason");
Assert.ok(
TelemetryEnvironment.setExperimentInactive.calledWith("foo"),
@@ -254,8 +245,6 @@ add_task(async function test_setExperimentInactive_called() {
"experiment should be inactive after unenroll"
);
assertEmptyStore(manager.store);
sandbox.restore();
});
@@ -281,7 +270,7 @@ add_task(async function test_send_unenroll_event() {
"no Glean unenrollment events before unenrollment"
);
manager.unenroll("foo", { reason: "some-reason" });
manager.unenroll("foo", "some-reason");
Assert.ok(TelemetryEvents.sendEvent.calledOnce);
Assert.deepEqual(
@@ -319,8 +308,6 @@ add_task(async function test_send_unenroll_event() {
"Glean.nimbusEvents.unenrollment recorded with correct reason"
);
assertEmptyStore(manager.store);
sandbox.restore();
});
@@ -362,8 +349,6 @@ add_task(async function test_undefined_reason() {
"Glean.nimbusEvents.unenrollment recorded with correct (unknown) reason"
);
assertEmptyStore(manager.store);
sandbox.restore();
});
@@ -384,7 +369,7 @@ add_task(async function test_remove_rollouts() {
await manager.onStartup();
manager.unenroll("foo", { reason: "some-reason" });
manager.unenroll("foo", "some-reason");
Assert.ok(
manager.store.updateExperiment.calledOnce,
@@ -397,48 +382,4 @@ add_task(async function test_remove_rollouts() {
}),
"Called with expected parameters"
);
assertEmptyStore(manager.store);
});
add_task(async function test_unenroll_individualOptOut_statusTelemetry() {
Services.fog.testResetFOG();
const manager = ExperimentFakes.manager();
await manager.onStartup();
await manager.enroll(
ExperimentFakes.recipe("foo", {
bucketConfig: {
...ExperimentFakes.recipe.bucketConfig,
count: 1000,
},
branches: [ExperimentFakes.recipe.branches[0]],
})
);
Services.fog.applyServerKnobsConfig(
JSON.stringify({
metrics_enabled: {
"nimbus_events.enrollment_status": true,
},
})
);
manager.unenroll("foo", { reason: "individual-opt-out" });
Assert.deepEqual(
Glean.nimbusEvents.enrollmentStatus
.testGetValue("events")
?.map(ev => ev.extra),
[
{
slug: "foo",
branch: "control",
status: "Disqualified",
reason: "OptOut",
},
]
);
});

View File

@@ -35,8 +35,6 @@ function setupTest({ recipes }) {
ExperimentAPI._resetForTests();
sandbox.restore();
Services.fog.testResetFOG();
},
};
}
@@ -156,14 +154,6 @@ add_task(async function test_enroll() {
const labs = await FirefoxLabs.create();
Services.fog.applyServerKnobsConfig(
JSON.stringify({
metrics_enabled: {
"nimbus_events.enrollment_status": true,
},
})
);
await Assert.rejects(
labs.enroll(),
/enroll: slug and branchSlug are required/,
@@ -194,20 +184,6 @@ add_task(async function test_enroll() {
"ExperimentManager.enroll called"
);
Assert.deepEqual(
Glean.nimbusEvents.enrollmentStatus
.testGetValue("events")
?.map(ev => ev.extra),
[
{
slug: recipe.slug,
branch: "control",
status: "Enrolled",
reason: "OptIn",
},
]
);
Assert.ok(manager.store.get(recipe.slug)?.active, "Active enrollment exists");
labs.unenroll(recipe.slug);
@@ -338,14 +314,6 @@ add_task(async function test_unenroll() {
await labs.enroll("opt-in", "control");
Assert.ok(manager.store.get("opt-in")?.active, "Enrolled in opt-in");
Services.fog.applyServerKnobsConfig(
JSON.stringify({
metrics_enabled: {
"nimbus_events.enrollment_status": true,
},
})
);
// Should not throw.
labs.unenroll("bogus");
@@ -362,20 +330,6 @@ add_task(async function test_unenroll() {
// Should not throw.
labs.unenroll("opt-in");
Assert.deepEqual(
Glean.nimbusEvents.enrollmentStatus
.testGetValue("events")
?.map(ev => ev.extra),
[
{
slug: "opt-in",
branch: "control",
status: "Disqualified",
reason: "OptOut",
},
]
);
manager.unenroll("rollout");
cleanup();
});

View File

@@ -330,7 +330,7 @@ add_task(async function test_experiment_optin_targeting() {
"Should enroll in experiment"
);
manager.unenroll(`optin-${recipe.slug}`);
manager.unenroll(`optin-${recipe.slug}`, "test-cleanup");
sandbox.restore();
Services.prefs.clearUserPref(DEBUG_PREF);

View File

@@ -23,9 +23,6 @@ const { TelemetryEnvironment } = ChromeUtils.importESModule(
const { TelemetryTestUtils } = ChromeUtils.importESModule(
"resource://testing-common/TelemetryTestUtils.sys.mjs"
);
const { UnenrollmentCause } = ChromeUtils.importESModule(
"resource://nimbus/lib/ExperimentManager.sys.mjs"
);
function assertEnrollments(store, expectedActive, expectedInactive) {
for (const slug of expectedActive) {
@@ -1302,12 +1299,7 @@ add_task(async function test_rollout_reenroll_optout() {
"Should enroll in rollout"
);
manager.unenroll(
rollout.slug,
UnenrollmentCause.fromReason(
NimbusTelemetry.UnenrollReason.INDIVIDUAL_OPT_OUT
)
);
manager.unenroll(rollout.slug, "individual-opt-out");
await loader.updateRecipes();
@@ -1446,8 +1438,8 @@ add_task(async function test_active_and_past_experiment_targeting() {
["experiment-a", "experiment-b", "rollout-a", "rollout-b"]
);
manager.unenroll("experiment-c");
manager.unenroll("rollout-c");
manager.unenroll("experiment-c", "test");
manager.unenroll("rollout-c", "test");
assertEmptyStore(manager.store);
cleanupFeatures();
@@ -2324,144 +2316,14 @@ add_task(async function test_updateRecipes_enrollmentStatus_telemetry() {
},
]);
manager.unenroll("stays-enrolled");
manager.unenroll("enrolls");
manager.unenroll("stays-enrolled", "test");
manager.unenroll("enrolls", "test");
assertEmptyStore(manager.store);
Services.fog.testResetFOG();
cleanupFeatures();
});
add_task(async function test_updateRecipes_enrollmentStatus_notEnrolled() {
const loader = ExperimentFakes.rsLoader();
const manager = loader.manager;
await manager.onStartup();
await loader.enable();
const features = [
new ExperimentFeature("test-feature-0", { variables: {} }),
new ExperimentFeature("test-feature-1", { variables: {} }),
new ExperimentFeature("test-feature-2", { variables: {} }),
new ExperimentFeature("test-feature-3", { variables: {} }),
new ExperimentFeature("test-feature-4", { variables: {} }),
new ExperimentFeature("test-feature-5", { variables: {} }),
new ExperimentFeature("test-feature-6", { variables: {} }),
new ExperimentFeature("test-feature-7", { variables: {} }),
new ExperimentFeature("test-feature-8", { variables: {} }),
];
const cleanupFeatures = ExperimentTestUtils.addTestFeatures(...features);
function recipe(slug, featureId) {
return ExperimentFakes.recipe(slug, {
bucketConfig: {
...ExperimentFakes.recipe.bucketConfig,
count: 1000,
},
branches: [
{
ratio: 1,
slug: "control",
features: [
{
featureId,
value: {},
},
],
},
],
});
}
const recipes = [
{
...recipe("enrollment-paused", "test-feature-0"),
isEnrollmentPaused: true,
},
{
...recipe("no-match", "test-feature-1"),
targeting: "false",
},
{
...recipe("targeting-only", "test-feature-2"),
bucketConfig: {
...ExperimentFakes.recipe.bucketConfig,
count: 0,
},
},
{
...recipe("already-enrolled-rollout", "test-feature-3"),
isRollout: true,
},
recipe("already-enrolled-experiment", "test-feature-3"),
];
await manager.enroll(
{ ...recipe("enrolled-rollout", "test-feature-3"), isRollout: true },
"force-enrollment"
);
await manager.enroll(
recipe("enrolled-experiment", "test-feature-3"),
"force-enrollment"
);
sinon.stub(loader.remoteSettingsClients.experiments, "get").resolves(recipes);
Services.fog.applyServerKnobsConfig(
JSON.stringify({
metrics_enabled: {
"nimbus_events.enrollment_status": true,
},
})
);
await loader.updateRecipes("timer");
Assert.deepEqual(
Glean.nimbusEvents.enrollmentStatus
.testGetValue("events")
?.map(ev => ev.extra),
[
{
slug: "enrollment-paused",
status: "NotEnrolled",
reason: "EnrollmentsPaused",
},
{
slug: "no-match",
status: "NotEnrolled",
reason: "NotTargeted",
},
{
slug: "targeting-only",
status: "NotEnrolled",
reason: "NotSelected",
},
{
slug: "already-enrolled-rollout",
status: "NotEnrolled",
reason: "FeatureConflict",
conflict_slug: "enrolled-rollout",
},
{
slug: "already-enrolled-experiment",
status: "NotEnrolled",
reason: "FeatureConflict",
conflict_slug: "enrolled-experiment",
},
]
);
manager.unenroll("enrolled-experiment");
manager.unenroll("enrolled-rollout");
assertEmptyStore(manager.store);
Services.fog.testResetFOG();
cleanupFeatures();
});
add_task(async function test_updateRecipesWithPausedEnrollment() {
const loader = ExperimentFakes.rsLoader();
const manager = loader.manager;
@@ -2482,19 +2344,19 @@ add_task(async function test_updateRecipesWithPausedEnrollment() {
.resolves([recipe]);
sinon.spy(manager, "onRecipe");
sinon.spy(manager, "_enroll");
sinon.spy(manager, "enroll");
await loader.updateRecipes("test");
Assert.ok(
manager.onRecipe.calledOnceWith(recipe, "rs-loader", {
ok: true,
status: MatchStatus.ENROLLMENT_PAUSED,
status: MatchStatus.TARGETING_ONLY,
}),
"Should call onRecipe with enrollments paused"
"Should call onRecipe with targeting match"
);
Assert.ok(
manager._enroll.notCalled,
manager.enroll.notCalled,
"Should not call enroll for paused recipe"
);
@@ -2699,8 +2561,8 @@ add_task(async function testUnenrollsFirst() {
await loader.updateRecipes("timer");
assertEnrollments(manager.store, ["e3", "r3"], ["e1", "e2", "r1", "r2"]);
manager.unenroll("e3");
manager.unenroll("r3");
manager.unenroll("e3", "test");
manager.unenroll("r3", "test");
assertEmptyStore(manager.store);
});

View File

@@ -1587,7 +1587,7 @@ add_task(async function test_prefFlips_unenrollment() {
for (const { slug, isRollout = false } of expectedEnrollments) {
const computedSlug = `${slug}-${isRollout ? "rollout" : "experiment"}`;
info(`Unenrolling from ${computedSlug}\n`);
manager.unenroll(computedSlug);
manager.unenroll(computedSlug, "cleanup");
}
assertEmptyStore(manager.store);
assertNoObservers(manager);
@@ -1963,13 +1963,6 @@ add_task(async function test_prefFlip_setPref_restore() {
for (const [i, { name, ...testCase }] of TEST_CASES.entries()) {
Services.fog.testResetFOG();
Services.fog.applyServerKnobsConfig(
JSON.stringify({
metrics_enabled: {
"nimbus_events.enrollment_status": true,
},
})
);
Services.telemetry.snapshotEvents(
Ci.nsITelemetry.DATASET_PRERELEASE_CHANNELS,
/* clear = */ true
@@ -2046,35 +2039,9 @@ add_task(async function test_prefFlip_setPref_restore() {
},
]
);
Assert.deepEqual(
Glean.nimbusEvents.enrollmentStatus
.testGetValue("events")
?.map(ev => ev.extra),
[
{
slug: enrollmentOrder[0],
branch: "control",
status: "Enrolled",
reason: "Qualified",
},
{
slug: enrollmentOrder[0],
branch: "control",
status: "Disqualified",
reason: "PrefFlipsConflict",
conflict_slug: enrollmentOrder[1],
},
{
slug: enrollmentOrder[1],
branch: "control",
status: "Enrolled",
reason: "Qualified",
},
]
);
info("Unenrolling...");
manager.unenroll(enrollmentOrder[1]);
manager.unenroll(enrollmentOrder[1], "test-cleanup");
info("Checking expected prefs...");
checkExpectedPrefBranches(expectedPrefs);
@@ -2153,7 +2120,7 @@ add_task(async function test_prefFlips_cacheOriginalValues() {
"originalValues cached on serialized enrollment"
);
manager.unenroll(recipe.slug);
manager.unenroll(recipe.slug, "test");
Assert.ok(
!Services.prefs.prefHasUserValue("test.pref.please.ignore"),
"pref unset after unenrollment"
@@ -2240,7 +2207,7 @@ add_task(async function test_prefFlips_restore_unenroll() {
null
);
manager.unenroll(recipe.slug);
manager.unenroll(recipe.slug, "test");
Assert.ok(
!Services.prefs.prefHasUserValue("test.pref.please.ignore"),
"pref unset after unenrollment"
@@ -2539,10 +2506,10 @@ add_task(async function test_prefFlips_failed_experiment_and_rollout() {
info("Unenrolling...");
if (expectedEnrollments.includes(ROLLOUT)) {
manager.unenroll(ROLLOUT);
manager.unenroll(ROLLOUT, "test-cleanup");
}
if (expectedEnrollments.includes(EXPERIMENT)) {
manager.unenroll(EXPERIMENT);
manager.unenroll(EXPERIMENT, "test-cleanup");
}
info("Cleaning up...");

View File

@@ -11,13 +11,11 @@ ChromeUtils.defineESModuleGetters(lazy, {
BranchedAddonStudyAction:
"resource://normandy/actions/BranchedAddonStudyAction.sys.mjs",
ExperimentManager: "resource://nimbus/lib/ExperimentManager.sys.mjs",
NimbusTelemetry: "resource://nimbus/lib/Telemetry.sys.mjs",
PreferenceExperiments:
"resource://normandy/lib/PreferenceExperiments.sys.mjs",
RecipeRunner: "resource://normandy/lib/RecipeRunner.sys.mjs",
RemoteSettingsExperimentLoader:
"resource://nimbus/lib/RemoteSettingsExperimentLoader.sys.mjs",
UnenrollmentCause: "resource://nimbus/lib/ExperimentManager.sys.mjs",
});
const SHIELD_LEARN_MORE_URL_PREF = "app.normandy.shieldLearnMoreUrl";
@@ -211,13 +209,8 @@ ChromeUtils.defineLazyGetter(AboutPages, "aboutStudies", () => {
}
},
async removeMessagingSystemExperiment(slug) {
lazy.ExperimentManager.unenroll(
slug,
lazy.UnenrollmentCause.fromReason(
lazy.NimbusTelemetry.UnenrollReason.INDIVIDUAL_OPT_OUT
)
);
async removeMessagingSystemExperiment(slug, reason) {
lazy.ExperimentManager.unenroll(slug, reason);
this._sendToAll(
"Shield:UpdateMessagingSystemExperimentList",
lazy.ExperimentManager.store.getAll()

View File

@@ -34,7 +34,10 @@ export class ShieldFrameParent extends JSWindowActorParent {
);
break;
case "Shield:RemoveMessagingSystemExperiment":
aboutStudies.removeMessagingSystemExperiment(msg.data.slug);
aboutStudies.removeMessagingSystemExperiment(
msg.data.slug,
msg.data.reason
);
break;
case "Shield:OpenDataPreferences":
aboutStudies.openDataPreferences();

View File

@@ -319,6 +319,7 @@ class MessagingSystemListItem extends React.Component {
handleClickRemove() {
sendPageEvent("RemoveMessagingSystemExperiment", {
slug: this.props.study.slug,
reason: "individual-opt-out",
});
}

View File

@@ -177,7 +177,7 @@ add_task(async function test_targeting_exists() {
await manager.onStartup();
await manager.store.addEnrollment(ExperimentFakes.experiment("foo"));
manager.unenroll("foo");
manager.unenroll("foo", "some-reason");
await manager.store.addEnrollment(
ExperimentFakes.experiment("bar", { active: false })
);
@@ -186,7 +186,7 @@ add_task(async function test_targeting_exists() {
);
manager.store.addEnrollment(ExperimentFakes.rollout("rol1"));
manager.unenroll("rol1");
manager.unenroll("rol1", "some-reason");
manager.store.addEnrollment(ExperimentFakes.rollout("rol2"));
let targetSnapshot = await ASRouterTargeting.getEnvironmentSnapshot({