Last Comment Bug 764585 - make it even harder to get enumerated histograms wrong
: make it even harder to get enumerated histograms wrong
Status: RESOLVED FIXED
:
Product: Toolkit
Classification: Components
Component: Telemetry (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla18
Assigned To: Nathan Froyd [:froydnj]
:
Mentors:
Depends on: 781200 785959
Blocks: 763359
  Show dependency treegraph
 
Reported: 2012-06-13 14:23 PDT by Nathan Froyd [:froydnj]
Modified: 2012-08-29 06:40 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (1.25 KB, patch)
2012-08-28 10:46 PDT, Nathan Froyd [:froydnj]
taras.mozilla: review+
Details | Diff | Splinter Review

Description Nathan Froyd [:froydnj] 2012-06-13 14:23:28 PDT
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.
Comment 1 (dormant account) 2012-06-13 15:45:08 PDT
r+
Comment 2 :Ehsan Akhgari 2012-06-14 14:22:08 PDT
there's at least one sample of this for the macros.
Comment 3 :Ehsan Akhgari 2012-06-14 14:44:42 PDT
(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!
Comment 4 David Bolter [:davidb] ***PTO until 29th*** 2012-06-19 10:55:23 PDT
This blocks a FF 14 tracked bug.
Comment 5 Nathan Froyd [:froydnj] 2012-08-28 10:46:45 PDT
Created attachment 656089 [details] [diff] [review]
patch

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?
Comment 6 (dormant account) 2012-08-28 10:55:23 PDT
Comment on attachment 656089 [details] [diff] [review]
patch

excellent
Comment 8 Ed Morley [:emorley] 2012-08-29 06:40:40 PDT
https://hg.mozilla.org/mozilla-central/rev/30c7ffa7bd97

Note You need to log in before you can comment on or make changes to this bug.