1477433 - (use-counters-in-release) Enable use-counters on release channel

Status: RESOLVED FIXED

(Toolkit :: Telemetry, enhancement, P1)

Description

[Dietrich Ayala (:dietrich)]

Developer Outreach has been working with Firefox product and platform engineering to enable measurability of all or part of the web platform in the wild.

The tool for this measurability on Nightly is use-counters.

This bug is for enabling use-counters on release channel.

We've been working with Georg to assess the ability and cost of enabling use-counters on the release channel, both client-side and data processing on the server, and it appears reasonable at this point.

Initial legal review was conducted, and we'll get second pass on legal review once the work on this bug is complete and we're ready to flip the switch.

More details on this initiative are available at https://docs.google.com/document/d/1ORhMf6CHiENxtrMq_HcqzZh8ghHj38eYafiPjouFK4s/edit

Comment 1

[Georg Fritzsche [:gfritzsche]]

I took a look through the current (raw) size impact of use counters on Beta 61:
https://gist.github.com/georgf/130fff0d991865d8d9905e5870d53822

The key takeaways:
- The 99th percentile is 0.26 kb.
- The max is 6.8 kb.

Given those, it seems fine to enable the existing use counters for release.

Mark, a heads-up for you for main ping impact.
Should we give ops a heads-up too?

Comment 2

[Jan-Erik Rediger [:janerik]]

To enable use counters on release we most likely need to touch [1] and add "releaseChannelCollection: opt-out" as the default.

[1]: https://searchfox.org/mozilla-central/rev/ad36eff63e208b37bc9441b91b7cea7291d82890/toolkit/components/telemetry/parse_histograms.py#131-132

Comment 3

[Mark Reid [:mreid]]

Yep! cc :whd for visibility.

Main ping impact looks reasonable to me.

Comment 4

[Mark Reid [:mreid]]

I've filed Bug 1477749 to make sure use counters are made available in the "main_summary" dataset.

Comment 5

[Georg Fritzsche [:gfritzsche]]

