collectMeasurements() only called at upload time

RESOLVED FIXED in Firefox 21

Status

defect
P1
normal
RESOLVED FIXED
6 years ago
8 months ago

People

(Reporter: gps, Assigned: gps)

Tracking

unspecified
Firefox 22
Dependency tree / graph

Firefox Tracking Flags

(firefox20 unaffected, firefox21 fixed)

Details

Attachments

(1 attachment)

Assignee

Description

6 years ago
Currently pull-based metrics are only collected at upload time. This means that if a user has opted out of FHR (data collection still occurs if they are opted out - only upload is disabled) that not all data will be pulled into the FHR database. This means that the value of about:healthreport is diminished for users that have opted out.

I'm asserting this is a beta blocker. Since code changes will be contained to FHR JS, I'm guessing we can uplift this to Aurora without much issue.
Also necessary for Android: "upload" should really be "document generation", with upload being a separate phase (that will differ between platforms).
Priority: -- → P1
Assignee

Comment 2

6 years ago
We could consider using nsIUpdateTimerManager to handle scheduling of collection. https://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/nsIUpdateTimerManager.idl
Assignee

Comment 3

6 years ago
Blair granted permission on IRC to use nsIUpdateTimerManager despite its name. Apparently there is precedent.
Assignee

Comment 4

6 years ago
This is somewhat hacky and fragile. More robust solutions were not chosen because this code will be refactored for Android and I didn't think the effort was worth it.

I wish I could deploy a more robust test. However, the timer manager does not easily allow itself to be tested. Perhaps one day somebody will write a testing-only module to help with testing installed timers. See https://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/test_timermanager/unit/test_0010_timermanager.js for what it would take.
Assignee: nobody → gps
Status: NEW → ASSIGNED
Attachment #724243 - Flags: review?(rnewman)
Comment on attachment 724243 [details] [diff] [review]
Install timer for daily collection, v1

Review of attachment 724243 [details] [diff] [review]:
-----------------------------------------------------------------

Movin'.

::: services/healthreport/healthreporter.jsm
@@ +220,5 @@
> +      // the timer ID.
> +      let timerName = this._branch.replace(".", "-", "g") + "lastDailyCollection";
> +      let tm = Cc["@mozilla.org/updates/timer-manager;1"]
> +                 .getService(Ci.nsIUpdateTimerManager);
> +      tm.registerTimer(timerName, this.collectMeasurements.bind(this),

Is there any chance that either of these will fail?

It seems sensible to wrap this in a try block.
Attachment #724243 - Flags: review?(rnewman) → review+
Assignee

Comment 6

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/7190b9dd7f51

with a try block.
Target Milestone: --- → mozilla22
https://hg.mozilla.org/mozilla-central/rev/7190b9dd7f51
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Assignee

Comment 8

6 years ago
Comment on attachment 724243 [details] [diff] [review]
Install timer for daily collection, v1

[Approval Request Comment]
Bug caused by (feature/regressing bug #): FHR initial landing.
User impact if declined: FHR won't collect data unless submission is enabled.
Testing completed (on m-c, etc.): It has baked for a few days. There is test coverage.
Risk to taking this patch (and alternatives if risky): Low risk from a technical standpoint.
String or UUID changes made by this patch: None

Unlike the uplift requests I just filed, this one is more important. A P1 launch requirement for FHR is that FHR collect data even if upload is disabled. This is to allow people to utilize the benefits of about:healthreport while not periodically submitting data to Mozilla. This is an important adherence to Mozilla's principle of users being in control of their data. This should have made the initial code drop of FHR but it fell through the cracks.
Attachment #724243 - Flags: approval-mozilla-aurora?
Attachment #724243 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Component: Metrics and Firefox Health Report → Client: Desktop
Flags: in-testsuite+
Product: Mozilla Services → Firefox Health Report
Target Milestone: mozilla22 → Firefox 22

Updated

8 months ago
Product: Firefox Health Report → Firefox Health Report Graveyard
You need to log in before you can comment on or make changes to this bug.