1635662 - Add ecosystemClientId to the Accounts Ecosystem Telemetry ping

Status: RESOLVED FIXED

(Toolkit :: Telemetry, enhancement, P3)

Description

[Ryan Kelly [:rfkelly]]

As described in Account Ecosysten Telemetry Identifiers, the AET ping needs to include an ecosystemClientId field that is very similar to, but not linked to, the main-ping clientId. This is a randomly-generated identifier that is unique to a particular Firefox profile.

We will need to:

• Manage persistence of such an identifier, creating a new random one if it doesn't already exist
• Arrange for it to be included in the ecosystem telemetry ping that results from Bug 1635659.
• Arrange for it to be included in the deletion-request ping for users who opt out of telemetry.

I haven't looked into the details of doing this at all, but I'm kind of hopeful that we can wholesale copy how the main telemetry clientId is managed...

Comment 1

[Chris H-C :chutten]

Chiming in to say: Another important aspect of any client-id-alike will be that it is cleared to a canary value on opt-out and is set to a new value if the user opts back in.

Comment 3

[Lina Butler [:lina]]

:chutten, since we want this to have the same semantics as the client ID (clear to canary on opt-out, reset on opt-in), we were wondering if we can add the ecosystemClientId to ClientID.jsm (it wouldn't be the client ID, but a new random ID that's reset and cleared together with it), and add accessors for it?

Comment 4

[Mark Hammond [:markh] [:mhammond]]

Another question I have relates to the async nature of this system. TelemetryControllerParent really doesn't want to await fetching the clientID. The patch in bug 1635659 already awaits on FxA as part of submitting the ping, but I'm not sure how much of a problem this is.

So if we assume that the response to Lina's question means that ClientID.jsm can be used and ends up with (say) getEcosystemClientID() and getCachedEcosystemClientID(), I'm not clear whether:

• EcosystemTelemetry.jsm can just use ClientID.jsm directly and call getCachedEcosystemClientID(), then await getEcosystemClientID() if that returned null, and stick the result directly in the payload.

or

