Send Telemetry events for use of the "Pause" and "Restart" buttons

RESOLVED FIXED in Firefox 46

Status

defect
P1
normal
Rank:
19
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: mancas, Assigned: mancas)

Tracking

unspecified
mozilla47
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox46 fixed, firefox47 fixed)

Details

(Whiteboard: [pause][btpp-fix-now])

User Story

Add telemetry events when user clicks on "Pause"/"Restart" buttons

Acceptance criteria:
- Send Telemetry events for use of the "Pause" and "Restart" buttons

User value:
The user value we are providing through this data collection is the ability for the Hello team to understand engagement with the product, and use that information to make the product more useful or engaging, or determine if the product doesn't have sufficient value to users.
With regards to the "Pause/Restart" sharing feature, we want to understand the following:
- Share of Hello sessions where "Pause sharing" gets used (validation of feature's user value)
- Share of Hello sessions where "Restart sharing gets used (validation that the user is using Hello for sharing purposes and not just as a Skype replacement)

Attachments

(2 attachments, 3 obsolete attachments)

Add telemetry events when user clicks on "Pause"/"Restart" buttons
Rank: 19
Comment on attachment 8717066 [details] [review]
[loop] mancas:bug1245486 > mozilla:master

Mike, the code is working as expected but I'm getting an exception since the histogram does not exist. Could you tell me where I need to create the new one to be recognized?
Attachment #8717066 - Flags: feedback?(mdeboer)
Posted patch MC_Patch (obsolete) — Splinter Review
Attachment #8717405 - Flags: review?(mdeboer)
Comment on attachment 8717405 [details] [diff] [review]
MC_Patch

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

This change requires review from a data collection peer.

::: toolkit/components/telemetry/Histograms.json
@@ +8387,5 @@
>      "releaseChannelCollection": "opt-out",
>      "description": "Number of sessions where at least one chat message was exchanged"
>    },
> +  "LOOP_INFOBAR_ACTION_BUTTONS": {
> +    "alert_emails": ["firefox-dev@mozilla.org", "mdeboer@mozilla.com"],

You might want to mbanner@moz there, instead of me.
Attachment #8717405 - Flags: review?(mdeboer)
Attachment #8717405 - Flags: review?(benjamin)
Attachment #8717405 - Flags: review+
Attachment #8717066 - Flags: feedback?(mdeboer) → feedback+
Comment on attachment 8717405 [details] [diff] [review]
MC_Patch

I'm a bit confused, because the bug is talking about two buttons (pause/restart), but this histogram is counting just one thing. Is that intended? The description here is very vague and needs to be clarified: are the pause/restart buttons the only buttons on the infobar, or are there other buttons and are they also counted?

Mike, will you be monitoring this value using the telemetry dashboards? As far as I know, those dashboards currently only show people who have opted into to extended telemetry (even on the release channel). If you need to look at the entire population, you will need to write your own queries.

In order for this to be opt-out, you need to have a statement of how this metric will provide user value. I suspect that you'll be using this to optimize other product decisions? If so can you provide some details about what those decisions are and how this data will influence it?
Flags: needinfo?(mdeboer)
Attachment #8717405 - Flags: review?(benjamin) → feedback-
Comment on attachment 8717066 [details] [review]
[loop] mancas:bug1245486 > mozilla:master

Comments addressed. Take a look when you get a chance

Thank you
Attachment #8717066 - Flags: review?(mdeboer)
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #5)
> I'm a bit confused, because the bug is talking about two buttons
> (pause/restart), but this histogram is counting just one thing. Is that
> intended? The description here is very vague and needs to be clarified: are
> the pause/restart buttons the only buttons on the infobar, or are there
> other buttons and are they also counted?

There is a 'Disconnect' button, which is not counted. The Pause/ Restart buttons are one and the same button; a two-state toggle.

> Mike, will you be monitoring this value using the telemetry dashboards? As
> far as I know, those dashboards currently only show people who have opted
> into to extended telemetry (even on the release channel). If you need to
> look at the entire population, you will need to write your own queries.

I won't be monitoring this value, our PM will (:RT) and Ian (:ibicking). I *think* they're using the dashboards.

> In order for this to be opt-out, you need to have a statement of how this
> metric will provide user value. I suspect that you'll be using this to
> optimize other product decisions? If so can you provide some details about
> what those decisions are and how this data will influence it?

Of course. I think :RT simply omitted that from the user story - let's ask!
Flags: needinfo?(mdeboer) → needinfo?(rtestard)
Status: NEW → ASSIGNED
Comment on attachment 8717066 [details] [review]
[loop] mancas:bug1245486 > mozilla:master

Just a few more things to address!
Attachment #8717066 - Flags: review?(mdeboer) → review-
User value details now added to the user story field.
Regarding the way we'll be monitoring the values, we used to be able to review on https://telemetry.mozilla.org/ although the data does not seem to appear there anymore for opt-out data. I assume that writing our own requests assumes the use of a Spark instance, I created bug 1247268 to follow that up.
User Story: (updated)
Flags: needinfo?(rtestard)
Comment on attachment 8717066 [details] [review]
[loop] mancas:bug1245486 > mozilla:master

I've addressed your comments. Please take a look at the PR

Thanks!
Attachment #8717066 - Flags: review- → review?(mdeboer)
Blocks: 1247268
Comment on attachment 8717066 [details] [review]
[loop] mancas:bug1245486 > mozilla:master

Thanks, looking good to me!
Attachment #8717066 - Flags: review?(mdeboer) → review+
How will this one counter here be able to satisfy the requirement "Share of Hello sessions where 'Restart sharing' gets used". If it's a simple counter, it seems that it would be hard to distinguish the case of multiple hello sessions, hit pause once in each, versus a single session where pause/restart was chosen.
Flags: needinfo?(rtestard)
Could we send just one instance of each event per Hello session when applicable ?
Flags: needinfo?(rtestard)
That's fine with me; I don't know how much engineering work that is.
Manu, can you please have a look and let me know if we could send just one instance of each event per Hello session when applicable ?
Flags: needinfo?(b.mcb)
Can we send this as two events, one "pause" and one "restart"?  Then 1 pause means they turned video off, 1 pause and 1 restart means they paused for a little while, etc.  Theoretically we can go by even/odd but that seems like it might be hard to analyze in Telemetry.

I'm not sure if we got the user value statement in: tracking these clicks allows us to analyze if the tab sharing feature of Hello is useful (e.g., if people turn it off and do not turn it back on, then it is not useful to that population).  Also we can see if the pause feature is used; if not we may redesign it or remove it.  If we make changes to the UI in the future this will provide a baseline for comparison.
Comment on attachment 8717066 [details] [review]
[loop] mancas:bug1245486 > mozilla:master

Mike I've addressed comments from Ian and Romain. Now the events are send one, of each type, per session

Would you mind taking a look at the patch again?

Thanks!
Flags: needinfo?(b.mcb)
Attachment #8717066 - Flags: review+ → review?(mdeboer)
To make sure the add-on doesn't raise an exception when installed on a Firefox without the Histograms.json (as will happen during QA), we should guard the Telemetry with a try/catch.
Comment on attachment 8717066 [details] [review]
[loop] mancas:bug1245486 > mozilla:master

Your filtering solution is nice & simple. Good work, thanks!
Attachment #8717066 - Flags: review?(mdeboer) → review+
Landed in master: https://github.com/mozilla/loop/commit/488153c83028340b219766c46a72cc6dd41b51e6

We still need to land mc_patch in mozilla-central
Attachment #8717405 - Flags: feedback- → feedback?(benjamin)
Comment on attachment 8717405 [details] [diff] [review]
MC_Patch

This patch doesn't seem to match the Hello change, which uses an enumerated histogram. Wrong patch perhaps?
Attachment #8717405 - Flags: feedback?(benjamin) → feedback-
Posted patch MC_Patch (obsolete) — Splinter Review
Attachment #8717405 - Attachment is obsolete: true
Attachment #8720681 - Flags: feedback?(benjamin)
Comment on attachment 8720681 [details] [diff] [review]
MC_Patch

Does this build? I'm pretty sure you need an n_values: 2 for this to build properly, and I'm certain it won't show up on telemetry.mozilla.org properly without that.

data-review=me with that change
Attachment #8720681 - Flags: feedback?(benjamin) → feedback+
Changes addressed so keeping f+
Attachment #8720681 - Attachment is obsolete: true
Attachment #8721237 - Flags: feedback+
https://hg.mozilla.org/integration/fx-team/rev/ac5ed9e7b4173585001b5691ea941d2afb95da4f
Bug 1245486 - Send Telemetry events for use of the "Pause" and "Restart" buttons. f=bsmedberg, r=mikedeboer
It compiles locally, please check if it works for you and land this and bug 1208416

Thank you!
Attachment #8721237 - Attachment is obsolete: true
Attachment #8724643 - Flags: review?(mdeboer)
https://hg.mozilla.org/integration/fx-team/rev/970aa793c5e2778172cd8e527f9dc04848400d40
Bug 1245486 - Send Telemetry events for use of the Pause and Restart buttons. r=mikedeboer
Attachment #8724643 - Flags: review?(mdeboer) → review+
https://hg.mozilla.org/mozilla-central/rev/970aa793c5e2
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Comment on attachment 8724643 [details] [diff] [review]
0001-Bug-1245486-Send-Telemetry-events-for-use-of-the-Pau.patch

Approval Request Comment
[Feature/regressing bug #]: Firefox Hello, telemetry for use of pause & restart buttons
[User impact if declined]: We won't be able to measure the use of the pause/restart buttons on tab sharing until 47 - taking longer to get the results out.
[Describe test coverage new/current, TreeHerder]: Has been in m-c & aurora for quite a while.
[Risks and why]: Low, small change to json file.
[String/UUID change made/needed]: None
Attachment #8724643 - Flags: approval-mozilla-beta?
Comment on attachment 8724643 [details] [diff] [review]
0001-Bug-1245486-Send-Telemetry-events-for-use-of-the-Pau.patch

extra patch for Hello telemetry. Please uplift to beta.
Attachment #8724643 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.