719167 - telemetry should periodically save histograms

Status: RESOLVED WORKSFORME

(Toolkit :: Telemetry, defect)

Description

[Nathan Froyd [:froydnj]]

Once bug 707320 goes in, we should periodically save the state of histograms to disk so that we don't lose useful information in the case of a browser crash or similar.

Comment 1

[Sid Stamm [:geekboy or :sstamm]]

This change is contrary to a risk resolution in the privacy review:

https://wiki.mozilla.org/Privacy/Reviews/Telemetry#Conformity_to_Private_Browsing_Mode (scroll down to the resolution box)

"Telemetry data always kept only in volatile memory or temporarily persisted to disk (see above). Telemetry collection and reporting is entirely suspended when private mode is entered, and resumed on private mode exit. See bug 661573"

Originally, nothing was persisted to disk.  We changed that in bug 707320 so that stuff could be measured during shut-down.  What is the problem we'd like to solve by persisting this data?

Comment 2

[Nathan Froyd [:froydnj]]

The problem is that between telemetry pings, the browser can crash, and we lose whatever information had accumulated since the last ping.  The accumulated information is useful.  Pings from saved sessions are distinguished from pings for the current session; saved-session pings without successful shutdowns are interesting as a secondary way of measuring crashes, perhaps.

Comment 3

[Sid Stamm [:geekboy or :sstamm]]

I believe (though I am trying to find the study), most people close their browsers pretty regularly -- daily.  

So is it your goal to identify telemetry data sets that lead to crashes?  What do we gain from this change other than "fewer lost datasets"?

Comment 4

[Nathan Froyd [:froydnj]]

I could easily believe the majority of people close their browsers daily.

I honestly don't know what the exact goals were; Taras suggested doing this in bug 707320 comment 39 and asked me to file it.  I was offering possible justifications, but maybe it's better to go to the source.  Taras, what did you have in mind?

Comment 5

[(dormant account)]

(In reply to Sid Stamm [:geekboy] from comment #3)

> So is it your goal to identify telemetry data sets that lead to crashes? 
> What do we gain from this change other than "fewer lost datasets"?

Indeed. Robustness is the main goal here.

Comment 6

[Sid Stamm [:geekboy or :sstamm]]

Makes sense.  Can we delete the persisted file after the data gets transmitted?  If we are simply making sure we don't lose data on crash, can we purge any loaded-from-disk data when we recover from a crash and do a ping?  This is the same as what I recommended in bug 707320 comment 30.

Comment 7

[(dormant account)]

(In reply to Sid Stamm [:geekboy] from comment #6)
> Makes sense.  Can we delete the persisted file after the data gets
> transmitted?  If we are simply making sure we don't lose data on crash, can
> we purge any loaded-from-disk data when we recover from a crash and do a
> ping?  This is the same as what I recommended in bug 707320 comment 30.

Yes we should do that in this bug.

Comment 8

[Nathan Froyd [:froydnj]]

(In reply to Sid Stamm [:geekboy] from comment #6)
> Makes sense.  Can we delete the persisted file after the data gets
> transmitted?  If we are simply making sure we don't lose data on crash, can
> we purge any loaded-from-disk data when we recover from a crash and do a
> ping?  This is the same as what I recommended in bug 707320 comment 30.

The deletion of the data file is being done in the patches already posted for bug 707320.

Comment 9

[Georg Fritzsche [:gfritzsche]]

We already cover the concerns here now:
* We submit "main" pings much more frequently.
* We periodically save Telemetry session payloads to disk and submit them with reason "aborted-session" in the next session.