1450715 - Add pref dom.ipc.plugins.sandbox-level.flash to telemetry

Status: RESOLVED FIXED

(Core :: Security: Process Sandboxing, enhancement, P1)

Description

[Haik Aftandilian [:haik]]

This bug is filed to add the the value of pref dom.ipc.plugins.sandbox-level.flash to telemetry so we can get data on how often users have to change this pref on Mac as we roll out a Mac Flash sandbox.

Comment 1

[Haik Aftandilian [:haik]]

Attachment: Bug 1450715 - Add pref dom.ipc.plugins.sandbox-level.flash to telemetry

Adds the Flash plugin process sandbox level to telemetry so we'll
know if users manually disable the sandbox which will help
us make informed decisions about Flash sandbox restrictions
and their impact on users.

Review commit: https://reviewboard.mozilla.org/r/237794/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/237794/

Comment 2

[Haik Aftandilian [:haik]]

Comment on attachment 8969104 [details]
Bug 1450715 - Add pref dom.ipc.plugins.sandbox-level.flash to telemetry

Requesting data review.

Comment 3

[Alex Gaynor [:Alex_Gaynor]]

Comment on attachment 8969104 [details]
Bug 1450715 - Add pref dom.ipc.plugins.sandbox-level.flash to telemetry

https://reviewboard.mozilla.org/r/237794/#review243692

I think this should be a histogram, not a scalar? Hopefully someone on the data team can weigh in.

Comment 4

[Chris H-C :chutten]

Comment on attachment 8969104 [details]
Bug 1450715 - Add pref dom.ipc.plugins.sandbox-level.flash to telemetry

https://reviewboard.mozilla.org/r/237794/#review243692

Depends what questions you want to answer with this data.

Certainly this should be using ScalarSet instead of ScalarAdd (they should behave identically if run only once, which this will, but the former is more semantically correct). And if this information is needed for all subsessions, this will be insufficient. ...the suitability of these things all depends on the questions this hopes to answer.

Luckily that is the first question on the Data Collection Review Request form which is required for data collection review, so we'll hopefully get all this sorted out quickly.

Comment 5

[Chris H-C :chutten]

Comment on attachment 8969104 [details]
Bug 1450715 - Add pref dom.ipc.plugins.sandbox-level.flash to telemetry

https://reviewboard.mozilla.org/r/237794/#review243786

Data Collection Review need a request form filled out. (see https://wiki.mozilla.org/Firefox/Data_Collection for full process details)

Comment 6

[Haik Aftandilian [:haik]]

Comment on attachment 8969104 [details]
Bug 1450715 - Add pref dom.ipc.plugins.sandbox-level.flash to telemetry

https://reviewboard.mozilla.org/r/237794/#review243692

The question we want to be able to answer is what is the value of the pref across the user base. For example, if the new Mac Flash sandbox causes problems for some users, users may end up manually setting dom.ipc.plugins.sandbox-level.flash to zero. If lots of users are making the change, that would influence how we roll out the sandbox to release. The sandbox is enabled by default on Nightly now and we plan to enable it by default on Beta.

Comment 7

[Chris H-C :chutten]

In this case it may make more sense to add it to the list of userPrefs[1]. They are only sent if a user changes it, and they are included on every ping that has an Environment section.

To collect a pref value in this way, you just need to add it to the list[2].

[1]: https://firefox-source-docs.mozilla.org/toolkit/components/telemetry/telemetry/data/environment.html#userprefs
[2]: https://searchfox.org/mozilla-central/rev/59a9a86553e9bfd9277202748ff791fd9bc0713b/toolkit/components/telemetry/TelemetryEnvironment.jsm#185

Comment 8

[Haik Aftandilian [:haik]]

(In reply to Chris H-C :chutten from comment #7)
> In this case it may make more sense to add it to the list of userPrefs[1].
> They are only sent if a user changes it, and they are included on every ping
> that has an Environment section.
> 
> To collect a pref value in this way, you just need to add it to the list[2].
> 
> [1]:
> https://firefox-source-docs.mozilla.org/toolkit/components/telemetry/
> telemetry/data/environment.html#userprefs
> [2]:
> https://searchfox.org/mozilla-central/rev/
> 59a9a86553e9bfd9277202748ff791fd9bc0713b/toolkit/components/telemetry/
> TelemetryEnvironment.jsm#185

Thanks. I'll update the code to use userPrefs and complete the request form.

Comment 9

[Haik Aftandilian [:haik]]

Attachment: Collection Review Request

Comment 10

[Haik Aftandilian [:haik]]

Comment on attachment 8969104 [details]
Bug 1450715 - Add pref dom.ipc.plugins.sandbox-level.flash to telemetry

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/237794/diff/1-2/

Comment 11

[Chris H-C :chutten]

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?

As much as for other parts of userPrefs.

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

Yes, standard Telemetry mechanism.

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

:haik will monitor it over time

    Using the category system of data types on the Mozilla wiki, what collection type of data do the requested measurements fall under?

The request says Category 2 which may be a little generous. A case could be made for it being as low as Cat1.

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

Default on

    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

    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? 

Yes, when Flash is EOL. :haik, is there a "Flash EOL" bug that we can annotate with a reminder to remove this userPref?

---
Result: datareview+


A quick note, if you're hoping to query this on sql.tmo in, say, main_summary, you will need pipeline-side changes as documented here: https://docs.telemetry.mozilla.org/datasets/batch_view/main_summary/reference.html#user-preferences

(It's a small change)

Comment 12

[Alex Gaynor [:Alex_Gaynor]]

Comment on attachment 8969104 [details]
Bug 1450715 - Add pref dom.ipc.plugins.sandbox-level.flash to telemetry

https://reviewboard.mozilla.org/r/237794/#review244310

Comment 13

[Haik Aftandilian [:haik]]

(In reply to Chris H-C :chutten from comment #11)
>     Does there need to be a check-in in the future to determine whether to
> renew the data? 
> 
> Yes, when Flash is EOL. :haik, is there a "Flash EOL" bug that we can
> annotate with a reminder to remove this userPref?

I filed bug 1455901 "Remove dom.ipc.plugins.sandbox-level.flash telemetry when Flash EOL's" and hooked it up to a Flash EOL bug.

> A quick note, if you're hoping to query this on sql.tmo in, say,
> main_summary, you will need pipeline-side changes as documented here:
> https://docs.telemetry.mozilla.org/datasets/batch_view/main_summary/
> reference.html#user-preferences

OK. Thanks.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2d79f6714086
Add pref dom.ipc.plugins.sandbox-level.flash to telemetry r=Alex_Gaynor

Comment 15

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/2d79f6714086