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)

enhancement

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
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)
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
Yep! cc :whd for visibility. Main ping impact looks reasonable to me.
Flags: needinfo?(mreid)
I've filed Bug 1477749 to make sure use counters are made available in the "main_summary" dataset.
Depends on: 1477749
(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
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
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.
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
Thanks Chris! Added to the list of steps to launch.
Depends on: 1425700
Alias: use-counters-in-release
Attached file use-counters.txt (deleted) β€”
Hello Chris, here's the data collection review form. Can you please evaluate the request? Thanks!
Attachment #9005539 - Flags: review?(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+
Attachment #9005539 - Flags: review?(chutten) → review+
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)
Flags: needinfo?(ryanvm)
Flags: needinfo?(ryanvm) → needinfo?(aryx.bugmail)
(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)
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)
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)
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)
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 :)
Found em, filter on USE_COUNTER2 in content processes in Histograms. Which means my build isn't working... will investigate further...
(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...
Enable use-counters on release channel
Setting this to P1 for visibility in our dashboard as it's actively being worked on.
Assignee: nobody → dietrich
Priority: P3 → P1
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)
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)
Pushed by dietrich@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/39706085e808 Enable use-counters on release channel (r=chutten)
Attachment #9023953 - Attachment description: Bug 1477433 - Enable use-counters on release channel (r=gfritzsche) → Bug 1477433 - Enable use-counters on release channel (r=chutten)
Pushed by dietrich@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/eff95fc19f19 Enable use-counters on release channel (r=chutten)
Flags: needinfo?(dietrich)
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
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)
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?
(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)
(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.
(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.
Regarding the same ping + client ID, this is okay by me if it meets the product needs.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: