Closed Bug 694447 Opened 8 years ago Closed 8 years ago

Broken idle telemetry reporting

Categories

(Toolkit :: Places, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla10

People

(Reporter: mak, Assigned: mreid)

References

Details

(Keywords: regression)

Attachments

(1 file, 3 obsolete files)

Looking at the metrics page I still can't find valid reports for PLACES_DATABASE_FILESIZE_MB. I tried manually firing gather-telemetry to the component and it has been correctly accumulated so the code now looks correct.
There may still be something in the middle that blocks the actual reporting though, should do some test with a fresh profile and follow all path through idle-daily to gather-telemetry to actually send data.
Attached patch Adds in init of the idleService (obsolete) — Splinter Review
The "idle" object wasn't being initialized before, so now it's called "idleService" and there's a lazy getter for it.
Attachment #568067 - Flags: review?
Use defineLazyServiceGetter?
Attachment #568067 - Flags: review? → review?(mak77)
Assignee: nobody → mreid
Blocks: 689142
Status: NEW → ASSIGNED
Keywords: regression
(In reply to Marco Bonardo [:mak] from comment #2)
> Use defineLazyServiceGetter?

and while there, please also fix the other lazygetter there
Attached patch Updated to address review items (obsolete) — Splinter Review
changed to use defineLazyServiceGetter
Comment on attachment 568092 [details] [diff] [review]
Updated to address review items

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

r=me with this small code style fix

::: toolkit/components/telemetry/TelemetryPing.js
@@ +75,5 @@
>           getSelectedLocale('global');
>  }
>  
> +XPCOMUtils.defineLazyServiceGetter(this, "Telemetry", "@mozilla.org/base/telemetry;1", "nsITelemetry");
> +XPCOMUtils.defineLazyServiceGetter(this, "idleService", "@mozilla.org/widget/idleservice;1", "nsIIdleService");

To cope with the 80 chars limit, we usually format these as:
XPCOMUtils.defineLazyServiceGetter(this, "Telemetry",
                                   "@mozilla.org/base/telemetry;1",
                                   "nsITelemetry");
Attachment #568092 - Flags: review+
Attachment #568067 - Attachment is obsolete: true
Attachment #568067 - Flags: review?(mak77)
Fixed long lines
Attachment #568092 - Attachment is obsolete: true
Attached patch completed patchSplinter Review
please check in if everything looks OK
Attachment #568101 - Attachment is obsolete: true
Attachment #568112 - Flags: checkin?(mak77)
Summary: is Places telemetry MIA yet? → Broken idle telemetry reporting
Done, thank you very much for this fix, you saved me a bunch of time :)

https://hg.mozilla.org/integration/mozilla-inbound/rev/1a4c18efd917

You probably want to add the -U option to your defaults for qnew, that will setup you as the patch author, so whoever pushes your patches does not have to do that. See https://developer.mozilla.org/en/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F
Attachment #568112 - Flags: checkin?(mak77)
(It's also best to have your checkin message ("Broken idle telemetry reporting") summarize the *change*, rather than summarizing the bug. Not a big deal, though. :))
Oh, nevermind -- looks like you did, but mak clobbered it when adding your username.  Bad mak. :)
hm, yeah my bad.
https://hg.mozilla.org/mozilla-central/rev/1a4c18efd917
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla10
You need to log in before you can comment on or make changes to this bug.