1901263 - Implement profile group ID

Status: RESOLVED FIXED

(Toolkit :: Startup and Profile System, task)

Description

[Dave Townsend [:mossop]]

Implement the group ID which will span across selectable profiles. All profiles should be assigned one, initially random. Once we implement profile creation we will assign the same group ID to the newly created profile. We may want to store the group ID in the database to ensure it is kept in sync but it also needs to be accessible from telemetry where we want to include it in all pings that currently include client id.

Comment 1

[Dave Townsend [:mossop]]

The current clientId is implemented in ClientID.sys.mjs, canonically stored in datareporting/state.json in the profile and cached for synchronous access in the toolkit.telemetry.cachedClientID preference. Storing the new group ID similarly makes sense. It probably makes sense to also manage this in the ClientID.sys.mjs module rather than creating a new module as we can share some aspects of the code. In particular we probably want to make sure we clear the group ID when the client ID is cleared.

At one point it was suggested that the group ID have a different form from a client ID so the two are easily distinguishable. This could be as simple as adding a prefix or using a UUID without the dashes.

Presumably there will need to be some server-side changes for this. I'm not sure what, chutten can you give some direction here?

While this is something that Firefox leadership has signed off on I am assuming we will still need to go through the sensitive data review process here?

Comment 2

[Chris H-C :chutten]

(In reply to Dave Townsend [:mossop] from comment #1)

The current clientId is implemented in ClientID.sys.mjs, canonically stored in datareporting/state.json in the profile and cached for synchronous access in the toolkit.telemetry.cachedClientID preference. Storing the new group ID similarly makes sense. It probably makes sense to also manage this in the ClientID.sys.mjs module rather than creating a new module as we can share some aspects of the code. In particular we probably want to make sure we clear the group ID when the client ID is cleared.

I concur that there's a lot of shared behaviour we want to reuse for group_id from client_id, but I don't think clearing it is one of those behaviours. To my knowledge the group id remains in force and unchanged on a profile even when data upload is disabled since prefs (including data upload) are per-profile, not per-group.

(( Of course, if the only use of the group id is for data upload then it doesn't matter whether or not it is present since it'll never enter a new ping. ))

Putting it in ClientID.sys.mjs is where I'd want to put it, too.

At one point it was suggested that the group ID have a different form from a client ID so the two are easily distinguishable. This could be as simple as adding a prefix or using a UUID without the dashes.

This has a good intention, but if we wish to transmit this using Glean it would likely make most sense as a uuid metric which normalizes its value to the standard hyphenated uuid (but without surrounding {}) format for storage and transmission. Setting an invalid or misshapen uuid value is not supported.

We could use uuid-valid but not-uuidv4-generatable values (at a higher risk of collision) by e.g. 1337-coding a prefix like 612009 (loosely "GROOP") ((this is what we do for the canary "c0ffeec0-ffee-c0ff-eec0-ffeec0ffeec0" value )), or we could put it in a string metric instead.

Presumably there will need to be some server-side changes for this. I'm not sure what, chutten can you give some direction here?

Only when/if there is some behaviour specific to the group id that we want different from all other uuid metrics.

While this is something that Firefox leadership has signed off on I am assuming we will still need to go through the sensitive data review process here?

Correct. At least as paperwork to show we've gone through our process and haven't cut any corners.

Comment 3

[Chris H-C :chutten]

Q: Does this bug include in its scope the design, impl, testing, and documentation of the instrumentation of the profile group identifier?

Comment 4

[Dave Townsend [:mossop]]

(In reply to Chris H-C :chutten from comment #3)

Q: Does this bug include in its scope the design, impl, testing, and documentation of the instrumentation of the profile group identifier?

I think this includes:

1. Generating a new group ID when one does not already exist (i.e. for all existing profiles).
2. Adding that to existing pings.
3. Documenting it.
4. Testing all that.

Though listing all that maybe I should separate out item 2 into some separate bugs, let's see how we go.

Is there other work I'm missing? I'm not totally sure I know what you mean by "instrumentation" here.

Comment 5

[Chris H-C :chutten]

"Instrumentation" is pretty much your step 2. I wanted to make sure it included both Legacy and Glean, and it included schema changes for Legacy (because it'll probably be going in either the Environment or Common Ping Data or something equally custom), and it included tests of the instrumentation, and maybe that it included changes to the Glean SDK if we need something from it (like we might need to have it respect include_client_id: false on pings, or have special behaviour when it's send_if_empty: false, or if we want to have a specific send_in_pings value for "included it everywhere the client_id is included", or what-have-you).

If this is the bug for that, then now it's all written down and that's enough for me for now.

Comment 6

[Dave Townsend [:mossop]]

Attachment: Bug 1901263: Support saving and loading the profile group ID in ClientID.sys.mjs. r=chutten!

Comment 7

[Dave Townsend [:mossop]]

Attachment: Bug 1901263: Add the profile group ID to legacy telemetry pings. r=chutten!

Comment 8

[Dave Townsend [:mossop]]

Attachment: data review

Comment 9

[Dave Townsend [:mossop]]

Attachment: Bug 1901263: Add the profile group ID to glean pings. r=chutten!

Comment 10

[BMO Github Automation]

Attachment: [mozilla-services/mozilla-pipeline-schemas] Bug 1901263: Add `profileGroupId` to schemas. (#822)

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2ab80f63edd5
Support saving and loading the profile group ID in ClientID.sys.mjs. r=chutten
https://hg.mozilla.org/integration/autoland/rev/be9a823b57b9
Add the profile group ID to legacy telemetry pings. r=chutten,data-stewards,afranchuk
https://hg.mozilla.org/integration/autoland/rev/bf43d01b36bd
Add the profile group ID to glean pings. r=chutten,data-stewards

Comment 12

[Norisz Fay [:noriszfay]]

Backed out for causing build bustages

Backout link

Push with failures

Failure log

Xpcshell failures present as well: https://treeherder.mozilla.org/logviewer?job_id=467991517&repo=autoland&lineNumber=2498

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8777b143cb18
Support saving and loading the profile group ID in ClientID.sys.mjs. r=chutten
https://hg.mozilla.org/integration/autoland/rev/fb86ed3484d1
Add the profile group ID to legacy telemetry pings. r=chutten,data-stewards,afranchuk
https://hg.mozilla.org/integration/autoland/rev/14cd024dde90
Add the profile group ID to glean pings. r=chutten,data-stewards

Comment 14

[Dave Townsend [:mossop]]

Attachment: Bug 1901263: Bustage fixes for crashreporter tests.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/73d37e644eb9
Bustage fixes for crashreporter tests. r=fix CLOSED TREE

Comment 16

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/8777b143cb18
https://hg.mozilla.org/mozilla-central/rev/fb86ed3484d1
https://hg.mozilla.org/mozilla-central/rev/14cd024dde90
https://hg.mozilla.org/mozilla-central/rev/73d37e644eb9