1445921 - Fix "optout" ping

Status: RESOLVED FIXED

(Toolkit :: Telemetry, enhancement, P1)

Description

[Mark Reid [:mreid]]

The "deletion" ping is currently mis-named, and was a holdover from the FHR days.

Rename "deletion" ping to "optout" ping, and remove the clientId from the ping.

Make a best effort to send it right away, then turn off data reporting (do not block or keep retrying).

Upon opting out, wipe out the locally-stored clientId to dissociate the profile from any previously collected data.

We may also want to add a boolean scalar or event upon opt-in if a client re-enables Telemetry.

Comment 1

[Mark Reid [:mreid]]

Matt, are there implications here for Shield? Should opting out of Telemetry disable any in-flight experiments?

Comment 2

[Mark Reid [:mreid]]

One more consequence here - we should generate a new clientId if someone opts back in to Telemetry reporting.

Comment 3

[Chris H-C :chutten]

Notes that come to mind:

1) We aren't the only thing what uses clientId. How do we want this to affect crash reports (for instance)?
2) We also should ensure we reset the session and subsession counters.
3) Should we leave the profile creation date?
4) We should ensure we clear and disable the Telemetry Archive and erase pending pings.
5) crash pings built for pingsender are stored in a non-profile-specific space (the directory named "Pending Pings/"). There shouldn't be anything in there when we're not currently crashing, but we might want to double-check?

Comment 4

[Matt Grimes [:Matt_G]]

IIUC option out of Telemetry should disable any in-flight experiments and prevent you from being enrolled in any new ones. Mythmon can confirm that behavior.

Comment 5

[Michael Cooper [:mythmon]]

Currently disabling telemetry will prevent enrollment in new experiments, but won't unenroll you from existing experiments. This probably isn't good, as it will basically make any experiments a user has active permanent once they turn off telemetry, unless they have built-in expiration. Most add-on experiments have these expirations, but preference experiments do not.

We'll need to rethink Normandy and Shield's relationship to this preference in the coming months.

Comment 6

[Chris H-C :chutten]

:mreid, when were you hoping to see the client work done for this? We're doing some quarter planing and are trying to figure out relative priorities.

Comment 7

[Mark Reid [:mreid]]

I would really like to see this work land this quarter, if possible.

Comment 9

[Chris H-C :chutten]

Event and TMO work is taking up my time for the next while. Deprioritizing and unassigning for now.

Comment 10

[Georg Fritzsche [:gfritzsche]]

Lining this up for the next iteration to fit it into the quarter.
We should see who can actually take this.

Comment 11

[Georg Fritzsche [:gfritzsche]]

Alessio and Jan-Erik will figure out how this fits in.

Comment 12

[Jan-Erik Rediger [:janerik]]

ni?:mreid

How will the optout ping be used in the pipeline?
Just for statistics how many people turn off telemetry explicitely?
If we don't have the clientId in the ping we can't associate it with previous data.

ni?mythmon

Re comment 5, was this discussed for Normandy/Shield yet?
Upon disabling Telemetry we could notify an event, e.g. "telemetry-disabled", on which Normandy/Shield could listen and take appropriate action. Would this help?

Comment 13

[Mark Reid [:mreid]]

