1791163 - [Experiment] Users are not enrolled into the QA Nightly Firefox MR Existing User Experience experiment on Firefox 105

Status: VERIFIED FIXED

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

Description

[Maria Heres, inactive, contact Ecosystem QA]

Attachment: mr 105.gif

[Affected Versions]:

• Firefox RC 2 105.0 (Build ID: 20220915150737)
• Firefox Nightly 105.0a1 (Build ID: 20220802094116)

[Affected Platforms]:

• Windows 10
• macOS 11.6.6
• Linux Mint 20.2

[Prerequisites]:

• If using a build other than Nightly, have the “channel-prefs” file of the build modified to have "app.update.channel" set to “nightly”.
• Have updates disabled (e.g. by selecting “Check for updates but let you choose to install them” from “about:preferences”).

[Steps to reproduce]:

1. Open the browser.
2. Open “about:config” and set the following prefs:
   messaging-system.rsexperimentloader.collection_id set to nimbus-preview.
   messaging-system.log set to all
3. Open the Multiprocess Browsing Console (using Ctrl/command + Shift + J).
4. Restart the browser (by using Ctrl/command + Alt + R with the Browsing Console focused).
5. Open “about:studies” and observe the page.
6. Focus the Browsing Console and observe the messages displayed for “qa-nightly-firefox-mr-existing-user-experience”.

[Expected result]:
Step 5. The “QA Nightly Firefox MR Existing User Experience” study is displayed in “Active studies”.
Step 6. There is a message displayed stating that “qa-nightly-firefox-mr-existing-user-experience” matched.

[Actual result]:
Step 5. The “QA Nightly Firefox MR Existing User Experience” study is not displayed.
Step 6. There are messages displayed stating that

• “qa-nightly-firefox-mr-existing-user-experience did not validate”
• properties do “not match additional properties schema.”

[Notes]:

• The Nightly version of the build displays the message that “Experiment qa-nightly-firefox-mr-existing-user-experience has unknown featureId: majorRelease2022”.
• The issue is not reproducible for Firefox Nightly 106.0a1 (Build ID: 20220915171049)
• Attached is a recording of the issue.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

:barret, since you are the author of the regressor, bug 1774022, could you take a look?

For more information, please visit auto_nag documentation.

Comment 2

[Ed Lee :Mardak]

[Tracking Requested - why for this release]: MR holdback experiment needs to enroll users during 105

Comment 3

[Dan Mosedale (:dmosedale, :dmose)]

The current hypothesis is below. I'm working up an integration test (which we will need for landing in any case) to verify that this is the actual mechanism at work here; then I'll post detailed options for addressing it.

https://experimenter.services.mozilla.com/nimbus/qa-nightly-firefox-mr-existing-user-experience
is a holdback experiment planned for MR2 / Firefox 106 release. This requires that enough existing Firefox 105 release users get enrolled into the experiment before 106 lands on the release channel.

A few months ago, bug 1774022 made validation of integer fields in “variables only schemas” more strict in the Nimbus Desktop Client.

It also made a change that caused validation of variables-only schemas to disallow unknown properties: https://searchfox.org/mozilla-central/source/toolkit/components/nimbus/lib/RemoteSettingsExperimentLoader.jsm#518

At the end of last week, QA discovered that the MR2 holdback experiment was not enrolling users in Firefox 105. That cause has been traced back to the above change.

Comment 4

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: Bug 1791163 - check RemoteSettingsExpLoader handles featureIds w/additional props, r=barret

Comment 5

[Dan Mosedale (:dmosedale, :dmose)]

The integration test I just attached confirms that reverting the change mentioned above would fix this problem: it fails on current trunk, and passes if one changes additionalProperties in the code linked to on searchfox above from false to true (though I don't know if other things should be changed in addition to that to make it a safe change).

It's derived from one of the existing tests, but with the JSON replaced by something extremely close to the exact JSON that's on experimenter for the QA test right now. This is overkill for just testing the additional property thing, so it might not be what really wants to land.

Comment 6

[Dan Mosedale (:dmosedale, :dmose)]

As far as options go, Shell has scheduled a meeting including people with more context tomorrow for us to dig into that in detail.

Comment 7

[Dan Mosedale (:dmosedale, :dmose)]

For completeness, here's a list of a things that came up as possible options for dealing with this so far:

a) revert the additionalProperties fix as discussed above and uplift that to the scheduled 105.x release
b) uplift the variables of the majorRelease2022 section of FeatureManifest.yaml to the scheduled 105.x release
c) Use a Normandy pref rollout to set the the schema verification 'kill switch' pref implemented for these sorts of situations.

