1138560 - Add telemetry for web pages launched from home screen shortcuts.

Status: RESOLVED DUPLICATE of bug 1023306

(Firefox for Android Graveyard :: General, defect)

Description

[Wesley Johnston (:wesj)]

We should add something to the "Add to homescreen" links in Firefox so that we can track when they're clicked. We can just put an extra on the intent and read it out quickly when we're launched.

Comment 1

[Alexander Ploner]

Attachment: bug-1138560-v1.patch

As suggested by mcomella in my first bug, I'd like to work on this. I already created a patch and need some feedback.

The aim of this bug is to measure the time spans between the creation of a home screen shortcut and all times the user opens the shortcut. Is that right?

I created a new string constant and named it "EXTRA_HOMESCREEN_SHORTCUT_CREATION_TIME" to store the creation time. The name describes exactly what it's used for, but I don't know whether it's too long or not.

The time span is calculated by using the values out of System.currentTimeMillis(). I'm using this function because the SystemClock values are reset on every system boot. The problematic part about this is that the value can and will change sometimes.

The measured time span gets submit with Telemetry.sendUIEvent(TelemetryContract.Event.ACTION, TelemetryContract.Method.HOMESCREEN, timeSpan);
All of that line is guessed by comparing it to other places. I probably have to change things here ;).

Additional note: When reading the value of the intent extra, there is an check whether the value equals zero, the default value. This is used to skip old shortcuts that do not have this extra set.

Comment 2

[Wesley Johnston (:wesj)]

Cool. I don't even think we need the timestamp code here TBH. I just wanted a count of how many times you launched from one of these. You'll need to add a key to the Telemetry.json file:

http://mxr.mozilla.org/mozilla-central/source/toolkit/components/telemetry/Histograms.json#7775

and then store it like you are here.

Comment 3

[Nick Alexander :nalexander [he/him]]

Alex: I'd really like to see this happen.  I see that you have a patch and some feedback -- can I help get this moving again?

Comment 4

[Alexander Ploner]

Attachment: Updated patch recording to feedback.

Sorry for not updating this for such a long time.

