Closed Bug 1137067 Opened 5 years ago Closed 5 years ago

[Metrics] Duplicate entries in the FTU ping

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:2.2+, b2g-v2.2 fixed, b2g-master fixed)

RESOLVED FIXED
2.2 S8 (20mar)
blocking-b2g 2.2+
Tracking Status
b2g-v2.2 --- fixed
b2g-master --- fixed

People

(Reporter: shinglyu, Assigned: thills)

References

Details

Attachments

(3 files)

*** 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
Flags: needinfo?(rdandu)
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.
Flags: needinfo?(rdandu)
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?
Flags: needinfo?(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
Flags: needinfo?(thills)
Assignee: nobody → thills
Status: NEW → ASSIGNED
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
Attachment #8577300 - Flags: feedback?(marshall)
(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 on attachment 8577300 [details] [diff] [review]
ftuduplicatedata.diff

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

This looks good!
Attachment #8577300 - Flags: feedback?(marshall) → feedback+
Attachment #8578075 - Flags: review?(marshall)
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
Attachment #8578075 - Flags: review?(marshall) → review+
https://github.com/mozilla-b2g/gaia/commit/b4066c9668fbc69af468c4f625d7bf0ee8607280
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Hi Tamara,

Will this patch be submitted to 2.2 branch too?
Flags: needinfo?(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
Flags: needinfo?(thills)
Attachment #8578075 - Flags: approval-gaia-v2.2?(bbajaj)
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?
blocking-b2g: --- → 2.2+
Attachment #8578075 - Flags: approval-gaia-v2.2?(bbajaj) → approval-gaia-v2.2+
Attached file 2.2 ftu ping payload
Verified for 2.2
The duplication happens in app usage ping too, are we going to remove them in app usage ping too?
Flags: needinfo?(rdandu)
yes, we should remove duplicates from  app usage info. Reducing packet size for this regularly sent packet gives more value.
Flags: needinfo?(rdandu)
yes, we should remove duplicates from  app usage info. Reducing packet size for this regularly sent packet gives more value.
OK then, I'll open a new bug for that so we can track it independently.
See Also: → 1148305
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
You need to log in before you can comment on or make changes to this bug.