• We should thread support for this ID into TelemetryControllerParent in the same way it already supports the plain old clientID (including the reporting of a histogram if it's not available like it does for clientID), and EcosystemTelemetry.jsm just passes (say) includeEcosystemClientId: true in options?

Comment 5

[Chris H-C :chutten]

Adding additional client_id-like ID support to ClientID.jsm was the original preferred design, and to my knowledge nothing has changed to invalidate that design. So please do go ahead! (( Originally I think we were going to make it a named key generator/store for client_id-like IDs that any subsystem might use/want, but I think with Glean coming that it makes sense to keep this specific to the ecosystem_client_id instead of generic ))

I prefer EcosystemTelemetry using ClientID directly to get the client id. If we added it to submitExternalPing it might make others think they should use it (I already have this qualm about the useEncryption options, but there were other factors there that made this the least bad design). Also: The ecosystem client id is inside the payload, yes? Then it makes more sense to have it out where the payload is being assembled. (submitExternalPing should treat the payload opaquely as much as possible).

Comment 6

[Lina Butler [:lina]]

Attachment: Bug 1635662 - Manage the ecosystem client ID in telemetry. r?chutten!

This commit adds a new ecosystem client ID to ClientID.jsm. It's
handled the same way as a regular telemetry client ID, in that it's a
random UUID that's persisted to disk, and reset together with the
client ID. Unlike the main client ID, we don't cache it in prefs.

Comment 7

[Lina Butler [:lina]]

For deletion-request handling, I guess we'll want to hook into here and include the old ecosystem client ID in the payload (along with scalars), right?

Would we also want to include the anonymized user ID (ecosystem_anon_id here)? Or is the ecosystem_client_id sufficient—IOW, if you opt out of telemetry collection on Desktop, we'll still retain data from iOS and Fenix with the same ecosystem_anon_id, just not the client ID?

Comment 8

[Ryan Kelly [:rfkelly]]

Would we also want to include the anonymized user ID (ecosystem_anon_id here)?

We should include only the device-specific identifier, not the account-level identifier.

IOW, if you opt out of telemetry collection on Desktop, we'll still retain data from iOS and Fenix with the same ecosystem_anon_id

Right, this is the intended behaviour. Users can delete the telemetry for their whole account by deleting their account.

Comment 9

[Lina Butler [:lina]]

Thanks, that matches my understanding!

Comment 10

[Chris H-C :chutten]

(In reply to Lina Cambridge [:lina] from comment #7)

For deletion-request handling, I guess we'll want to hook into here and include the old ecosystem client ID in the payload (along with scalars), right?

Would we also want to include the anonymized user ID (ecosystem_anon_id here)? Or is the ecosystem_client_id sufficient—IOW, if you opt out of telemetry collection on Desktop, we'll still retain data from iOS and Fenix with the same ecosystem_anon_id, just not the client ID?

The intention with the "deletion-request" ping is for any ride-along ids to be stored as a Scalar in the "deletion-request" store so that it'll get picked up automagically (See these other ids for example). If you set the ride along scalar value on ClientID.jsm's updateEcosystemClientId and that should be that.

Comment 11

[Lina Butler [:lina]]

Attachment: deletion-request-data-review.md

Slavishly copied from Ryan's data review request in bug 1604844, with appropriate changes (ecosystem client ID will only be sent if the user has signed in to FxA).

Comment 12

[Chris H-C :chutten]

Comment on attachment 9165268 [details]
deletion-request-data-review.md

PRELIMINARY NOTES:

There is a new version of the Data Review Request: https://github.com/mozilla/data-review/blob/master/request.md

This new version differs from the version you used in only Q6 where the new form asks for the location of documentation. In this case I know the location and will address it in my first answer below.

For future Data Review Requests, please use the new form.

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. This collection is Telemetry so is documented in its definitions file [Scalars.yaml](https://hg.mozilla.org/mozilla-central/file/tip/toolkit/components/telemetry/Scalars.yaml) and the [Probe Dictionary](https://telemetry.mozilla.org/probe-dictionary/).

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

Yes. This collection is Telemetry so can be controlled through Firefox's Preferences.

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

Yes, Lina is responsible.

    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. (( The identifier itself is Cat4 but it has previously been approved for inclusion in "deletion-request" pings and other parts of Accounts Ecosystem Telemetry. This categorization is for its inclusion in the "deletion-request" ping only ))

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

Default on for all channels.

    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. This collection is permanent.

---
Result: datareview+

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bb8a399944a5
Manage the ecosystem client ID in telemetry. r=chutten

Comment 14

[Natalia Csoregi [:nataliaCs]]

Backed out for failures on test_client_id.js

backout: https://hg.mozilla.org/integration/autoland/rev/c68ce4b312265e9f640e4d72c84f41b9b45bbc4f

push: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedTaskRun=LGz_pYDdRsqdnHO_ZbZEUg.0&searchStr=android%2C7.0%2Cx86-64%2Cdebug%2Cxpcshell%2Ctests%2Ctest-android-em-7.0-x86_64%2Fdebug-geckoview-xpcshell-e10s%2Cx2&revision=bb8a399944a54d3844a7d05502e2755ac153aef1

failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=311009164&repo=autoland&lineNumber=3017

[task 2020-07-24T22:33:20.244Z] 22:33:20 INFO - TEST-START | toolkit/components/telemetry/tests/unit/test_client_id.js
[task 2020-07-24T22:33:21.271Z] 22:33:21 WARNING - TEST-UNEXPECTED-FAIL | toolkit/components/telemetry/tests/unit/test_client_id.js | xpcshell return code: 0
[task 2020-07-24T22:33:21.271Z] 22:33:21 INFO - TEST-INFO took 1026ms
[task 2020-07-24T22:33:21.271Z] 22:33:21 INFO - >>>>>>>
[task 2020-07-24T22:33:21.272Z] 22:33:21 INFO - toolkit/components/telemetry/tests/unit/test_client_id.js | xpcw: cd /data/local/tmp/test_root/xpc/toolkit/components/telemetry/tests/unit
[task 2020-07-24T22:33:21.273Z] 22:33:21 INFO - toolkit/components/telemetry/tests/unit/test_client_id.js | xpcw: xpcshell -r /data/local/tmp/test_root/xpc/c/httpd.manifest --greomni /data/local/tmp/test_root/xpcb/geckoview-androidTest.apk -m -e const _HEAD_JS_PATH = "/data/local/tmp/test_root/xpc/head.js"; -e const _MOZINFO_JS_PATH = "/data/local/tmp/test_root/xpc/p/mozinfo.json"; -e const _PREFS_FILE = "/data/local/tmp/test_root/xpc/user.js"; -e const _TESTING_MODULES_DIR = "/data/local/tmp/test_root/xpc/m"; -f /data/local/tmp/test_root/xpc/head.js -e const _HEAD_FILES = ["/data/local/tmp/test_root/xpc/toolkit/components/telemetry/tests/unit/head.js"]; -e const _JSDEBUGGER_PORT = 0; -e const _TEST_FILE = ["test_client_id.js"]; -e const _TEST_NAME = "toolkit/components/telemetry/tests/unit/test_client_id.js"; -e _execute_test(); quit(0);
[task 2020-07-24T22:33:21.273Z] 22:33:21 INFO - toolkit/components/telemetry/tests/unit/test_client_id.js | [6836, Unnamed thread 75a2681219b0] WARNING: XPCOM objects created/destroyed from static ctor/dtor: file /builds/worker/checkouts/gecko/xpcom/base/nsTraceRefcnt.cpp, line 202
[task 2020-07-24T22:33:21.273Z] 22:33:21 INFO - toolkit/components/telemetry/tests/unit/test_client_id.js | [6836, Unnamed thread 75a2681219b0] WARNING: XPCOM objects created/destroyed from static ctor/dtor: file /builds/worker/checkouts/gecko/xpcom/base/nsTraceRefcnt.cpp, line 202
[task 2020-07-24T22:33:21.274Z] 22:33:21 INFO - toolkit/components/telemetry/tests/unit/test_client_id.js | [6836, Unnamed thread 75a2681219b0] WARNING: XPCOM objects created/destroyed from static ctor/dtor: file /builds/worker/checkouts/gecko/xpcom/base/nsTraceRefcnt.cpp, line 202
[task 2020-07-24T22:33:21.274Z] 22:33:21 INFO - toolkit/components/telemetry/tests/unit/test_client_id.js | [6836, Unnamed thread 75a2681219b0] WARNING: XPCOM objects created/destroyed from static ctor/dtor: file /builds/worker/checkouts/gecko/xpcom/base/nsTraceRefcnt.cpp, line 202
[task 2020-07-24T22:33:21.274Z] 22:33:21 INFO - toolkit/components/telemetry/tests/unit/test_client_id.js | [6836, Main Thread] WARNING: No Android crash handler set: file /builds/worker/checkouts/gecko/toolkit/crashreporter/nsExceptionHandler.cpp, line 1959
[task 2020-07-24T22:33:21.274Z] 22:33:21 INFO - toolkit/components/telemetry/tests/unit/test_client_id.js | [6836, Main Thread] WARNING: No default pref files found.: file /builds/worker/checkouts/gecko/modules/libpref/Preferences.cpp, line 4240
[task 2020-07-24T22:33:21.275Z] 22:33:21 INFO - toolkit/components/telemetry/tests/unit/test_client_id.js | [6836, Main Thread] WARNING: Couldn't get the user appdata directory. Crash events may not be produced.: file /builds/worker/checkouts/gecko/toolkit/crashreporter/nsExceptionHandler.cpp, line 2914
[task 2020-07-24T22:33:21.275Z] 22:33:21 INFO - (xpcshell/head.js) | test MAIN run_test pending (1)

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cf7bb4a22eb9
Manage the ecosystem client ID in telemetry. r=chutten

Comment 16

[Lina Butler [:lina]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c55865515683af5f1b6f167a1edd8318a9d9be60

Comment 17

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/cf7bb4a22eb9