(In reply to Jan-Erik Rediger [:janerik] from comment #12)
> ni?:mreid
> 
> How will the optout ping be used in the pipeline?
> Just for statistics how many people turn off telemetry explicitely?
> If we don't have the clientId in the ping we can't associate it with
> previous data.

Correct - we just want aggregate counts of how many profiles are opting out. I don't think it matters which ones, specifically, they are.

Comment 14

[Michael Cooper [:mythmon]]

(In reply to Jan-Erik Rediger [:janerik] from comment #12)
> ni?mythmon
> 
> Re comment 5, was this discussed for Normandy/Shield yet?
> Upon disabling Telemetry we could notify an event, e.g.
> "telemetry-disabled", on which Normandy/Shield could listen and take
> appropriate action. Would this help?

Emitting an event, like "telemetry-disabled" should work well for Normandy.

Comment 15

[Jan-Erik Rediger [:janerik]]

Attachment: Bug 1445921 - Allow client ID to be set and reset.

MozReview-Commit-ID: K4O9SXlmQkI

Comment 16

[Jan-Erik Rediger [:janerik]]

Attachment: Bug 1445921 - Implement and test new optout ping logic.

This follows the steps from the specification and also ensures sending
the optout ping is only tried once and discarded if that fails.

Depends on D1947

MozReview-Commit-ID: 99peURNq9jx

Comment 17

[Jan-Erik Rediger [:janerik]]

Attachment: Bug 1445921 - Test for canary client id in received pings.

MozReview-Commit-ID: 8aPTGmYv6Wf

Depends on D1948

Comment 18

[Jan-Erik Rediger [:janerik]]

Attachment: Bug 1445921 - Document new optout ping

MozReview-Commit-ID: FCjCpozJ5yS

Depends on D1949

Comment 19

[Phabricator Automation]

Comment on attachment 8989773 [details]
Bug 1445921 - Test for canary client id in received pings.

Alessio Placitelli [:Dexter] has approved the revision.

https://phabricator.services.mozilla.com/D1949

Comment 20

[Phabricator Automation]

Comment on attachment 8989771 [details]
Bug 1445921 - Allow client ID to be set and reset.

Alessio Placitelli [:Dexter] has approved the revision.

https://phabricator.services.mozilla.com/D1947

Comment 21

[Georg Fritzsche [:gfritzsche]]

One question that came up in conversation here:
Scenario:
- disable telemetry upload
- restart the browser
- re-enable telemetry upload
  - generate a new client id
  - ... now crash before we async saved it to disk

What happens when we restart?
Do we create a new client id again?

Comment 22

[Chris H-C :chutten]

Looks like we will create a new one, but we won't have sent the other one so there shouldn't be double-counting: https://searchfox.org/mozilla-central/rev/6ef785903fee6c0b16a1eab79d722373d940fd78/toolkit/modules/ClientID.jsm#115

Comment 23

[Phabricator Automation]

Comment on attachment 8989774 [details]
Bug 1445921 - Document new optout ping

Alessio Placitelli [:Dexter] has approved the revision.

https://phabricator.services.mozilla.com/D1950

Comment 24

[Phabricator Automation]

Comment on attachment 8989772 [details]
Bug 1445921 - Implement and test new optout ping logic.

Alessio Placitelli [:Dexter] has approved the revision.

https://phabricator.services.mozilla.com/D1948

Comment 25

[Jan-Erik Rediger [:janerik]]

Attachment: data-review-request.txt

Comment 26

[Jan-Erik Rediger [:janerik]]

(In reply to Georg Fritzsche [:gfritzsche] [away June 19 - July 3] from comment #21)
> One question that came up in conversation here:
> Scenario:
> - disable telemetry upload
> - restart the browser
> - re-enable telemetry upload
>   - generate a new client id
>   - ... now crash before we async saved it to disk
> 
> What happens when we restart?
> Do we create a new client id again?

The very first thing we do when resetting the client ID is to remove the disk-stored ID.
If at boot this file doesn't exist a new id is created (and written to that file, as mentioned by :chutten).
But we also depend on the Telemetry upload preference. So if that isn't synced to disk yet, after a restart of the browser upload will still be disabled through the preference and the user would need to re-enable it again, triggering the whole resetting once more.

Comment 27

[Chris H-C :chutten]

Comment on attachment 8990237 [details]
data-review-request.txt

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?

Yes, Scalars.yaml

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

Yes, standard Telemetry mechanisms apply.

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

Yes, :janerik will.

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

Category 2, interaction.

    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? 

No, it is permanent.

---
Result: datareview+

Comment 28

[Phabricator Automation]

Comment on attachment 8989772 [details]
Bug 1445921 - Implement and test new optout ping logic.

Chris H-C :chutten has approved the revision.

https://phabricator.services.mozilla.com/D1948

Comment 29

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f3ef58bc6b38
Allow client ID to be set and reset. r=Dexter

Comment 30

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7c23db3a5f53
Implement and test new optout ping logic. r=Dexter,chutten

Comment 31

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b859e76fcc67
Test for canary client id in received pings. r=Dexter

Comment 32

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b5bff80b9d18
Document new optout ping r=Dexter

Comment 33

[Raul Gurzau (:RaulG)]

Backed out 4 changesets (bug 1445921) for Linting failure on components/telemetry/tests/unit/test_TelemetryController.js on a CLOSED TREE

Backout link: https://hg.mozilla.org/integration/autoland/rev/c73eb22a077df9a4238426135ab4e4025f954abe

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=7c23db3a5f530f001bfb1dc989ab61e3b882804f

Log link: https://treeherder.mozilla.org/logviewer.html#?job_id=187107238&repo=autoland&lineNumber=240

Log snippet:

[vcs 2018-07-09T09:43:17.885Z] 245506 files updated, 0 files merged, 0 files removed, 0 files unresolved
[vcs 2018-07-09T09:43:18.052Z] updated to 7c23db3a5f530f001bfb1dc989ab61e3b882804f
[vcs 2018-07-09T09:43:18.057Z] PERFHERDER_DATA: {"framework": {"name": "vcs"}, "suites": [{"extraOptions": ["c3.xlarge"], "lowerIsBetter": true, "name": "clone", "shouldAlert": false, "subtests": [], "value": 106.30561518669128}, {"extraOptions": ["c3.xlarge"], "lowerIsBetter": true, "name": "pull", "shouldAlert": false, "subtests": [], "value": 7.696439027786255}, {"extraOptions": ["c3.xlarge"], "lowerIsBetter": true, "name": "update", "shouldAlert": false, "subtests": [], "value": 77.21055388450623}, {"extraOptions": ["c3.xlarge"], "lowerIsBetter": true, "name": "overall", "shouldAlert": false, "subtests": [], "value": 192.3637878894806}]}
[vcs 2018-07-09T09:43:18.486Z] TinderboxPrint:<a href=https://hg.mozilla.org/integration/autoland/rev/7c23db3a5f530f001bfb1dc989ab61e3b882804f title='Built from autoland revision 7c23db3a5f530f001bfb1dc989ab61e3b882804f'>7c23db3a5f530f001bfb1dc989ab61e3b882804f</a>
[task 2018-07-09T09:43:18.486Z] executing ['bash', '-cx', 'cd /builds/worker/checkouts/gecko/ && cp -r /build/node_modules_eslint node_modules && ln -s ../tools/lint/eslint/eslint-plugin-mozilla node_modules && ln -s ../tools/lint/eslint/eslint-plugin-spidermonkey-js node_modules && ./mach lint -l eslint -f treeherder --quiet\n']
[task 2018-07-09T09:43:18.494Z] + cd /builds/worker/checkouts/gecko/
[task 2018-07-09T09:43:18.495Z] + cp -r /build/node_modules_eslint node_modules
[task 2018-07-09T09:43:19.061Z] + ln -s ../tools/lint/eslint/eslint-plugin-mozilla node_modules
[task 2018-07-09T09:43:19.063Z] + ln -s ../tools/lint/eslint/eslint-plugin-spidermonkey-js node_modules
[task 2018-07-09T09:43:19.063Z] + ./mach lint -l eslint -f treeherder --quiet
[task 2018-07-09T09:43:19.792Z] New python executable in /builds/worker/checkouts/gecko/obj-x86_64-pc-linux-gnu/_virtualenvs/init/bin/python2.7
[task 2018-07-09T09:43:19.792Z] Also creating executable in /builds/worker/checkouts/gecko/obj-x86_64-pc-linux-gnu/_virtualenvs/init/bin/python
[task 2018-07-09T09:43:21.344Z] Installing setuptools, pip, wheel...done.
[task 2018-07-09T09:43:22.430Z] running build_ext
[task 2018-07-09T09:43:22.430Z] building 'psutil._psutil_linux' extension
[task 2018-07-09T09:43:22.430Z] creating build
[task 2018-07-09T09:43:22.430Z] creating build/temp.linux-x86_64-2.7
[task 2018-07-09T09:43:22.430Z] creating build/temp.linux-x86_64-2.7/psutil
[task 2018-07-09T09:43:22.430Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_common.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_common.o
[task 2018-07-09T09:43:22.430Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_posix.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_posix.o
[task 2018-07-09T09:43:22.430Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_linux.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_linux.o
[task 2018-07-09T09:43:22.430Z] creating build/lib.linux-x86_64-2.7
[task 2018-07-09T09:43:22.430Z] creating build/lib.linux-x86_64-2.7/psutil
[task 2018-07-09T09:43:22.430Z] x86_64-linux-gnu-gcc -pthread -shared -Wl,-O1 -Wl,-Bsymbolic-functions -Wl,-Bsymbolic-functions -Wl,-z,relro -fno-strict-aliasing -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -Wl,-Bsymbolic-functions -Wl,-z,relro -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security build/temp.linux-x86_64-2.7/psutil/_psutil_common.o build/temp.linux-x86_64-2.7/psutil/_psutil_posix.o build/temp.linux-x86_64-2.7/psutil/_psutil_linux.o -o build/lib.linux-x86_64-2.7/psutil/_psutil_linux.so
[task 2018-07-09T09:43:22.430Z] building 'psutil._psutil_posix' extension
[task 2018-07-09T09:43:22.430Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_common.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_common.o
[task 2018-07-09T09:43:22.430Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_posix.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_posix.o
[task 2018-07-09T09:43:22.430Z] x86_64-linux-gnu-gcc -pthread -shared -Wl,-O1 -Wl,-Bsymbolic-functions -Wl,-Bsymbolic-functions -Wl,-z,relro -fno-strict-aliasing -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -Wl,-Bsymbolic-functions -Wl,-z,relro -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security build/temp.linux-x86_64-2.7/psutil/_psutil_common.o build/temp.linux-x86_64-2.7/psutil/_psutil_posix.o -o build/lib.linux-x86_64-2.7/psutil/_psutil_posix.so
[task 2018-07-09T09:43:22.430Z] copying build/lib.linux-x86_64-2.7/psutil/_psutil_linux.so -> psutil
[task 2018-07-09T09:43:22.430Z] copying build/lib.linux-x86_64-2.7/psutil/_psutil_posix.so -> psutil
[task 2018-07-09T09:43:22.430Z] 
[task 2018-07-09T09:43:22.430Z] Error processing command. Ignoring because optional. (optional:packages.txt:comm/build/virtualenv_packages.txt)
[task 2018-07-09T09:48:05.298Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/toolkit/components/telemetry/tests/unit/test_TelemetryController.js:177:11 | 'snapshot' is already declared in the upper scope. (no-shadow)
[taskcluster 2018-07-09 09:48:05.714Z] === Task Finished ===
[taskcluster 2018-07-09 09:48:05.715Z] Unsuccessful task run with exit code: 1 completed in 511.35 seconds

Comment 34

[Eliza Balazs [:ebalazs_]]

This also caused some xpcshell failures, e.g.: https://treeherder.mozilla.org/logviewer.html#?job_id=187109862&repo=autoland&lineNumber=1880
The culprit seems to be: https://hg.mozilla.org/integration/autoland/rev/7c23db3a5f530f001bfb1dc989ab61e3b882804f

Comment 35

[Jan-Erik Rediger [:janerik]]

lint failure fixed, xpcshell failures fixed. Try run for Linux: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5263bb29e624d005dcd1b783f1557443f60de3dd

Waiting for it before landing again.

Comment 36

[Jan-Erik Rediger [:janerik]]

Attachment: PR 180: Add schema for optout ping

Comment 37

[Jan-Erik Rediger [:janerik]]

linux64 & windows try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5fa0372e5640754a6a20fc766dc55e4cf21efd15
Will rebase after bug 1472715 lands.

Comment 38

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1f157650a9f5
Allow client ID to be set and reset. r=Dexter

Comment 39

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0218f7afb67f
Implement and test new optout ping logic. r=Dexter,chutten

Comment 40

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cd46cd235092
Test for canary client id in received pings. r=Dexter

Comment 41

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/68e29b0840e7
Document new optout ping r=Dexter

Comment 42

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/1f157650a9f5
https://hg.mozilla.org/mozilla-central/rev/0218f7afb67f
https://hg.mozilla.org/mozilla-central/rev/cd46cd235092
https://hg.mozilla.org/mozilla-central/rev/68e29b0840e7