Closed Bug 1381591 Opened 7 years ago Closed 7 years ago

Refactor plugin initialization telemetry

Categories

(Core Graveyard :: Plug-ins, defect)

defect
Not set
normal

Tracking

(firefox55 verified, firefox56 verified)

VERIFIED FIXED
mozilla56
Tracking Status
firefox55 --- verified
firefox56 --- verified

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.
Attached file Data collection doc
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.
Attachment #8887213 - Flags: review?(rweiss)
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 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 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
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 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+
This needs a rebased patch for Beta.
Flags: needinfo?(benjamin)
Blocks: 1384932
(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)
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)
Flags: qe-verify+
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+
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.