1037495 - Telemetry support for App Usage Metrics in FxOS

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::System, defect)

Description

[Marshall Culpepper [:marshall_law]]

Just like we do for FTU pings, we'll need to make sure we can support payloads being submitted by the app usage metrics service in Gaia which recently landed in 2.1.

Currently, payloads look like this (I realize we may need to update this)

    {
      start: timestamp of batch start time (in millisecond format),
      stop: timestamp of batch end time,
      deviceID: "0i9n4ffh" // a unique id used to link batches together,
      locale: "en_US" // the user's language from navigator.language,
      screen: {
        width: the screen width in css pixels,
        height: the screen height in css pixels,
        devicePixelRatio: device pixels per css pixel
      },
      deviceinfo: {
        'deviceinfo.update_channel': update channel,
        'deviceinfo.platform_version': gecko version number (I think)
        'deviceinfo.platform_build_id': more version details,
        'developer.menu.enabled': if true user probably has abnormal app usage patterns!
      },
      apps: {
         // the properties of this object are application origins and the values of those properties are
         // the usage data for those applications.  For example:
         'homescreen.gaiamobile.org': {
           usageTime: total app usage time during this reporting period, in seconds
           invocations: number of times the app has been opened in this period
           installs: number of times this app has been installed in this period
           uninstalls: number of times this app has been uninstalled in this period
           activities: { // the activity handlers this app has invoked, and how many times
             'gallery.gaiamobile.org': 1,    // gallery handled an activity once
             'wallpaper.gaiamobile.org': 4   // wallpaper app handled activity 4 times
           }
         },
         // more app data here
      }
    }

Comment 1

[Mark Reid [:mreid]]

Please add top-level keys for:

ver: 1
info: {
  reason: whatever reason you want to use for the app usage metrics pings
  appUpdateChannel: nightly, aurora, beta, release, etc
  appBuildID: build id
  appName: FirefoxOS
  appVersion: 33.0a1
  // Any other info keys such as "locale" can go here too, but please
  // try to match field names with existing telemetry payloads.
}

Comment 2

[Marshall Culpepper [:marshall_law]]

Per mreid, it looks like we'll also need to update the URL scheme to:
 
    /submit/telemetry/<uuid>/reason/appname/appversion/channel/buildid

Comment 3

[Marshall Culpepper [:marshall_law]]

Attachment: Factor out common telemetry code (pull request)

Updating AppUsageMetrics to use the proper URL and data format meant copying quite a bit of code from the FTU ping, so instead I factored that code out into a shared Telemetry module. In the future this module should handle getting the proper settings automatically, but I wanted to bring a little bit of sanity before going too deep down the rabbit hole.

