1156355 - Telemetry redesign: Consolidate file logic in TelemetryStorage.jsm

Status: RESOLVED FIXED

(Toolkit :: Telemetry, defect, P2)

Description

[Georg Fritzsche [:gfritzsche]]

Per: https://docs.google.com/document/d/1lKRfNl5aJ0W2flOp67Y5O3WDz2dSSz7a6r23DQ89poU/edit?pli=1

TelemetryStorage.jsm should be a helper module with almost no state.

This module should be in charge of
* the saving/loading pings & scanning ping dirs
* serializing writes to the same file
* pruning & deleting of persisted pings

This module should know how to do higher-level operations such as saving/removing the aborted session file.
This module should handle the details of saving/loading/scanning ping directories:
* TelemetryController will use this for pending pings
  * we won’t be able to load them all into memory, so we should handle them just like the archived pings for cheap scanning
* TelemetryController will use this for the archived pings

The interface is at least:
* savePendingPing(ping) called by TelemetryPing
* loadPendingPing(id) called by TelemetryPing
* getPendingPingList()
* saveArchivedPing(ping) (done in bug 1150134)
* loadArchivedPing(id) (done in bug 1150134)
* getArchivedPingList() (done in bug 1150134)
* saveAbortedSessionPing()
* loadAbortedSessionPing()
* removeAbortedSessionPing()

Comment 1

[Georg Fritzsche [:gfritzsche]]

Also we should:
* rename loadHistograms()
* rename |get pingsLoaded()| to something more fitting with its name
* change loadSavedPings() to not load all pings from this (here or in bug 1041616 or ...)
  * pending pings should use a file name scheme that contains their id, type & creation timestamp (like archived pings)
  * we should build a list of pending pings
  * we can clean old pings etc. out just from that cheap-to-build list
  * when we have send opportunities etc. we can load pings from that list and send them

Comment 2

[Georg Fritzsche [:gfritzsche]]

Attachment: Part 1: Refactor aborted-session ping handling

Move aborted-session file handling details into TelemetryStorage.

Comment 3

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 8600473 [details] [diff] [review]
Part 1: Refactor aborted-session ping handling

I moved this part over to bug 1156253 to not block that one.

Comment 4

[Alessio Placitelli [:Dexter]]

I think this bug can be safely closed as we've got all the stuff we wanted in TelemetryStorage, with the other modules relying on this one for Telemetry I/O operations.

- Saving, loading and removing the aborted-session ping [1]
- Saving, loading, removing a pending ping and scanning the pending ping directory [2]
- Saving, loading an archived ping and scanning the archived ping directory [3]
- Pruning/Enforcing quota on the pending ping directory [4]
- Pruning/Enforcing quota on the archived ping directory [5]

It also seems that the changes from comment 1 were already addressed.

[1] - https://dxr.mozilla.org/mozilla-central/rev/c2256ee8ae9a8ee0bf7ab49a8b1924720d846cc7/toolkit/components/telemetry/TelemetryStorage.jsm#306,316,349
[2] - https://dxr.mozilla.org/mozilla-central/rev/c2256ee8ae9a8ee0bf7ab49a8b1924720d846cc7/toolkit/components/telemetry/TelemetryStorage.jsm#230,257,267,281,295
[3] - https://dxr.mozilla.org/mozilla-central/rev/c2256ee8ae9a8ee0bf7ab49a8b1924720d846cc7/toolkit/components/telemetry/TelemetryStorage.jsm#150,160,171
[4] - https://dxr.mozilla.org/mozilla-central/rev/c2256ee8ae9a8ee0bf7ab49a8b1924720d846cc7/toolkit/components/telemetry/TelemetryStorage.jsm#191,200
[5] - https://dxr.mozilla.org/mozilla-central/rev/c2256ee8ae9a8ee0bf7ab49a8b1924720d846cc7/toolkit/components/telemetry/TelemetryStorage.jsm#182