Fix SCRIPT_BLOCK_WRONG_MIME histogram

RESOLVED FIXED in Firefox 51

Status

()

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: evilpie, Assigned: ckerschb)

Tracking

unspecified
mozilla51
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox51 fixed)

Details

(Whiteboard: [domsecurity-active])

Attachments

(1 attachment)

Reporter

Description

3 years ago
Something went wrong, suddenly we get huge amount of results for the new added types, far exceeding the previous unknown values. And for some reason we get 11/12 instead of 10/11, which I added in bug 1299267.

I would like to understand what is going on. Do we I have to rename the probe?
Hoo boy. The Telemetry system _really_ doesn't like it when you change the number of buckets in a histogram.

The short answer is: Yes. If you want a histogram that does the same thing, but with a different number of buckets, you must write a new histogram (with a new name) that has the desired parameters.

+gfritzshce, who might know off the top of his head what (if anything) else needs to be done with the current, odd, histogram.
I would just rename the existing probe, e.g. to SCRIPT_BLOCK_WRONG_MIME_2.
rvitillo would know more about how this affects the aggregator etc.

I'd love to avoid people stumbling over this, bug 1261865 is the best option i can think of though (and i didn't have time to dig into commit hooks yet).
(In reply to Georg Fritzsche [:gfritzsche] from comment #2)
> I would just rename the existing probe, e.g. to SCRIPT_BLOCK_WRONG_MIME_2.

Renaming the probe would work. The aggregation system is based on the assumption that probes are immutable.
Assignee: nobody → ckerschb
Status: NEW → ASSIGNED
Whiteboard: [domsecurity-active]
Blocks: 1299267
No longer depends on: 1299267
Georg, can you sign off on that so we can get correct probes again?
Attachment #8789251 - Flags: review?(gfritzsche)
Comment on attachment 8789251 [details] [diff] [review]
bug_1300891_rename_script_block_mime_type.patch

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

I'm not a data collection peer [0], but this is just updating & renaming an existing probe.

For future measurements, note that we added "categorical" histograms, which improve these kind of histograms.
See e.g.: "TELEMETRY_TEST_CATEGORICAL", `LABELS_TELEMETRY_TEST_CATEGORICAL`, `AccumulateCategorical(id)`.
This currently shows up like an enumerated probe on t.m.o, but soon you'll see the labels displayed there.

0: https://wiki.mozilla.org/Firefox/Data_Collection

::: toolkit/components/telemetry/Histograms.json
@@ +5560,5 @@
>      "alert_emails": ["ckerschbaumer@mozilla.com"],
>      "bug_numbers": [1288361, 1299267],
>      "expires_in_version": "56",
>      "kind": "enumerated",
>      "n_values": 15,

Side note:
This is using a higher bucket count than you need. That is helpful as it leaves room for adding more entries in the future without the same issues.
Attachment #8789251 - Flags: review?(gfritzsche) → review+
(In reply to Georg Fritzsche [:gfritzsche] from comment #5)
> For future measurements, note that we added "categorical" histograms, which
> improve these kind of histograms.

Thanks for the info and the speedy review :-)

Comment 7

3 years ago
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/074fc3a0ac1f
Rename SCRIPT_BLOCK_WRONG_MIME to avoid confusing the telemetry probe. r=gfritzsche
Keywords: checkin-needed
Reporter

Comment 8

3 years ago
Thanks for fixing, I didn't realize everything would get so screwed up.

Comment 9

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/074fc3a0ac1f
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in before you can comment on or make changes to this bug.