982663 - App Usage metrics collection on FxOS (Phase 2 of metrics project)

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia, defect)

Description

[Ravi Dandu]

Collection of Metrics Phase 2 for Firefox OS is needed to security patches and bugs to improve user experience:
In this phase Reporting of info at a regular interval
1) System and App crashes
2) Apps usage (length of time each app is used)
3) System and App errors

Comment 1

[David Flanagan [:djf]]

Attachment: Marshall's WIP patch

This is Marshall's work-in-progress patch. I will see if I can move this forward while he is OOO.

Comment 2

[David Flanagan [:djf]]

I've added review comments to the patch, and will use those as the basis for my work on it.

Comment 3

[David Flanagan [:djf]]

I've left a bunch of comments on github that are sort of review comments and sort of a way to just organize my own thoughts about this feature.

Before we can land this, I'd like to see at least these changes:

1) a modification to the settings app that enables the user to disable data collection and transmission. Also, do we need to integrate this with the telemetry opt-in in the FTU app?  Do we need to distinguish this kind of app usage telemetry from the existing telemetry?  If the user does not opt-in to telemetry, do we send this information?

2) reporting of crashes hooked up.

3) verification that the code does not report time while an app is open but the phone is alseep. That is, when the phone sleeps or the user locks the phone, we need to treat that as an app closing. (Possibly, however, it should not increment the closed count but should just agregate the time.)

4) Use existing system app code (if there is any) for querying multiple settings values

5) A cleanup of the retry logic so that if data submission fails we just keep collecting data and send it later. We shouldn't be collecting a second batch of data while we're still waiting for a previous batch to send. 

6) Some mechanism for giving up if we are never able to send the data. Some users will not have wifi access and will never turn on data and will be content to use the phone as a phone or to install apps by sdcard.  We don't want to collect years worth of usage data in that case. Perhaps if we are unable to transmit any data for a week (or a month?) then we either discard the data and start a new batch, or better, we just give up and disable the feature.

7) We need to decide whether we want to include a unique id (a random # generated on first run) with our data so that our daily batches of app usage data can be tied together. Is that okay, or would it be too invasive. Who can we ask about this?

8) I want to avoid persisting our data to async storage each time an app is launched because that will make the system app and db busy at exactly the time that the app is trying to run. It will probably cause launch time regressions. I think it is sufficient to only update the data structure and persist it when an app closes or crashes.  When an app opens we just need to record the start time and wait for it to close.

9) The timestamps associated with each report should probably be the start and end of the reporting period, not the first and last timestamps contained in the report.

10) I think we should include the user's language along with the device information in each report.

11) I think there is a bug in the existing code where if the service shuts down it can not restart. We'll need to fix that.

I had a day full of meetings and blocking bug code reviews, so am out of time to actually write any code for this feature today. I'm happy to continue to work on it on Monday, however.  Marshall, if you're back on Monday, please let me know if you'd like me to start coding up some of my ideas here.

Comment 4

[David Flanagan [:djf]]

I just read mozilla.org/telemetry and found this quote:

  Telemetry does not collect any bookmarks, browsing history or passwords. 

It seems to me that the app usage data we're collecting here could be considered a bookmark or browsing history. So what we're collecting here is more invasive than the telemetry information we send. If telemetry is off by default, then I think this feature should be off by default.  But in that case we'll never get anyone to use it.

Setting needinfo for Adam and Will. Guys: do either of you know about these metrics? Have the privacy implications been hashed out? Is this opt-in or opt-out, and is it tied to the existing telemetry opt-in?  Do we have a coherent explanation for why collecting app usage data is not like collecting bookmarks or browsing history?

Comment 5

[David Flanagan [:djf]]

Hmm. Looks like John is probably the right person to ask about Metrics. John, do you have answers to the questions in comment #4?

Comment 6

[David Flanagan [:djf]]

Marshall,

As I recall, you said that you'd really like to get some integration tests written for this feature. That's actually something that I have little experience with. And I have some ideas about how I'd like to tackle a lot of the items in my list in comment #3. So how about I take a crack at refactoring metrics.js and, when you're back you work on the integration tests. Does that sounds like a good way to divide this up?

Comment 7

[Marshall Culpepper [:marshall_law]]

(In reply to David Flanagan [:djf] from comment #3)
> I've left a bunch of comments on github that are sort of review comments and
> sort of a way to just organize my own thoughts about this feature.

I agree with mostly everything here, just filling in some extra comments below

 
> 1) a modification to the settings app that enables the user to disable data
> collection and transmission. Also, do we need to integrate this with the
> telemetry opt-in in the FTU app?  Do we need to distinguish this kind of app
> usage telemetry from the existing telemetry?  If the user does not opt-in to
> telemetry, do we send this information?

Ravi was in the process of getting sign off for any UX changes necessary, but I can't recall where we dropped off on that. Flagging him..

> 
> 4) Use existing system app code (if there is any) for querying multiple
> settings values

I haven't been able to find any common code for getting a group of setting values, but I did write a utility function for this in ftu_ping.js called 'getSettings'. We could move that to an existing utility namespace or class..


> 5) A cleanup of the retry logic so that if data submission fails we just
> keep collecting data and send it later. We shouldn't be collecting a second
> batch of data while we're still waiting for a previous batch to send. 

This is perfectly fine, and in retrospect obvious :)

> 6) Some mechanism for giving up if we are never able to send the data. Some
> users will not have wifi access and will never turn on data and will be
> content to use the phone as a phone or to install apps by sdcard.  We don't
> want to collect years worth of usage data in that case. Perhaps if we are
> unable to transmit any data for a week (or a month?) then we either discard
> the data and start a new batch, or better, we just give up and disable the
> feature.

Agreed, maybe the cutoff could just be 1 week (with the default value of daily submissions)
 
> 7) We need to decide whether we want to include a unique id (a random #
> generated on first run) with our data so that our daily batches of app usage
> data can be tied together. Is that okay, or would it be too invasive. Who
> can we ask about this?

We do generate a UUID for FTU (first time use) pings, and the server keeps them around for 24 hours before wiping them. I was thinking we'd probably only report app usage in 1 daily batch, how do you think the UUID would be associated? I'm definitely considered about the privacy implications..
 