(In reply to Jan-Erik Rediger [:janerik] from comment #2)
> To enable use counters on release we most likely need to touch [1] and add
> "releaseChannelCollection: opt-out" as the default.
> 
> [1]:
> https://searchfox.org/mozilla-central/rev/
> ad36eff63e208b37bc9441b91b7cea7291d82890/toolkit/components/telemetry/
> parse_histograms.py#131-132

The effects can be tested locally by building an "official release" build and verifying use counters are recording using about:telemetry.
At least `MOZILLA_OFFICIAL` and `MOZ_UPDATE_CHANNEL` need to be set. [1] RyanVM might have some example mozconfigs handy though.

1: https://firefox-source-docs.mozilla.org/toolkit/components/telemetry/telemetry/internals/preferences.html

Comment 6

[Georg Fritzsche [:gfritzsche]]

This is mostly driven by Dietrich, setting this as P3 for now (for our dashboard).
Dietrich, just let us know if you need any support from our side.

Comment 7

[Dietrich Ayala (:dietrich)]

Ok! Thanks to you and Jan-Erik for pointing towards the right places to make a patch. Next week I'm away at an event, but will put a patch up after that.

Comment 8

[Chris H-C :chutten]

A quick note that this will require Data Collection Review[1] to extend Use Counter collections to Release.

[1]: https://wiki.mozilla.org/Firefox/Data_Collection

Comment 9

[Dietrich Ayala (:dietrich)]

Thanks Chris! Added to the list of steps to launch.

Comment 10

[Dietrich Ayala (:dietrich)]

Attachment: use-counters.txt

Hello Chris, here's the data collection review form. Can you please evaluate the request? Thanks!

Comment 11

[Chris H-C :chutten]

Comment on attachment 9005539 [details]
use-counters.txt

Preliminary notes:
Ideally we'd have a list of each and every use counter in the data review request, but for now I can draw any future visitor's attention to the Probe Dictionary's list of 246 use counters recorded on current Nightly: https://telemetry.mozilla.org/probe-dictionary/?search=use_counter2&channel=nightly&version=63

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? 

Standard Telemetry mechanisms apply.

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

Standard Telemetry mechanisms apply.

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

Yes, Dietrich Ayala

    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. (It is possible through some level of domain inference we could infer from short sessions which pages were visited based on the tracked features that were recorded. To do that would require we perform a web survey of these features which we have not done and would be expensive to perform, so I don't think there's enough risk to be concerned about here.)

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

Default on.

    Does the instrumentation include the addition of any new identifiers?

No.

    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. Permanent collection.

---
Result: datareview+

Comment 12

[Dietrich Ayala (:dietrich)]

Thanks for review, Chris!

(In reply to Jan-Erik Rediger [:janerik] from comment #2)
> To enable use counters on release we most likely need to touch [1] and add
> "releaseChannelCollection: opt-out" as the default.
> 
> [1]:
> https://searchfox.org/mozilla-central/rev/
> ad36eff63e208b37bc9441b91b7cea7291d82890/toolkit/components/telemetry/
> parse_histograms.py#131-132

Thanks Jan-Erik. Do you mean like adding this to the "if self._is_use_counter:" bit?

definition.setdefault('releaseChannelCollection', 'opt-out')

(In reply to Georg Fritzsche [:gfritzsche] from comment #5)
> The effects can be tested locally by building an "official release" build
> and verifying use counters are recording using about:telemetry.
> At least `MOZILLA_OFFICIAL` and `MOZ_UPDATE_CHANNEL` need to be set. [1]
> RyanVM might have some example mozconfigs handy though.
> 
> 1:
> https://firefox-source-docs.mozilla.org/toolkit/components/telemetry/
> telemetry/internals/preferences.html

RyanVM, do any other options need to be set to be able to test this?

Comment 13

[Jan-Erik Rediger [:janerik]]

(In reply to Dietrich Ayala (:dietrich) from comment #12)
> Thanks Jan-Erik. Do you mean like adding this to the "if self._is_use_counter:" bit?
> 
> definition.setdefault('releaseChannelCollection', 'opt-out')

Yip, that sounds about right.

Comment 14

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

To get early beta builds and tests on Try, import the base config patch and early patch from: https://treeherder.mozilla.org/#/jobs?repo=try&resultStatus=testfailed,busted,exception,retry,usercancel,runnable&revision=0994133cb8f13a00f4febd1b7694a6818d28e502

To get the same for late beta (EARLY_BETA_OR_EARLIER not set), import the following patch on top of base config and early beta:
https://treeherder.mozilla.org/#/jobs?repo=try&resultStatus=testfailed,busted,exception,retry,usercancel,runnable&revision=2f96aa6549d62875616bb40af2c714c619f4fa9f

Let me know if you run into issues.

Comment 15

[Dietrich Ayala (:dietrich)]

Thanks Sebastian! I'm just getting to this now, but thanks for the guidance. Are these steps still valid, or different now that we're 19 days later in the cycle?

Comment 16

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

The base config and early beta one might have been update. Just import the latest ones from https://treeherder.mozilla.org/#/jobs?repo=try&resultStatus=testfailed,busted,exception,retry,usercancel,runnable&revision=0733e9ce5377d72a14d0609d19db8979155be24a Also import 481171081b4b if you want to get rid of some wpt failures.

The early beta patch must be the tip what you push to Try.

If you are interested in build where EARLY_BETA_OR_EARLIER is false, do the above mentioned imports for the early beta and import the following beta patch on top before the push to Try: https://treeherder.mozilla.org/#/jobs?repo=try&resultStatus=testfailed,busted,exception,retry,usercancel,runnable&revision=abe33ef03345af2ff608ac1d8503bbbe153221e4

You can fin the recent beta simulations at https://docs.google.com/document/d/1zOgJc1_6DF7j78WTU7Ai1dWSv-A1RU563M8A3AGlU5A/edit#

Comment 17

[Dietrich Ayala (:dietrich)]

Have a beta branch build with official flags, and this patch applied. Where in about:telemetry do use-counters show up? I should be able to find in Nightly right?

Also doing fun things like reviving my commit access so I can push to try, learning Arcanist/Phabrictor workflow... I guess it had been a while since I committed anything :)

Comment 18

[Dietrich Ayala (:dietrich)]

Found em, filter on USE_COUNTER2 in content processes in Histograms.

Which means my build isn't working... will investigate further...

Comment 19

[Dietrich Ayala (:dietrich)]

(In reply to Dietrich Ayala (:dietrich) from comment #18)
> Found em, filter on USE_COUNTER2 in content processes in Histograms.
> 
> Which means my build isn't working... will investigate further...

Ha, I just hadn't loaded any page that triggered one. Patch does work! Now to submit through this new review process...

Comment 20

[Dietrich Ayala (:dietrich)]

Attachment: Bug 1477433 - Enable use-counters on release channel (r=chutten)

Enable use-counters on release channel

Comment 21

[Georg Fritzsche [:gfritzsche]]

Setting this to P1 for visibility in our dashboard as it's actively being worked on.

Comment 22

[Dietrich Ayala (:dietrich)]

Hi Chris, I wrote a test like the others and added to the patch, but it isn't really testing this change, afaict.

Do you know how we can test whether the set of generated histograms from a default build config would contain USE_COUNTER2_* histograms?

Comment 23

[Chris H-C :chutten]

I left some comments on the patch about how to test your code by checking if the parsed "dataset" is the correct one. (The nomenclature here has a nasty case of history, so the wording's a little obtuse)

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/39706085e808
Enable use-counters on release channel (r=chutten)

Comment 25

[Dorel Luca [:dluca]]

Backed out changeset 39706085e808 (bug 1477433) for lining failure. CLOSED TREE

Log:
https://treeherder.mozilla.org/logviewer.html#?job_id=211721203&repo=autoland&lineNumber=267

Push with failure:
https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=39706085e80850e86e77ff4b5f70ed69d7288782

Backout:
https://hg.mozilla.org/integration/autoland/rev/9c50102149e038a64843f6f38a436c82aeaa58fe

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/eff95fc19f19
Enable use-counters on release channel (r=chutten)

Comment 27

[Eliza Balazs [:ebalazs_]]

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

Comment 28

[Dietrich Ayala (:dietrich)]

We had a second legal review, and following questions arose:

* See if we can use a separate ping

* See if we can remove the client id, if it's not needed (which I don't think it is)

* Ensure the feature is documented with our other telemetry docs

The separate ping and removal of client id are not *required* by legal, as this is typical category 1 data. However it would be ideal.

Georg, we'd talked about a separate ping for this data before, but I don't remember where we ended up. Should I file a bug for that?

Comment 29

[Tim Smith (inactive) 👨‍🔬 [:tdsmith]]

Tying a client identifier to these counters is important from a data science perspective because it lets us count
- the fraction of users that use a feature, and
- the average per-user fraction of sites that use a feature

Without a client_id, we can make some statements about the fraction of pageviews that implicated a feature, but we lose the ability to filter out or identify unusual clients and usage patterns, which makes it harder to make claims about what the counters tell us.

Was the rationale for using a separate ping type to separate these counters from a client_id, or something else? If "something else," can you elaborate?

Comment 30

[Georg Fritzsche [:gfritzsche]]

(In reply to Dietrich Ayala (:dietrich) from comment #28)
> We had a second legal review, and following questions arose:
> 
> * See if we can use a separate ping

(1) For this first step (of only enabling the limited existing set of user counters for release), i would recommend to not use a separate ping. Otherwise this would mean additional work for design, engineering, testing and pipeline.

(2) For the next planned step, where you want to add broad coverage & recording of many/all CSS features etc., we should consider a separate ping.
This would mitigate concerns like impact on the ping size of the main ping.

> 
> * See if we can remove the client id, if it's not needed (which I don't
> think it is)

Following (1), removing the client id is not an option, as it is on the main ping.
When proceeding to (2), we should consider what is needed. We could consider using a separate unique per client if that helps with the concerns. That would still allow doing a census-like report that gives us "per user" numbers, but would avoid being able to directly tie it to other telemetry data.

> 
> * Ensure the feature is documented with our other telemetry docs

Ok, the use counter mechanism is documented here:
https://firefox-source-docs.mozilla.org/toolkit/components/telemetry/telemetry/collection/use-counters.html

We should make sure to clearly call out the collection on release there and that data is included in the main ping.

In the main ping documentation, we should call out in the histogram section that use counters are part of the histogram data and link to the use-counter article:
https://firefox-source-docs.mozilla.org/toolkit/components/telemetry/telemetry/data/main-ping.html#histograms

You can find the docs here:
https://searchfox.org/mozilla-central/source/toolkit/components/telemetry/docs

You can build and view them using `mach doc`.

> The separate ping and removal of client id are not *required* by legal, as
> this is typical category 1 data. However it would be ideal.
> 
> Georg, we'd talked about a separate ping for this data before, but I don't
> remember where we ended up. Should I file a bug for that?

Let's make sure we track that for the next step of (2), yes.

Comment 31

[Dietrich Ayala (:dietrich)]

(In reply to Tim Smith 👨‍🔬 [:tdsmith] from comment #29)

Hi Tim! Thanks, this is super helpful input.

The idea behind separate ping was two-fold: Separate data from client id to minimize risk of PII as much as possible and to reduce ping size as we increase use-counter feature coverage.

However, our PII scenario with telemetry in the status quo is already approved, so it's likely not necessary to disassociate from client id.

If the product needs are better met by keeping client id, then lets assume we'll keep it for now, regardless of separate ping or not.

As Georg said in comment #30, we'll figure out separate ping in later updates.

Comment 32

[Dietrich Ayala (:dietrich)]

(In reply to Georg Fritzsche [:gfritzsche] from comment #30)

Thanks Georg.

Filed bug 1510566 for updating the docs.

Expanding feature coverage of use-counters will be done in bug 1479121.

I'll chat w/ legal on same-ping + client id.

Comment 33

[Merwin]

Regarding the same ping + client ID, this is okay by me if it meets the product needs.