Closed Bug 1451814 Opened 3 years ago Closed 3 years ago

Document Telemetry Event Summarization in the in-tree docs

Categories

(Toolkit :: Telemetry, enhancement, P1)

enhancement

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: 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 chutten@mozilla.com:
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+
https://hg.mozilla.org/mozilla-central/rev/e122d6c67217
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in before you can comment on or make changes to this bug.