I expect that Tuesday's meeting will solidify a plan, likely a).

Comment 8

[Simona Rosu [:srosu], Ecosystem QA]

This issue is also reproducible when trying to enroll in the new users' study.

Comment 9

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

(In reply to Maria Heres, [:mheres], Ecosystem QA from comment #0)

[snip]
[Notes]:

• The Nightly version of the build displays the message that “Experiment qa-nightly-firefox-mr-existing-user-experience has unknown featureId: majorRelease2022”.

This is because the nightly build you are using (20220802094116) does not have the feature. The feature was added on Aug 11 so any nightly from Aug 12 onward should work.

(In reply to Dan Mosedale (:dmosedale, :dmose) from comment #7)

For completeness, here's a list of a things that came up as possible options for dealing with this so far:

a) revert the additionalProperties fix as discussed above and uplift that to the scheduled 105.x release
b) uplift the variables of the majorRelease2022 section of FeatureManifest.yaml to the scheduled 105.x release
c) Use a Normandy pref rollout to set the the schema verification 'kill switch' pref implemented for these sorts of situations.

I expect that Tuesday's meeting will solidify a plan, likely a).

I agree that option a is the best way forward. It is the minimal amount of work and needs to be fixed anyway. Lifting the variables into 105 is not an option, because any logic that uses majorRelease2022 variables will turn on once we enroll people in 105. See searchfox -- there are a lot of uses.

Comment 10

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

Attachment: Bug 1791163 - Allow additional variables when validating features that are variables-only r?jlockhart

Comment 11

[Ed Lee :Mardak]

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

any logic that uses majorRelease2022 variables will turn on once we enroll people in 105. See searchfox -- there are a lot of uses.

That logic only exists in 106 (beta). The query checking 105 (release) shows only reference is in FeatureManifest.yaml:
https://searchfox.org/mozilla-release/search?q=majorRelease2022

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ba3343338252
Allow additional variables when validating features that are variables-only r=jlockhart

Comment 13

[Dan Mosedale (:dmosedale, :dmose)]

Here are the 105 and 106 builds from those branches, with the patch here built in.

They are all visibly branded Nightly, but you’ll definitely need to edit channel-prefs, because these builds all have a channel of “nightly-try”. If you look at the "Firefox Nightly" -> "About Nightly" dialog, you should see the version number of the actual branch it was built from (ie 105.0.1 for release, 106.0b2 for beta).

These builds are not signed, which means that on Mac, you’ll likely need to do something along the lines of https://support.apple.com/guide/mac-help/open-a-mac-app-from-an-unidentified-developer-mh40616/mac
to start them.

The 105 try run link in case the links below are insufficient for whatever reason:
https://treeherder.mozilla.org/jobs?repo=try&revision=c0fa97c522d4951ced33764da33ba95b0d05379b

105 builds:

Mac: OS X Cross Compiled Shippable Bpgo(B*)
Windows: Windows 2012 x64 Shippable Bpgo(B)
Linux: Linux x64 Shippable Bpgo(B)

The 106 try run link in case the links below are insufficient for whatever reason:
https://treeherder.mozilla.org/jobs?repo=try&revision=765aca5113c935fd4fd61abe3c611d850eb5f25a

106 builds:
Mac: OS X Cross Compiled Shippable Bpgo(B*)
Windows: Windows 2012 x64 Shippable Bpgo(B)
Linux: Linux x64 Shippable Bpgo(B)

Comment 14

[Dan Mosedale (:dmosedale, :dmose)]

The things most worth testing (presumably both for new & existing users, given that, as I understand it, both have issues):

• Enrolls in 105 build above
• Same profile stays enrolled after starting 106 build above

These above two will help give us confidence for uplifts. If things go well with QA, I'll nominate the patch for uplift to 106 Wednesday.

