Bug 1271378 - Report session times in core ping

[thebnich]

This sends an array of usage times with the core ping. A single usage time is defined as the time, in seconds, the application has been in the foreground.

One useful metric this can help capture is determining how long users are using the app before they uninstall. Since sending the ping on each application foreground isn't sufficient to measure a user who only uses the application once, this PR changes the core ping triggers. Rather than sending the core ping on each application foreground, we'll now send the core ping to be sent after 1) the first application startup, and 2) each application background.

Sending a ping on each background allows us to include the usage interval for that session. Sending a ping on the first run (or more specifically, first run with this patch applied) gives us a soon-as-possible data point that we can use to uniquely identify a user (so they'll at least be on the map after a crash-then-uninstall scenario). Other than the initial launch, I can't think of a reason we'd want to send pings on app startup/foreground, but happy to hear any arguments.

The usage array works by appending the current usage time to the stored array, then clearing the stored array after a successful ping. That means the very first ping will have an empty usage array on the first run, and all subsequent pings will contain at least one value in the array. Generally, the array will have just one item since pings will usually succeed.

[mcomella]

Sending a ping on each background allows us to include the usage interval for that session.

How do you handle the case that the ping could not be uploaded to the server?

edit: It looks like you store the usage data in a preference and remove it if the upload is successful. However, this could cause you to double-count if the server got and stored the ping but was unable to notify the client (e.g. the final ACK packets are lost). Since you don't think you successfully uploaded, you'd try to upload these sessions again. Are you okay with that?

An alternative would be to only attempt to upload the session data once (i.e. you can only guarantee the server receives the data at most once or at least once but not exactly once).

fwiw, on Android, we use sequence numbers to uniquely identify a ping and its data. To do this, when we create a ping for upload, we immediately store it to disk and when we try to upload, we try to upload all of the pings stored on disk. We keep a running tally for measurements stored on disk and reset them when a new ping is created (e.g. session usage data & search counts).

Sending a ping on the first run (or more specifically, first run with this patch applied) gives us a soon-as-possible data point that we can use to uniquely identify a user (so they'll at least be on the map after a crash-then-uninstall scenario).

This seems reasonable.

Other than the initial launch, I can't think of a reason we'd want to send pings on app startup/foreground, but happy to hear any arguments.

I assume iOS allows you to run (potentially long-running) code in the background on app closed without getting interrupted? We wouldn't want the app close blocked on network access or the app killed while we're doing network access.

[mcomella]

Other than the initial launch, I can't think of a reason we'd want to send pings on app startup/foreground, but happy to hear any arguments.

If we're really getting into the nitty gritty, here are some more ideas:

• When the app is opened, we'll very likely activate the radio (we're a browser after all) making it efficient to activate the radio to upload this. However, when the app is closing, the radio may not have been used for a while and may not be used for a while afterwards so we're causing the radio to use a lot of energy that could have been avoided.
• How does the OS handle device shutdown? If the app is open and someone requests shutdown, will you have enough time to upload?

[mcomella]

nit: foregroundTime makes me think "time in the foreground", i.e. elapsed. I'd call it timeWhenForegrounded or something.

[mcomella]

This sends an array of usage times with the core ping. A single usage time is defined as the time, in seconds, the application has been in the foreground.

Did you check with Georg about this? In the previous meeting, we discussed how we wanted to handle this field and we decided to change this into two separate fields, see how it felt, and iterate if necessary:

• session time in seconds (aggregate)
• number of sessions

[mcomella]

It's a little unfortunate these version numbers aren't aligning with the android implementation but perhaps that's to be expected – we should probably have iOS-specific docs. I'd talk to Georg.

[mcomella]

I assume record & reset can never be called concurrently (given that I don't see synchronization code)?

[thebnich]

For all practical scenarios, correct. We record the usage only when entering the background, and we reset only when we send the core ping (which is on the first startup and for each background, after we record). It's theoretically possible for these to overlap if the user manages to background the app very quickly after starting it for the first time, or if they background multiple times very quickly and have queued core pings fire when resetting, but I'm not convinced this is a real concern.