Closed
Bug 1477433
(use-counters-in-release)
Opened 6 years ago
Closed 6 years ago
Enable use-counters on release channel
Categories
(Toolkit :: Telemetry, enhancement, P1)
Toolkit
Telemetry
Tracking
()
RESOLVED
FIXED
mozilla65
Tracking | Status | |
---|---|---|
firefox65 | --- | fixed |
People
(Reporter: dietrich, Assigned: dietrich)
References
Details
Attachments
(2 files)
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•6 years ago
|
||
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?
Flags: needinfo?(mreid)
Comment 2•6 years ago
|
||
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•6 years ago
|
||
Yep! cc :whd for visibility.
Main ping impact looks reasonable to me.
Flags: needinfo?(mreid)
Comment 4•6 years ago
|
||
I've filed Bug 1477749 to make sure use counters are made available in the "main_summary" dataset.
Comment 5•6 years ago
|
||
(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•6 years ago
|
||
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.
Priority: -- → P3
Assignee | ||
Comment 7•6 years ago
|
||
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•6 years ago
|
||
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
Assignee | ||
Comment 9•6 years ago
|
||
Thanks Chris! Added to the list of steps to launch.
Assignee | ||
Updated•6 years ago
|
Alias: use-counters-in-release
Assignee | ||
Comment 10•6 years ago
|
||
Hello Chris, here's the data collection review form. Can you please evaluate the request? Thanks!
Attachment #9005539 -
Flags: review?(chutten)
Comment 11•6 years ago
|
||
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+
Attachment #9005539 -
Flags: review?(chutten) → review+
Assignee | ||
Comment 12•6 years ago
|
||
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?
Flags: needinfo?(jrediger)
Assignee | ||
Updated•6 years ago
|
Flags: needinfo?(ryanvm)
Updated•6 years ago
|
Flags: needinfo?(ryanvm) → needinfo?(aryx.bugmail)
Comment 13•6 years ago
|
||
(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.
Flags: needinfo?(jrediger)
Comment 14•6 years ago
|
||
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.
Flags: needinfo?(aryx.bugmail)
Assignee | ||
Comment 15•6 years ago
|
||
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?
Flags: needinfo?(aryx.bugmail)
Comment 16•6 years ago
|
||
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#
Flags: needinfo?(aryx.bugmail)
Assignee | ||
Comment 17•6 years ago
|
||
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 :)
Assignee | ||
Comment 18•6 years ago
|
||
Found em, filter on USE_COUNTER2 in content processes in Histograms.
Which means my build isn't working... will investigate further...
Assignee | ||
Comment 19•6 years ago
|
||
(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...
Assignee | ||
Comment 20•6 years ago
|
||
Enable use-counters on release channel
Comment 21•6 years ago
|
||
Setting this to P1 for visibility in our dashboard as it's actively being worked on.
Assignee: nobody → dietrich
Priority: P3 → P1
Assignee | ||
Comment 22•6 years ago
|
||
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?
Flags: needinfo?(chutten)
Comment 23•6 years ago
|
||
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)
Flags: needinfo?(chutten)
Comment 24•6 years ago
|
||
Pushed by dietrich@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/39706085e808
Enable use-counters on release channel (r=chutten)
Comment 25•6 years ago
|
||
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
Flags: needinfo?(dietrich)
Updated•6 years ago
|
Attachment #9023953 -
Attachment description: Bug 1477433 - Enable use-counters on release channel (r=gfritzsche) → Bug 1477433 - Enable use-counters on release channel (r=chutten)
Comment 26•6 years ago
|
||
Pushed by dietrich@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/eff95fc19f19
Enable use-counters on release channel (r=chutten)
Assignee | ||
Updated•6 years ago
|
Flags: needinfo?(dietrich)
Comment 27•6 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox65:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
Assignee | ||
Comment 28•6 years ago
|
||
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?
Flags: needinfo?(gfritzsche)
Comment 29•6 years ago
|
||
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•6 years ago
|
||
(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.
Flags: needinfo?(gfritzsche)
Assignee | ||
Comment 31•6 years ago
|
||
(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.
Assignee | ||
Comment 32•6 years ago
|
||
(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•6 years ago
|
||
Regarding the same ping + client ID, this is okay by me if it meets the product needs.
Updated•6 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•