1218327 - "installs" in payload of app usage metric is not recorded when installing packaged app from Marketplace

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Metrics, defect)

Description

[Gerry Chang [:gchang]]

If installing the packaged app from Marketplace, like Line/ Skate Jumper, we found that the installs is not count in payload. However, if we uninstall the app, the "uninstalls" will be count. This won't happen in hosted apps.

### Steps:
1. Modify the apps/system/js/app_usage_metrics.js and reset gaia
   AUM.REPORT_URL = 'https://incoming-telemetry-mozilla-org-koge918u861h.runscope.net/submit/telemetry';
   AUM.DEBUG = true;
   AUM.REPORT_INTERVAL = 60 * 1000;

2. Search "Line" or "Skate Jumper" packaged app in Marketplace
3. Install the app
4. Wait for more than 1 minute until the data is sent
5. Check the payload of appusage.

### Actual result:
The payload of this app is shown in below. The installs is 0.

"https://marketplace.firefox.com/app/8bcde521-3e5b-4e0c-879d-26eccfa0b607/manifest.webapp": {"20151026": {"usageTime": 59,"invocations": 1,"installs": 0,"uninstalls": 0,"enables": 0,"disables": 0,"activities": {},"addOn": false}}

### Expected result:
The "installs" should be 1.

However, if the app is uninstalled, the "uninstalls" is 1.
"https://marketplace.firefox.com/app/8bcde521-3e5b-4e0c-879d-26eccfa0b607/manifest.webapp": {"20151026": {"usageTime": 7,"invocations": 1,"installs": 0,"uninstalls": 1,"enables": 0,"disables": 0,"activities": {},"addOn": false}},


Build ID               20151025085903
Gaia Revision          2d361e57a349b79d6b459541d316922f7d5b1a2c
Gaia Date              2015-10-26 08:31:16
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/d53a52b39a95dced722cca90ac74529b66dd5253
Gecko Version          44.0a1
Device Name            aries
Firmware(Release)      4.4.2

Comment 1

[Gerry Chang [:gchang]]

Attachment: packaged_app_install.log

Comment 2

[Cynthia Tang [:cynthiatang]]

This issue can also be reproduced in TV.

Comment 3

[Tamara Hills [:thills]]

I can see what the problem is here. The app.manifest is coming up as undefined when the install event is fired.  I will debug some more and try and find out the root cause.

Comment 4

[GitHub Autolander Bot]

Attachment: [gaia] tamarahills:bugfix/1218327-installs-not-counted > mozilla-b2g:master

Comment 5

[Tamara Hills [:thills]]

Comment on attachment 8679473 [details] [review]
[gaia] tamarahills:bugfix/1218327-installs-not-counted > mozilla-b2g:master

Hi Fabrice/Marshall,

I believe what has happened here is when we are installing apps from the marketplace, it looks like app.updateManifest is used instead of app.manifest.  

See this comment here: https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/app_install_manager.js#L227

Also, manifest.role is not a required attribute in the manifest so I guarded against this against exception.

Thanks,

-tamara

Comment 6

[[:fabrice] Fabrice Desré]

For packaged apps, the full manifest is only available when the downloadsuccess event is fired, not when the install event fires. So I think you should rather wait for downloadsuccess, unless you're happy with counting potentially aborted installations in the metrics.

Comment 7

[Tamara Hills [:thills]]

Ravi,

From looking at the code, it looks like we've always been counting canceled installs.

I have a patch which fixes the problem introduced with the addon changes that we are counting NO installs.

Are you ok with this counting canceled installs (as we've been doing) and filing a follow up to fix this?

Thanks,

-tamara

Comment 8

[Ravi Dandu]

Tamara,   
   The plan:  
1) Fix this current problem of not counting any installs for addons
2) A different bug will be filed for differentiating between successful and aborted/cancelled installs. 

The requirements are a) know how many apps/addons were successfully installed
                     b) know how many installations were started, but aborted before completely installing.

Comment 9

[Tamara Hills [:thills]]

Hi Ravi,

Ok, yes, that makes sense. Just to be clear the current problem you list in 1) is that it's not counting any installs for packaged apps from the marketplace.

Based on this, I will flag Fabrice again for review on my current patch. 

Thanks,

-tamara

Comment 10

[Tamara Hills [:thills]]

Comment on attachment 8679473 [details] [review]
[gaia] tamarahills:bugfix/1218327-installs-not-counted > mozilla-b2g:master

Hi Fabrice,

Please see Ravi's comment on what the plan is.

Given that, can you take a look at current patch?

Thanks much,

-tamara

Comment 11

[[:fabrice] Fabrice Desré]

Comment on attachment 8679473 [details] [review]
[gaia] tamarahills:bugfix/1218327-installs-not-counted > mozilla-b2g:master

