Closed Bug 1261830 Opened 4 years ago Closed 4 years ago

Content notifications: Start telemetry sessions

Categories

(Firefox for Android :: General, defect)

All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 48
Tracking Status
firefox48 --- fixed

People

(Reporter: sebastian, Assigned: sebastian)

References

Details

Attachments

(1 file)

No description provided.
No longer blocks: content-notifications
The very first action is when we subscribe to a feed. That's where I start the session now. Is this the best place? Or should the session start later when we show a notification or the user clicks on it? I guess I should start to look at the data and do some evaluations to understand what we are looking for.
(In reply to Sebastian Kaspari (:sebastian) from comment #2)
> The very first action is when we subscribe to a feed. That's where I start
> the session now. Is this the best place? Or should the session start later
> when we show a notification or the user clicks on it? I guess I should start
> to look at the data and do some evaluations to understand what we are
> looking for.

We should understand when sessions end. I thought they ended automatically when gecko is killed, so this may not work as expected, but maybe I'm wrong.

I wonder how well this even works if it's called from an Android background service. The implementation leads me to here:
http://mxr.mozilla.org/mozilla-central/source/widget/android/nsAppShell.cpp#714

And it looks like this ends up calling into UITelemetry.startSession:
http://mxr.mozilla.org/mozilla-central/source/toolkit/components/telemetry/UITelemetry.jsm#129

Looking in this file, I don't actually see logic where we automatically end sessions, so maybe this works as expected?

NI to mfinkle to see if he has more context around the behavior of these UI telemetry sessions.
Flags: needinfo?(mark.finkle)
Sessions can be a PITA, especially when we don't really have a "session", like this situation. Typically, sessions are used for things like "showing History panel" which has a start and end. 

--
Some history: We thought Sessions would be important to track in their own right, but turns out, they are not that useful. We also stamped all Events with the active Sessions. That turned out to be more useful, because it adds a different kind of context to an Event.

A good example of this is "firstrun.1" Sessions. Started, but never stopped because we never know when Android stops the application. So "firstrun.1" never appears as a Session itself (must be stopped for that), but it is stamped on every Event that fires in the user's initial launch of Fennec. Yes, Sessions are cleared on subsequent app launches.

We don't even save the standalone Sessions in Redash. Just the Session stamped into Events.
--

In this case, we are looking to stamp Sessions (Experiments) onto Events to help with tracking A/B experiments. Given that we are stretching the definition of a Session used in a background service and we only really care about the stamped Event, why not use this simple minded pattern:

startTelemetrySession();
Telemetry.sendUIEvent(TelemetryContract.Event.SAVE, TelemetryContract.Method.SERVICE, "content_update");
stopTelemetrySession();

That will give us the Event stamping we want. We will be able to tell that the content_update was saved using a specific heuristic.

Wait, as I write this it occurs to me that this Event is no the one we care about is it? We want to stamp the (SHOW, NOTIFICATION, "content_update") event, right? That's the one that is controlled by the experiment, right?
Flags: needinfo?(mark.finkle)
Comment on attachment 8738103 [details]
MozReview Request: Bug 1261830 - Content notifications: Stamp telemetry events with experiment session. r?margaret

https://reviewboard.mozilla.org/r/44319/#review41289

Given mfinkle's last comment, let's just surround the events we care about with sessions, so that we can tag them appropriately.
Attachment #8738103 - Flags: review?(margaret.leibovic)
Attachment #8738103 - Attachment is obsolete: true
Comment on attachment 8738103 [details]
MozReview Request: Bug 1261830 - Content notifications: Stamp telemetry events with experiment session. r?margaret

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/44319/diff/1-2/
Attachment #8738103 - Attachment description: MozReview Request: Bug 1261830 - Start telemetry session when subscribing to feed. r?margaret → MozReview Request: Bug 1261830 - Content notifications: Stamp telemetry events with experiment session. r?margaret
Attachment #8738103 - Attachment is obsolete: false
Attachment #8738103 - Flags: review?(margaret.leibovic)
Attachment #8738103 - Flags: review?(margaret.leibovic) → review+
Comment on attachment 8738103 [details]
MozReview Request: Bug 1261830 - Content notifications: Stamp telemetry events with experiment session. r?margaret

https://reviewboard.mozilla.org/r/44319/#review42007

Clunky, but gets the job done. We should think about if there are better patterns for this type of analysis that might emerge in the future.

::: mobile/android/base/java/org/mozilla/gecko/preferences/GeckoPreferences.java:391
(Diff revision 2)
>          }
>  
>          // Launched from "Notifications settings" action button in a notification.
>          if (intentExtras != null && intentExtras.containsKey(CheckForUpdatesAction.EXTRA_CONTENT_NOTIFICATION)) {
> +            Telemetry.startUISession(TelemetryContract.Session.EXPERIMENT, FeedService.getEnabledExperiment(this));
>              Telemetry.sendUIEvent(TelemetryContract.Event.ACTION, Method.BUTTON, "notification-settings");

This is the only notification with this button right now, right?
(In reply to :Margaret Leibovic from comment #7)
> This is the only notification with this button right now, right?

Yeah, correct!
https://hg.mozilla.org/integration/fx-team/rev/adbe62e4a9748eb9570e9bf72eaf7bd19af4da2b
Bug 1261830 - Content notifications: Stamp telemetry events with experiment session. r=margaret
https://hg.mozilla.org/mozilla-central/rev/adbe62e4a974
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
You need to log in before you can comment on or make changes to this bug.