Closed Bug 1256415 Opened 4 years ago Closed 4 years ago

FENNEC_GECKOAPP_STARTUP_ACTION data is wrong after removing WEBAPP startup action

Categories

(Firefox for Android :: Metrics, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 48
Tracking Status
firefox47 --- fixed
firefox48 --- fixed
fennec 48+ ---

People

(Reporter: Margaret, Assigned: Margaret)

References

Details

Attachments

(2 files)

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.
Flags: needinfo?(mark.finkle)
Flags: needinfo?(bbermes)
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.
Flags: needinfo?(bbermes)
tracking-fennec: ? → 48+
I'm open to suggestions, just wanted to put up a proposal.
Attachment #8732431 - Flags: review?(mark.finkle)
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.
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.
Flags: needinfo?(mark.finkle)
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/
Attachment #8732431 - Flags: review?(mark.finkle)
(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.
Attachment #8732431 - Flags: review?(mark.finkle) → review+
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.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/7875a8acb8c4
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
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.
Attachment #8751310 - Flags: approval-mozilla-beta?
Comment on attachment 8751310 [details] [diff] [review]
patch for beta uplift

Improving telemetry data quality, Beta47+
Attachment #8751310 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.