1517469 - Enable Telemetry events for low latency uptake monitoring

Status: RESOLVED FIXED

(Firefox :: Remote Settings Client, enhancement)

Description

[Mathieu Leplatre [:leplatrem]]

Currently, the Uptake Telemetry [0] uses histograms to send feedback.

With the objective of having real-time monitoring and alerts of thresholds, the uptake telemetry helper should use the Events API instead [1]

[0] https://firefox-source-docs.mozilla.org/main/latest/toolkit/components/telemetry/telemetry/collection/uptake.html 

[1] https://firefox-source-docs.mozilla.org/main/latest/toolkit/components/telemetry/telemetry/collection/events.html

Comment 1

[Mathieu Leplatre [:leplatrem]]

Attachment: telemetry-approval-form.txt

Comment 2

[Chris H-C :chutten]

Comment on attachment 9041469 [details]
telemetry-approval-form.txt

This should prove to be a straightforward review, but it needs a full list of the data being collected. It might be a better idea to pursue this review when there's a patch in place to help?

Comment 3

[Mathieu Leplatre [:leplatrem]]

Attachment: Bug 1517469 - Enable Events Telemetry for Uptake monitory

Enable Events Telemetry for Uptake monitoring

Comment 4

[Mathieu Leplatre [:leplatrem]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7ad245ee594df962ae8951eeeaff4a8fd03be8c3

Comment 5

[Mathieu Leplatre [:leplatrem]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=dce893f2d8e3c74fe6c4347dd59a81f3c7f778ba

Comment 6

[Chris H-C :chutten]

Comment on attachment 9041469 [details]
telemetry-approval-form.txt

DATA COLLECTION REVIEW RESPONSE:

    Is there or will there be documentation that describes the schema for the ultimate data set available publicly, complete and accurate?

Yes. This collection is Telemetry so is documented in its definitions file ([Events.yaml](https://hg.mozilla.org/mozilla-central/file/tip/toolkit/components/telemetry/Events.yaml)) and the [Probe Dictionary](https://telemetry.mozilla.org/probe-dictionary/).

    Is there a control mechanism that allows the user to turn the data collection on and off?

Yes. This collection is Telemetry so can be controlled through Firefox's Preferences.

    If the request is for permanent data collection, is there someone who will monitor the data over time?

Yes. Mathieu Leplatre is responsible.

    Using the category system of data types on the Mozilla wiki, what collection type of data do the requested measurements fall under?

Category 1, Technical.

    Is the data collection request for default-on or default-off?

Default on for all channels.

    Does the instrumentation include the addition of any new identifiers?

No. (It does include the name of the source whose uptake we're reporting and whether it's via normandy or remotesettings.

    Is the data collection covered by the existing Firefox privacy notice?

Yes.

    Does there need to be a check-in in the future to determine whether to renew the data?

No. This collection is permanent.

---
Result: datareview+

Comment 7

[Chris H-C :chutten]

Will this supplant the older histogram-based collection? Should we file a bug to remove that after this lands?

Comment 8

[Mathieu Leplatre [:leplatrem]]

Will this supplant the older histogram-based collection? Should we file a bug to remove that after this lands?

Well, when the population running this patch (Fx67!?) becomes big enough to do proper monitoring I guess that yes, we will remove the histogram ping. It's also possible that we'll have to wait the next ESR...

Comment 9

[Mathieu Leplatre [:leplatrem]]

The current approach where we enable events in RemoteSettings.init() isn't good enough. nsBrowserGlue calls us very late and some collection might try to sync before that (eg. Tippytop icons on new tab).

Is enabling events something expensive? Should I called it like lazily (eg. on first call to UptakeTelemetry.report())?
And BTW, should I disable it at some point (eg. after synchronization is finished)?

What would you recommend?

Thanks!

Comment 10

[Mathieu Leplatre [:leplatrem]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f7bfbc1e51d963f83d5f4ce9996baf5b4a541c0e

Comment 11

[Jan-Erik Rediger [:janerik]]

Enabling is not particularly expensive if done once (it stores the category in a hash set).
Disabling just removes it from the hash set. Therefore never disabling it again is usually fine.

If possible, calling it lazily just once in UptakeTelemetry seems like a good solution.

Comment 12

[Mathieu Leplatre [:leplatrem]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=dbfe3f6c0362a7d3a15178f0129b259df3b9a8c2

Comment 13

[Mathieu Leplatre [:leplatrem]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=237c26e1a107e5009c25be07998018f2610931d0

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/113c037637b6
Enable Events Telemetry for Uptake monitory r=janerik

Comment 15

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/113c037637b6