It appears we stopped getting Fennec telemetry since around the 4-18 Nightly. One cause I see is that the "datareporting.healthreport.uploadEnabled" pref is *not* set in Fennec, but there may be other causes.

Can someone look into this, Vladan? We didn't notice this for about a month, so we should get it fixed asap.

I don't think "datareporting.healthreport.uploadEnabled" controls the FHR system in Fennec, which is a Java background service. That said, Telemetry is not connected to that pref either, and we do depend on Gecko's Telemetry system to upload telemetry.

(In reply to Mark Finkle (:mfinkle) from comment #2) > I don't think "datareporting.healthreport.uploadEnabled" controls the FHR > system in Fennec, which is a Java background service. That said, Telemetry > is not connected to that pref either, and we do depend on Gecko's Telemetry > system to upload telemetry. Unified Telemetry (FHR + Telemetry) does look at the "datareporting.healthreport.uploadEnabled" pref: http://hg.mozilla.org/mozilla-central/annotate/4fb7ff694bf5/toolkit/components/telemetry/TelemetryController.jsm#l1186

(In reply to Vladan Djeric (:vladan) -- please needinfo! from comment #3) > Unified Telemetry (FHR + Telemetry) does look at the > "datareporting.healthreport.uploadEnabled" pref: > > http://hg.mozilla.org/mozilla-central/annotate/4fb7ff694bf5/toolkit/ > components/telemetry/TelemetryController.jsm#l1186 Fennec is not using Unified Telemetry, right?

(In reply to Vladan Djeric (:vladan) -- please needinfo! from comment #3) > Unified Telemetry (FHR + Telemetry) does look at the > "datareporting.healthreport.uploadEnabled" pref: Guess that's the bug then: don't do that. :D Fennec ships Gecko's telemetry but not Gecko's FHR.

Clearly we've specified that this is how desktop unified telemetry is supposed to behave. So what you want is an android-specific exception.

(In reply to Benjamin Smedberg [:bsmedberg] from comment #6) > Clearly we've specified that this is how desktop unified telemetry is > supposed to behave. So what you want is an android-specific exception. If the pref is unused for any Java FHR impl, but could be used to easily re-enable Telemetry then let's add it to mobile.js Richard - Sound safe?

Safe for these purposes, potentially confusing for users or distributions that think they can use that pref to control health report upload on Android, or for future developers. If you take that avenue, comment the heck out of the pref value. Some alternate solutions: * don't check d.h.uE if toolkit.telemetry.unified is false (which is the case for Fennec). * introduce a second pref that can unilaterally allow telemetry upload, and check that alongside. * don't assume that the _absence_ of d.h.uE (which I presume is the case for Fennec) means no telemetry upload. There are a few different alternatives: look for a second pref; assume true; maybe others.

(In reply to Richard Newman [:rnewman] from comment #8) > * don't check d.h.uE if toolkit.telemetry.unified is false (which is the > case for Fennec). That is what should probably happen (unless someone objects). That would also cover other uses, say Thunderbird and B2G.

Looking at the implementation, this was simply a bug in the send condition check.

A bundled try push showed android test failures: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ed63d9d3caf8 Fixed those and repushed: https://treeherder.mozilla.org/#/jobs?repo=try&revision=49e97a729591

Comment on attachment 8608844 [details] [diff] [review] Don't rely on datareporting.healthreport.uploadEnabled to control telemetry submission on Android Approval Request Comment [Feature/regressing bug #]: Unified Telemetry, https://wiki.mozilla.org/Unified_Telemetry This is part of the first (main) batch of uplifts to 40 to enable shipping on that train, see bug 1120356, comment 2. [User impact if declined]: Data & measurement insight projects delayed or blocked with direct impact on projects depending on this. [Describe test coverage new/current, TreeHerder]: We have good automation coverage of the feature. We also had manual tests of the main tasks as well as confirmation of correct behavior on Nightly for the patches here. [Risks and why]: Low-risk - these patches are rather isolated to Telemetry and have been on Nightly for a while with no bad reports. We intend to track on-going data quality and other issues during the 40 aurora & beta and flip the new behavior off when it doesn't meet the requirements. [String/UUID change made/needed]: The only string changes were to the about:telemetry page. We decided that we can live with missing translations on that page for a cycle as that page is not exactly user-facing.

Comment on attachment 8608844 [details] [diff] [review] Don't rely on datareporting.healthreport.uploadEnabled to control telemetry submission on Android Unified telemetry is an important new feature. It is blocking some other projects. Taking it.