add animated image specific probes for several imagelib telemetry probes

RESOLVED FIXED in Firefox 55

Status

()

Core
ImageLib
RESOLVED FIXED
9 months ago
8 months ago

People

(Reporter: tnikkel, Assigned: tnikkel)

Tracking

Trunk
mozilla55
Points:
---

Firefox Tracking Flags

(firefox55 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

9 months ago
To verify that discarding of animated images is performing as expected we need a bit more telemetry data. The current probes mix animated with non-animated. We expect non-animated numbers to stay the same. So we won't be able to really tell how the numbers are changing for animated images.
(Assignee)

Comment 1

9 months ago
Created attachment 8847291 [details] [diff] [review]
patch
Attachment #8847291 - Flags: review?(aosmond)
(Assignee)

Updated

9 months ago
Attachment #8847291 - Flags: review?(gfritzsche)
Comment on attachment 8847291 [details] [diff] [review]
patch

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

::: toolkit/components/telemetry/histogram-whitelists.json
@@ +363,5 @@
>      "HTTP_TRANSACTION_USE_ALTSVC",
>      "HTTP_TRANSACTION_USE_ALTSVC_OE",
>      "IMAGE_DECODE_CHUNKS",
>      "IMAGE_DECODE_COUNT",
> +    "IMAGE_ANIMATED_DECODE_COUNT",

You should not add any new entries to the whitelist.
Please just fill out the required fields in Histograms.json.
Attachment #8847291 - Flags: review?(gfritzsche)
Are there any other specific questions for me?

Note that you will still need data review after technical review:
https://wiki.mozilla.org/Firefox/Data_Collection
(Assignee)

Comment 4

8 months ago
Created attachment 8847985 [details] [diff] [review]
patch v2

Removed the whilelist bits.
Attachment #8847985 - Flags: review?(aosmond)
Attachment #8847985 - Flags: feedback?(benjamin)
(Assignee)

Comment 5

8 months ago
(In reply to Georg Fritzsche [:gfritzsche] [at workweek] from comment #2)
> You should not add any new entries to the whitelist.
> Please just fill out the required fields in Histograms.json.

Okay, I don't know what the whitelist is for, I just cargo-culted the existing probes.

(In reply to Georg Fritzsche [:gfritzsche] [at workweek] from comment #3)
> Are there any other specific questions for me?

I don't know, is there anything else I need to do?

> Note that you will still need data review after technical review:
> https://wiki.mozilla.org/Firefox/Data_Collection

I asked bsmedberg for feedback as that page says.
(Assignee)

Updated

8 months ago
Attachment #8847291 - Attachment is obsolete: true
Attachment #8847291 - Flags: review?(aosmond)
Comment on attachment 8847985 [details] [diff] [review]
patch v2

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

::: toolkit/components/telemetry/Histograms.json
@@ +1455,5 @@
> +    "low": 50,
> +    "high": 50000000,
> +    "n_buckets": 100,
> +    "description": "Time from starting a decode of an animated image to it showing up on the screen (us)"
> +  },

Maybe it has changed, but without the whitelist bits, I think you need to specify the bug_numbers and alert_emails fields (see my removal of a probe in bug 1332005). The alert_email at least will cause an email to be sent when the probe is near/at expiry as a reminder which is handy.
Attachment #8847985 - Flags: review?(aosmond) → review+

Updated

8 months ago
Attachment #8847985 - Flags: feedback?(benjamin) → feedback+
(Assignee)

Comment 7

8 months ago
(In reply to Andrew Osmond [:aosmond] from comment #6)
> Maybe it has changed, but without the whitelist bits, I think you need to
> specify the bug_numbers and alert_emails fields (see my removal of a probe
> in bug 1332005). The alert_email at least will cause an email to be sent
> when the probe is near/at expiry as a reminder which is handy.

Yeah, this appears to be enforced by the build system. I got build errors until I fixed this. Now I know what the whitelist is for.

Comment 8

8 months ago
Pushed by tnikkel@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ea7ac56cdb2e
Add animated image specific probes for several imagelib telemetry probes. r=aosmond f=bsmedberg

Comment 9

8 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/ea7ac56cdb2e
Status: NEW → RESOLVED
Last Resolved: 8 months ago
status-firefox55: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.