Closed Bug 1791163 Opened 1 year ago Closed 1 year ago

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

Categories

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

Product:
Firefox
Component:
Nimbus Desktop Client
Platform:
Desktop
All
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
107 Branch
Iteration:
107.1 - Sept 19 - Sept 30
Tracking Flags:
Tracking Status
firefox-esr102 --- unaffected
firefox105 + verified
firefox106 + verified
firefox107 + verified

People

(Reporter: mheres, Assigned: dmosedale)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: regression)

Attachments

(3 files)

mr 105.gif
4.61 MB, image/gif
Details
Bug 1791163 - check RemoteSettingsExpLoader handles featureIds w/additional props, r=barret
48 bytes, text/x-phabricator-request
Details | Review
Bug 1791163 - Allow additional variables when validating features that are variables-only r?jlockhart
48 bytes, text/x-phabricator-request
RyanVM
: approval-mozilla-beta+
RyanVM
: approval-mozilla-release+
Details | Review
Attached image 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.
Severity: -- → S2
Blocks: message-schema
Regressed by: 1774022

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

For more information, please visit auto_nag documentation.

Flags: needinfo?(brennie)

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

tracking-firefox105: --- → ?
Assignee: nobody → dmosedale
Iteration: --- → 107.1 - Sept 19 - Sept 30
Priority: -- → P1
Component: Messaging System → Nimbus Desktop Client

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.

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.

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

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).

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

(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.

Flags: needinfo?(brennie)

(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

Pushed by dmosedale@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ba3343338252
Allow additional variables when validating features that are variables-only r=jlockhart

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)

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.

toolkit.telemetry.overrideUpdateChannel

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

Status: NEW → RESOLVED
Closed: 1 year ago
status-firefox107: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → 107 Branch

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.

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.

Flags: needinfo?(dmosedale)

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.

Status: RESOLVED → VERIFIED
status-firefox107: fixed → verified

[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.

tracking-firefox106: --- → ?
Flags: needinfo?(dmosedale)

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
Attachment #9295433 - Flags: approval-mozilla-release?
Attachment #9295433 - Flags: approval-mozilla-beta?
Flags: qe-verify+

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.

Attachment #9295433 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

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

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.

status-firefox106: fixed → verified
QA Whiteboard: [qa-triaged]
Attachment #9295333 - Attachment description: WIP: Bug 1791163 - integration test failing on additional props → Bug 1791163 - integration test failing on additional props, r?barret
Attachment #9295333 - Attachment description: Bug 1791163 - integration test failing on additional props, r?barret → Bug 1791163 - integration test should pass if additional on additional props given, r?barret

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.

Attachment #9295433 - Flags: approval-mozilla-release? → approval-mozilla-release+

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.

status-firefox105: fixed → verified
Flags: qe-verify+
Attachment #9295333 - Attachment description: Bug 1791163 - integration test should pass if additional on additional props given, r?barret → Bug 1791163 - check RemoteSettingsExpLoader handles featureIds w/additional props, r?barret
Attachment #9295333 - Attachment description: Bug 1791163 - check RemoteSettingsExpLoader handles featureIds w/additional props, r?barret → Bug 1791163 - check RemoteSettingsExpLoader handles featureIds w/additional props, r=barret
Pushed by dmosedale@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f869451e1a3e
check RemoteSettingsExpLoader handles featureIds w/additional props, r=barret
You need to log in before you can comment on or make changes to this bug.