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

VERIFIED FIXED

Status

Firefox OS
Metrics
VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: gchang, Assigned: thills)

Tracking

(Blocks: 2 bugs)

unspecified
ARM
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:2.5+, b2g-v2.5 fixed)

Details

(Whiteboard: [ft:conndevices][partner-blocker])

Attachments

(2 attachments, 5 obsolete attachments)

(Reporter)

Description

2 years ago
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
(Reporter)

Comment 1

2 years ago
Created attachment 8678750 [details]
packaged_app_install.log
(Reporter)

Updated

2 years ago
QA Whiteboard: [COM=Telemetry]
This issue can also be reproduced in TV.
Whiteboard: [ft:conndevices]
(Assignee)

Comment 3

2 years ago
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.
Created attachment 8679473 [details] [review]
[gaia] tamarahills:bugfix/1218327-installs-not-counted > mozilla-b2g:master
(Assignee)

Updated

2 years ago
Assignee: nobody → thills
Status: NEW → ASSIGNED
(Assignee)

Comment 5

2 years ago
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
Attachment #8679473 - Flags: review?(marshall)
Attachment #8679473 - Flags: review?(fabrice)
(Assignee)

Updated

2 years ago
Blocks: 1152000
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.
Attachment #8679473 - Flags: review?(fabrice)

Updated

2 years ago
Blocks: 1199319
blocking-b2g: --- → 2.5+
Whiteboard: [ft:conndevices] → [ft:conndevices][partner-blocker]
(Assignee)

Comment 7

2 years ago
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
Flags: needinfo?(rdandu)

Comment 8

2 years ago
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.
Flags: needinfo?(rdandu) → needinfo?(thills)
(Assignee)

Comment 9

2 years ago
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
Flags: needinfo?(thills) → needinfo?(rdandu)
(Assignee)

Comment 10

2 years ago
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
Attachment #8679473 - Flags: review?(fabrice)
Comment on attachment 8679473 [details] [review]
[gaia] tamarahills:bugfix/1218327-installs-not-counted > mozilla-b2g:master

I left a comment on github.
Attachment #8679473 - Flags: review?(fabrice)
(Assignee)

Updated

2 years ago
Attachment #8679473 - Flags: review?(marshall)
(Assignee)

Comment 12

2 years ago
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
Flags: needinfo?(fabrice)
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.
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.
Flags: needinfo?(fabrice)
(Assignee)

Comment 15

2 years ago
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
Attachment #8679473 - Flags: review?(marshall)
Attachment #8679473 - Flags: review?(fabrice)
Attachment #8679473 - Flags: review?(fabrice) → review-
Component: Gaia::System → Metrics

Comment 16

2 years ago
(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.
Flags: needinfo?(rdandu)
(Assignee)

Comment 17

2 years ago
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
Attachment #8679473 - Flags: review- → review?(fabrice)
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
Attachment #8679473 - Flags: review?(marshall) → review+
(Assignee)

Comment 19

2 years ago
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
QA Whiteboard: [COM=Telemetry] → [COM=Telemetry][COM=TV Metrics]
(Assignee)

Comment 20

2 years ago
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
Flags: needinfo?(fabrice)
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', ...).
Flags: needinfo?(fabrice)
(Assignee)

Comment 22

2 years ago
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
Attachment #8679473 - Flags: review?(fabrice) → review+
(Assignee)

Comment 23

2 years ago
https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=94f70908a0e316518a6ba27d2c6ab9db8efa04d8
(Assignee)

Comment 24

2 years ago
https://github.com/mozilla-b2g/gaia/commit/f7157546731eaa7a865b59ab0bf7669b0438580f
(Assignee)

Updated

2 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
(Assignee)

Comment 25

2 years ago
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.
Attachment #8679473 - Flags: approval-gaia-v2.5?
Comment on attachment 8679473 [details] [review]
[gaia] tamarahills:bugfix/1218327-installs-not-counted > mozilla-b2g:master

Approved for 2.5.

Thank you
Attachment #8679473 - Flags: approval-gaia-v2.5? → approval-gaia-v2.5+
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.
Status: RESOLVED → VERIFIED
(Assignee)

Updated

2 years ago
Keywords: checkin-needed
Hi Luke, 
Could you please uplift this patch for TV? Thank you.
Flags: needinfo?(lchang)
Created attachment 8685767 [details] [review]
[gaia] luke-chang:1218327_tv_installs_not_counted > mozilla-b2g:master
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.
Flags: needinfo?(lchang) → needinfo?(ctang)
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.
Flags: needinfo?(ctang) → needinfo?(lchang)
(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 ?
Hi Rex,

I won't be in the office next week. Could you please take over this? Thanks a lot.
Flags: needinfo?(lchang) → needinfo?(rexboy)
removing checkin-needed for comment #32
Keywords: checkin-needed
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.
Flags: needinfo?(cbook)
Keywords: checkin-needed

Updated

2 years ago
Attachment #8685767 - Attachment is obsolete: true
ah ok, Luke could you create a 2.5 PR request that i could merge to 2.5 ? Thanks!
Flags: needinfo?(cbook) → needinfo?(lchang)
Blocks: 1224813
(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
status-b2g-v2.5: --- → fixed
Flags: needinfo?(lchang)
Created attachment 8688353 [details] [review]
[gaia] rexboy7:bug-1224813 > mozilla-b2g:master
Comment on attachment 8688353 [details] [review]
[gaia] rexboy7:bug-1224813 > mozilla-b2g:master

Sorry. please ignore this PR.
Attachment #8688353 - Attachment is obsolete: true
Flags: needinfo?(rexboy)
I'll open a PR on bug 1224813 carrying the same patch for TV side.
Created attachment 8688357 [details] [review]
[gaia] rexboy7:bug-1224813 > mozilla-b2g:master
Created attachment 8688358 [details] [review]
[gaia] rexboy7:bug-1224813 > mozilla-b2g:master
Attachment #8688357 - Attachment is obsolete: true
Attachment #8688358 - Attachment is obsolete: true

Updated

2 years ago
Keywords: checkin-needed
Created attachment 8719391 [details] [review]
[gaia] luke-chang:1218327_tv_installs_not_counted > mozilla-b2g:master

Updated

2 years ago
Attachment #8719391 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.