I left a comment on github.

Comment 12

[Tamara Hills [:thills]]

Hi Fabrice,

One question for you.  Are "gaiamobile.org" apps all packaged apps or are some hosted?  Right now, it looks like we support marketplace apps, certified apps installed on the phone and 'gaiamobile.org' apps.  Just want to clarify because fix will be much simpler if we can always look at downloadsuccess event vs. trying to correlate the two events.

Thanks,
-tamara

Comment 13

[Russ Nicoletti [:russn]]

The way I understand it, to satisfy the first requirement (to collect successful app and addon installs), we should listen to the 'downloaded' event instead of the 'install' event. To satisfy the second requirement we need to listen to both. Initially, for 2.5 RA, I would think we only target the first requirement.

Comment 14

[[:fabrice] Fabrice Desré]

For hosted app, you will only get a "install" event on mozApps.mgmt, so in any case you need to listen to this one. For packaged apps (including addons), the manifest may not be available yet at this point. That's why you have to listen for "downloadsuccess" on the app object to get the manifest. You can also listen for "downloaderror" to track aborted installs.

Comment 15

[Tamara Hills [:thills]]

Comment on attachment 8679473 [details] [review]
[gaia] tamarahills:bugfix/1218327-installs-not-counted > mozilla-b2g:master

Hi Fabrice,

Thanks for your help on this.

This patch is *just* correcting the install count.  The canceled install will be in a separate patch.

Essentially, install event only counts hosted app installs.  The downloadsuccess counts everything else.

I tested and it works fine.

Thanks,

-tamara

Comment 16

[Ravi Dandu]

(In reply to Tamara Hills [:thills] from comment #9)
> Hi Ravi,
> 
> Ok, yes, that makes sense. Just to be clear the current problem you list in
> 1) is that it's not counting any installs for packaged apps from the
> marketplace.
> 
ok. Understood.

Comment 17

[Tamara Hills [:thills]]

Comment on attachment 8679473 [details] [review]
[gaia] tamarahills:bugfix/1218327-installs-not-counted > mozilla-b2g:master

Hi Fabrice,
I took your advice on this and added/removed the event Listener depending on whether the manifest is available.

Thanks,
-tamara

Comment 18

[Marshall Culpepper [:marshall_law]]

Comment on attachment 8679473 [details] [review]
[gaia] tamarahills:bugfix/1218327-installs-not-counted > mozilla-b2g:master

Looks mostly good to me, but I'll defer to Fabrice's review for behavior here. I left one comment about the MockApp's new addEventListener, but it isn't a blocker IMO

Comment 19

[Tamara Hills [:thills]]

Comment on attachment 8679473 [details] [review]
[gaia] tamarahills:bugfix/1218327-installs-not-counted > mozilla-b2g:master

Hi Fabrice,

Updated to use the app.dispatch vs. window.

Thanks,
-tamara

Comment 20

[Tamara Hills [:thills]]

Hi Fabrice,

When I change to not rebroadcast the ondownloadsuccess event inside of app_install_manager.js, I get the event inside of app_usage_metrics.js using |app.ondownloadsuccess = function()...|.  However, I noticed that this masks the event that is received in app_install_manager.js.  

