commit 56b39b51d2f92c37c7c892f7c76282c7aa4b5f83
parent 309094844ef89311db58001760275fb46f96b662
Author: Daniel Thorn <dthorn@mozilla.com>
Date: Tue, 6 Jan 2026 19:36:28 +0000
Bug 2006752 - Move enrollment status events to targeting context ping r=nimbus-reviewers,beth
Differential Revision: https://phabricator.services.mozilla.com/D277107
Diffstat:
11 files changed, 115 insertions(+), 95 deletions(-)
diff --git a/toolkit/components/nimbus/lib/RemoteSettingsExperimentLoader.sys.mjs b/toolkit/components/nimbus/lib/RemoteSettingsExperimentLoader.sys.mjs
@@ -396,82 +396,89 @@ export class RemoteSettingsExperimentLoader {
// See-also: https://bugzilla.mozilla.org/show_bug.cgi?id=1936317
// See-also: https://bugzilla.mozilla.org/show_bug.cgi?id=1936319
if (lazy.TARGETING_CONTEXT_TELEMETRY_ENABLED) {
- lazy.recordTargetingContext();
+ await lazy.recordTargetingContext();
}
- // Since this method is async, the enabled pref could change between await
- // points. We don't want to half validate experiments, so we cache this to
- // keep it consistent throughout updating.
- const validationEnabled = this.validationEnabled;
+ try {
+ // Since this method is async, the enabled pref could change between await
+ // points. We don't want to half validate experiments, so we cache this to
+ // keep it consistent throughout updating.
+ const validationEnabled = this.validationEnabled;
- let recipeValidator;
+ let recipeValidator;
- if (validationEnabled) {
- recipeValidator = new lazy.JsonSchema.Validator(
- await SCHEMAS.NimbusExperiment
- );
- }
+ if (validationEnabled) {
+ recipeValidator = new lazy.JsonSchema.Validator(
+ await SCHEMAS.NimbusExperiment
+ );
+ }
- let allRecipes = null;
- try {
- allRecipes = await this.getRecipesFromAllCollections({
- forceSync,
- trigger,
- });
- } catch (e) {
- lazy.log.debug("Failed to update", e);
- }
+ let allRecipes = null;
+ try {
+ allRecipes = await this.getRecipesFromAllCollections({
+ forceSync,
+ trigger,
+ });
+ } catch (e) {
+ lazy.log.debug("Failed to update", e);
+ }
- if (allRecipes !== null) {
- const unenrolledExperimentSlugs = lazy.NimbusEnrollments
- .syncEnrollmentsEnabled
- ? await lazy.NimbusEnrollments.loadUnenrolledExperimentSlugsFromOtherProfiles()
- : undefined;
+ if (allRecipes !== null) {
+ const unenrolledExperimentSlugs = lazy.NimbusEnrollments
+ .syncEnrollmentsEnabled
+ ? await lazy.NimbusEnrollments.loadUnenrolledExperimentSlugsFromOtherProfiles()
+ : undefined;
- const enrollmentsCtx = new EnrollmentsContext(
- this.manager,
- recipeValidator,
- {
- validationEnabled,
- labsEnabled: lazy.ExperimentAPI.labsEnabled,
- studiesEnabled: lazy.ExperimentAPI.studiesEnabled,
- shouldCheckTargeting: true,
- unenrolledExperimentSlugs,
- }
- );
+ const enrollmentsCtx = new EnrollmentsContext(
+ this.manager,
+ recipeValidator,
+ {
+ validationEnabled,
+ labsEnabled: lazy.ExperimentAPI.labsEnabled,
+ studiesEnabled: lazy.ExperimentAPI.studiesEnabled,
+ shouldCheckTargeting: true,
+ unenrolledExperimentSlugs,
+ }
+ );
- const { existingEnrollments, recipes } =
- this._partitionRecipes(allRecipes);
+ const { existingEnrollments, recipes } =
+ this._partitionRecipes(allRecipes);
- for (const { enrollment, recipe } of existingEnrollments) {
- const result = recipe
- ? await enrollmentsCtx.checkRecipe(recipe)
- : CheckRecipeResult.Ok(MatchStatus.NOT_SEEN);
+ for (const { enrollment, recipe } of existingEnrollments) {
+ const result = recipe
+ ? await enrollmentsCtx.checkRecipe(recipe)
+ : CheckRecipeResult.Ok(MatchStatus.NOT_SEEN);
- await this.manager.updateEnrollment(
- enrollment,
- recipe,
- this.SOURCE,
- result
- );
- }
+ await this.manager.updateEnrollment(
+ enrollment,
+ recipe,
+ this.SOURCE,
+ result
+ );
+ }
- for (const recipe of recipes) {
- const result = await enrollmentsCtx.checkRecipe(recipe);
- await this.manager.onRecipe(recipe, this.SOURCE, result);
- }
+ for (const recipe of recipes) {
+ const result = await enrollmentsCtx.checkRecipe(recipe);
+ await this.manager.onRecipe(recipe, this.SOURCE, result);
+ }
- lazy.log.debug(`${enrollmentsCtx.matches} recipes matched.`);
- }
+ lazy.log.debug(`${enrollmentsCtx.matches} recipes matched.`);
+ }
- if (trigger !== "timer") {
- const lastUpdateTime = Math.round(Date.now() / 1000);
- Services.prefs.setIntPref(TIMER_LAST_UPDATE_PREF, lastUpdateTime);
- }
+ if (trigger !== "timer") {
+ const lastUpdateTime = Math.round(Date.now() / 1000);
+ Services.prefs.setIntPref(TIMER_LAST_UPDATE_PREF, lastUpdateTime);
+ }
- if (allRecipes !== null) {
- // Enrollments have not changed, so we don't need to notify.
- Services.obs.notifyObservers(null, "nimbus:enrollments-updated");
+ if (allRecipes !== null) {
+ // Enrollments have not changed, so we don't need to notify.
+ Services.obs.notifyObservers(null, "nimbus:enrollments-updated");
+ }
+ } finally {
+ if (lazy.TARGETING_CONTEXT_TELEMETRY_ENABLED) {
+ // Submit targeting context ping after all enrollment status events should be generated
+ GleanPings.nimbusTargetingContext.submit();
+ }
}
}
diff --git a/toolkit/components/nimbus/lib/TargetingContextRecorder.sys.mjs b/toolkit/components/nimbus/lib/TargetingContextRecorder.sys.mjs
@@ -401,6 +401,4 @@ export async function recordTargetingContext() {
// This will ensure that the profile group ID metric has been set.
await lazy.ClientID.getProfileGroupID();
-
- GleanPings.nimbusTargetingContext.submit();
}
diff --git a/toolkit/components/nimbus/metrics.yaml b/toolkit/components/nimbus/metrics.yaml
@@ -1043,10 +1043,12 @@ nimbus_events:
- https://bugzilla.mozilla.org/show_bug.cgi?id=1817481
- https://bugzilla.mozilla.org/show_bug.cgi?id=1955169
- https://bugzilla.mozilla.org/show_bug.cgi?id=1997467
+ - https://bugzilla.mozilla.org/show_bug.cgi?id=2006752
data_reviews:
- https://bugzilla.mozilla.org/show_bug.cgi?id=1817481
- https://bugzilla.mozilla.org/show_bug.cgi?id=1955169
- https://bugzilla.mozilla.org/show_bug.cgi?id=1997467
+ - https://bugzilla.mozilla.org/show_bug.cgi?id=2006752
data_sensitivity:
- technical
notification_emails:
@@ -1054,6 +1056,7 @@ nimbus_events:
- project-nimbus@mozilla.com
expires: never
disabled: true
+ send_in_pings: *targeting_context_pings
migration:
type: event
diff --git a/toolkit/components/nimbus/test/unit/test_ExperimentManager_enroll.js b/toolkit/components/nimbus/test/unit/test_ExperimentManager_enroll.js
@@ -359,7 +359,7 @@ add_task(async function test_failure_name_conflict() {
Assert.deepEqual(
Glean.nimbusEvents.enrollmentStatus
- .testGetValue("events")
+ .testGetValue("nimbus-targeting-context")
.map(ev => ev.extra),
[
{
@@ -708,7 +708,7 @@ add_task(async function test_forceEnroll_cleanup() {
Assert.deepEqual(
Glean.nimbusEvents.enrollmentStatus
- .testGetValue("events")
+ .testGetValue("nimbus-targeting-context")
?.map(ev => ev.extra),
[
{
diff --git a/toolkit/components/nimbus/test/unit/test_ExperimentManager_prefs.js b/toolkit/components/nimbus/test/unit/test_ExperimentManager_prefs.js
@@ -1869,7 +1869,9 @@ add_task(async function test_prefChange() {
}
const enrollmentStatusEvents = (
- Glean.nimbusEvents.enrollmentStatus.testGetValue("events") ?? []
+ Glean.nimbusEvents.enrollmentStatus.testGetValue(
+ "nimbus-targeting-context"
+ ) ?? []
).map(ev => ev.extra);
for (const expectedEvent of expectedEnrollmentStatusEvents) {
diff --git a/toolkit/components/nimbus/test/unit/test_ExperimentManager_unenroll.js b/toolkit/components/nimbus/test/unit/test_ExperimentManager_unenroll.js
@@ -363,7 +363,7 @@ add_task(async function test_unenroll_individualOptOut_statusTelemetry() {
Assert.deepEqual(
Glean.nimbusEvents.enrollmentStatus
- .testGetValue("events")
+ .testGetValue("nimbus-targeting-context")
?.map(ev => ev.extra),
[
{
@@ -408,7 +408,7 @@ add_task(async function testUnenrollBogusReason() {
Assert.deepEqual(
Glean.nimbusEvents.enrollmentStatus
- .testGetValue("events")
+ .testGetValue("nimbus-targeting-context")
?.map(ev => ev.extra),
[
{
diff --git a/toolkit/components/nimbus/test/unit/test_FirefoxLabs.js b/toolkit/components/nimbus/test/unit/test_FirefoxLabs.js
@@ -245,7 +245,7 @@ add_task(async function test_enroll() {
Assert.deepEqual(
Glean.nimbusEvents.enrollmentStatus
- .testGetValue("events")
+ .testGetValue("nimbus-targeting-context")
?.map(ev => ev.extra),
[
{
@@ -378,7 +378,7 @@ add_task(async function test_unenroll() {
Assert.deepEqual(
Glean.nimbusEvents.enrollmentStatus
- .testGetValue("events")
+ .testGetValue("nimbus-targeting-context")
?.map(ev => ev.extra),
[
{
diff --git a/toolkit/components/nimbus/test/unit/test_Migrations.js b/toolkit/components/nimbus/test/unit/test_Migrations.js
@@ -1706,7 +1706,7 @@ add_task(async function testGraduateFirefoxLabsAutoPip() {
Assert.deepEqual(
Glean.nimbusEvents.enrollmentStatus
- .testGetValue("events")
+ .testGetValue("nimbus-targeting-context")
.map(event => event.extra),
[
{
diff --git a/toolkit/components/nimbus/test/unit/test_RemoteSettingsExperimentLoader_updateRecipes.js b/toolkit/components/nimbus/test/unit/test_RemoteSettingsExperimentLoader_updateRecipes.js
@@ -1880,7 +1880,9 @@ add_task(async function test_updateRecipes_enrollmentStatus_telemetry() {
await loader.updateRecipes("test");
- const events = Glean.nimbusEvents.enrollmentStatus.testGetValue("events");
+ const events = Glean.nimbusEvents.enrollmentStatus.testGetValue(
+ "nimbus-targeting-context"
+ );
Assert.deepEqual(events?.map(ev => ev.extra) ?? [], [
{
@@ -2067,7 +2069,7 @@ add_task(async function test_updateRecipes_enrollmentStatus_notEnrolled() {
Assert.deepEqual(
Glean.nimbusEvents.enrollmentStatus
- .testGetValue("events")
+ .testGetValue("nimbus-targeting-context")
?.map(ev => ev.extra),
[
{
@@ -2444,7 +2446,7 @@ add_task(async function testUnenrolledInAnotherProfileBeforeUpdate() {
Assert.deepEqual(
Glean.nimbusEvents.enrollmentStatus
- .testGetValue("events")
+ .testGetValue("nimbus-targeting-context")
?.map(ev => ev.extra),
[
{
@@ -2614,7 +2616,7 @@ add_task(async function testUnenrolledInAnotherProfileBetweenUpdates() {
Assert.deepEqual(
Glean.nimbusEvents.enrollmentStatus
- .testGetValue("events")
+ .testGetValue("nimbus-targeting-context")
?.map(ev => ev.extra),
[
{
diff --git a/toolkit/components/nimbus/test/unit/test_TargetingContextRecorder.js b/toolkit/components/nimbus/test/unit/test_TargetingContextRecorder.js
@@ -59,6 +59,14 @@ async function setupTest({ ...args } = {}) {
}
/**
+ * Call recordTargetingContext and submit nimbusTargetingContext ping
+ */
+async function recordTargetingContextAndSubmit() {
+ await recordTargetingContext();
+ GleanPings.nimbusTargetingContext.submit();
+}
+
+/**
* Return an object containing each nimbus_targeting_context metric mapped to
* its recorded value.
*
@@ -206,7 +214,7 @@ add_task(async function testNimbusTargetingContextAllKeysPresent() {
`nimbusTargetingContext.${metric} was recorded ${JSON.stringify(values[metric])}`
);
}
- }, recordTargetingContext);
+ }, recordTargetingContextAndSubmit);
manager.unenroll("experiment");
manager.unenroll("rollout");
@@ -231,7 +239,7 @@ add_task(async function testNimbusTargetingEnvironmentUserSetPrefs() {
!prefs.includes("nimbus.testing.testSetString"),
"nimbus.testing.testInt is not set and not in telemetry"
);
- }, recordTargetingContext);
+ }, recordTargetingContextAndSubmit);
// This pref is a fallbackPref, so should not appear in the list.
Services.prefs.setIntPref("nimbus.testing.testInt", 123);
@@ -252,7 +260,7 @@ add_task(async function testNimbusTargetingEnvironmentUserSetPrefs() {
prefs.includes("nimbus.testing.testSetString"),
"nimbus.testing.testSetString is set and in telemetry"
);
- }, recordTargetingContext);
+ }, recordTargetingContextAndSubmit);
Services.prefs.deleteBranch("nimbus.testing.testInt");
Services.prefs.deleteBranch("nimbus.testing.testSetString");
@@ -275,7 +283,7 @@ add_task(async function testNimbusTargetingEnvironmentPrefValues() {
!Object.hasOwn(prefs, PREF_KEY),
`${PREF} not set and not present in telemetry`
);
- }, recordTargetingContext);
+ }, recordTargetingContextAndSubmit);
Services.prefs.getDefaultBranch(null).setStringPref(PREF, "default");
@@ -288,7 +296,7 @@ add_task(async function testNimbusTargetingEnvironmentPrefValues() {
"default",
`${PREF} set on the default branch and present in telemetry`
);
- }, recordTargetingContext);
+ }, recordTargetingContextAndSubmit);
Services.prefs.setStringPref(PREF, "user");
@@ -301,7 +309,7 @@ add_task(async function testNimbusTargetingEnvironmentPrefValues() {
"user",
`${PREF} set on the user branch and present in telemetry`
);
- }, recordTargetingContext);
+ }, recordTargetingContextAndSubmit);
Services.prefs.deleteBranch(PREF);
@@ -323,7 +331,7 @@ add_task(async function testExperimentMetrics() {
Assert.deepEqual(values.activeExperiments, []);
Assert.deepEqual(values.activeRollouts, []);
Assert.deepEqual(values.enrollmentsMap, []);
- }, recordTargetingContext);
+ }, recordTargetingContextAndSubmit);
await manager.enroll(
NimbusTestUtils.factories.recipe.withFeatureConfig("experiment-1", {
@@ -371,7 +379,7 @@ add_task(async function testExperimentMetrics() {
{ experimentSlug: "rollout-1", branchSlug: "rollout" },
].sort()
);
- }, recordTargetingContext);
+ }, recordTargetingContextAndSubmit);
manager.unenroll("experiment-1", { reason: "test" });
manager.unenroll("experiment-2", { reason: "test" });
@@ -392,7 +400,7 @@ add_task(async function testExperimentMetrics() {
{ experimentSlug: "rollout-1", branchSlug: "rollout" },
].sort()
);
- }, recordTargetingContext);
+ }, recordTargetingContextAndSubmit);
await cleanup();
});
@@ -423,7 +431,7 @@ add_task(async function testErrorMetrics() {
!Object.hasOwn(prefs, PREF_KEY),
`${PREF_KEY} not set and not present in telemetry`
);
- }, recordTargetingContext);
+ }, recordTargetingContextAndSubmit);
info(
"testing prefs with the wrong type are recorded in the pref_type_errors metric"
@@ -441,7 +449,7 @@ add_task(async function testErrorMetrics() {
!Object.hasOwn(prefs, PREF_KEY),
"nimbus.qa.pref-1 not set and not present in telemetry"
);
- }, recordTargetingContext);
+ }, recordTargetingContextAndSubmit);
Services.prefs.deleteBranch(PREF);
@@ -465,7 +473,7 @@ add_task(async function testErrorMetrics() {
assertRecordingFailures({
attrEvalErrors: ["currentDate", "isFirstStartup"],
});
- }, recordTargetingContext);
+ }, recordTargetingContextAndSubmit);
await cleanup();
@@ -514,7 +522,7 @@ add_task(async function testRecordingErrors() {
null,
"The targetingContextValue metric is not recorded by default."
);
- }, recordTargetingContext);
+ }, recordTargetingContextAndSubmit);
// We triggered glean to record error metrics. Ensure that we don't double count.
Services.fog.testResetFOG();
@@ -576,7 +584,7 @@ add_task(async function testRecordingErrors() {
],
"activeExperiments should have the invalid value in the targetingContextValue metric"
);
- }, recordTargetingContext);
+ }, recordTargetingContextAndSubmit);
await cleanup();
@@ -604,7 +612,7 @@ add_task(async function testAddonsInfo() {
[],
"No recorded addon info"
);
- }, recordTargetingContext);
+ }, recordTargetingContextAndSubmit);
const ext1 = ExtensionTestUtils.loadExtension({
useAddonManager: "temporary",
@@ -629,7 +637,7 @@ add_task(async function testAddonsInfo() {
"hasInstalledAddons is true"
);
Assert.deepEqual(values.addonsInfo.addons, [ext1.id], "Has one addon");
- }, recordTargetingContext);
+ }, recordTargetingContextAndSubmit);
const ext2 = ExtensionTestUtils.loadExtension({
useAddonManager: "temporary",
@@ -658,7 +666,7 @@ add_task(async function testAddonsInfo() {
[ext1.id, ext2.id].sort(),
"Has two addons"
);
- }, recordTargetingContext);
+ }, recordTargetingContextAndSubmit);
await ext1.unload();
await ext2.unload();
@@ -680,7 +688,7 @@ add_task(async function testAddonsInfo() {
[],
"No recorded addon info"
);
- }, recordTargetingContext);
+ }, recordTargetingContextAndSubmit);
await cleanup();
});
diff --git a/toolkit/components/nimbus/test/unit/test_prefFlips.js b/toolkit/components/nimbus/test/unit/test_prefFlips.js
@@ -2253,7 +2253,7 @@ add_task(async function test_prefFlip_setPref_restore() {
);
Assert.deepEqual(
Glean.nimbusEvents.enrollmentStatus
- .testGetValue("events")
+ .testGetValue("nimbus-targeting-context")
?.map(ev => ev.extra),
[
{
