1271378 - Report session times in core ping

Status: RESOLVED FIXED

(Firefox for iOS :: Telemetry, defect)

Description

[Brian Nicholson (:bnicholson)]

Draft: https://docs.google.com/document/d/1z3kD4EBO5_xFke2NTar2GvlCIkJaTHxwz3IjRGpWC2U

Comment 1

[Brian Nicholson (:bnicholson)]

Attachment: Link to Github pull-request: https://github.com/mozilla/firefox-ios/pull/1811

Comment 2

[Brian Nicholson (:bnicholson)]

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.

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

Georg and Michael talked a bit more about saving session times, since the spec was not what we thought it was. I think we did settle on using "usage" and an array of times only.

Check with mcomella for more details, and you should probably redirect the review to him or someone else.

Comment 4

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8751835 [details] [review]
Link to Github pull-request: https://github.com/mozilla/firefox-ios/pull/1811

This looks on the right track if we're okay with potentially double-counting session data (my naive thoughts think it'd be better to drop data rather than double-count though).

One thing we need to resolve before a proper r+ though: data format (https://bugzilla.mozilla.org/show_bug.cgi?id=1243595#c2 seemed to mostly resolve it, but do you feel we should discuss this more?).

Comment 5

[Brian Nicholson (:bnicholson)]

Comment on attachment 8751835 [details] [review]
Link to Github pull-request: https://github.com/mozilla/firefox-ios/pull/1811

Updated the PR to tally session counts/durations instead of an array of times. Not requesting review yet since we'll try to keep the iOS and Android ping version numbers in sync, and this is among the last of the changes Android is making.

Comment 6

[Brian Nicholson (:bnicholson)]

Comment on attachment 8751835 [details] [review]
Link to Github pull-request: https://github.com/mozilla/firefox-ios/pull/1811

Think this is ready for re-review now. Note that the PR currently includes bug 1271380, so just look at the last commit.

Comment 7

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8751835 [details] [review]
Link to Github pull-request: https://github.com/mozilla/firefox-ios/pull/1811

Seems reasonable to me, with my comments taken into account.

Comment 8

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Comment on attachment 8751835 [details] [review]
Link to Github pull-request: https://github.com/mozilla/firefox-ios/pull/1811

LGTM

Comment 9

[Brian Nicholson (:bnicholson)]

https://github.com/mozilla/firefox-ios/commit/9077695a7a7d51e77190c4da4cd72b606e2bfef2