847499 - Make sure telemetry knows to write out saved-session data when fennec goes in background

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[(dormant account)]

10:30 <taras> so atm telemetry reports on idle-daily
10:30 <taras> which many android sessions dont live long enough for
10:30 <taras> the other time it saves data
10:30 <taras> is on app shutdown
10:30 <taras> which also doesnt happen
10:30 <taras> so we need to trigger telemetry flush
10:31 <taras> on when app gets pushed into background

This should get us bursty saved-session submissions. Follow up would be to have the fennec background service send telemetry in.

Comment 1

[Matt Brubeck (:mbrubeck)]

We might have similar issues in Firefox for Metro...

Comment 3

[Kartikaya Gupta (email:[e-mail])]

Attachment: WIP

Just an outline of how this could be implemented. I think the current telemetry code assumes the pings are either in _pendingPings or saved to file (but not both), and they are only flushed to file on exit. If we flush to disk on going to background we'll be violating this assumption and so we have to deal with the fallout by re-loading them into memory before we send it out. I'll flesh this out more and test it.

Comment 4

[Nathan Froyd [:froydnj]]

I think you should be able to get away with:

1) Flush the current session to disk when backgrounding the application;
2) When resuming from background, delete the ping file corresponding to the current session.  There's no need to send that ping file now, since we're still running the current session.

I'm assuming backgrounding/resuming doesn't create new session IDs (seems logical).  I don't think there's any need to load all the saved pings every time we try to send.

Comment 5

[Kartikaya Gupta (email:[e-mail])]

Attachment: Save session data on backgrounding

Updated patch. Still needs testing; will request review once that's done.

Comment 6

[Kartikaya Gupta (email:[e-mail])]

Attachment: Save session data on backgrounding

This seems to do the right thing, although in order to test it I had to hack up the code a bunch so I'm not 100% sure I tested it correctly.

Also, if I'm reading the code correctly, it seems that when you quit desktop firefox, the quit-application-granted and profile-before-change2 handlers will get run, and will unconditionally save the ping file for the session, even if that ping has already been sent. This means the ping will get re-sent in the next session. Is that correct? I added the _pingsSubmitted flag to guard against this scenario before I realized it affected desktop as well, so maybe it doesn't need to be there.

Comment 7

[Nathan Froyd [:froydnj]]

Comment on attachment 729329 [details] [diff] [review]
Save session data on backgrounding

Review of attachment 729329 [details] [diff] [review]:
-----------------------------------------------------------------

You are reading the code correctly, and it's OK that a sequence like:

send ping at idle
save ping
<time elapses, browser restarts>
load saved ping from previous session, send that

happens, because the last (saved) ping should contain at least as much data as the ping sent out at idle.  In fact, the telemetry server only stores one ping per session anyway.

r=me if you take out the code dealing with _pingsSubmitted.

Comment 8

[Kartikaya Gupta (email:[e-mail])]

That makes sense, thanks. Took out _pingsSubmitted and landed:

https://hg.mozilla.org/integration/mozilla-inbound/rev/eed15a51c2d8

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/eed15a51c2d8