Closed
Bug 1300891
Opened 6 years ago
Closed 6 years ago
Fix SCRIPT_BLOCK_WRONG_MIME histogram
Categories
(Core :: DOM: Security, defect)
Core
DOM: Security
Tracking
()
RESOLVED
FIXED
mozilla51
Tracking | Status | |
---|---|---|
firefox51 | --- | fixed |
People
(Reporter: evilpie, Assigned: ckerschb)
References
Details
(Whiteboard: [domsecurity-active])
Attachments
(1 file)
5.59 KB,
patch
|
gfritzsche
:
review+
|
Details | Diff | Splinter Review |
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•6 years ago
|
||
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•6 years ago
|
||
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•6 years ago
|
||
(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 | ||
Updated•6 years ago
|
Assignee: nobody → ckerschb
Status: NEW → ASSIGNED
Whiteboard: [domsecurity-active]
Assignee | ||
Updated•6 years ago
|
Assignee | ||
Comment 4•6 years ago
|
||
Georg, can you sign off on that so we can get correct probes again?
Attachment #8789251 -
Flags: review?(gfritzsche)
Comment 5•6 years ago
|
||
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+
Assignee | ||
Comment 6•6 years ago
|
||
(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 :-)
Assignee | ||
Updated•6 years ago
|
Keywords: checkin-needed
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•6 years ago
|
||
Thanks for fixing, I didn't realize everything would get so screwed up.
Comment 9•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/074fc3a0ac1f
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox51:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in
before you can comment on or make changes to this bug.
Description
•