Comment 15

[Ed Lee :Mardak]

toolkit.telemetry.overrideUpdateChannel

Comment 16

[Iulian Moraru]

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

Comment 17

[Simona Rosu [:srosu], Ecosystem QA]

Thank you, Dan, for the detailed information provided! I‘ve verified this issue using the try builds on Windows 10 x64, macOS 12.5.1, and Linux Mint 20.2 x64., and I confirm that the users can be successfully enrolled in the “QA Nightly Firefox MR Existing User Experience experiment” holdback study.

Since both experiments (new users and existing users) have the same targeting, I couldn’t enroll in the “QA Nightly Firefox MR New User” study, and to be able to verify this, I created a clone of the recipe on the Stage server ((link)[https://stage.experimenter.nonprod.dataops.mozgcp.net/nimbus/qa-nightly-firefox-mr-new-user-stage/details]) where the users can be successfully enrolled in this study as well.

Also, the users are still enrolled in the “QA Nightly Firefox MR Existing User Experience experiment” study after opening Firefox 106 with the same profile.

Comment 18

[BugBot [:suhaib / :marco/ :calixte]]

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

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

For more information, please visit auto_nag documentation.

Comment 19

[Simona Rosu [:srosu], Ecosystem QA]

I also verified this issue on the latest Firefox Nightly build downloaded from treeherder (Build ID: 20220921035608) on Windows 10 x64, macOS 12.5.1, and Linux Mint 20.2 x64 where the users can be enrolled in the “QA Nightly Firefox MR New User” (stage) and “QA Nightly Firefox MR Existing User Experience experiment” exeperiments.

Comment 20

[Dan Mosedale (:dmosedale, :dmose)]

[Tracking Requested - why for this release]:
We plan to uplift this to the 105.0.x release so that the holdback experiment will work correctly. To get extra testing, the correct behavior, and to keep things easy to reason about as we do the uplifting and as the experiments run, it should be uplifted to 106 as well.

Comment 21

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 9295433 [details]
Bug 1791163 - Allow additional variables when validating features that are variables-only r?jlockhart

Beta/Release Uplift Approval Request

• User impact if declined: MR 2022 Holdback experiment will fail.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: Detailed in comment 0, comment 14, and comment 17 of this bug.
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Two line code change with automated testing slightly loosens overzealous experiment enrollment rejection criteria (ie makes things more likely to work, rather than less). This patch has already been manually QAed in try builds from the 105 and 106 branches as described in comment 17.
• String changes made/needed: None
• Is Android affected?: No

Comment 22

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9295433 [details]
Bug 1791163 - Allow additional variables when validating features that are variables-only r?jlockhart

Needed for an eventual dot release. Approved for 106.0b3.

Comment 23

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/960b7c49053c

Comment 24

[Dan Mosedale (:dmosedale, :dmose)]

It can be QAed in beta once 106.b3 is released, and I believe that is currently scheduled for Thursday the 22nd.

Comment 25

[Simona Rosu [:srosu], Ecosystem QA]

I’ve verified this issue using Firefox Beta 106.0b3 (Build ID: 20220922185808) on Windows 10 x64, macOS 12.5.1, and Linux Mint 20.2 x64.
I confirm that the users can be enrolled in the “QA Nightly Firefox MR New User” (stage) and “QA Nightly Firefox MR Existing User Experience experiment” experiments.

Comment 26

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9295433 [details]
Bug 1791163 - Allow additional variables when validating features that are variables-only r?jlockhart

Approved for 105.0.2 so QA can start testing the builds, but it would be nice to get the test landed soon if possible.

Comment 27

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/ac867a5225dd

Comment 28

[Simona Rosu [:srosu], Ecosystem QA]

I‘ve verified this issue using the Firefox Release 105.0.2 (Build ID: 20220929212302) downloaded from Treeherder on Windows 10 x64, macOS 12.5.1, and Ubuntu 20.04 x64., and I confirm that the users can be successfully enrolled in the “QA Nightly Firefox MR Existing User Experience experiment” holdback study.

Comment 29

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f869451e1a3e
check RemoteSettingsExpLoader handles featureIds w/additional props, r=barret

Comment 30

[Marian-Vasile Laza]

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