1689438 - Instrument IO Errors and inconsistencies in ClientID.jsm

Status: RESOLVED FIXED

(Toolkit :: Telemetry, task, P1)

Description

[Chris H-C :chutten|PTO (back July 13)]

While investigating bug 1678146 :tdsmith encouraged me to look closer at a very rare but perhaps-significant issue: sometimes a single Glean client_id would report being associated with several Telemetry client_ids.

This should be more-or-less impossible without copying just ProfD/datareporting/glean/* into a new profile while leaving ProfD/datareporting/state.json behind.

After some thinking out loud while reading code I noticed that we appear to only ever attempt to save the Telemetry client_id to state.json exactly once (unless you opt out of Telemetry, in which case we also try to save it after we set it to c0ffeec0-ffee-c0ff-eec0-ffeec0ffeec0). What if that fails? What if that always fails?

I think we should instrument ClientID.jsm to report a few things:

• The number of times an IO Error happens while trying to save state.json to disk
• Whether we generated a new client_id this subsession due to an inability to load state.json
• The number of times we successfully load a client_id from state.json that doesn't match the value cached in the profile's preferences

This way we can see if IO Errors lead towards additional generation, whether the client_id in the current ping is "fresh" (so we can compare it to other indicators that the profile isn't fresh, like first use date and profileSubsessionCount), and whether there's something causing a disagreement between the two storage mechanisms for Telemetry's client_id.

Comment 1

[Chris H-C :chutten|PTO (back July 13)]

Attachment: Bug 1689438 - Instrument ClientID.jsm for potential failures r?janerik!

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cb8d52e34d80
Instrument ClientID.jsm for potential failures r=janerik

Comment 3

[Narcis Beleuzu [:NarcisB]]

Backed out for xpcshell failures on test_client_id.js

Backout link: https://hg.mozilla.org/integration/autoland/rev/f6e71a0eec550054607f5ff7796ae7461dd0189d
Log link: https://treeherder.mozilla.org/logviewer?job_id=328458388&repo=autoland&lineNumber=1803

Comment 4

[Chris H-C :chutten|PTO (back July 13)]

Attachment: data collection review

(test failure appears to be a product check needing override. Patch forthcoming)

Oops, also forgot a data collection review. Bad chutten. No biscuit.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/211158e3f477
Instrument ClientID.jsm for potential failures r=janerik

Comment 6

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/211158e3f477

Comment 7

[Travis Long [:travis_]]

Comment on attachment 9200465 [details]
data collection review

1. Is there or will there be documentation that describes the schema for the ultimate data set in a public, complete, and accurate way?
   Yes

2. Is there a control mechanism that allows the user to turn the data collection on and off?
   Yes, through the Data Choices preference

3. If the request is for permanent data collection, is there someone who will monitor the data over time?
   N/A, expires in Firefox 90

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

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

6. Does the instrumentation include the addition of any new identifiers?
   No

7. Is the data collection covered by the existing Firefox privacy notice?
   Yes

8. Does there need to be a check-in in the future to determine whether to renew the data?
   Set to expire in Firefox 90, may renew after that.

9. Does the data collection use a third-party collection tool?
   No

Result: data-review+

Comment 8

[Chris H-C :chutten|PTO (back July 13)]

Comment on attachment 9199884 [details]
Bug 1689438 - Instrument ClientID.jsm for potential failures r?janerik!

Beta/Release Uplift Approval Request

• User impact if declined: Delay in finding out the impact of data weirdness caused by IO Errors in the client_id module.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): On one hand, this has sat in Nightly for a few days and everything points to it only instrumenting things, not changing them. On the other hand, the client_id is the most fundamental unit of deduplication we have in Firefox Telemetry, so any change to it will have ramifications in anything we do with Data.

Thus, I call it Medium and am glad we're in Early Beta.

• String changes made/needed:

Comment 9

[Pascal Chevrel (relman team) -> :pascalc]

Comment on attachment 9199884 [details]
Bug 1689438 - Instrument ClientID.jsm for potential failures r?janerik!

This is medium risk and we are actually past early betas. Given our shorter release cycle and the fact that we are ending the 86 beta cycle this week, I think this should ride the 87 cycle, thanks.