1850127 - User is unenrolled from a rollout if the user unenrolls from an experiment, both using the opaqueResponseBlocking feature config

Status: VERIFIED FIXED

(Firefox :: Nimbus Desktop Client, defect, P1)

Description

[Valentin Bandac [:vbandac], Ecosystem QA]

[Affected versions]:

• Firefox Release 116
• Firefox Beta 117

[Affected Platforms]:

• Windows 10x64
• macOS Ventura 13.5
• Ubuntu 22.04 x64

[Prerequisites]:

• Have an experiment that sets the feature “off” on the control branch and “on” on the treatment branch, and a rollout that sets the feature on for the opaqueResponseBlocking feature config.
• Have a user.js file for the experiment to be able to enroll in the control branch.

[Steps to reproduce]:

1. Use the user.js file to enroll in the Treatment A branch of the experiment and restart the browser.
2. Navigate to “about:telemetry#events-tab” and observe the events.
3. Navigate to “about:studies” and remove the experiment.
4. Navigate to “about:telemetry#events-tab” and observe the events.

[Expected result]:

• Step 2:
  • An enroll event is registered for the experiment.
  • An enroll event is registered for the rollout.
• Step 3: The experiment is removed and the user is still enrolled in the rollout.

[Actual result]:

• Step 2:
  • An enroll event is registered for the experiment.
  • An enroll event is registered for the rollout.
• Step 3: The user is unenrolled from both the experiment and the rollout.

[Notes]:

• The issue might be reproducible on Firefox Nightly also, if the rollout sets a value different from the default value for the "browser.opaqueResponseBlocking" pref.

Comment 1

[Sean Feng [:sefeng211]]

Should we ping someone from the experiment team about this? I don't think necko team will do anything to this bug right?

Comment 2

[Kershaw Chang [:kershaw]]

