Closed Bug 1163764 Opened 5 years ago Closed 5 years ago

Report telemetry for EME opt-out ("Play DRM content" checkbox is checked)

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
firefox38 --- wontfix
firefox38.0.5 --- wontfix
firefox39 --- affected
firefox40 --- affected
firefox41 --- fixed

People

(Reporter: cpeterson, Assigned: eflores)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

Can we include or differentiate EME telemetry from downstream Firefox builds?
Do we also want to differentiate regular build getting the pref turned off and repack getting the pref turned on from the defaults for each?
(In reply to Chris Peterson [:cpeterson] from comment #0)
> Can we include or differentiate EME telemetry from downstream Firefox builds?

Downstream would have to be deliberately enabling telemetry reporting in their mozconfig. I have no idea if any downstream users are doing that.

Once bug 1160101 lands, we can do this only for builds that are built with ac_add_options --enable-adone-eme, i.e. only for builds that ship with Adobe EME preffed on by default. Then we are less likely to catch downstream in this.

(In reply to Henri Sivonen (:hsivonen) from comment #1)
> Do we also want to differentiate regular build getting the pref turned off
> and repack getting the pref turned on from the defaults for each?

In a different telemetry probe perhaps?
Assignee: cpearce → edwin
Attached patch 1163764-opt-out-telemetry.patch (obsolete) — Splinter Review
Telemetry for EME being disabled.

We only want to measure this where the Adobe CDM is available, so the "boolean" histogram type is used. It should work roughly the same as the "flag" type since we're add()ing from GMPProvider.startup(), which is only called once AFAICT.
Attachment #8607897 - Flags: review?(cpearce)
Comment on attachment 8607897 [details] [diff] [review]
1163764-opt-out-telemetry.patch

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

Will also need telemetry peer (like vladan) to r+.
Attachment #8607897 - Flags: review?(cpearce) → review+
Comment on attachment 8607897 [details] [diff] [review]
1163764-opt-out-telemetry.patch

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

::: toolkit/components/telemetry/Histograms.json
@@ +7873,5 @@
>      "kind": "enumerated",
>      "n_values": 5,
>      "description": "The result of trying to download a document to show in reader view (0=Success, 1=Error XHR, 2=Error no document)"
> +  },
> +  "VIDEO_EME_DISABLED": {

Can you add an alert_email so we know who "owns" the histogram and we can inform you when the histogram is about to expire or if suddenly a lot more people start opting out
We can also file bugs in a Bugzilla component instead of sending emails, but that only works well if the component is well-monitored

::: toolkit/mozapps/extensions/internal/GMPProvider.jsm
@@ +557,5 @@
>          this._log.warn("startup - adding clearkey CDM failed", e);
>        }
>      }
>  
> +#ifdef MOZ_ADOBE_EME

I think you could also only report the value to telemetry if the pref value is non-default (whatever the default might be)

::: toolkit/mozapps/extensions/internal/moz.build
@@ +20,5 @@
>          'PluginProvider.jsm',
>      ]
>  
>  EXTRA_PP_JS_MODULES.addons += [
> +    'GMPProvider.jsm',

what's this change about?
Attachment #8607897 - Flags: review?(vdjeric)
Carry r+ from cpearce.

(In reply to Vladan Djeric (:vladan) -- please needinfo! from comment #5)
> Can you add an alert_email so we know who "owns" the histogram and we can
> inform you when the histogram is about to expire or if suddenly a lot more
> people start opting out

Done.

> I think you could also only report the value to telemetry if the pref value
> is non-default (whatever the default might be)

It will be useful to have this data as a proportion of users, and I don't think we otherwise track how many compatible builds are out there.

> ::: toolkit/mozapps/extensions/internal/moz.build
> @@ +20,5 @@
> >          'PluginProvider.jsm',
> >      ]
> >  
> >  EXTRA_PP_JS_MODULES.addons += [
> > +    'GMPProvider.jsm',
> 
> what's this change about?

Tells the build system that GMPProvider.jsm should be preprocessed; that is, it makes the #ifdef work.
Attachment #8607897 - Attachment is obsolete: true
Attachment #8615105 - Flags: review?(vdjeric)
Comment on attachment 8615105 [details] [diff] [review]
1163764-opt-out-telemetry.patch v2

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

> It will be useful to have this data as a proportion of users, and I don't
> think we otherwise track how many compatible builds are out there.

What you're doing is fine, but I meant you could use a flag histograms and only set the flag if the pref's value is non-default. Then the accumulation code wouldn't need to be in an "#ifdef MOZ_ADOBE_EME" because the default value of the media.gmp-eme-adobe.enabled pref is already determined by "#ifdef MOZ_ADOBE_EME" in firefox.js. Just a small, optional suggestion
Attachment #8615105 - Flags: review?(vdjeric) → review+
https://hg.mozilla.org/mozilla-central/rev/1639c268ce9b
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
You need to log in before you can comment on or make changes to this bug.