1379627 - Re-evaluate filter expressions for preference experiments before setting prefs on startup

Status: NEW

(Shield :: Shield Study, enhancement)

Description

[Nicolas B. Pierron [:nbp]]

Bug 1373241 requested the test of pref-flip experiment on Nightly, and we are seeing a few Beta crashes where this specific pref-flip experiment is running:

https://crash-stats.mozilla.com/report/index/24b5b757-0274-49f3-9a25-52d520170710

Without knowing how the addons works, the most plausible hypothesis involves users switching from Nightly to beta, or running both alternatively.

I will note that this stack trace should have became a top-top-top-crasher on beta if the pref-flip experiment had been enabled on beta. It got fixed before the request to run the pref-flip experiment on nightly.

Comment 1

[Osmose [:osmose, :mkelly]]

Looking into this now.

Comment 2

[Osmose [:osmose, :mkelly]]

The recipe appears to be correctly targeted to Nightly, although the title in the admin says Beta for some reason. Revision history doesn't show it ever being targeted to Beta, and I can't get a fresh profile in Beta to fetch the recipe, so the description's guess at users switching channels seems plausible; while users should be swapped out of the experiment next time they fetch recipes while on the new channel, there's up to 24 hours in which the experiment may still be running while they're on a new channel. 

I'm going to see if it's possible to confirm this hypothesis with telemetry pings.

Comment 3

[Osmose [:osmose, :mkelly]]

My telemetry-fu isn't great, but I searched the past four days of telemetry pings on Nightly and couldn't find any from the client in the crash linked above. I still think the likely culprit is switching channels/sharing a profile across channels, though.

nbp: I've rarely used crash-stats, so I dunno how to check this myself, but how many crashes are we seeing? Like, <5 a day?

Comment 4

[Osmose [:osmose, :mkelly]]

Potential solution to this: Store the last filter expression seen for each experiment, and re-evaluate the expression on startup before modifying the preference. We'll have to cache client data from the server (geolocation and time), but otherwise I think this is a workable solution.

Matt_G: Thoughts on the solution and priority? IMO unless we're seeing a bunch of crashes/users who cross channels running experiments in the wrong channel, this can probably wait for a few weeks.

Comment 5

[Matt Grimes [:Matt_G]]

That seems like the right approach. +1

Comment 6

[Nicolas B. Pierron [:nbp]]

(In reply to Michael Kelly [:mkelly,:Osmose] from comment #3)
> My telemetry-fu isn't great, but I searched the past four days of telemetry
> pings on Nightly and couldn't find any from the client in the crash linked
> above. I still think the likely culprit is switching channels/sharing a
> profile across channels, though.
> 
> nbp: I've rarely used crash-stats, so I dunno how to check this myself, but
> how many crashes are we seeing? Like, <5 a day?

For this signature, yes. We only have 4 reports on Beta (1%) from 4 different users.

Otherwise I am no expert my-self and I do not yet have access to rawdump to filter all crashes based on the experiment flags.
Julien Cristau reported this issue to me earlier today, he might know better.

Comment 7

[Julien Cristau [:jcristau]]

I'm not seeing a way to filter on the TelemetryEnvironment field in crash-stats, I just looked at the few crashes on beta manually since I was surprised to see that particular signature show up there.

Comment 8

[Osmose [:osmose, :mkelly]]

Given the impact, this isn't high-priority, but I think we should implement the fix eventually anyway:

- When a preference experiment starts, store the filter expression used to evaluate it.
- Modify the client information we fetch during recipe runs (geolocation and current time) to be cached between runs/
- On startup, before we modify preferences, re-evaluate the filter expression to ensure that it is still valid. If it is not, disable the experiment. If the filters use geolocation or current time, use the cached versions.

This will probably be heavily affected by how bug 1371350 is changing out startup preference changing stuff, so we should wait until that is resolved before moving to this.