1256415 - FENNEC_GECKOAPP_STARTUP_ACTION data is wrong after removing WEBAPP startup action

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Metrics, defect)

Description

[:Margaret Leibovic]

I missed this in my review, but removing this startup action from this enum affects our histogram telemetry data:
https://hg.mozilla.org/mozilla-central/rev/93156962855d#l10.27

I'd actually like to get rid of this histogram and replace it with UI telemetry.

Barbara, what value do you currently get from this probe? What would you like to know?

I propose that we add individual events for the things that we're currently tracking with this histogram probe. Using the enum ordinal values makes it hard to analyze this data, and leads to off-by-one errors if we update the enum.

Mark, should we add a new STARTUP action to track these startup events? Or should we use the existing LAUNCH method? I see we're already using that for the search widget:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/search/java/org/mozilla/search/SearchWidget.java#73

Maybe something like...

Normal launch: Action.LAUNCH, Method.HOMESCREEN, "app_icon"
Homescreen shortcut launch: Action.LAUNCH, Method.HOMESCREEN, "shortcut"
URL intent: Aciton.LAUNCH, Method.INTENT, "external_app"

For guest/restricted profiles, maybe we should use sessions instead of events.

Comment 1

[Barbara Bermes [:barbara] - NI please!]

I'd like to track (over time) how many times Firefox was opened based on the different scenarios to detect trends, issues or new directions.

For example, if we end up adding more progressive apps features into our product, I'd like to see the Homescreen shortcut going up. Same goes for FFB mode.

I hope that helps.

Comment 2

[:Margaret Leibovic]

Attachment: MozReview Request: Bug 1256415 - Replace FENNEC_GECKOAPP_STARTUP_ACTION histogram with UI telemetry. r=mfinkle

Review commit: https://reviewboard.mozilla.org/r/41169/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/41169/

Comment 3

[:Margaret Leibovic]

I'm open to suggestions, just wanted to put up a proposal.

Comment 4

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

Comment on attachment 8732431 [details]
MozReview Request: Bug 1256415 - Replace FENNEC_GECKOAPP_STARTUP_ACTION histogram with UI telemetry. r=mfinkle

https://reviewboard.mozilla.org/r/41169/#review37821

::: mobile/android/base/java/org/mozilla/gecko/BrowserApp.java:4006
(Diff revision 1)
>  
>      @Override
> -    protected StartupAction getStartupAction(final String passedURL, final String action) {
> +    protected void recordStartupActionTelemetry(final String passedURL, final String action) {
>          final boolean inGuestMode = GeckoProfile.get(this).inGuestMode();
>          if (inGuestMode) {
> -            return StartupAction.GUEST;
> +            Telemetry.startUISession(TelemetryContract.Session.GUEST);

Need to think about this a bit more. This will add "guest" or "restricted" to every event triggered while running the application. This works well for some cases, like "firstrun".

::: mobile/android/base/java/org/mozilla/gecko/BrowserApp.java:4012
(Diff revision 1)
> -        if (Restrictions.isRestrictedProfile(this)) {
> +            Telemetry.startUISession(TelemetryContract.Session.RESTRICTED);
> -            return StartupAction.RESTRICTED;
>          }
> +
>          if (ACTION_HOMESCREEN_SHORTCUT.equals(action)) {
> -            return StartupAction.SHORTCUT;
> +            Telemetry.sendUIEvent(TelemetryContract.Event.LAUNCH, TelemetryContract.Method.HOMESCREEN, "shortcut");

This use of "launch.1" seems to work well.

Comment 5

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

Other thoughts:

* We already get shortcut usage via "loadurl.1" > "homescreen"
* We use "loadurl.1" > "intent" for loading URLs from other apps. We use "" or "tabqueue-*" for the extras.

Maybe we could just use "launch.1" for "launcher" or "guest" or "restricted"? There is always some conflation between launching via homescreen or external app, and normal, guest or restricted so maybe that's too confusing or just wouldn't capture the data.

I notice that we use "launch.1" from the Widget:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/search/java/org/mozilla/search/SearchWidget.java#73

Not sure how that's working with a very separate activity from Gecko.

Comment 6

[:Margaret Leibovic]

Comment on attachment 8732431 [details]
MozReview Request: Bug 1256415 - Replace FENNEC_GECKOAPP_STARTUP_ACTION histogram with UI telemetry. r=mfinkle

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/41169/diff/1-2/

Comment 7

[:Margaret Leibovic]

(In reply to Mark Finkle (:mfinkle) from comment #5)

> Maybe we could just use "launch.1" for "launcher" or "guest" or
> "restricted"? There is always some conflation between launching via
> homescreen or external app, and normal, guest or restricted so maybe that's
> too confusing or just wouldn't capture the data.

I did this in the new version of this patch, but it feels kinda gross... I'm not sure if this is the best way to record this data, but you have a better understanding of how we analyze it.

Comment 8

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

Comment on attachment 8732431 [details]
MozReview Request: Bug 1256415 - Replace FENNEC_GECKOAPP_STARTUP_ACTION histogram with UI telemetry. r=mfinkle

https://reviewboard.mozilla.org/r/41169/#review40177

Wow. You are right. The code is a bit convoluted, but I think you captured the states correctly. We should be able to use these probes to create metrics for how Fennec is opened.

You can test this using ADB logcat to view the output and make sure it matches your expectations for some simple use cases.

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/7875a8acb8c4

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7875a8acb8c4

Comment 11

[:Margaret Leibovic]

Attachment: patch for beta uplift

Approval Request Comment
[Feature/regressing bug #]: None.

[User impact if declined]: We can't understand the ways people launch the browser, including guest or restricted profiles.

[Describe test coverage new/current, TreeHerder]: No automated tests, baked on Nightly/Aurora.

[Risks and why]: Low-risk, there should be no functional change besides adding telemetry probes, but there is a bit of refactoring here.

[String/UUID change made/needed]: None.

Comment 12

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8751310 [details] [diff] [review]
patch for beta uplift

Improving telemetry data quality, Beta47+

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-beta/rev/d92c793bfa61