1300891 - Fix SCRIPT_BLOCK_WRONG_MIME histogram

Status: RESOLVED FIXED

(Core :: DOM: Security, defect)

Description

[Tom Schuster [:evilpie]]

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?

Comment 1

[Chris H-C :chutten]

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.

Comment 2

[Georg Fritzsche [:gfritzsche]]

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).

Comment 3

[Roberto Agostino Vitillo (:rvitillo)]

(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.

Comment 4

[Christoph Kerschbaumer [:ckerschb]]

Attachment: bug_1300891_rename_script_block_mime_type.patch

Georg, can you sign off on that so we can get correct probes again?

Comment 5

[Georg Fritzsche [: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": ["[e-mail]"],
>      "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.

Comment 6

[Christoph Kerschbaumer [:ckerschb]]

(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

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/074fc3a0ac1f
Rename SCRIPT_BLOCK_WRONG_MIME to avoid confusing the telemetry probe. r=gfritzsche

Comment 8

[Tom Schuster [:evilpie]]

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

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/074fc3a0ac1f