1490945 - Evaluate event telemetry improvement proposal

Status: RESOLVED FIXED

(Toolkit :: Telemetry, enhancement, P2)

Description

[Georg Fritzsche [:gfritzsche]]

In this document:
https://docs.google.com/document/d/1ZcKaTEXx8MWrqp2xXU8xModbvFo30UPig0hLekvMx94/edit?usp=sharing

Comment 1

[Chris H-C :chutten|PTO (back July 6)]

I'll look into this quickly this iteration.

Comment 2

[Chris H-C :chutten|PTO (back July 6)]

I mentioned in email about how multiple method and object values currently operate. :shong, has that explanation addressed the concerns in the document to your satisfaction?

Comment 3

[Su-Young Hong]

Hi Chris, 

Not exactly. 

My main concern is that there isn't a specified field for the "name" of the event. 

As it stands now, all Telemetry events are basically generic events, and we have to specify some combination of field values to identify a particular event type. That means: 

1) We cannot have 2 event types with a similar schema, even when it makes sense to do so. 
2) Future event's schema could overlap with an existing event's schema, and essentially make it impossible to distinguish between the two. 

It looks like we DO name event types in the YAML file, but in the data schema (timestamp | category | method | object | value | extra) there is nowhere to put that name. 

We should really have some name field in the data schema. This issue will definitely become more problematic in the future, so the earlier we can address it, the better. 

Sorry if I'm not explaining things clearly enough, maybe we should have a vidyo call to touch base if it's not clear or you have any disagreements. 

- Su

Comment 4

[Chris H-C :chutten|PTO (back July 6)]

Oh no, I forgot to submit this comment last week:

:shong and I had a conversation about this today (Friday Sept 28). We covered many topics here and came to the following points of agreement:

1) It's tricky to discover code and definitions for events when you're starting from the data side. Events and event records lack a singular identifying "metric name" that Scalars and Histograms have to aid in these sorts of situations. (e.g. try using searchfox to discover information about events in the "devtools" category)

2) It is unclear (undocumented?) that any given {category, method, object} tuple is unique, so there were worries of overlap.

3) Events are a relatively-new and relatively-complex metric type. Best practices and even vocabulary are still underdeveloped compared to Histograms and Scalars.

4) Given size considerations it is unlikely that we're going to be too interested in adding new identifiers to event records and instead should focus on best practices in using the ones already present.

This led to some interesting ideas

* Calling events within a single event definition (with multiple methods and objects and extra keys, but a single description) an Event Family so we can talk about them independently of their definition, any Event Records of recorded data, and Category it belongs to.

* Perhaps developing a taxonomy that favours having a single Family per Category. Using a rich category name like "toplevel.secondlevel" to provide a "metric name" for the Family that makes it easier to find using available tools.

* Adding more documentation on the data side (docs.tmo) and updating existing in-tree docs to be clear on relevant implementation details like the enforced uniqueness of {category, method, object}.

:shong has taken the first action to improve the Event documentation on docs.tmo. After that we can see what updates are needed in-tree and where this bug needs to be taken.

Comment 5

[Su-Young Hong]

Hey chutten, sorry for the long delay. I've made a PR with the documentation: 

https://github.com/mozilla/firefox-data-docs/pull/197

Let me know what you think

Comment 6

[Chris H-C :chutten|PTO (back July 6)]

Reviews are on Github. Looks like there's room for improving the source docs by at least noting in [1] that category+method+object are required to be unique, and that events are submitted by the "event" ping (the doc for which has plenty of other relevant information about event behaviour)

This bug can now be about adding those two things to the source docs and we can split the event best practices discussion out to the PR in Comment#5.

[1]: https://firefox-source-docs.mozilla.org/toolkit/components/telemetry/telemetry/collection/events.html#the-yaml-definition-file

Comment 7

[Chris H-C :chutten|PTO (back July 6)]

Attachment: bug 1490945 - Improve Telemetry Event docs slightly r?Dexter

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1dc4651530f8
Improve Telemetry Event docs slightly r=Dexter

Comment 9

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/1dc4651530f8

Comment 10

[Byron Campen [:bwc]]

https://hg.mozilla.org/projects/larch/rev/1dc4651530f8290007b9440c9013493cc0ef65f2
bug 1490945 - Improve Telemetry Event docs slightly r=Dexter