> 8) I want to avoid persisting our data to async storage each time an app is
> launched because that will make the system app and db busy at exactly the
> time that the app is trying to run. It will probably cause launch time
> regressions. I think it is sufficient to only update the data structure and
> persist it when an app closes or crashes.  When an app opens we just need to
> record the start time and wait for it to close.

I agree with your concern, but a few other things that I'm worried about with this approach:
- In the event of a system app crash, we will lose any data about any app that was started before it was closed. That might be an acceptable trade off..

- In the case of a new device, the system clock can be set to epoch+0 (1970) until it is updated with NTP. In my experience from FTU ping, this can actually cause two timestamps between those events to be erratically different (currently, about 44 years different)

- I'm already posting the async storage access off until the next loop tick, but we can probably get a little more conservative and post the job that writes to a specified interval after app startup, say 5 seconds.

 
> 10) I think we should include the user's language along with the device
> information in each report.

Sounds good, we are also including this in the FTU ping

Comment 8

[David Flanagan [:djf]]

I think we can collect all the data we need by listening to events. This will hopefully be more stable and reliable than instrumenting the window management code...

We get "appopened" (with e.detail.origin) when the user switches to a new app from the homescreen or task switcher or with edge gestures.

We get "homescreenopened" (with e.detail.origin) when the user switches to the homescreen.

We get "lockscreen-appopened" (e.detail.origin) when we switch to the lockscreen.

We get "lockscreen-appclosed" when the lockscreen is unlocked. We don't get appopened or homescreenopened in this case, so will have to remember which app is current when that happens.

We get "screenchange" (e.detail.screenEnabled) when the phone sleeps or wakes

We get applicationuninstall when an app is uninstalled (though there is probably a more definitive event from the mozApps API as well.)

In addition to these app events, we can use "online" and "offline" events to tell when we have a network connection and we can use navigator.addIdleObserver to tell when the user is idle. We can use those events to help us decide when to transmit our data.  I think we don't want to use timers, but when we get events, we want to see if it has been long enough that we need to transmit the current batch of data. If it has been long enough, and if we are online and if the user is idle, then we should send a batch.

Comment 9

[John Jensen]

(In reply to David Flanagan [:djf] from comment #4)

[adding Jishnu]

Ravi and Jishnu know more than I do, but my understanding of the proposal is that users would be prompted at the device activation stage as to whether or not they want to send information to Mozilla ("opt-in"). The wording of any existing prompts would likely have to be adjusted to cover the larger set of information collected.

Comment 10

[Marshall Culpepper [:marshall_law]]

David and I are moving forward with an event based redesign, clearing my flag

Comment 11

[David Flanagan [:djf]]

Attachment: link to patch on github

Alive: this is a preliminary version of a patch to collect app usage metrics. Product really wants this to be uplifted to 2.0 so we're trying to finish it up and get it landed quickly.

The patch relies on events from the window manager, so I'm asking for your feedback on this implementation to make sure, for example, that I am not depending on events that you plan to remove or deprecate.

Comment 12

[David Flanagan [:djf]]

Tim: it looks like Alive is OOO, so I'm wondering if you could take or delegate this feedback request. Is there anyone on your team who should review this metrics bug before landing, or are you okay with reviews going through Fabrice and/or Etienne, who (I think) have been involved in similar patches for ftu_ping.js

Comment 13

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8446733 [details] [review]
link to patch on github

I cannot give a feedback+ because the script is entirely written against what have been agreed on in System app. Even though it looks fine and functional now I must block this from landing so system app owners will not paying the prices of maintaining this in the long run.

I also against ANY late feature. I am very interested to know who is above project/product planning and is allowed to push this feature to 2.0, and have you spend time working on this feature in the stabilization phase. That said, based on the script I am seeing right now, I don't see huge risk for this feature to cause any regression on 2.0 -- the worse thing could happen is probably it breaks itself, since it does not call anything other system modules.

Comment 14

[David Flanagan [:djf]]

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #13)
> Comment on attachment 8446733 [details] [review]
> link to patch on github
> 
> I cannot give a feedback+ because the script is entirely written against
> what have been agreed on in System app. Even though it looks fine and
> functional now I must block this from landing so system app owners will not
> paying the prices of maintaining this in the long run.

When you say "entirely written against what has been agreed" you just mean that you want me to export a constructor rather than an init() function, right?  What's the proper convention for uninit()? Can I make that an instance method named destroy()?  Are you okay with me storing constants within an anonymous scope, or are you going to insist that everything be accessible through the constructor?

Also, I'd note that the "have been agreed" must be a recent or weak kind of agreement because as recently as March, the ftu_ping.js code was written in the old "export an object" style.
 
> I also against ANY late feature. I am very interested to know who is above
> project/product planning and is allowed to push this feature to 2.0, and
> have you spend time working on this feature in the stabilization phase. 

Yes, we are all against late features. In this case, it is Chris Lee who is very interested in getting this uplifted to 2.0 if at all possible.

That
> said, based on the script I am seeing right now, I don't see huge risk for
> this feature to cause any regression on 2.0 -- the worse thing could happen
> is probably it breaks itself, since it does not call anything other system
> modules.

Thanks for this. How thoroughly did you look this over? From some of your comments on github it looks like you basically did a review rather than just a feedback pass. In particular are you satisfied with the window manager events I'm listening to here?

Comment 15

[Marshall Culpepper [:marshall_law]]

Attachment: Initial integration tests

This needs to be updated to work with David's code once his refactor is done, but currently this works for everything except uninstallation on my branch.

Comment 16

[David Flanagan [:djf]]

Comment on attachment 8447367 [details] [diff] [review]
Initial integration tests

This looks good to me, though I have to confess that I haven't written a marionette test in something like 9 months, so I barely understand what is going on.

I'm guessing that you're going to need a way to get at the AppUsageMetrics instance in order to make this test work. If so, let's move the constructor invocation to bootstrap.js and assign the instance to a global variable there. 

If there is an easy way in marionette to switch apps with an edge gesture or with the task switcher, it would be good to test that those are recorded correctly.

You might check that homescreen time and lockscreen time are both recorded as well.

And if you can get an app to launch an activity, you could check that the activity is recorded. Maybe there is an existing test that selects wallpaper from the homescreen that you could copy activity launching code from.

Comment 17

[David Flanagan [:djf]]

Status update: this patch is close to done.  Marshall and I are working on tests, but the metrics gathering and reporting code is done.

Before we can land this we need to:

1) finish tests
2) update strings in the FTU and Settings app to indicate that the telemetry is not just performance data but performance and usage data
3) get technical review (from Tim or perhaps Etienne)
4) get UI review for the string changes. I'm not sure who handles the UX of the FTU and Settings apps.