(In reply to Sean Feng [:sefeng] from comment #1)

Should we ping someone from the experiment team about this? I don't think necko team will do anything to this bug right?

This looks like not a networking bug to me.
I changed the component to Firefox::Nimbus Desktop Client in order to get a right one to have a look.

Comment 3

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Hi Valentin,

Am I getting this issue right that there's a bug, when and only when an experiment's control branch sets the feature to a value, which is different from the value set for a rollout?

For example, if the prerequisite becomes have an experiment that sets the feature “on” on the control branch and “off” on the treatment branch, and a rollout that sets the feature "off" for the opaqueResponseBlocking feature config , we will still get unexpected results, right?

Also, what if our experiment has only a single control branch, not like your prerequisites which have 2 branches? If an experiment that sets the feature “on” on the single control branch, which is the same value that a rollout sets, what is the testing result? I am asking for clarification because we are currently running a Nimbus experiment which has only a single-branch to set the ORB feature "on", while we are planning to land a patch to turn ORB on and ship to all users by default. After feature shipping, we will plan to end that single-branch experiment (setting ON). And I assume "end the single-branch experiment" means the same as "users unenroll from the experiment". Therefore, I want to ensure and confirm, in our scenario, users will still have ORB on, after that experiment ends, because that's the default setting we deployed. Thank you.

Comment 4

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Hsin-Yi Tsai (she/her) [:hsinyi] from comment #3)

Hi Valentin,

Am I getting this issue right that there's a bug, when and only when an experiment's control branch sets the feature to a value, which is different from the value set for a rollout?

For example, if the prerequisite becomes have an experiment that sets the feature “on” on the control branch and “off” on the treatment branch, and a rollout that sets the feature "off" for the opaqueResponseBlocking feature config , we will still get unexpected results, right?

Also, what if our experiment has only a single control branch, not like your prerequisites which have 2 branches? If an experiment that sets the feature “on” on the single control branch, which is the same value that a rollout sets, what is the testing result? I am asking for clarification because we are currently running a Nimbus experiment which has only a single-branch to set the ORB feature "on", while we are planning to land a patch to turn ORB on and ship to all users by default. After feature shipping, we will plan to end that single-branch experiment (setting ON). And I assume "end the single-branch experiment" means the same as "users unenroll from the experiment". Therefore, I want to ensure and confirm, in our scenario, users will still have ORB on, after that experiment ends, because that's the default setting we deployed. Thank you.

:cmuresan already cleared this up for me on slack.
This bug is a different scenario from the deploy case I asked above.

Comment 5

[Beth Rennie [:beth] (she/her)]

I'm looking into this.

Comment 6

[Beth Rennie [:beth] (she/her)]

Please set the messaging-system.log pref to debug and try to re-verify on a clean profile. This will include relevant logs that tells us why the experiments are unenrolling. I'm not enable to reproduce successfully.

Comment 7

[Beth Rennie [:beth] (she/her)]

Note: this is the script I was using to try to reproduce:

(async function() {
  const PREF = "browser.opaqueResponseBlocking";
  const ROLLOUT_SLUG = "orb-rollout";
  const EXPERIMENT_SLUG = "orb-experiment";

  const { ExperimentManager } = ChromeUtils.importESModule("resource://nimbus/lib/ExperimentManager.sys.mjs");
  const { NimbusFeatures } = ChromeUtils.importESModule("resource://nimbus/ExperimentAPI.sys.mjs");

  const cleanup = () => {
    ExperimentManager.store._deleteForTests("orb-rollout");
    ExperimentManager.store._deleteForTests("orb-experiment");
  };

  cleanup();

  const recipeTemplate = ({isRollout = false, enabled} = {}) => ({
    appName: "firefox_desktop",
    appId: "firefox-desktop",
    application: "firefox-desktop",
    channel: "nightly",
    slug: isRollout ? ROLLOUT_SLUG : EXPERIMENT_SLUG,
    userFacingName: isRollout ? ROLLOUT_SLUG : EXPERIMENT_SLUG,
    userFacingDescription: isRollout ? ROLLOUT_SLUG : EXPERIMENT_SLUG,
    bucketConfig: {
      namespace: "nimmbus-test",
      randomizationUnit: "normandy_id",
      start: 0,
      count: 1000,
      total: 1000,
    },
    isEnrollmentPaused: false,
    branches: [
      {
        slug: "control",
        ratio: 1,
        features: [
          {
            featureId: "opaqueResponseBlocking",
            value: { enabled },
          }
        ],
      },
    ],
    isRollout,
    featureIds: ["opaqueResponseBlocking"],
  });

  Services.prefs.setBoolPref(PREF, false);

  const value = () => Services.prefs.getBoolPref(PREF);
  const enrollments = () => ExperimentManager.store.getAll().filter(e => e.active).map(e => e.slug).filter(slug => [ROLLOUT_SLUG, EXPERIMENT_SLUG].includes(slug));

  const dbg = () => `${PREF} = ${value()}; enrolled = ${enrollments().join(", ")}`;

  const rollout = recipeTemplate({ isRollout: true, enabled: true });
  const exp = recipeTemplate({ enabled: false });

  console.log("Before enrollment: ", dbg());

  await ExperimentManager.enroll(rollout, "test");
  console.log("After rollout enrollment: ", dbg());

  await ExperimentManager.enroll(exp, "test");
  console.log("After experiment enrollment: ", dbg());

  ExperimentManager.unenroll(EXPERIMENT_SLUG, "test");
  console.log("After experiment unenrollment: ", dbg());

  ExperimentManager.unenroll(ROLLOUT_SLUG, "test");
  console.log("After rollout unenrollment: ", dbg());


  cleanup();
})();

If you paste it into the browser console:

Before enrollment:  browser.opaqueResponseBlocking = false; enrolled = debugger eval code:58:11
ExperimentManager: New rollout started: orb-rollout, control ExperimentManager.sys.mjs:473
After rollout enrollment:  browser.opaqueResponseBlocking = true; enrolled = orb-rollout debugger eval code:61:11
ExperimentManager: New experiment started: orb-experiment, control ExperimentManager.sys.mjs:473
After experiment enrollment:  browser.opaqueResponseBlocking = false; enrolled = orb-rollout, orb-experiment debugger eval code:64:11
ExperimentManager: Recipe unenrolled: orb-experiment ExperimentManager.sys.mjs:669
After experiment unenrollment:  browser.opaqueResponseBlocking = true; enrolled = orb-rollout debugger eval code:67:11
ExperimentManager: Recipe unenrolled: orb-rollout ExperimentManager.sys.mjs:669
After rollout unenrollment:  browser.opaqueResponseBlocking = false; enrolled = 

Comment 8

[Ciprian Muresan [:cmuresan], Formerly of Ecosystem QA - please needinfo @mcoman for things regarding the team]

Hey @barret, Valentin is currently in PTO for the remainder of the week.

I've staged two sets of experiment+rollout in both preview and live states.

• Live: Experiment, Rollout
• Preview: Experiment, Rollout

Using a new profile or refreshed profile, set the following prefs:

• "app.normandy.user_id" to "1a4a4e94-c1f4-4876-96a7-7f1c46875a5a" or "a9828181-9584-417d-863c-7dd297dc19e9" (IDs only for Live)
• "messaging-system.log" to "all"
• "services.settings.server" to "https://settings-cdn.stage.mozaws.net/v1"
• "security.content.signature.root_hash" to "3C:01:44:6A:BE:90:36:CE:A9:A0:9A:CA:A3:A5:20:AC:62:8F:20:A7:AE:32:CE:86:1C:B2:EF:B7:0F:A0:C7:45"

OR

• "messaging-system.rsexperimentloader.collection_id" to "nimbus-preview" if using the preview experiments
• "app.normandy.user_id" to "4ebd49fe-e202-43e5-bcf6-94bd36d6a887" or "b7fb0b2b-eb56-4b9b-8df2-44f860b5308b" (IDs only for Preview)
• "messaging-system.log" to "all"
• "services.settings.server" to "https://settings-cdn.stage.mozaws.net/v1"
• "security.content.signature.root_hash" to "3C:01:44:6A:BE:90:36:CE:A9:A0:9A:CA:A3:A5:20:AC:62:8F:20:A7:AE:32:CE:86:1C:B2:EF:B7:0F:A0:C7:45"

The two things of note in the Browser Console are:

• Immediate unenrollment from both Experiment and Rollout
  • ExperimentManager: Recipe unenrolled: feature-configuration-testing-for-opaqueresponseblocking-rollout-v2 ExperimentManager.sys.mjs:669
  • ExperimentManager: Recipe unenrolled: feature-configuration-testing-for-opaqueresponseblocking-experiment-v2 ExperimentManager.sys.mjs:669
• Followed up by a re-enrollment of the Rollout after some time has passed
  • RSLoader: [experiment] feature-configuration-testing-for-opaqueresponseblocking-experiment-v2 matched targeting RemoteSettingsExperimentLoader.sys.mjs:477
  • ExperimentManager: Enrollment feature-configuration-testing-for-opaqueresponseblocking-experiment-v2 has expired, aborting. ExperimentManager.sys.mjs:536
  • RSLoader: Testing targeting expression: (version|versionCompare('116.!') >= 0) RemoteSettingsExperimentLoader.sys.mjs:540
  • RSLoader: [rollout] feature-configuration-testing-for-opaqueresponseblocking-rollout-v2 matched targeting RemoteSettingsExperimentLoader.sys.mjs:477
  • ExperimentManager: Re-enrolling in rollout "feature-configuration-testing-for-opaqueresponseblocking-rollout-v2 ExperimentManager.sys.mjs:551
  • ExperimentManager: New rollout started: feature-configuration-testing-for-opaqueresponseblocking-rollout-v2, rollout-single-branch

about:telemetry events:

• 24292 normandy unenroll nimbus_experiment feature-configuration-testing-for-opaqueresponseblocking-experiment-v2 {"reason": "individual-opt-out", "branch": "treatment-b", "enrollmentId": "28def9eb-f73a-4e1f-8e30-f0b563c79fe6"}
• 24307 normandy unenroll nimbus_experiment feature-configuration-testing-for-opaqueresponseblocking-rollout-v2 {"reason": "changed-pref", "branch": "rollout-single-branch", "enrollmentId": "7817cab3-eee7-46ce-8098-18e5d3979d9b", "changedPref": "browser.opaqueResponseBlocking"}

The issue is not reproducible if the Experiment branch sets the same values as the Rollout configuration.

Comment 9

[Beth Rennie [:beth] (she/her)]

I've managed to reproduce the issue and I've tracked it down to this piece of code:

      if (entry) {
        entry.enrollmentChanging = true;
      }

      lazy.PrefUtils.setPref(pref.name, newValue, {
        branch: pref.branch,
      });

      if (entry) {
        entry.enrollmentChanging = false;
      }

When the pref callback is called, entry.enrollmentChanging has already been reset to false. AFAIK the callbacks should be called synchronously in response to the setPref call, but I'm investigating.

Comment 11

[Beth Rennie [:beth] (she/her)]

Attachment: Bug 1850127 - Handle setPref prefs which are nested r?emcminn

Pref observers registered for prefs like "foo" will also fire for pref changes
on "foo.bar", etc. If a feature registered nested prefs like these, then a
change to pref "foo.bar" would trigger the pref listener for "foo.bar" and
cause spurious unenrollments, especially when attempting to enroll in both an
experiment and rollout at the same time, and when trying to unenroll from an
experiment when also enrolled in a rollout.

Depends on D187789

Comment 12

[Beth Rennie [:beth] (she/her)]

(In reply to Barret Rennie [:barret] (they/them) from comment #9)

I've managed to reproduce the issue and I've tracked it down to this piece of code:

      if (entry) {
        entry.enrollmentChanging = true;
      }

      lazy.PrefUtils.setPref(pref.name, newValue, {
        branch: pref.branch,
      });

      if (entry) {
        entry.enrollmentChanging = false;
      }

When the pref callback is called, entry.enrollmentChanging has already been reset to false. AFAIK the callbacks should be called synchronously in response to the setPref call, but I'm investigating.

The issue here is that a pref observer for browser.opaqueResponseBlocking was also being called for any pref under browser.opaqueResponseBlocking, such as browser.opaqueResponseBlocking.filterFetchResponse, causing spurious unenrollments. This also caused bug 1850120.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c59e4f361ba0
Handle setPref prefs which are nested r=emcminn

Comment 14

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/c59e4f361ba0

Comment 15

[BugBot [:suhaib / :marco]]

The patch landed in nightly and beta is affected.
:barret, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox118 to wontfix.

For more information, please visit BugBot documentation.

Comment 16

[Beth Rennie [:beth] (she/her)]

Comment on attachment 9352558 [details]
Bug 1850127 - Handle setPref prefs which are nested r?emcminn

Beta/Release Uplift Approval Request

• User impact if declined: This will affect our ability to run pref-setting experiments
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Only affects pref-settings experiments. Code change is small (2 loc) and heavily covered by tests.
• String changes made/needed:
• Is Android affected?: No

Comment 17

[Pascal Chevrel (relman team) -> :pascalc]

Comment on attachment 9352558 [details]
Bug 1850127 - Handle setPref prefs which are nested r?emcminn

Approved for 118.0b9 (last beta) thanks.

Comment 18

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/1d6a66bf2e08

Comment 19

[Pascal Chevrel (relman team) -> :pascalc]

https://hg.mozilla.org/releases/mozilla-beta/rev/1d6a66bf2e08

Comment 20

[Valentin Bandac [:vbandac], Ecosystem QA]

We have verified this issue on Firefox Release 118.0 (Build ID: 20230918143747) and Firefox Release 119.0.1 (Build ID: 20231106151204) on Windows 10 x64, Windows 11 x64, macOS 13.6, Ubuntu 22.04 x64.

• The user is NOT unenrolled from a rollout if the user unenrolls from an experiment, both using the opaqueResponseBlocking feature config.