Closed Bug 1451814 Opened 3 years ago Closed 3 years ago
Document Telemetry Event Summarization in the in-tree docs
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: nobody → chutten
Status: NEW → ASSIGNED
Priority: -- → P1
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+
rs=me for this :) Thanks for changing it!
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/e122d6c67217 Document Event Summarization in the Event Documentation r=Dexter
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]
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)
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 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+
You need to log in before you can comment on or make changes to this bug.