Closed Bug 1460893 Opened 3 years ago Closed 3 years ago
No telemetry event created when the recipe preference type doesn't match the Firefox preference type
59 bytes, text/x-review-board-request
[Affected versions]: - 62.0a1 (2018-05-11) - 61.0b4 build1 (20180510160705) [Affected platforms]: - Windows 10 x54 - macOS 10.13.3 - Ubuntu 16.04 x86 [Preconditions] - You need access to SHIELD Control Panel ( https://normandy-admin.stage.mozaws.net/ ) - Set the 'app.normandy.dev_mode' preference to 'true' to run recipes immediately on startup. - Set the 'app.normandy.logging.level' preference to '0' to enable more logging. - 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'. - Set the preference value for 'app.normandy.api_url' to 'https://normandy.stage.mozaws.net/api/v1'. [Steps to reproduce]: 1. Open the SHIELD Control Panel. 2. Start creating a new pref-experiment recipe (e.g. for a boolean type preference). 3. For the preference name use a preference whose type is different (e.g. integer) 4. Approve and publish the recipe. 5. Launch Firefox having the pre-requisites set and open: - browser console - about:telemetry (locate the Events tab and choose "dynamic" in top right corner dropdown) [Expected result]: - about:telemetry states the enroll failed status for the current experiment - browser console shows that the recipe could not be executed [Actual result]: - No telemetry event is created for this particular recipe - starting with 2018-04-04, the "Could not execute recipe [...]" error is not displayed anymore in browser console (possible regressed by bug 1450153) [Regression range]: - Not a regression introduced by bug 1436113 [Additional notes]: - See also bug 1447254
Assignee: nobody → mcooper
Status: NEW → ASSIGNED
Attachment #8979371 - Attachment is obsolete: true
Comment on attachment 8979372 [details] Bug 1460893 - Add event telemetry to preference study error cases https://reviewboard.mozilla.org/r/245530/#review251760
Attachment #8979372 - Flags: review?(gijskruitbosch+bugs) → review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/c28aa36127ad Add event telemetry to preference study error cases r=Gijs
Please request Beta approval on this when you get a chance.
Comment on attachment 8979372 [details] Bug 1460893 - Add event telemetry to preference study error cases Approval Request Comment [Feature/Bug causing the regression]: None [User impact if declined]: Very little. If a faulty recipe is sent out, Mozilla will receive less useful debugging information. [Is this code covered by automated tests?]: Yes [Has the fix been verified in Nightly?]: Yes [Needs manual test from QE? If yes, steps to reproduce]: Yes, see comment 0 [List of other uplifts needed for the feature/fix]: None [Is the change risky?]: No [Why is the change risky/not risky?]: It is a very minor change to error handling. [String changes made/needed]: None
Attachment #8979372 - Flags: approval-mozilla-beta?
Comment on attachment 8979372 [details] Bug 1460893 - Add event telemetry to preference study error cases Simple preference study fix. Approved for 61.0b8.
Attachment #8979372 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
I can confirm that the fix is implemented on 62.0a1 (2018-05-25) and 61.0b8 build1 (20180524181234), across platforms (Windows 10 x64, Ubuntu 16.04 x64 and macOS 10.13.4). Now about:telemetry successfully displays the enrollFailed event when the recipe preference type doesn't match the Firefox preference type. There is still one potential problem regarding Browser Console, as the following error is triggered: "Previous preference value is of type "128", but was given "64" (integer) PreferenceExperiments.jsm:337" (the recipe preference type is integer and the Firefox preference type is boolean in this particular case). Any thoughts about this?
This code is designed to throw in the case of invalid data. I'm not sure if this is a good or bad design, but it isn't something I want to change right now. We'll likely be doing some significant rewriting of this code soon anyways.
Thank you Michael! Per comment 11 and comment 12, I will mark 62 and 61 as verified fixed.
You need to log in before you can comment on or make changes to this bug.