Closed Bug 1300891 Opened 6 years ago Closed 6 years ago

Fix SCRIPT_BLOCK_WRONG_MIME histogram

Categories

(Core :: DOM: Security, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox51 --- fixed

People

(Reporter: evilpie, Assigned: ckerschb)

References

Details

(Whiteboard: [domsecurity-active])

Attachments

(1 file)

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 :-)
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
Thanks for fixing, I didn't realize everything would get so screwed up.
https://hg.mozilla.org/mozilla-central/rev/074fc3a0ac1f
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in before you can comment on or make changes to this bug.