Closed Bug 1179226 Opened 6 years ago Closed 6 years ago
Support Backend should not rely on FHR prefs
With FHR possibly being turned off when unified telemetry lands, we need to make sure selfsupport keeps working. Right now, it depends on FHR prefs . * We should make SelfSupport work if either FHR is on or Unified Telemetry is on. * We should check the tests and make sure the use the correct prefs.  https://hg.mozilla.org/mozilla-central/annotate/079b6f1ae1c3/browser/modules/SelfSupportBackend.jsm#l82
6 years ago
6 years ago
Assignee: nobody → alessio.placitelli
This patch changes the SelfSupportBackend.jsm  module for Heartbeat to make it work with FHR turned off. The tests don't need to be changed. SelfSupportService.js  doesn't need to be changed now: about:healthreport needs to be changed to stop using the FHR helper function from MozSelfSupport first. I've manually checked that MozSelfSupport works (for the telemetry part) by opening about:healthreport and issuing MozSelfSupport.getTelemetry* calls from the web console.  - https://hg.mozilla.org/mozilla-central/annotate/cef11c3e86c3/browser/modules/SelfSupportBackend.jsm  - https://hg.mozilla.org/mozilla-central/annotate/cef11c3e86c3/browser/components/selfsupport/SelfSupportService.js
Attachment #8630060 - Flags: review?(gfritzsche)
Comment on attachment 8630060 [details] [diff] [review] bug1179226.patch Review of attachment 8630060 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/modules/SelfSupportBackend.jsm @@ +25,5 @@ > const PREF_URL = "browser.selfsupport.url"; > // FHR status. > const PREF_FHR_ENABLED = "datareporting.healthreport.service.enabled"; > +// Unified Telemetry status. > +const PREF_TELEMETRY_UNIFIED = "toolkit.telemetry.unified"; Too much whitespace before the `=`.
Attachment #8630060 - Flags: review?(gfritzsche) → review+
Thanks. Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5cc5b47f5350
Whiteboard: [b5] [unifiedTelemetry] [uplift2] → [rC] [unifiedTelemetry] [uplift3]
Comment on attachment 8631435 [details] [diff] [review] bug1179226.patch - v2 Approval Request Comment [Feature/regressing bug #]: This is part of the uplift3 batch for the unified Telemetry project: http://bit.ly/1TCl4r8 This is targetting 41 now and these are the last minor "feature" patches blocking shipping - any further patches will be fixes for data quality etc. with very limited scope & impact. [User impact if declined]: Unified Telemetry can't ship. This is a fix needed to keep selfsupport working. [Describe test coverage new/current, TreeHerder]: This has automated test-coverage. [Risks and why]: Low-risk, well understood small behavior change limited to selfsupport code. [String/UUID change made/needed]: No string changes.
Attachment #8631435 - Flags: approval-mozilla-aurora?
Comment on attachment 8631435 [details] [diff] [review] bug1179226.patch - v2 Seems safe to uplift as this code has been in m-c for more than 2 weeks. Approved.
Attachment #8631435 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Tracked. Requesting QE team to verify when possible. Thanks!
Alessio, could you please provide more detailed instructions on how to verify this fix? Based on comment 1, after calling MozSelfSupport.getTelemetry* (both Ping and Pinglist) calls from the Web Console, I get: http://i.imgur.com/ixzez9k.png with latest 41.0b8 and “ReferenceError: MozSelfSupport is not defined” is thrown with 40.0b9, but nothing different visible in about:healthreport. Thanks in advance!
Hi Alexandra! MozSelfSupport is not directly involved with SelfSupportBackend. To test the latter, I think your best shot is to check if it gets disabled due to FHR or UT being disabled , by disabling both the unified telemetry pref and the FHR service pref. Please do not hesitate to contact me back if you need more info!  - http://mxr.mozilla.org/mozilla-central/source/browser/modules/SelfSupportBackend.jsm#90
Note: to check if it gets disabled, just check the browser console for the "init - Disabling SelfSupport because FHR and Unified Telemetry are disabled." log line.
Verified fixed with latest 41.0b8, across platforms  - with FHR & UT disabled and 'browser.selfsupport.log.level' set to 'Trace', the following lines are visible in the console: 1441718116253 Browser.SelfSupportBackend TRACE init 1441718116253 Browser.SelfSupportBackend CONFIG init - Disabling SelfSupport because FHR and Unified Telemetry are disabled.  Ubuntu 14.04 32-bit, Mac OS X 10.10.4, Windows 7 64-bit
Also confirming this fix with 42.0b1 (Build ID: 20150921151815), across platforms .  Windows 7 64-bit, Mac OS X 10.11 Beta and Ubuntu 14.04 32-bit
You need to log in before you can comment on or make changes to this bug.