We also need to get signoff from the legal and privacy teams, but I don't know if we need to wait for that before landing on master.

Comment 18

[David Flanagan [:djf]]

I have finished the unit tests for this.

Comment 19

[David Flanagan [:djf]]

Comment on attachment 8446733 [details] [review]
link to patch on github

Tim,

Since you've already given feedback on this patch, I wanted to offer the review to you, if you've got time.  (Marshall has suggested I ask Etienne, however, so if you are not able to do it I could ask him instead.)

I still need to make a couple of string changes in FTU and settings, but the system app part of the patch should be done now, refactored as you requested, and there is a long set of unit tests.

Comment 20

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8446733 [details] [review]
link to patch on github

Please find Etienne for review since the timezone difference might prevent this patch from being iterated at the fastest possible speed.

(I will reply the needinfo in the next comment)

Comment 21

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to David Flanagan [:djf] from comment #14)
> (In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from
> comment #13)
> > Comment on attachment 8446733 [details] [review]
> > link to patch on github
> > 
> > I cannot give a feedback+ because the script is entirely written against
> > what have been agreed on in System app. Even though it looks fine and
> > functional now I must block this from landing so system app owners will not
> > paying the prices of maintaining this in the long run.
> 
> When you say "entirely written against what has been agreed" you just mean
> that you want me to export a constructor rather than an init() function,
> right?  

Yes and no. The constructor should avoid creating any side effect (by start "running" directly) and allow users of the instance to attach it to the right place and overwrite properties before it actually starts. A example was written back then.

https://wiki.mozilla.org/Gaia/System/Refactoring_Plan#Stage_1_example

> What's the proper convention for uninit()? Can I make that an
> instance method named destroy()? 

|start()| and |stop()| is probably the best naming.

> Are you okay with me storing constants
> within an anonymous scope, or are you going to insist that everything be
> accessible through the constructor?

I would like to see everything as a property of the prototype object, to prevent one instance from messing up with another. (Maybe const and Object.freeze() can prevent that?)

> Also, I'd note that the "have been agreed" must be a recent or weak kind of
> agreement because as recently as March, the ftu_ping.js code was written in
> the old "export an object" style.

That was ... a mistake. We did not make enough effort to communicate this rule to everyone r+'ing system app code. I will certainly try to communicate that again since you pointed out that. Another problem is that many of us are also responsible of product feature/scheduling so conflicting identities sometimes takes precedence.

Also, while we try to enforce the pattern convention, we also try not ask people to rewrite what has been written just for the sake of that. Since you have already complete the patch and this is a late feature, I think that applies to you to.

(Please, Please schedule some time after 2.0 to make this module comply with the pattern we try to convert to afterwards.)

> > I also against ANY late feature. I am very interested to know who is above
> > project/product planning and is allowed to push this feature to 2.0, and
> > have you spend time working on this feature in the stabilization phase. 
> 
> Yes, we are all against late features. In this case, it is Chris Lee who is
> very interested in getting this uplifted to 2.0 if at all possible.
> 
> > That said, based on the script I am seeing right now, I don't see huge risk for
> > this feature to cause any regression on 2.0 -- the worse thing could happen
> > is probably it breaks itself, since it does not call anything other system
> > modules.
> 
> Thanks for this. How thoroughly did you look this over? From some of your
> comments on github it looks like you basically did a review rather than just
> a feedback pass. In particular are you satisfied with the window manager
> events I'm listening to here?

I do not know if the events you are listen to does indeed means the data you can to collect.

Maybe Alive should feedback? the patch?

Comment 22

[David Flanagan [:djf]]

Comment on attachment 8446733 [details] [review]
link to patch on github

Alive,

I'm requesting your feedback on this patch mostly to let you know that this metrics collection module depends on events (like appopened and homescreeenopened) from the window manager. If any of these events are things that you are planning to change soon, please let us know.

Comment 23

[David Flanagan [:djf]]

I've updated the pull request to include string changes in the Settings and FTU apps to disclose app usage data collection as part of the Telemetry opt-in.

Comment 24

[David Flanagan [:djf]]

Attachment: screenshot of FTU app before string change

Comment 25

[David Flanagan [:djf]]

Attachment: screenshot of FTU app after string change

Comment 26

[David Flanagan [:djf]]

Attachment: screeshot of FTU "Learn more" screen before string change

Comment 27

[David Flanagan [:djf]]

Attachment: screeshot of FTU "Learn more" screen after string change

Comment 28

[David Flanagan [:djf]]

Attachment: screenshot of Settings app before string change

Comment 29

[David Flanagan [:djf]]

Attachment: screenshot of Settings app after string change

Stephany,

I'm not sure which UX designer(s) is/are in charge of the FTU and Settings apps, so I'm flagging you for the UX review of these 6 screenshots hoping that you will pass the request on as appropriate.

Comment 30

[Etienne Segonzac (:etienne)]

Comment on attachment 8446733 [details] [review]
link to patch on github

What a exquisite read :)
I made some minor comments on github but Tim clearly did most of the review.

The events looks ok, I'm keeping the f? on Alive to double check, also we might want to look into the appcrashed event (maybe a follow up).

Now, I don't want this to turn into bikesheding since it was previously discussed, but if you look at the bootstrap.js change the AppUsageMetrics module kinda stands out :/

Ideally, for the sake of consistency:
- start() would be renamed startCollecting()
- stop() would be renamed stopCollecting()

- setting the metricsEnabledListener would move to a .start() method (returning |this|)
- destroy() would be renamed stop()


But I can settle for the first 2 :)

Comment 31

[David Flanagan [:djf]]

Review comments addressed, commits squashed, PR updated.
Etienne: you didn't set r+ or r- on this. I've made the changes you requested, so I guess I'm going to assume that you meant to give a conditional r+ here...

Comment 32

[David Flanagan [:djf]]

In comment #3 I had a list of goals for this patch. I think the code here addresses all of them except:

2) we already have crash reporting for real crashes. It would be nice to be able to report things like OOMs with these app usage metrics, but I can't find a way to do that, so for now this patch does no crash reporting at all.  It might be possible to at least add reporting of the hard crashes that are already reported by the crash reporter.

6) I punted on this one. If the user opts in to telemetry and then never ever goes online, we'll collect data and never send it. I'm guessing this will be rare. When I wrote that goal I was imagining the size of the collected data growing to take up more and more storage room. But that is unlikely since our data collection consists mostly of incrementing numbers. Also, there is code now so that we don't even attempt to send data when offline.

Comment 33

[David Flanagan [:djf]]

This feature was originally targeted for the 2.0 release, and I know that the product team would still really like to get it into that release. So I expect that once this lands, the patch will be nominated for uplift to 2.0.

In order to do that uplift, we will need to:

1) Let it sit for a few days on master to verify that we are successfully getting useful data reported to the servers.

2) Sort out the late l10n issues involved in the Settings and FTU string changes

3) Modify the patch and the unit tests to use old-style lockscreen events instead of new style lockscreen events.

4) Modify the integration tests to work with the 2.0 homescreen instead of the 2.1 homescreen

5) Edit mozilla.org/telemetry again to change the version number from 2.1 to 2.0

Comment 34

[Jason Smith [:jsmith]]

(In reply to David Flanagan [:djf] from comment #33)
> This feature was originally targeted for the 2.0 release, and I know that
> the product team would still really like to get it into that release. So I
> expect that once this lands, the patch will be nominated for uplift to 2.0.

I don't think this should be considered for 2.0 at this point. We're string frozen right now & no longer going to accept features into the 2.0 release unless they are part of the stop ship 2.0 feature list (visual refresh, vertical homescreen, loop).

Comment 35

[David Flanagan [:djf]]

Filed bug 1032485 to track the necessary update to mozilla.org/telemetry

Comment 36

[David Flanagan [:djf]]

Filed bug 1032494 for legal review

Comment 37

[David Flanagan [:djf]]

Looks like bug 1021259 is the correct already filed legal bug.

Comment 38

[David Flanagan [:djf]]

Comment on attachment 8448088 [details]
screenshot of Settings app after string change

Rob,

Ravi tells me that Jaime told him that you are the UX point person for this feature, so I'm asking you to review the wording changes in the screenshots here. There are 6 screenshots here. Three befores and three afters.  (One of the screenshots says B2G OS instead of FirefoxOS but that is just because I created that screenshot on a developer build instead of a production build.)

Basically, we're using the existing telemetry opt-in checkboxes in FTU and Settings apps to also opt in to app usage data collection, so instead of just saying "performance data" we now need to say "performance and app usage data".

Comment 39

[David Flanagan [:djf]]

I've gotten verbal sign off from legal to land this. Presumably bug 1021259 will be updated to reflect that.

Comment 40

[David Flanagan [:djf]]

Handing the bug back to Marshall now to land once he's done with integration tests and we have r+ and ux-review+

Comment 41

[Rob MacDonald [:robmac]]

Comment on attachment 8448088 [details]
screenshot of Settings app after string change

Flagging Francis as I'm out until Wednesday, July 2.

Comment 42

[David Flanagan [:djf]]

Jason,

I hear from Ravi that you are the QA contact for this feature. If so, I'd be happy to answer any questions you have about how to test it.  (But I'll be on PTO from soon, so Marshall is probably the better person to talk to.)

To me, the key thing to test here is that we're only collecting data when the user has opted in. The module has good debugging output.  If you do adb logcat | grep AppUsage you'll see messages each time you switch from one application to another.  And if you turn telemetry on or off, you should see messages about data collection starting up and shutting down.  There are unit tests to cover this stuff, and Marshall is working on integration tests as well, but presumably QA will want to check this feature out as well.

Comment 43

[Jason Smith [:jsmith]]

Sounds good. (In reply to David Flanagan [:djf] from comment #42)
> Jason,
> 
> I hear from Ravi that you are the QA contact for this feature. If so, I'd be
> happy to answer any questions you have about how to test it.  (But I'll be
> on PTO from soon, so Marshall is probably the better person to talk to.)

Yup, that's right. I'll be testing this.

> 
> To me, the key thing to test here is that we're only collecting data when
> the user has opted in. The module has good debugging output.  If you do adb
> logcat | grep AppUsage you'll see messages each time you switch from one
> application to another.  And if you turn telemetry on or off, you should see
> messages about data collection starting up and shutting down.  There are
> unit tests to cover this stuff, and Marshall is working on integration tests
> as well, but presumably QA will want to check this feature out as well.

I'm thinking I'll likely use a similar test methodology I used with the FTU ping here. What I did there was I pointed Gaia to my own hosted test server (https://github.com/jds2501/mockmetricsserver) & analyze the output based on actions I take on the client side. The main question I have is that I need to understand what the client side is expecting the server to respond with in any of the ping interactions involved here, as I will need to make sure my server does similar behavior. The other question I have is I'd like to understand the JSON structure used in the ping requests & what each data point means (i.e. something similar to what's written up here - https://wiki.mozilla.org/FirefoxOS/Metrics#FTU_ping, but for the metrics ping here instead).

Comment 44

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8446733 [details] [review]
link to patch on github

I didn't read the full code since I am not asked to do so,
but I have the same opinion as Tim:

If everyone is rushing and dumping stuff into system app as PM requested, system app will become something nobody could maintain.

BTW after reading app_usage_metrics.js I think the infomation should be calculated in a sub module of app_window therefore you don't need to listen to the events on your own. All metric should be only done when
* The app is in foreground state
* The app has no front window (inline activity or popup)
* There's no higher priority UI upon current active app window

I suspect use those events are not correct:
What if an app is using window.open to open another page? Will you stop recording the caller?
What if the user pull down the utility tray to read notification? Will you stop recording the current app?
What if the call comes and overlay the current app?

Comment 45

[Marshall Culpepper [:marshall_law]]

Attachment: Patch w/ tests

This is the new pull request including my integration tests, and updates to Dave's implementation to use ftu.pingURL as the basis for the reporting URL w/ unit tests.

Tagging Jason for feedback because of his github review comment, and Etienne for review again just because it wasn't clear if the original patch (which my commit is based on) was given r+..

Comment 46

[Etienne Segonzac (:etienne)]

Comment on attachment 8448587 [details] [review]
Patch w/ tests

Impressive work guys!
r=me for the system part, you might want to ask for a formal stamp of approval from a settings and ftu peer.

I'm not aware of the back story of this feature but I don't feel like this is being "dumped" on the system app. Having telemetry be a a completely separate module only consuming events sounds about right.
It's always worrying to see this amount of code land, but I'm confident the thoughtful automated testing work that went into this patch will greatly mitigate any maintenance issue.

We should keep our eyes open regarding potential performance issues before uplifting, but the work done on 'appopened' is kept to a minimum so we should be alright.

And yes, we'll need to follow up regarding sheets since you can get 2 or more consecutive appopened for the same app but I don't think it's blocking.

Comment 47

[Jason Smith [:jsmith]]

Comment on attachment 8448587 [details] [review]
Patch w/ tests

Looks okay to me. A nit I would suggest is to consider renaming the variable ftu.pingURL to just pingURL, since we are now using it across multiple different apps for different purposes. That's not a blocker to landing this though.

Comment 48

[David Flanagan [:djf]]

(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #44)
> Comment on attachment 8446733 [details] [review]
> link to patch on github
> 
> If everyone is rushing and dumping stuff into system app as PM requested,
> system app will become something nobody could maintain.

Are you suggesting that there is somewhere other than the system app that this feature could be implemented in?

> 
> BTW after reading app_usage_metrics.js I think the infomation should be
> calculated in a sub module of app_window therefore you don't need to listen
> to the events on your own. 

Given the rush to implement this, I thought a decoupled event-based implementation would be safer, and would be substantially easier to get reviewed since it does not need to insert any code into the window manager.

All metric should be only done when
> * The app is in foreground state
> * The app has no front window (inline activity or popup)
> * There's no higher priority UI upon current active app window
> 

> I suspect use those events are not correct:
> What if an app is using window.open to open another page? Will you stop
> recording the caller?

I didn't test this case. Does window.open cause another appopened event? 

> What if the user pull down the utility tray to read notification? Will you
> stop recording the current app?

Ideally, we could record the amount of time the user spends reading notifications. But that doesn't seem like a high priority to me.  I think it is fine to record time to the underlying app while the notification tray is down.

> What if the call comes and overlay the current app?

I don't understand the incoming call architecture. Doesn't an app get launched when the user answers a call? I didn't actually test this case. I think that we would like to record time the user spends on the phone, so if time on an incoming call is being accounted for as time in an overlaid app, then that seems worth fixing to me.

Alive: do you have pointers for events we should be listening to to correctly handle incoming calls?

Comment 49

[Marshall Culpepper [:marshall_law]]

Rebased against latest master and pushed to try and address some of the (seemingly) unrelated failures in Travis

Comment 50

[Marshall Culpepper [:marshall_law]]

Comment on attachment 8448587 [details] [review]
Patch w/ tests

Requesting feedback of the FTU code from Francisco, and Settings code from Vivien.. let me know if someone else would be better

Comment 51

[Marshall Culpepper [:marshall_law]]

Try build is also going:
https://tbpl.mozilla.org/?tree=Try&rev=2ad99dd9cca9

Comment 52

[David Flanagan [:djf]]

Forgot to set needinfo for Alive. Alive, please see comment #48: can you offer suggestions on how we can properly record time spent on incoming calls? That seems like an important enough use-case that we should file a followup bug for it.

Comment 53

[David Flanagan [:djf]]

(In reply to Jason Smith [:jsmith] from comment #43)

> I'm thinking I'll likely use a similar test methodology I used with the FTU
> ping here. What I did there was I pointed Gaia to my own hosted test server
> (https://github.com/jds2501/mockmetricsserver) & analyze the output based on
> actions I take on the client side. The main question I have is that I need
> to understand what the client side is expecting the server to respond with
> in any of the ping interactions involved here, as I will need to make sure
> my server does similar behavior. 

We ignore the server response completely. If the request errors or times out, we retry. Otherwise, we assume it is successful without ever inspecting the response.

The other question I have is I'd like to
> understand the JSON structure used in the ping requests & what each data
> point means (i.e. something similar to what's written up here -
> https://wiki.mozilla.org/FirefoxOS/Metrics#FTU_ping, but for the metrics
> ping here instead).

Here's the JSON:

    {
      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 54

[David Flanagan [:djf]]

(In reply to Marshall Culpepper [:marshall_law] from comment #50)
> Comment on attachment 8448587 [details] [review]
> Patch w/ tests
> 
> Requesting feedback of the FTU code from Francisco, and Settings code from
> Vivien.. let me know if someone else would be better

Given that these are string changes only, I think that if we get ui-review+ on them, we can land even without feedback from Franciso and Vivien. Its a good idea to bring these changes to their attention, but I don't think we need to wait for them to land.

Comment 55

[[:fabrice] Fabrice Desré]

Please please please, stop using origin as a way to identify apps. Use manifest urls instead. r- for using origins

Comment 56

[David Flanagan [:djf]]

Fabrice: I'm not sure that the events generated by the window management code include the manifest URL as part of the event detail. If they all do, then the change you're asking for is relatively easy. If not, it makes the patch substantially harder.

Could you explain why you care?

Note that I think the value we're actually getting is the origin plus launch path. Here are some example strings that we get with the current patch:

Homescreen: app://verticalhome.gaiamobile.org
Lockscreen: app://lockscreen.gaiamobile.org
Dialer: app://communications.gaiamobile.org/dialer/index.html#keyboard-view
Facebook app from marketplace: https://m.facebook.com
Notes app from marketplace: app://{68b79fc2-03f9-4612-bd73-4dcf5915c46d}

Those all seem sufficiently unique for data collection purposes

Comment 57

[David Flanagan [:djf]]

Marshall,

I've confirmed what Alive pointed out about not recording data when on the call screen. For outgoing calls, the user goes to the dialer, so time on the call counts toward the dialer app, which seems reasonable enough. But for incoming calls, we think we're still at whatever app that was in use when the call came in.

Also, when the call screen blanks the screen because of the proximity sensor (when you hold the phone up to your ear) the code thinks that the phone is sleeping and stops recording time to any app.  This is probably better than improperly accounting for time in the app you were using, but it isn't right. The "screenchange" event has a reason code, and I guess we need to check that to distinguish this kind of on call screen blanking from actual going to sleep.  Sorry I didn't anticipate that.

Comment 58

[David Flanagan [:djf]]

Jason,

See comment #53 for the testing info you requested.  Also, if you're going to test reporting, you'll need to change the metrics.appusage.reportInterval setting so that you don't have to wait 24 hours to generate a report. The value is in ms, so 300000 would send a report every 5 minutes. Use metrics.appusage.retryInterval to set the retry interval.  If you don't have a way to modify settings like that in your own builds, you may have to ask Marshall to add appropriate code to the build system.

Comment 59

[Marshall Culpepper [:marshall_law]]

(In reply to David Flanagan [:djf] from comment #57)

> .. for incoming calls, we think we're still at whatever app that was in use when
> the call came in.
> 
> Also, when the call screen blanks the screen because of the proximity sensor
> (when you hold the phone up to your ear) the code thinks that the phone is
> sleeping and stops recording time to any app.  

Those both sound serious enough to warrant fixing and additional tests for those use cases. How do you feel about putting them in a follow up so we can get this patch landed? I can heavily document the known problems in comments w/ bugzilla link and work on that next.

>  Sorry I didn't anticipate that.

It's impossible to anticipate everything :)

Comment 60

[Jason Smith [:jsmith]]

(In reply to David Flanagan [:djf] from comment #58)
> Jason,
> 
> See comment #53 for the testing info you requested.  Also, if you're going
> to test reporting, you'll need to change the metrics.appusage.reportInterval
> setting so that you don't have to wait 24 hours to generate a report. The
> value is in ms, so 300000 would send a report every 5 minutes. Use
> metrics.appusage.retryInterval to set the retry interval.  If you don't have
> a way to modify settings like that in your own builds, you may have to ask
> Marshall to add appropriate code to the build system.

Okay, makes sense.

Comment 61

[(inactive after 6/18) Alive Kuo [:alive]]

(In reply to David Flanagan [:djf] from comment #57)
> Marshall,
> 
> I've confirmed what Alive pointed out about not recording data when on the
> call screen. For outgoing calls, the user goes to the dialer, so time on the
> call counts toward the dialer app, which seems reasonable enough. But for
> incoming calls, we think we're still at whatever app that was in use when
> the call came in.
> 
> Also, when the call screen blanks the screen because of the proximity sensor
> (when you hold the phone up to your ear) the code thinks that the phone is
> sleeping and stops recording time to any app.  This is probably better than
> improperly accounting for time in the app you were using, but it isn't
> right. The "screenchange" event has a reason code, and I guess we need to
> check that to distinguish this kind of on call screen blanking from actual
> going to sleep.  Sorry I didn't anticipate that.

David, I disagree. No matter the call is incoming or outgoing, you could not use your app anymore when callscreen is on top of your active app. So I proposed to stop recording once 'attentionscreen' is active.

Note: Use 'attentionscreenshow' and 'attentionscreenhide' event.

Comment 62

[(inactive after 6/18) Alive Kuo [:alive]]

(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #61)
> (In reply to David Flanagan [:djf] from comment #57)
> > Marshall,
> > 
> > I've confirmed what Alive pointed out about not recording data when on the
> > call screen. For outgoing calls, the user goes to the dialer, so time on the
> > call counts toward the dialer app, which seems reasonable enough. But for
> > incoming calls, we think we're still at whatever app that was in use when
> > the call came in.
> > 
> > Also, when the call screen blanks the screen because of the proximity sensor
> > (when you hold the phone up to your ear) the code thinks that the phone is
> > sleeping and stops recording time to any app.  This is probably better than
> > improperly accounting for time in the app you were using, but it isn't
> > right. The "screenchange" event has a reason code, and I guess we need to
> > check that to distinguish this kind of on call screen blanking from actual
> > going to sleep.  Sorry I didn't anticipate that.
> 
> David, I disagree. No matter the call is incoming or outgoing, you could not
> use your app anymore when callscreen is on top of your active app. So I
> proposed to stop recording once 'attentionscreen' is active.
> 
> Note: Use 'attentionscreenshow' and 'attentionscreenhide' event.

The procedure is when attentionscreen appears, the app who uses it will be launched 'at background', and system app opens the attention window just on current active app.
So no new appopened event is fired, hence you need to take care these events.

window.open is the same: there will be popupopened event, but noted that, a background app could also open window so you should just care about the foreground one.

inline activity is the same: there will be activityopened event, but noted that only foreground app is able to launch activity.

Due to the complexity above I still recommand to have a submodule inside AppWindow,
hence you don't need to record the app by origin anymore: the submodule is app by app.

Comment 63

[Francis Djabri [:djabber]]

Comment on attachment 8448088 [details]
screenshot of Settings app after string change

This seems ok to me, but I only see one screen shot in this attachment, not the 3 before and 3 afters that David mentioned.

Comment 64

[[:fabrice] Fabrice Desré]

(In reply to David Flanagan [:djf] from comment #56)
> Fabrice: I'm not sure that the events generated by the window management
> code include the manifest URL as part of the event detail. If they all do,
> then the change you're asking for is relatively easy. If not, it makes the
> patch substantially harder.
> 
> Could you explain why you care?

We will soon enable multiple apps per origin. This means that you will have duplicates in your data and won't be able to distinguish apps from the same origin.

> Note that I think the value we're actually getting is the origin plus launch
> path. Here are some example strings that we get with the current patch:
> 
> Homescreen: app://verticalhome.gaiamobile.org
> Lockscreen: app://lockscreen.gaiamobile.org

These ones don't have launch path (their are actually origins).

> Dialer: app://communications.gaiamobile.org/dialer/index.html#keyboard-view

Full launch path.

> Facebook app from marketplace: https://m.facebook.com
> Notes app from marketplace: app://{68b79fc2-03f9-4612-bd73-4dcf5915c46d}

origins there. That doesn't seem consistent.

> Those all seem sufficiently unique for data collection purposes

Maybe, but using the manifest url is 100% reliable. The current data looks not that reliable. For instance the comm app could be called with another fragment identifier.

What do you get from apps launched as activities?

Comment 65

[(inactive after 6/18) Alive Kuo [:alive]]

(In reply to Fabrice Desré [:fabrice] from comment #64)
> (In reply to David Flanagan [:djf] from comment #56)
> > Fabrice: I'm not sure that the events generated by the window management
> > code include the manifest URL as part of the event detail. If they all do,
> > then the change you're asking for is relatively easy. If not, it makes the
> > patch substantially harder.
> > 
> > Could you explain why you care?
> 
> We will soon enable multiple apps per origin. This means that you will have
> duplicates in your data and won't be able to distinguish apps from the same
> origin.
> 
> > Note that I think the value we're actually getting is the origin plus launch
> > path. Here are some example strings that we get with the current patch:
> > 
> > Homescreen: app://verticalhome.gaiamobile.org
> > Lockscreen: app://lockscreen.gaiamobile.org
> 
> These ones don't have launch path (their are actually origins).
> 
> > Dialer: app://communications.gaiamobile.org/dialer/index.html#keyboard-view
> 
> Full launch path.
> 
> > Facebook app from marketplace: https://m.facebook.com
> > Notes app from marketplace: app://{68b79fc2-03f9-4612-bd73-4dcf5915c46d}
> 
> origins there. That doesn't seem consistent.
> 
> > Those all seem sufficiently unique for data collection purposes
> 
> Maybe, but using the manifest url is 100% reliable. The current data looks
> not that reliable. For instance the comm app could be called with another
> fragment identifier.
> 
> What do you get from apps launched as activities?

For activity we are sending the activity's manifestURL.
For example, 'pick' activity will fire 'activityopened' event with gallery's manifestURL.

That is to say, if we WANT to record the activity as well as the app, then nothing bad.
But IF we want to separate the record of the activity and the app, we could use an internal instance ID.

I have no idea how is the record output though.

Comment 66

[Marshall Culpepper [:marshall_law]]

(In reply to Francis Djabri [:djabber] from comment #63) 
> I only see one screen shot in this attachment, not
> the 3 before and 3 afters that David mentioned.

I think he attached those as separate images on this bug

Comment 67

[Francisco Jordano [:arcturus] [:francisco]]

Comment on attachment 8448587 [details] [review]
Patch w/ tests

FTU part f+, just left a comment regarding that fact that we could lazy load the library, and just have a tiny script loading in system to check if the user enabled this stats sharing or not and load the rest of the library after this check.

Great work Ravikumar, it reminds me to an open source project I started months ago, same concept of collecting stats, but at application level:
https://github.com/arcturus/eventpouchhttps://github.com/arcturus/eventpouch

Comment 68

[Marshall Culpepper [:marshall_law]]

I've confirmed that e.detail.manifestURL is for at least:

* appopened
* homescreen-opened
* lockscreen-appopened
* lockscreen-appclosed

And was null/undefined for:
* active
* idle
* screenchange

We're only using origin in our appopened, homescreen-opened, and lockscreen-appopened event listeners (we're using application.origin for install/uninstall..)

Comment 69

[Marshall Culpepper [:marshall_law]]

OK, the final update here was to use manifestURL instead of origin for tracking apps, the change was not too large, and mostly just required test changes. I've rebased and pushed pending one more Travis run, after which I'll get this landed on master.

Comment 70

[Marshall Culpepper [:marshall_law]]

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

Comment 71

[Marshall Culpepper [:marshall_law]]

Attachment: v2.0 patch

Seeking approval to land app the usage metrics feature in 2.0 based on the product team's need

[User impact] if declined:
No app usage metrics collected in 2.0 (mostly our problem)

[Testing completed]:
Unit tests and Marionette / integration tests included in patch
QA test plan is being formed

[Risk to taking this patch] (and alternatives if risky):
The feature is mostly event driven, and decoupled from other services, so should present minimal risk as features go.

[String changes made]:
Minor updates to wording at the end of the FTU process and in the Settings app under 'Performance data' -> 'Performance and usage data'

Comment 72

[Jason Smith [:jsmith]]

(In reply to Marshall Culpepper [:marshall_law] from comment #71)
> Created attachment 8452811 [details] [review]
> v2.0 patch
> 
> Seeking approval to land app the usage metrics feature in 2.0 based on the
> product team's need
> 
> [User impact] if declined:
> No app usage metrics collected in 2.0 (mostly our problem)

The problem here is that we're past 6/20. 6/20 was the deadline to deliver late-breaking features that were granted an exception post FL. Why can't we let this just ride the trains?

> 
> [Testing completed]:
> Unit tests and Marionette / integration tests included in patch
> QA test plan is being formed

Note - I don't have the test plan done yet. All I've got right now is just indication of an idea of how could I test this informally.

> 
> [Risk to taking this patch] (and alternatives if risky):
> The feature is mostly event driven, and decoupled from other services, so
> should present minimal risk as features go.
> 
> [String changes made]:
> Minor updates to wording at the end of the FTU process and in the Settings
> app under 'Performance data' -> 'Performance and usage data'

We're string frozen at this point, so I don't think we can accept any string changes at this point of the release.

Comment 73

[bhavana bajaj [:bajaj]]

(In reply to Jason Smith [:jsmith] from comment #72)
> (In reply to Marshall Culpepper [:marshall_law] from comment #71)
> > Created attachment 8452811 [details] [review]
> > v2.0 patch
> > 
> > Seeking approval to land app the usage metrics feature in 2.0 based on the
> > product team's need
> > 
> > [User impact] if declined:
> > No app usage metrics collected in 2.0 (mostly our problem)
> 
> The problem here is that we're past 6/20. 6/20 was the deadline to deliver
> late-breaking features that were granted an exception post FL. Why can't we
> let this just ride the trains?
> 
> > 
> > [Testing completed]:
> > Unit tests and Marionette / integration tests included in patch
> > QA test plan is being formed
> 
> Note - I don't have the test plan done yet. All I've got right now is just
> indication of an idea of how could I test this informally.
> 
> > 
> > [Risk to taking this patch] (and alternatives if risky):
> > The feature is mostly event driven, and decoupled from other services, so
> > should present minimal risk as features go.
> > 
> > [String changes made]:
> > Minor updates to wording at the end of the FTU process and in the Settings
> > app under 'Performance data' -> 'Performance and usage data'
> 
> We're string frozen at this point, so I don't think we can accept any string
> changes at this point of the release.

+1 to the comments by :jsmith here and NI on marshall to help clarify.

Comment 74

[Marshall Culpepper [:marshall_law]]

(In reply to bhavana bajaj [:bajaj] [NOT reading Bugmail, needInfo please] from comment #73)
> (In reply to Jason Smith [:jsmith] from comment #72)
> > We're string frozen at this point, so I don't think we can accept any string
> > changes at this point of the release.
> 
> +1 to the comments by :jsmith here and NI on marshall to help clarify.

This has been requested by Product Management to land in 2.0 (even though it is late..). I'll flag Axel on the patch to make sure he's aware..

Jason, I'd be happy to meet with you and provide any additional info you need for a more formal testing plan.

Comment 75

[Marshall Culpepper [:marshall_law]]

Comment on attachment 8452811 [details] [review]
v2.0 patch

Requesting a string change for a late-landing feature in FxOS v2.0 based on Product Management's request..

Axel, I was told you would be the right person to tag..

Comment 76

[Axel Hecht [:Pike]]

I really think we should not take this.

With these changes, I'd invite the responsible product person to engage in a conversation with Bhavana and chofmann about the schedule impact this change would have. I don't expect that we'll wave privacy-related strings that are front-and-center in ftu and settings as not-localized.

Comment 77

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8452811 [details] [review]
v2.0 patch

While I would like to have improved reporting, we're well past feature and string freeze. This feature should be targeted at a future release, like 2.1.

Comment 78

[Stephany Wilkes]

Flagging Adam to ensure he's aware of the decision made in comment #77.

Comment 79

[John Jensen]

(In reply to Lawrence Mandel [:lmandel] from comment #77)
> This feature should be targeted at a future release, like 2.1.

Hi Lawrence,

Understanding whether and how our users use FxOS devices has always been a high priority. I am not close to the QA process or the codebase, but based on my understanding, there is low implementation risk. One could argue that, even as it it is, this functionality is over a year late. Is there no way to get it into 2.0?

Comment 80

[Jason Smith [:jsmith]]

(In reply to John Jensen from comment #79)
> (In reply to Lawrence Mandel [:lmandel] from comment #77)
> > This feature should be targeted at a future release, like 2.1.
> 
> Hi Lawrence,
> 
> Understanding whether and how our users use FxOS devices has always been a
> high priority. I am not close to the QA process or the codebase, but based
> on my understanding, there is low implementation risk. One could argue that,
> even as it it is, this functionality is over a year late. Is there no way to
> get it into 2.0?

Anything that involves string changes is *not* low risk. We're way past string freeze at this point.

Comment 81

[Adam Rogers (:arog)]

confirmed with Ravi and Chris Lee.  Product wants this in 2.0.  

All concerns are noted. Let's uplift.

Comment 82

[Jason Smith [:jsmith]]

(In reply to Adam Rogers (:arog) from comment #81)
> confirmed with Ravi and Chris Lee.  Product wants this in 2.0.  
> 
> All concerns are noted. Let's uplift.

I don't think there's agreement offline that we'll be pursuing this option. This needs to be further resolved offline.

Comment 83

[Lawrence Mandel [:lmandel] (use needinfo)]

(In reply to John Jensen from comment #79)
> Understanding whether and how our users use FxOS devices has always been a
> high priority. I am not close to the QA process or the codebase, but based
> on my understanding, there is low implementation risk. One could argue that,
> even as it it is, this functionality is over a year late. Is there no way to
> get it into 2.0?

In my opinion, no. We are far too late in the schedule (the FL milestone, which marked the end of new feature work, was June 9). At this point we need to keep the team focused on stabilization and our commitment to our agreed upon milestone dates, next up being the FC milestone on July 21. Things are not all doom and gloom for this feature. As noted in comment 70, this feature has already landed and will be shipped with 2.1.

(In reply to Adam Rogers (:arog) from comment #81)
> confirmed with Ravi and Chris Lee.  Product wants this in 2.0.  
> 
> All concerns are noted. Let's uplift.

Adam, due to requirements that include string changes (which are not yet well enough defined for an estimate from l10n) and additional QA effort, I don't think this change can be made without corresponding changes to the 2.0 schedule. Does the product team have a proposal for extending the schedule to accommodate this change and has that proposal been reviewed with our partners?

Comment 84

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 8448587 [details] [review]
Patch w/ tests

Etienne and Francisco are likely enough to give feedback on that. My personal data, without having digged too much in the code would be: Why do we have this big component, instead of splitting reports into telemetry probes inside the right part of the code ?

For example a probe can be added into BrowserElementParent to see how long a particular iframes has been displayed, etc..

Not doing this means that we rely on possible implementation details of the system app about what is the meaning of such events to match a specific behaviors. It would force any change in the underlying architecture to be reflected as an expected state for statistics that may or may not match.

Comment 85

[[:fabrice] Fabrice Desré]

I agree with Vivien. The ftu ping was done outside of telemetry and that made sense. But what's being done here should not be tied that much to the UI side of things. What we need is a way for gaia to augment telemetry for things that are gaia specific.

As far as process is concerned, I'm extremely upset to see that some people believe that it's ok to let a bug linger for 3 months and then ask for a wildcard to land. We've been fighting against that kind of events since we're out of 1.1 and that is not acceptable. And no, this feature is not a blocker for users. It's a nice to have for us.

Comment 86

[chris hofmann]

I've made these comments in e-mail threads but making sure it gets recorded here as well.

The screen shots reflect string changes that will be needed in the ftu and settings app, but they also contain a link to "learn more at m.o/telemetry" that I'm guessing will also have localized content, and there appears to be enough of a difference in the way this feature currently works and the kind of data that it collects, and the way it is stored and used that all that content needs to reviewed and updated and translated.  This maybe especially important for privacy sensitive places in the world like Germany and Brasil.

Is anyone assigned to do that work and/or is there a bug open on it?

It would be really good to recap all the data that's collected, and summarize how the transfer mechanism works, and at what rate the data is sent, with associate estimates on bandwidth use for the transfers since users may bear the cost for sending us this data.

Comment 87

[Marshall Culpepper [:marshall_law]]

(In reply to chris hofmann from comment #86)
> Is anyone assigned to do that work and/or is there a bug open on it?
> 

There's a bug open, but no one is assigned yet:
https://bugzilla.mozilla.org/show_bug.cgi?id=1032485

Comment 88

[Marshall Culpepper [:marshall_law]]

This landed for v2.1

Comment 89

[Ravi Dandu]

The information collected and design for app usage metrics is documented at https://wiki.mozilla.org/FirefoxOS/Metrics/App_Usage