1451814 - Document Telemetry Event Summarization in the in-tree docs

Status: RESOLVED FIXED

(Toolkit :: Telemetry, enhancement, P1)

Description

[Chris H-C :chutten]

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.

Comment 1

[Chris H-C :chutten]

Attachment: bug 1451814 - Document Event Summarization in the Event Documentation

Review commit: https://reviewboard.mozilla.org/r/238070/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/238070/

Comment 2

[Alessio Placitelli [:Dexter]]

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.

Comment 3

[Chris H-C :chutten]

Comment on attachment 8969334 [details]
bug 1451814 - Document Event Summarization in the Event Documentation

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/238070/diff/1-2/

Comment 4

[Alessio Placitelli [:Dexter]]

rs=me for this :) Thanks for changing it!

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e122d6c67217
Document Event Summarization in the Event Documentation r=Dexter

Comment 6

[Code Review Bot [:reviewbot]]

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]

Comment 7

[Chris H-C :chutten]

Attachment: 0001-bug-1451814-Fix-spelling-mistake.patch

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.

Comment 8

[Sylvestre Ledru [:Sylvestre] back june 18th]

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

[Sylvestre Ledru [:Sylvestre] back june 18th]

Comment on attachment 8971695 [details] [diff] [review]
0001-bug-1451814-Fix-spelling-mistake.patch

Rubber stamp as it is trivial

Comment 10

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/e122d6c67217