Other than the new shared code (and updating AUM / FTU ping to use it, there are a handful of other changes I'd like a sanity check on:

* AppUsageMetrics now uses uuid() to generate it's Device ID, since AFAICT the Telemetry server requires a UUID in it's URL format. This also now matches FTU ping

* For the FTU ping, we used version number of '3' for the FTU ping so the Telemetry server could shuffle data from the payload into the proper 'info' key. I've changed this back to '1', because the FTU ping and AUM are now using the same code to construct the Telemetry URL and payload

* Some unit tests that were in ftu_ping_test.js have been removed in favor of tests that now exist indepently in telemetry_test.js

Comment 4

[Marshall Culpepper [:marshall_law]]

Shing, just a heads up that this change will rearrange the data slightly for both AppUsageMetrics and FTU ping, since the data is now being properly formatted for the Telemetry server. In general, the change takes the form of:

{
  ver: 1,
  info: {
    reason: (ftu|appusage)
    appName: 'FirefoxOS',
    appUpdateChannel: 'x',
    appVersion: 'x',
    appBuildID: 'x',

    [all other AUM or FTU ping payload data we are used to seeing will go here]
  }
}

Comment 5

[Marshall Culpepper [:marshall_law]]

Ravi, can you mark this as blocking for 2.2? We need to make sure that App Usage Metrics is using the right URL and payload format for the Telemetry Server.

Comment 6

[Ravi Dandu]

[Tracking Requested - why for this release]: We need App Usage Metrics for 2.2 release as it is essential to compute Monthly Active Usage (MAU). MAU is critical metric identified by Mozilla management to track across Mozilla products this year.

Comment 7

[Ravi Dandu]

This is 2.2 blocking. Since the component is telemetry, blocking-b2g flag is not showing up on the bugzilla to note there. Ccing Kevin Hu, and Bhavana Bajaj on the bug to track it for 2.2.

Comment 8

[Mark Reid [:mreid]]

Comment on attachment 8555994 [details] [review]
Factor out common telemetry code (pull request)

This looks good to me - I commented on the version-change back from V3 to V1, but my question is answered by comment 3.

Generally the "id" used in Telemetry is expected to be a document id, not a device id, though pretty much any non-empty string will do. You could just generate a new uuid for each submission if you prefer.

If possible, could you post a sample payload + submission URL here so I can test it?

Comment 9

[Etienne Segonzac (:etienne)]

Comment on attachment 8555994 [details] [review]
Factor out common telemetry code (pull request)

I like the (thin and well tested) TelemetryRequest a lot, and the move to a true uuid() was probably overdue, so r+ for me!

(I'd like to see an integration test for the ftu_ping eventually but its a bit outside of the scope of this bug.)

Comment 10

[Dave Zeber [:dzeber]]

Comment on attachment 8555994 [details] [review]
Factor out common telemetry code (pull request)

Looks good to me too. Please post a sample payload when you get a chance so we can get an idea of what it looks like now.

Comment 11

[David Flanagan [:djf]]

Comment on attachment 8555994 [details] [review]
Factor out common telemetry code (pull request)

r- from me because you don't handle the upgrade case from 2.1 where a device already has a deviceID that is not a uuid. 

And TelemetryRequest.sendRequest() modifies the object that is passed in. If that object is reused, there won't be a proper timeout.

I did not attempt to review the FTU changes or the tests because I'm not familiar with those.

Overall, though, this looks good, and if you fix the issues identified above, I'll be happy to give r+

Comment 12

[Marshall Culpepper [:marshall_law]]

Comment on attachment 8555994 [details] [review]
Factor out common telemetry code (pull request)

Hey David, I'm using a different string for the DEVICE_ID_KEY to simply force a new UUID to be created for v2.1->v2.2 users, and no longer deleting the timeout key, amongst your other suggestions.. Do you want to take a look back over the PR before I land it?

Comment 13

[David Flanagan [:djf]]

Comment on attachment 8555994 [details] [review]
Factor out common telemetry code (pull request)

I have not tried testing this code, but you've addressed my issues from last time, and it looks good to me.

Comment 14

[David Flanagan [:djf]]

Comment on attachment 8555994 [details] [review]
Factor out common telemetry code (pull request)

Removing the old (and now out-dated) r-. Not sure why bugzilla has me down for two different reviews

Comment 15

[Stephany Wilkes]

Blocking in B2G triage based on importance of correct payload format.

Comment 16

[Marshall Culpepper [:marshall_law]]

master - https://github.com/mozilla-b2g/gaia/commit/985651c7781210aab5badf19d78de1711279a17e

Comment 17

[GitHub Autolander Bot]

Attachment: [gaia] marshall:bug1037495_sharedTelemetry_v2.2 > mozilla-b2g:v2.2

Comment 18

[Marshall Culpepper [:marshall_law]]

Comment on attachment 8566742 [details] [review]
[gaia] marshall:bug1037495_sharedTelemetry_v2.2 > mozilla-b2g:v2.2

Updates URL and payload format for AppUsageMetrics and FTU ping

[Bug caused by] (feature/regressing bug #):Bug 1037495

[User impact] if declined: AppUsageMetrics will continue to post to a wrong path on the Telemetry server

[Testing completed]: new unit tests, and verified data was properly received on the telemetry server w/ Dave Zeber

[Risk to taking this patch] (and alternatives if risky): Code for building the Telemetry URL and payload have been put in a common place, which means a new component.

[String changes made]: None

Comment 19

[Ryan VanderMeulen [:RyanVM]]

v2.2: https://github.com/mozilla-b2g/gaia/commit/71c76f36dc572e69cf5d35a6173a555dc55e1b7d

Comment 20

[Shing Lyu [:shinglyu]]

Comment on attachment 8555994 [details] [review]
Factor out common telemetry code (pull request)

Clear my flag since it has been merged

Comment 21

[Shing Lyu [:shinglyu]]

Attachment: MozTrap Cases