Closed
Bug 1451814
Opened 5 years ago
Closed 5 years ago
Document Telemetry Event Summarization in the in-tree docs
Categories
(Toolkit :: Telemetry, enhancement, P1)
Toolkit
Telemetry
Tracking
()
RESOLVED
FIXED
mozilla61
Tracking | Status | |
---|---|---|
firefox61 | --- | fixed |
People
(Reporter: chutten, Assigned: chutten)
References
Details
Attachments
(2 files)
We should add this to the event documentation: https://firefox-source-docs.mozilla.org/toolkit/components/telemetry/telemetry/collection/events.html Alessio says: > Let's call out what this is about, why, highlight that static/dynamic summaries > go to different scalars and mention the pref to tweak the number of allowed > summary keys.
Assignee | ||
Updated•5 years ago
|
Assignee: nobody → chutten
Status: NEW → ASSIGNED
Priority: -- → P1
Comment hidden (mozreview-request) |
Comment 2•5 years ago
|
||
mozreview-review |
Comment on attachment 8969334 [details] bug 1451814 - Document Event Summarization in the Event Documentation https://reviewboard.mozilla.org/r/238070/#review244112 Hey Chris, outstanding documentation! Let's just address the point below before landing this. ::: toolkit/components/telemetry/docs/collection/events.rst:244 (Diff revision 1) > + > +Calling ``recordEvent`` on any non-expired registered event will accumulate to a > +:doc:`Scalar <scalars>` for ease of analysing uptake and usage patterns. Even if the event category > +isn't enabled. > + > +The scalar is ``telemetry.event_counts`` for statically-registered events (the ones in Would you kindly call out that there's a limit to the number of keys and and document the purpose of "toolkit.telemetry.maxEventSummaryKeys"? Let's add the pref to the preferences.rst doc too.
Attachment #8969334 -
Flags: review?(alessio.placitelli) → review+
Comment hidden (mozreview-request) |
Comment 4•5 years ago
|
||
rs=me for this :) Thanks for changing it!
Pushed by chutten@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/e122d6c67217 Document Event Summarization in the Event Documentation r=Dexter
Comment 6•5 years ago
|
||
mozreview-review |
Comment on attachment 8969334 [details] bug 1451814 - Document Event Summarization in the Event Documentation https://reviewboard.mozilla.org/r/238070/#review246140 Code analysis found 1 defect in this patch: - 1 defect found by mozlint You can run this analysis locally with: - `./mach lint path/to/file` (JS/Python) If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx ::: toolkit/components/telemetry/docs/collection/events.rst:264 (Diff revision 2) > + > + // Let us suppose in the parent process this happens: > + Services.telemetry.recordEvent("interaction", "click", "document", "xuldoc"); > + Services.telemetry.recordEvent("interaction", "click", "document", "xuldoc-neighbour"); > + > + // And in each of child proccesses 1 through 4, this happens: Warning: Proccesses ==> processes [codespell]
Assignee | ||
Comment 7•5 years ago
|
||
I tried running lint on the file and it complained codespell wasn't around. I installed codespell, but then it complained about not supporting --ignore-words. So... Here's hoping this is the only spelling mistake. The other linters had nothing to say, so there's that.
Attachment #8971695 -
Flags: review?(alessio.placitelli)
Comment 8•5 years ago
|
||
Chris, you need as least codespell v1.11.0 (Ubuntu has an old version). Is there any other typos, the code review bot will tell you!
Comment 9•5 years ago
|
||
Comment on attachment 8971695 [details] [diff] [review] 0001-bug-1451814-Fix-spelling-mistake.patch Rubber stamp as it is trivial
Attachment #8971695 -
Flags: review?(alessio.placitelli) → review+
Comment 10•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/e122d6c67217
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in
before you can comment on or make changes to this bug.
Description
•