1137067 - [Metrics] Duplicate entries in the FTU ping

Status: RESOLVED FIXED

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

Description

[Shing Lyu [:shinglyu]]

*** Description
  appUpdateChannel, appBuildID and appVersion are duplicates to deviceinfo.* fields.

*** Steps to Reproduce
  1. Reset/Flash gaia
  2. Go through the FTU, enable WiFi (with at least one SIM card inserted)
  3. Wait for the FTU ping to reach the server

*** Expected Results
  No duplicate/redundant information in the payload

*** Actual Results
  appUpdateChannel, appBuildID and appVersion are duplicates to deviceinfo.* fields.

*** Other Notes
  I guess these fields are for backward compatibility. Just open a bug to see if we need to remove them in the future to reduce FTU ping size.

*** Reproduction Frequency
100%

*** Build
  Gaia-Rev        8e98fe665f3821d10d4d982cbb14cbe5b94d0be5
  Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/62c80c92b39e
  Build-ID        20150215162503
  Version         37.0a2
  Device-Name     flame
  FW-Release      4.4.2
  FW-Incremental  65
  FW-Date         Mon Dec 15 18:51:29 CST 2014
  Bootloader      L1TC000118D0

Comment 1

[Ravi Dandu]

Duplicate information should be removed. 
Did a quick character count removing the 3 duplicated values, and it turned out to be 13% to 17% reduction in payload characters. This is good return of investment. Lets put it on the backlog, and plan for it.

Comment 2

[Marshall Culpepper [:marshall_law]]

Yeah, the appUpdateChannel / appBuildId / etc fields exist for integration with the telemetry server. I'd be fine with removing the corresponding deviceinfo.* fields to save space, and it should be low risk enough to land for v2.2.

Tamara, do you want to take a look at this?

Comment 3

[Tamara Hills [:thills]]

Hi Marshall, I want to take a look at this but i will probably look at Bug 1141891 first.  My .02 is that it's probably more important.  Let me know if you disagree.

Thanks,

-tamara

Comment 4

[Tamara Hills [:thills]]

Attachment: ftuduplicatedata.diff

Hi Marshall,

Do you mind taking a look before I add the tests?

Also, just to be clear, I removed the duplicate device* entries and I realized the they are not only duplicated in the info portion of the JSON, but that they are also in the URL of the actual telemetry request.  I'm assuming server side needs it in the URL.  Does it also need it in the info of the telemetry request?

Thanks,

-tamara

Comment 5

[Marshall Culpepper [:marshall_law]]

(In reply to Tamara Hills [:thills] from comment #4)
> Also, just to be clear, I removed the duplicate device* entries and I
> realized the they are not only duplicated in the info portion of the JSON,
> but that they are also in the URL of the actual telemetry request.  I'm
> assuming server side needs it in the URL.  Does it also need it in the info
> of the telemetry request?

Yes, they are needed in both the URL and the appXXX keys of the 'info' dict in the payload. Taking a look at the patch now..

Comment 6

[Marshall Culpepper [:marshall_law]]

Comment on attachment 8577300 [details] [diff] [review]
ftuduplicatedata.diff

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

This looks good!

Comment 7

[GitHub Autolander Bot]

Attachment: [gaia] tamarahills:bugfix/1137067-remove-duplicate-ftu-entries > mozilla-b2g:master

Comment 8

[Marshall Culpepper [:marshall_law]]

Comment on attachment 8578075 [details] [review]
[gaia] tamarahills:bugfix/1137067-remove-duplicate-ftu-entries > mozilla-b2g:master

Good work Tamara :)

I made one minor suggestion, but this looks good to me. Can you also update the Metrics wiki with a new sample payload?

https://wiki.mozilla.org/FirefoxOS/Metrics/App_Usage

Comment 9

[Tamara Hills [:thills]]

Green try:  https://treeherder.mozilla.org/#/jobs?repo=gaia-try&revision=5b06297df291

Comment 10

[Tamara Hills [:thills]]

https://github.com/mozilla-b2g/gaia/commit/b4066c9668fbc69af468c4f625d7bf0ee8607280

Comment 11

[Shing Lyu [:shinglyu]]

Hi Tamara,

Will this patch be submitted to 2.2 branch too?

Comment 12

[Tamara Hills [:thills]]

Comment on attachment 8578075 [details] [review]
[gaia] tamarahills:bugfix/1137067-remove-duplicate-ftu-entries > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): remove data duplication
[User impact] if declined: User could wind up paying for extra bytes being transmitted during ftu ping.  This can potentially reduce cost to user that agrees to send data
[Testing completed]: Yes, tested the code and unit tests on 2.2 branch as well
[Risk to taking this patch] (and alternatives if risky):  Low.  
[String changes made]:no

Comment 13

[Tamara Hills [:thills]]

Hi Shing Lyu,

Yes, I have requested the uplift.

Thank you,

-tamara

(In reply to Shing Lyu [:slyu] from comment #11)
> Hi Tamara,
> 
> Will this patch be submitted to 2.2 branch too?

Comment 14

[Ryan VanderMeulen [:RyanVM]]

v2.2: https://github.com/mozilla-b2g/gaia/commit/8938c1175a02cb5442da4fa4f540101dd1481005

Comment 15

[Shing Lyu [:shinglyu]]

Attachment: 2.2 ftu ping payload

Verified for 2.2

Comment 16

[Shing Lyu [:shinglyu]]

The duplication happens in app usage ping too, are we going to remove them in app usage ping too?

Comment 17

[Ravi Dandu]

yes, we should remove duplicates from  app usage info. Reducing packet size for this regularly sent packet gives more value.

Comment 18

[Ravi Dandu]

yes, we should remove duplicates from  app usage info. Reducing packet size for this regularly sent packet gives more value.

Comment 19

[Shing Lyu [:shinglyu]]

OK then, I'll open a new bug for that so we can track it independently.

Comment 20

[Shing Lyu [:shinglyu]]

Verified on 2.2 

{
    "info": {
        "activationTime": 1428476193576,
        "appBuildID": "20150407002501",
        "appName": "FirefoxOS",
        "appUpdateChannel": "default",
        "appVersion": "37.0",
        "devicePixelRatio": 3,
        "deviceinfo.firmware_revision": "",
        "deviceinfo.hardware": "hammerhead",
        "deviceinfo.os": "2.2.0.0-prerelease",
        "deviceinfo.product_model": "AOSP on HammerHead",
        "deviceinfo.software": "Boot2Gecko 2.2.0.0-prerelease",
        "icc": null,
        "locale": "en-US",
        "network": null,
        "pingID": "db47939d-ea2e-4289-a5c5-18fecd944bb9",
        "pingTime": 1428476345214,
        "preinstalled": {
            "https://marketplace.firefox.com/packaged.webapp": "Marketplace",
            "https://moztrap.mozilla.org/owa/manifest.webapp": "MozTrap"
        },
        "reason": "ftu",
        "screenHeight": 640,
        "screenWidth": 360
    },
    "ver": 1
}

Build: 
  Gaia-Rev        ea735c21bfb0d78333213ff0376fce1eac89ead6
  Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/245f37f44017
  Build-ID        20150407002501
  Version         37.0
  Device-Name     hammerhead
  FW-Release      5.0
  FW-Incremental  eng.cltbld.20150407.040951
  FW-Date         Tue Apr  7 04:10:07 EDT 2015
  Bootloader      HHZ12d