I've updated the patch now:
- removed the timestamp code
- removed the intent extra (I'm now using the existing intent action ACTION_HOMESCREEN_SHORTCUT only)
- added a new histogram FENNEC_HOMESCREEN_SHORTCUT_LAUNCHES to Histograms.json

Is the histogram entry and its kind "count" okay?
(I also found FENNEC_STARTUP_GECKOAPP_ACTION, should those two histograms be combined?)



Now I'm searching for a way to record those launches within the Java code.

If I use the following line, there is still a connection to the histogram needed, but I don't know where to do this.
> Telemetry.sendUIEvent(TelemetryContract.Event.ACTION, TelemetryContract.Method.HOMESCREEN);

(There is also the method addToHistogram() where I can specify the histogram name directly, but this method is probably not suitable for this, because I have to increase the value.)
> Telemetry.addToHistogram("FENNEC_HOMESCREEN_SHORTCUT_LAUNCHES", 1);

I've searched at places like the Mozilla Wiki and in related Android bugs but I wasn't successful in finding a solution for that. Can someone point me to an Java example or give me some help in here?

Comment 5

[Alexander Ploner]

I'm not sure who "wants" the needinfo flag ;).

Comment 6

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8632552 [details] [diff] [review]
Updated patch recording to feedback.

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

Alexander: thanks for pushing this forward!

For UI Telemetry, I think everything is set up: there's no need to add to Histograms.json.  Flagging liuche to confirm.

::: mobile/android/base/GeckoApp.java
@@ +1837,5 @@
>          } else if (ACTION_HOMESCREEN_SHORTCUT.equals(action)) {
>              String uri = getURIFromIntent(intent);
>              GeckoAppShell.sendEventToGecko(GeckoEvent.createBookmarkLoadEvent(uri));
> +
> +            Telemetry.sendUIEvent(TelemetryContract.Event.ACTION, TelemetryContract.Method.HOMESCREEN);

I think this is right: we want Event.LOAD_URL and Method.HOMESCREEN here.  There's no need to reference a particular histogram, IIUC.

Comment 7

[Alexander Ploner]

Attachment: bug-1138560-v3.patch

(In reply to Nick Alexander :nalexander from comment #6)
> For UI Telemetry, I think everything is set up: there's no need to add to
> Histograms.json.  Flagging liuche to confirm.
Oooh, okay, that might be the reason why I haven't found an example with Telemetry.sendUIEvent() and Histograms.json together :D. Now I probably understand the difference between those two ;).


> I think this is right: we want Event.LOAD_URL and Method.HOMESCREEN here.
Yep, now you are saying it, it totally makes sense to me.


I've updated the patch so that it's immediately ready if liuche says that everything is right.

Comment 8

[Chenxia Liu [:liuche]]

Comment on attachment 8632870 [details] [diff] [review]
bug-1138560-v3.patch

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

Yep, that's correct! You don't need a histogram for UI Telemetry in Fennec.

Comment 9

[Alexander Ploner]

Ok, now that the patch seems okay, how should I go on? :)

Last time (bug 1148549) someone else pushed my patch to the try servers. (That's my second bug.)

Comment 10

[Nick Alexander :nalexander [he/him]]

(In reply to Alexander Ploner from comment #9)
> Ok, now that the patch seems okay, how should I go on? :)
> 
> Last time (bug 1148549) someone else pushed my patch to the try servers.
> (That's my second bug.)

I don't think we'll need a try push here.  liuche gave the r+, so we just need to add [checkin-needed] to the keywords.  (Which I'll do now.)  Then the sheriffs will come and land the patch.

Comment 11

[Alexander Ploner]

Perfect, thank you!

Comment 12

[Nick Alexander :nalexander [he/him]]

(In reply to Alexander Ploner from comment #11)
> Perfect, thank you!

Excellent.  Alex, I'd like to work with you on some additional things...  but I don't know much about what you want to work on, and I'm going to be on holiday until the end of July.  So I'd like to try to connect when I get back, and in the meantime, perhaps liuche has a good next ticket for you.

liuche: can you chime in?

Comment 13

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/2d12d2f2f450

Comment 14

[Chenxia Liu [:liuche]]

Hey Alexander! What sort of bugs would you be interested in working on? If you want to do a few frontend bugs, here are a few that you could take a look at: bug 1179020, bug 1183149, bug 1175375 ?

Comment 15

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/2d12d2f2f450

Comment 16

[Alexander Ploner]

Attachment: Telemetry 2015-08-01.png

If I look into the stats (see screenshot from [1]) there are already values for all release channels. Is this intended? Where does this data come from? I'm wondering if this patch mixes up two different things?


(In reply to Chenxia Liu [:liuche] from comment #14)
> Hey Alexander! What sort of bugs would you be interested in working on? If
> you want to do a few frontend bugs, here are a few that you could take a
> look at: bug 1179020, bug 1183149, bug 1175375 ?

Thanks for your suggestions. Seems like all bugs are taken already :D. I had to learn for exams so I didn't have time to have a look into them. I'll write you an email as soon as I feel able to take new bugs ;).

[1] https://people.mozilla.org/~mfinkle/uitelemetry/

Comment 17

[Mark Finkle (:mfinkle) (use needinfo?)]

I see four uses of HOMESCREEN in the tree:
http://mxr.mozilla.org/mozilla-central/search?string=Method.HOMESCREEN

Two are related to launching from homescreen icons:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/BrowserApp.java#3664
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/GeckoApp.java#1877

These are both fired in onNewIntent, and I think we are now double counting. The existing BrowserApp version should have been good enough. Looks like LucasR added the probe in bug 1023306.

I think we need to back this patch out. Sorry.

Comment 18

[Chenxia Liu [:liuche]]

Ah, sorry about that, I should have caught that. Good catch, mfinkle.

I'm duping this, and I've backed it out: https://hg.mozilla.org/integration/fx-team/rev/4fe45a08aeba