1435231 - [Normandy Telemetry] Pref-experiments/Studies - telemetry event not logged for case "user-preference-changed-sideload"/"uninstalled-sideload"

Status: VERIFIED FIXED

(Firefox :: Normandy Client, defect, P1)

Description

[Adrian Florinescu [:aflorinescu]]

[Environments:]
Ubuntu 16.04 x64
Firefox Nightly 60.0a1 20180201220225

[Prerequisites]
You need access to Admin interface (https://normandy-admin.stage.mozaws.net/control) or New Admin interface (https://normandy-admin.stage.mozaws.net/control-new)
1. Obtain a copy of Firefox with the SHIELD recipe client system add-on installed. You can check about:support to ensure that you have it.
2. Set the extensions.shield-recipe-client.dev_mode preference to true to run recipes immediately on startup.
3. Set the extensions.shield-recipe-client.logging.level preference to 0 to enable more logging.
4. Set the security.content.signature.root_hash preference to DB:74:CE:58:E4:F9:D0:9E:E0:42:36:BE:6C:C5:C4:F6:6A:E7:74:7D:C0:21:42:7A:03:BC:2F:57:0C:8B:9B:90.
5. Set the preference value for extensions.shield-recipe-client.api_url set to https://normandy.stage.mozaws.net/api/v1.

[Steps:]
1. Open Admin Interface.
2. Create/Approve/Publish a new preference experiment.
3. Create a new profile, set the pre-requisites, restart browser.
4. Open profile location, edit prefs.js and remove or update the value for the preference that the pref-experiment modifies or adds.
5. Open about:telemetry /Current ping/Events


[Actual Result:]
3. Preference experiment recipe is executed: Opening "profile location"/shield-preference-experiments.json will list the pref-experiment with "expired":false".
5. There is no Events section, the "profile location"/shield-preference-experiments.json will list the pref-experiment with "expired":true".

[Expected Result:]
5. There is telemetry Event for the unenroll with reason: "user-preference-changed-sideload"

Comment 1

[Adrian Florinescu [:aflorinescu]]

Note: Similar result for Studies type recipes, there is no telemetry Event for unenroll.

Comment 2

[Michael Cooper [:mythmon]]

Attachment: Bug 1435231 - Initialize Normandy events before other subcomponents

Review commit: https://reviewboard.mozilla.org/r/218202/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/218202/

Comment 3

[:Gijs (he/him)]

Comment on attachment 8948828 [details]
Bug 1435231 - Initialize Normandy events before other subcomponents

https://reviewboard.mozilla.org/r/218202/#review224018

r=me in that this is essentially a no-brainer thing to do + review.

However, I haven't manually tested this - I assume you have - but is it possible to add automated tests for this? That'd help ensure we don't miss something like this again...

(Also, did you push this to try? You can use mozreview for this or post a link if doing it with ./mach try .)

Comment 4

[Michael Cooper [:mythmon]]

Comment on attachment 8948828 [details]
Bug 1435231 - Initialize Normandy events before other subcomponents

https://reviewboard.mozilla.org/r/218202/#review224018

> However, I haven't manually tested this - I assume you have - but is it
> possible to add automated tests for this? That'd help ensure we don't miss
> something like this again...

I have manually tested this.

I'd like to have automate tests, but I'm not sure if there is a reasonable way to do it. Currently we initialize Events at the beginning of the entire test run, so that unrelated tests don't have to do that, so it would be tough to write tests that initialization happens in general. There are tests of this initialization function, but I think the best we could do is say that the Events initializer is called first. That doesn't seem like a very valuable test to me though.

This won't be needed for long anyways. Once Normandy is converted to a component, in bug 1436113, we can use the static yaml registration for events instead of this dynamic one. I think I'd prefer to lean on that instead of making this robust.

> 
> (Also, did you push this to try? You can use mozreview for this or post a
> link if doing it with ./mach try .)

I haven't yet, but I'll do that before merging.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/30a4e99f34af
Initialize Normandy events before other subcomponents r=Gijs

Comment 6

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/30a4e99f34af

Comment 7

[:Gijs (he/him)]

(In reply to Mike Cooper [:mythmon] from comment #4)
> There are tests of this initialization function, but I think the best
> we could do is say that the Events initializer is called first. That
> doesn't seem like a very valuable test to me though.

Ah, right. I concur that that probably isn't a worthwhile thing to invest in.

Comment 8

[Adrian Florinescu [:aflorinescu]]

Verified on OSX 10.12, Windows 8.1 and Ubuntu 16.04 using latest Nightly 2018-02-09 for both studies and experiments;

example of logged event:
"1423 	normandy 	unenroll 	addon_study 	231 	{"addonId": "{60B7679C-BED9-11E5-998D-8526BB8E7F8B}", "addonVersion": "9.5", "reason": "uninstalled-sideload"}"