Closed Bug 1314220 Opened 8 years ago Closed 7 years ago

Add the Telemetry for blocking autoplay media

Categories

(Core :: Audio/Video: Playback, defect)

Other Branch
defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla53
Tracking Status
firefox53 --- verified

People

(Reporter: alwu, Assigned: alwu)

References

Details

Attachments

(1 file)

Due to the UX's request, we want to measure the following information
(1) usage amount for mute/unmuted/unblocking tab icon
(2) blocking time (the duration from blocking to unblocking)
(3) amount of unblocking media via visiting the tab, not clicking unblocking icon
Comment on attachment 8810796 [details]
Bug 1314220 - add telemetry for blocking autoplay media.

https://reviewboard.mozilla.org/r/93132/#review93910

::: toolkit/components/telemetry/Histograms.json:10783
(Diff revision 2)
> +    "alert_emails": ["alwu@mozilla.com"],
> +    "expires_in_version": "58",
> +    "kind": "count",
> +    "keyed": true,
> +    "bug_numbers": [1314220],
> +    "description": "The total usage amount of the operations of tab audio indicator, eg. mute/unmuted/unblock.",

This description doesn't say what the possible key values are. If this is just the three options mute/unmute/unblock, this should be an enumerated histogram instead of keyed, because keying is significantly more expensive.

::: toolkit/components/telemetry/Histograms.json:10792
(Diff revision 2)
> +    "alert_emails": ["alwu@mozilla.com"],
> +    "expires_in_version": "58",
> +    "kind": "enumerated",
> +    "n_values": 90,
> +    "bug_numbers": [1314220],
> +    "description": "The time duration from media was blocked to unblocking. Now we record from 1 to 90 seconds.",

1) this is intended to record a scalar value, so it sohuld either be a linear or exponential histogram

2) is anything recorded/what is recorded if the user mutes and never unmutes? Or does this only record the case where the user undoes their previous decision?

3) a linear or exponential histogram will record a value for "above the max" which would almost certainly help here

Are you going to be doing the analysis of this data?
Attachment #8810796 - Flags: review?(benjamin) → review-
Comment on attachment 8810796 [details]
Bug 1314220 - add telemetry for blocking autoplay media.

https://reviewboard.mozilla.org/r/93132/#review94174

