Closed Bug 764585 Opened 8 years ago Closed 7 years ago
make it even harder to get enumerated histograms wrong
Bug 763359, comment 3: Because multiple teams got tripped up by this, I highly recommend that we change the API so that one **cannot** create a linear histogram from (1-n) with n buckets, by adding a MOZ_STATIC_ASSERT for that condition. For the case where this is a false positive, the person who defines the telemetry can just change the bucket count to (n+1). For the case where this check would prevent a problem, it will avoid collecting bad data.
there's at least one sample of this for the macros.
(In reply to Ehsan Akhgari [:ehsan] from comment #2) > there's at least one sample of this for the macros. That is, http://mxr.mozilla.org/mozilla-central/source/toolkit/components/telemetry/Telemetry.cpp#196. Turns out that bugzilla comments typed in on a phone aren't that useful!
This blocks a FF 14 tracked bug.
This patch adds a check for high > n_buckets for both linear and exponential histograms, in an attempt to eliminate the common mistake of using a linear histogram with high == n_buckets == N to record values from 1 to N. It's still possible to make errors, of course, but I think this is the best we can do short of rewriting how the histogram code at its core works. WDYT?
Attachment #656089 - Flags: review?(taras.mozilla)
Comment on attachment 656089 [details] [diff] [review] patch excellent
Attachment #656089 - Flags: review?(taras.mozilla) → review+
Green build: https://tbpl.mozilla.org/?tree=Try&rev=5d50758559d2 https://hg.mozilla.org/integration/mozilla-inbound/rev/30c7ffa7bd97
Assignee: nobody → nfroyd
Status: NEW → ASSIGNED
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
You need to log in before you can comment on or make changes to this bug.