Improve telemetry and logging coverage
Categories
(Firefox :: Session Restore, task, P2)
Tracking
()
Tracking | Status | |
---|---|---|
firefox126 | --- | fixed |
People
(Reporter: sclements, Assigned: kcochrane)
References
(Blocks 2 open bugs)
Details
(Whiteboard: [fidefe-session-restore])
Attachments
(2 files)
48 bytes,
text/x-phabricator-request
|
Details | Review | |
3.21 KB,
text/plain
|
chutten
:
data-review+
|
Details |
During my investigation of probes in bug 1854391, I found a few important pieces of information missing. These are some changes we should make to ensure better coverage and if we have ways of logging that are not console.error, we should add that too:
-
FX_SESSION_RESTORE_ALL_FILES_CORRUPT needs to be added to the release channel.
-
FX_SESSION_RESTORE_CORRUPT_FILE is of limited utility, because it only tracks if a failure occurs. We should create a new probe so we can track the reason for a corrupt file as it iterates through the various files/backups stored. We do log the errors in the try/catch block but that is only to console.error and users won't always know to look for it. If we keep this histogram, it should be added to the release channel and we might want to clean up use of it (it could potentially log false, then true if there's an except in the catch block).
-
SHUTDOWN_OK could use additional reporting since the value of this boolean histogram is based on whether the previous session crashed but it doesn't log the reasons.
-
Additionally, would be worth looking into if this check is good enough? Should the lastSessionState be deleted before knowing whether initialState.session exists or not?
-
Add new probes and logging:
- If at any stage in SessionWriter.write we hit an exception with writing to JSON, we should monitor it with a probe and log with the reason why rather than just throwing an error.
- In _onSessionFileRead here we should have some way of tracking if a session file has been modified by an add-on, because if that stateString is not parseable that would cause a session to not be restorable. Lets also log how often a sessionType is set to NO_SESSION and if there is a reason for it (it could be because the user purged history, but it could also be for the previously mentioned reason that an initialState string is invalid).
Updated•1 year ago
|
Comment 1•1 year ago
|
||
Do we want to keep bug 1854373 as separate, or should I close it as a dupe of this one?
Reporter | ||
Comment 2•1 year ago
|
||
(In reply to Sam Foster [:sfoster] (he/him) from comment #1)
Do we want to keep bug 1854373 as separate, or should I close it as a dupe of this one?
I'll close bug 1854373 out as a dupe.
Updated•1 year ago
|
Assignee | ||
Updated•11 months ago
|
Assignee | ||
Comment 4•11 months ago
|
||
Updated•11 months ago
|
Assignee | ||
Comment 5•10 months ago
|
||
Requesting data review for two new telemetry events for session restore.
Comment 7•10 months ago
|
||
Comment on attachment 9394250 [details]
Session Restore Data Collection 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.
Is there a control mechanism that allows the user to turn the data collection on and off?
Yes. This collection 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, :kcochrane 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 2, Interaction.
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 the data collection use a third-party collection tool?
No.
Result: datareview+
Description
•