Closed Bug 1174565 Opened 6 years ago Closed 6 years ago

[EME] Add telemetry to report which EME files are missing

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla42
Tracking Status
firefox42 --- fixed

People

(Reporter: cpearce, Assigned: gerald)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 6 obsolete files)

We have telemetry to report when GMP DLL files disappear, but we don't know report when the plugin DLL's voucher file disappears. We've had reports from the field that the voucher file can disappear but the DLL and info files are still present. We'll want to drive down this number over time, so we need telemetry to report when they disappear.
Priority: -- → P1
Follow-up to bug 1156566.
Adding telemetry histograms to track exactly which GMP files are missing. I kept the previous coarse-grained histograms, so we can see the overall trend (and also because I was not sure I could just remove them while they may still be used by previous versions).
Attachment #8627538 - Flags: review?(vdjeric)
Assignee: nobody → gsquelart
Depends on: 1156566
Adding telemetry reporting about which GMP files are missing.
Attachment #8627539 - Flags: review?(cpearce)
Comment on attachment 8627538 [details] [diff] [review]
1174565-plugin-missing-files-histograms.patch

Just thought of something. Delaying review for now.
Attachment #8627538 - Flags: review?(vdjeric)
Comment on attachment 8627539 [details] [diff] [review]
1174565-plugin-missing-files-telemetry.patch

Just thought of something. Delaying review for now.
Attachment #8627539 - Flags: review?(cpearce)
Follow-up to bug 1156566.
Adding telemetry histogram to track exactly which GMP files are missing. I kept the previous coarse-grained histograms, so we can see the overall trend (and also because I was not sure I could just remove them while they may still be used by previous versions).
Attachment #8627538 - Attachment is obsolete: true
Attachment #8627666 - Flags: review?(vdjeric)
Adding telemetry reporting about which GMP files are missing.
Attachment #8627539 - Attachment is obsolete: true
Attachment #8627668 - Flags: review?(cpearce)
Comment on attachment 8627668 [details] [diff] [review]
1174565-plugin-missing-files-telemetry.patch

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

LGTM, spohl should also review this, he maintains GMPProvider.
Attachment #8627668 - Flags: review?(spohl.mozilla.bugs)
Attachment #8627668 - Flags: review?(cpearce)
Attachment #8627668 - Flags: review+
Comment on attachment 8627666 [details] [diff] [review]
1174565-plugin-missing-files-histograms.patch

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

::: toolkit/components/telemetry/Histograms.json
@@ +7833,5 @@
> +  "VIDEO_ADOBE_GMP_MISSING_FILES": {
> +    "alert_emails": ["cpearce@mozilla.com"],
> +    "expires_in_version": "42",
> +    "kind": "enumerated",
> +    "n_values" : 8,

you might want to to declare "n_values" larger than you need it to future-proof it.. it's not possible to re-declare this histogram property later

@@ +7834,5 @@
> +    "alert_emails": ["cpearce@mozilla.com"],
> +    "expires_in_version": "42",
> +    "kind": "enumerated",
> +    "n_values" : 8,
> +    "description": "Adobe EME GMP files missing (sum of: 1:library, 2:info, 4:voucher)",

will you log a "0" a value when no files are missing? it could provide a denominator for the error rate

@@ +7848,5 @@
> +  "VIDEO_OPENH264_GMP_MISSING_FILES": {
> +    "alert_emails": ["cpearce@mozilla.com", "rjesup@mozilla.com"],
> +    "expires_in_version": "42",
> +    "kind": "enumerated",
> +    "n_values" : 4,

ditto
Attachment #8627666 - Flags: review?(vdjeric) → review+
Comment on attachment 8627668 [details] [diff] [review]
1174565-plugin-missing-files-telemetry.patch

Following Vladan's comments, I'll change the code a bit to have "0" values; I'll also split this reporting into separate platforms.
Attachment #8627668 - Flags: review?(spohl.mozilla.bugs)
(In reply to Vladan Djeric (:vladan) -- please needinfo! from comment #8)
> Comment on attachment 8627666 [details] [diff] [review]
> 1174565-plugin-missing-files-histograms.patch
> 
> Review of attachment 8627666 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: toolkit/components/telemetry/Histograms.json
> @@ +7833,5 @@
> > +    "n_values" : 8,
> you might want to to declare "n_values" larger than you need it to
> future-proof it.. it's not possible to re-declare this histogram property
> later

After discussing it with :cpearce, we're confident this number won't grow larger, as the number of files in plugin most likely won't change.

> @@ +7834,5 @@
> > +    "description": "Adobe EME GMP files missing (sum of: 1:library, 2:info, 4:voucher)",
> 
> will you log a "0" a value when no files are missing? it could provide a
> denominator for the error rate

Good idea! Added "0" value.

We've also split histograms between Windows/Mac/Linux/Other systems.
Attachment #8627666 - Attachment is obsolete: true
Attachment #8628054 - Flags: review?(vdjeric)
Reporting GMP missing files, or 0 for no missing files.
Split telemetry between Windows/Mac/Linux/Other systems.
Attachment #8627668 - Attachment is obsolete: true
Attachment #8628055 - Flags: review?(spohl.mozilla.bugs)
Attachment #8628055 - Flags: review?(cpearce)
Comment on attachment 8628055 [details] [diff] [review]
1174565-plugin-missing-files-telemetry.patch

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

Make sure to check `./mach xpcshell-test toolkit/mozapps/extensions/test/xpcshell/test_gmpProvider.js`.
Attachment #8628055 - Flags: review?(cpearce) → review+
(In reply to Gerald Squelart [:gerald] from comment #10)
> After discussing it with :cpearce, we're confident this number won't grow
> larger, as the number of files in plugin most likely won't change.

That's fine then

> We've also split histograms between Windows/Mac/Linux/Other systems.

You don't need to do that, the Telemetry ping already reports OS details (including OS version) and the Telemetry dash shows the distributions separated by OS, e.g. GC_MAX_PAUSE_MS for Windows systems only http://mzl.la/1T4010p
Attachment #8628054 - Flags: review?(vdjeric) → review-
Comment on attachment 8628055 [details] [diff] [review]
1174565-plugin-missing-files-telemetry.patch

Will need another round of changes, as telemetry is already split by OSes.
Attachment #8628055 - Flags: review?(spohl.mozilla.bugs)
Adding histograms to track which Adobe/OpenH264 GMP files are missing.
(As per comment 13, no need to manually split by OS.)
Attachment #8628054 - Attachment is obsolete: true
Attachment #8628055 - Attachment is obsolete: true
Attachment #8628090 - Flags: review?(vdjeric)
Attachment #8628090 - Flags: review?(vdjeric) → review+
Reporting telemetry for GMP missing files, or 0 for no missing files.
Updated xpcshell tests.
Attachment #8628555 - Flags: review?(spohl.mozilla.bugs)
Attachment #8628555 - Flags: review?(cpearce)
Summary: [EME] Add telemetry to report which EME files missing → [EME] Add telemetry to report which EME files are missing
Attachment #8628555 - Flags: review?(cpearce) → review+
Attachment #8628555 - Flags: review?(spohl.mozilla.bugs) → review+
https://hg.mozilla.org/mozilla-central/rev/64ed41dd62fe
https://hg.mozilla.org/mozilla-central/rev/edb7fc17176f
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
You need to log in before you can comment on or make changes to this bug.