I also tried just |app.addEventListener('ondownloadsuccess'...| inside of app_usage_metrics.js.  This does not receive the event.

I'm probably missing something really basic.

Thanks,

-tamara

Comment 21

[[:fabrice] Fabrice Desré]

Tamara, you need to use app.addEventListener('downloadsuccess', ...) in app_usage_metrics.js
In general we have obj.onXXX and the matching obj.addEventListener('XXX', ...).

Comment 22

[Tamara Hills [:thills]]

Comment on attachment 8679473 [details] [review]
[gaia] tamarahills:bugfix/1218327-installs-not-counted > mozilla-b2g:master

Thanks Fabrice,

This fixed it.  I pushed a new version of the patch.

-tamara

Comment 23

[Tamara Hills [:thills]]

https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=94f70908a0e316518a6ba27d2c6ab9db8efa04d8

Comment 24

[Tamara Hills [:thills]]

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

Comment 25

[Tamara Hills [:thills]]

Comment on attachment 8679473 [details] [review]
[gaia] tamarahills:bugfix/1218327-installs-not-counted > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Adding of feature for tracking addons
[User impact] if declined: We will not be able to track installs in AppUsage Metrics
[Testing completed]: Local testing completed.
[Risk to taking this patch] (and alternatives if risky):  Relatively low.  It's a rather small change isolated to the Telemetry files.  
[String changes made]: None.

Comment 26

[Mahendra Potharaju [:mahe]]

Comment on attachment 8679473 [details] [review]
[gaia] tamarahills:bugfix/1218327-installs-not-counted > mozilla-b2g:master

Approved for 2.5.

Thank you

Comment 27

[Iris Hsiao [:ihsiao]]

Verified on
Build ID               20151110023426
Gaia Revision          3180bbe2f2e94809c1fcef0e92a01da99cdfb530
Gaia Date              2015-11-10 03:31:51
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/4a7526d26bd47ce2e01f938702b91c95424026ed
Gecko Version          45.0a1
Device Name            aries
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.worker.20151110.015538
Firmware Date          Tue Nov 10 01:55:46 UTC 2015
Bootloader             s1

###Verify result###
"installs" in payload of app usage metric will be recorded when installing packaged app from Marketplace.

Comment 28

[Cynthia Tang [:cynthiatang]]

Hi Luke, 
Could you please uplift this patch for TV? Thank you.

Comment 29

[GitHub Autolander Bot]

Attachment: [gaia] luke-chang:1218327_tv_installs_not_counted > mozilla-b2g:master

Comment 30

[Luke Chang [:lchang] (inactive)]

Comment on attachment 8685767 [details] [review]
[gaia] luke-chang:1218327_tv_installs_not_counted > mozilla-b2g:master

Hi Cynthia,

I've uploaded the patch for porting it to TV. Could you please help verifying it before landing? Thanks.

Comment 31

[Cynthia Tang [:cynthiatang]]

Hi Luke,
The installs is still not count in payload. 

"https://marketplace.firefox.com/app/7e382154-a942-4e45-ae0a-fc6d82be9a0f/manifest.webapp": {
    "20151111": {
    "usageTime": 4,
    "invocations": 1,
    "installs": 0,
    "uninstalls": 0,
    "enables": 0,
    "disables": 0,
    "activities": {},
    "addOn": false
  }
},


(In reply to Luke Chang [:lchang] from comment #30)
> Comment on attachment 8685767 [details] [review]
> [gaia] luke-chang:1218327_tv_installs_not_counted > mozilla-b2g:master
> 
> Hi Cynthia,
> 
> I've uploaded the patch for porting it to TV. Could you please help
> verifying it before landing? Thanks.

Comment 32

[Carsten Book [:Tomcat]]

(In reply to Luke Chang [:lchang] from comment #30)
> Comment on attachment 8685767 [details] [review]
> [gaia] luke-chang:1218327_tv_installs_not_counted > mozilla-b2g:master
> 
> Hi Cynthia,
> 
> I've uploaded the patch for porting it to TV. Could you please help
> verifying it before landing? Thanks.

does this means we should hold the PR checkins back till this is verified ?

Comment 33

[Luke Chang [:lchang] (inactive)]

Hi Rex,

I won't be in the office next week. Could you please take over this? Thanks a lot.

Comment 34

[Carsten Book [:Tomcat]]

removing checkin-needed for comment #32

Comment 35

[Luke Chang [:lchang] (inactive)]

Hi Carsten,

My patch (attachment 8685767 [details] [review]) is actually written for TV build on master branch. What needs to be uplifted to v2.5 for now is attachment 8679473 [details] [review]. It has been verified in comment 27 and I think it's ready to uplift.

Rex will help to file another bug for porting this patch to TV build. Sorry about the confusion.

Comment 36

[Carsten Book [:Tomcat]]

ah ok, Luke could you create a 2.5 PR request that i could merge to 2.5 ? Thanks!

Comment 37

[Carsten Book [:Tomcat]]

(In reply to Carsten Book [:Tomcat] from comment #36)
> ah ok, Luke could you create a 2.5 PR request that i could merge to 2.5 ?
> Thanks!

never mind figured it out :) landed as https://github.com/mozilla-b2g/gaia/commit/9473dbcbebf4e758a3b73200968efc69071b4312

Comment 38

[GitHub Autolander Bot]

Attachment: [gaia] rexboy7:bug-1224813 > mozilla-b2g:master

Comment 39

[KM Lee [:rexboy] (inactivate after Sep. 22)]

Comment on attachment 8688353 [details] [review]
[gaia] rexboy7:bug-1224813 > mozilla-b2g:master

Sorry. please ignore this PR.

Comment 40

[KM Lee [:rexboy] (inactivate after Sep. 22)]

I'll open a PR on bug 1224813 carrying the same patch for TV side.

Comment 41

[GitHub Autolander Bot]

Attachment: [gaia] rexboy7:bug-1224813 > mozilla-b2g:master

Comment 42

[GitHub Autolander Bot]

Attachment: [gaia] rexboy7:bug-1224813 > mozilla-b2g:master

Comment 43

[GitHub Autolander Bot]

Attachment: [gaia] luke-chang:1218327_tv_installs_not_counted > mozilla-b2g:master