Closed
Bug 1381591
Opened 7 years ago
Closed 7 years ago
Refactor plugin initialization telemetry
Categories
(Core Graveyard :: Plug-ins, defect)
Core Graveyard
Plug-ins
Tracking
(firefox55 verified, firefox56 verified)
VERIFIED
FIXED
mozilla56
People
(Reporter: benjamin, Assigned: benjamin)
References
Details
Attachments
(2 files)
We currently have the following telemetry on plugin initialization: BLOCKED_ON_PLUGIN_MODULE_INIT_MS BLOCKED_ON_PLUGIN_INSTANCE_INIT_MS BLOCKED_ON_PLUGIN_STREAM_INIT_MS BLOCKED_ON_PLUGINASYNCSURROGATE_WAITFORINIT_MS BLOCKED_ON_PLUGIN_INSTANCE_DESTROY_MS What we really want for the remainder of Flash's lifetime is a simpler signal for how much Flash is used: the detailed performance metrics are now much less interesting. This bug will track fixing this all up.
Comment hidden (mozreview-request) |
Assignee | ||
Comment 2•7 years ago
|
||
rweiss, I'm going to attempt to fill out the new data review form here as a text attachment, to see how easy we can make it.
Assignee | ||
Updated•7 years ago
|
Attachment #8887213 -
Flags: review?(rweiss)
Comment 3•7 years ago
|
||
mozreview-review |
Comment on attachment 8887209 [details] Bug 1381591 - Refactor plugin initialization/performance telemetry to measure the things we care about, data-r?rweiss https://reviewboard.mozilla.org/r/157990/#review163142
Attachment #8887209 -
Flags: review?(kyle) → review+
Comment 4•7 years ago
|
||
mozreview-review |
Comment on attachment 8887209 [details] Bug 1381591 - Refactor plugin initialization/performance telemetry to measure the things we care about, data-r?rweiss https://reviewboard.mozilla.org/r/157990/#review164458 1) Has the analysis plan passed analysis review? n/a 2) Is there documentation that describes the schema for the ultimate data set available publicly, complete and accurate? (see here, here, and here) No, though this is an incremental change to existing telemetry. Final details around measurements will need to be included when the code is landed. This does not need to block data review at this time. 3) Is there a control mechanism that allows the user to turn the data collection on and off? (Note, for data collection not needed for security purposes, Mozilla provides such a control mechanism) Yes 4) Who will monitor if request is for permanent data collection? :bsmedberg has indicated he will publish period public updates to the roadmap and monitor on STMO. 5) What type of data do the requested instruments or measurements fall under? Please check the following document to understand the following options: No assertion about data collection type was provided. Given the nature of the data as described ("number of plugin instances"), I assert this is category 1 data. 6) Is the data collection default-on or default-off? (Technical data and Interaction data mentioned above can be default-on. Web activity data should be default-off) Default-on 7) Does the instrumentation include the addition of any new identifiers (whether anonymous or otherwise; e.g., username, random IDs, etc. See the appendix for more details)? No. 8) Is the data collection covered by the existing Firefox privacy notice? If unsure: escalate to legal. Yes 9) Does there need to be a check-in in the future to determine whether to renew the data? (Yes/No) (If yes, set a todo reminder or file a bug if appropriate) No, as :bsmedberg indicates that he will be responsible for monitoring this data moving forward.
Attachment #8887209 -
Flags: review?(rweiss) → review+
Comment 5•7 years ago
|
||
Comment on attachment 8887213 [details]
Data collection doc
1) Has the analysis plan passed analysis review?
n/a
2) Is there documentation that describes the schema for the ultimate data set available publicly, complete and accurate? (see here, here, and here)
No, though this is an incremental change to existing telemetry. Final details around measurements will need to be included when the code is landed. This does not need to block data review at this time.
3) Is there a control mechanism that allows the user to turn the data collection on and off? (Note, for data collection not needed for security purposes, Mozilla provides such a control mechanism) Yes
4) Who will monitor if request is for permanent data collection?
:bsmedberg has indicated he will publish period public updates to the roadmap and monitor on STMO.
5) What type of data do the requested instruments or measurements fall under? Please check the following document to understand the following options:
No assertion about data collection type was provided. Given the nature of the data as described ("number of plugin instances"), I assert this is category 1 data.
6) Is the data collection default-on or default-off? (Technical data and Interaction data mentioned above can be default-on. Web activity data should be default-off)
Default-on
7) Does the instrumentation include the addition of any new identifiers (whether anonymous or otherwise; e.g., username, random IDs, etc. See the appendix for more details)?
No.
8) Is the data collection covered by the existing Firefox privacy notice? If unsure: escalate to legal.
Yes
9) Does there need to be a check-in in the future to determine whether to renew the data? (Yes/No) (If yes, set a todo reminder or file a bug if appropriate)
No, as :bsmedberg indicates that he will be responsible for monitoring this data moving forward.
Attachment #8887213 -
Flags: review?(rweiss) → review+
Pushed by bsmedberg@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/59cca9197907 Refactor plugin initialization/performance telemetry to measure the things we care about, r=qdot data-r=rweiss
Pushed by bsmedberg@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/9ea7e992bf52 merge bustage - purge removed histograms from the whitelist file, r=bustage on a CLOSED TREE
Assignee | ||
Comment 8•7 years ago
|
||
Comment on attachment 8887209 [details] Bug 1381591 - Refactor plugin initialization/performance telemetry to measure the things we care about, data-r?rweiss Approval Request Comment [Feature/Bug causing the regression]: Not a regression; this is data collection requested by Jeff Griffiths to monitor the effects of plugin click-to-play which we're rolling out in FF55 [User impact if declined]: no *user* impact hopefully. Product impact of not being able to demonstrate Flash reduction over time as well. [Is this code covered by automated tests?]: No [Has the fix been verified in Nightly?]: Manual verification before landing [Needs manual test from QE? If yes, steps to reproduce]: A quick double-check of correct data would be good. Can be done by using Flash sites and checking the scalar in about:telemetry [List of other uplifts needed for the feature/fix]: histogram-whitelists bustage fix should land with this [Is the change risky?]: It is very unlikely to cause any user-visible regression. It is unlikely to cause invisible data collection issues but basic verification would help mitigate that risk [Why is the change risky/not risky?]: Changes are entirely in data collection code. [String changes made/needed]: None
Attachment #8887209 -
Flags: approval-mozilla-beta?
Comment 9•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/59cca9197907 https://hg.mozilla.org/mozilla-central/rev/9ea7e992bf52
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Comment 10•7 years ago
|
||
Comment on attachment 8887209 [details] Bug 1381591 - Refactor plugin initialization/performance telemetry to measure the things we care about, data-r?rweiss plugin telemetry update, beta55+ should be in 55.0b12
Attachment #8887209 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 11•7 years ago
|
||
This needs a rebased patch for Beta.
status-firefox55:
--- → affected
Flags: needinfo?(benjamin)
Assignee | ||
Comment 12•7 years ago
|
||
uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/1538eed11e2dee98e05f3ea05fb11b2118d52eae
Flags: needinfo?(benjamin)
Comment 13•7 years ago
|
||
(In reply to Benjamin Smedberg [:bsmedberg] from comment #8) > [Is this code covered by automated tests?]: No > [Has the fix been verified in Nightly?]: Manual verification before landing > [Needs manual test from QE? If yes, steps to reproduce]: A quick > double-check of correct data would be good. Can be done by using Flash sites > and checking the scalar in about:telemetry How do we tell if the data displayed in about:telemetry is correct or not? Should we check both Scalars and Keyed Scalars? If so, which sub-sections are relevant?
Flags: needinfo?(benjamin)
Assignee | ||
Comment 14•7 years ago
|
||
Andrei, when you read the patch you'll see that there is only one scalar being added, which counts the number of plugin instantiations (browser.usage.plugin_instantiated). That scalar should be recorded in the content process if e10s is on and the main process if e10s is off. It should be incremented only if Flash is active on a site.
Flags: needinfo?(benjamin)
Updated•7 years ago
|
Flags: qe-verify+
Comment 15•7 years ago
|
||
Based on the infos provided in comment 14, I was able to verify the issue on 56.0a1 (2017-08-01) and 55.0b13 build1 (20170727114534) using Windows 10 x64, Ubuntu 16.04 x86 and Mac OS X 10.11.6.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Updated•2 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•