::: browser/base/content/tabbrowser.xml:5162
(Diff revision 2)
>  
>            if (tab.hasAttribute("blocked")) {
>              tab.removeAttribute("blocked");
>              this._tabAttrModified(tab, ["blocked"]);
> +            Services.telemetry.getKeyedHistogramById("TAB_AUDIO_INDICATOR_OPERATION_USED").add("unblockByVisitingTab", 1);
> +            Services.telemetry.getHistogramById("TAB_AUDIO_INDICATOR_BLOCKING_TIME_SECOND").add(tab.finishMediaBlockTimer());

Please wrap these lines at 80 characters.

Also, you should use TelemetryTimestamps.jsm instead of rolling your own start/stop implementation.
Attachment #8810796 - Flags: review?(jaws) → review-
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #4)
> Also, you should use TelemetryTimestamps.jsm instead of rolling your own
> start/stop implementation.

Hi, Jared,
I need to have multiple different timers for each tab which was blocked, to measure the time from blocked to unblocked. But the TelemetryTimestamps doesn't fit my requirement, from its description it was used to measure the process's start time?

I also tried to use TelemetryStopwatch, but it was only one instance, I can't measure the multiple tabs at the same time.
Flags: needinfo?(jaws)
Ah yes, TelemetryStopwatch is what I had meant to say. Felipe, do you know how Alastor could get it to work?
Flags: needinfo?(jaws) → needinfo?(felipc)
Ah yeah, TelemetryStopwatch supports measuring multiple instances of the same measurement. There's a second parameter that you can pass as an argument, which can be any object to be keyed to that measurement. So that you can use the tab/browser object that you have to have one measurement going per tab.

(Unless you're saying that one tab might have more than 1 measurement too? But you could find another thing to use as a key)

Here is one example: http://searchfox.org/mozilla-central/rev/59bb309e38b10aba63dea8505fb800e99fe821d6/browser/base/content/browser.js#4795

More than 1 pageload might be happening at the same time, and this uses FX_PAGE_LOAD_MS and the browser to track that.
Flags: needinfo?(felipc)
(In reply to :Felipe Gomes (needinfo me!) from comment #7)
> Ah yeah, TelemetryStopwatch supports measuring multiple instances of the
> same measurement. There's a second parameter that you can pass as an
> argument, which can be any object to be keyed to that measurement. So that
> you can use the tab/browser object that you have to have one measurement
> going per tab.
> 
> (Unless you're saying that one tab might have more than 1 measurement too?
> But you could find another thing to use as a key)
> 
> Here is one example:
> http://searchfox.org/mozilla-central/rev/
> 59bb309e38b10aba63dea8505fb800e99fe821d6/browser/base/content/browser.js#4795
> 
> More than 1 pageload might be happening at the same time, and this uses
> FX_PAGE_LOAD_MS and the browser to track that.

Thanks! I'll try to use the TelemetryStopwatch instead of using my own implementation.
(In reply to Benjamin Smedberg [:bsmedberg] from comment #3)
> This description doesn't say what the possible key values are. If this is
> just the three options mute/unmute/unblock, this should be an enumerated
> histogram instead of keyed, because keying is significantly more expensive.

I'll change it to an enumerated histogram.

> 1) this is intended to record a scalar value, so it sohuld either be a
> linear or exponential histogram

I'll change it to a linear histogram.

> 2) is anything recorded/what is recorded if the user mutes and never
> unmutes? Or does this only record the case where the user undoes their
> previous decision?

No, we only record the blocking time, it's not related with mute/unmute.
We want to know how long user would unblock the tab.

> Are you going to be doing the analysis of this data?

Yes, I would provide these data to our UX and discuss the related analysis.
Comment on attachment 8810796 [details]
Bug 1314220 - add telemetry for blocking autoplay media.

https://reviewboard.mozilla.org/r/93132/#review97568

data-review=me (I did not review the code)
Attachment #8810796 - Flags: review?(benjamin) → review+
review ping?
Flags: needinfo?(jaws)
Comment on attachment 8810796 [details]
Bug 1314220 - add telemetry for blocking autoplay media.

https://reviewboard.mozilla.org/r/93132/#review98628

r=me with the following change.

::: browser/base/content/tabbrowser.xml:6900
(Diff revision 3)
> +          let histName = "TAB_MEDIA_BLOCKING_TIME_SECONDS";
> +          let ms = TelemetryStopwatch.timeElapsed(histName, this);
> +          let seconds = Math.round(ms / 1000);
> +          Services.telemetry.getHistogramById(histName).add(seconds);
> +          TelemetryStopwatch.cancel(histName, this);

This should be using TelemetryStopwatch.finish, which is designed to calculate the time delta, add to the histogram, and will delete the timer.
Attachment #8810796 - Flags: review?(jaws) → review+
Flags: needinfo?(jaws)
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #13) 
> This should be using TelemetryStopwatch.finish, which is designed to
> calculate the time delta, add to the histogram, and will delete the timer.

The time delta which TelemetryStopwatch calculates is by milli-second, but I want to record the duration by second.
Flags: needinfo?(jaws)
We should just record the number as milliseconds and then we can convert to seconds when analyzing the data.
Flags: needinfo?(jaws)
Pushed by alwu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e51e234ad01e
add telemetry for blocking autoplay media. r=bsmedberg,jaws
https://hg.mozilla.org/mozilla-central/rev/e51e234ad01e
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Verified as fixed using the latest Nightly 53.0a1 (Build ID: 20170111030235) on Windows 10 x64, Ubuntu 16.04 x64 and Mac OS X 10.11 (parameters "TAB_AUDIO_INDICATOR_USED" and "TAB_MEDIA_BLOCKING_TIME_MS" are added in about:telemetry -> Histograms and are properly updated).
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: