Skip to content

Commit

Permalink
Change default histogram buckets from {-infinity,0} to {-infinity,fir…
Browse files Browse the repository at this point in the history
…st_defined} (#61)
  • Loading branch information
katezaps authored and robskillington committed Sep 30, 2017
1 parent 0789c8c commit 95078a8
Show file tree
Hide file tree
Showing 3 changed files with 3 additions and 38 deletions.
21 changes: 3 additions & 18 deletions histogram.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ func (v DurationBuckets) AsDurations() []time.Duration {
func BucketPairs(buckets Buckets) []BucketPair {
if buckets == nil || buckets.Len() < 1 {
return []BucketPair{

bucketPair{
lowerBoundValue: -math.MaxFloat64,
upperBoundValue: math.MaxFloat64,
Expand All @@ -134,24 +135,6 @@ func BucketPairs(buckets Buckets) []BucketPair {
}
}

if durationBuckets, ok := buckets.(DurationBuckets); ok {
// If using duration buckets separating negative times and
// positive times is very much desirable as depending on the
// reporter will create buckets "-infinity,0" and "0,{first_bucket}"
// instead of just "-infinity,{first_bucket}" which for time
// durations is not desirable nor pragmatic
hasZero := false
for _, b := range buckets.AsDurations() {
if b == 0 {
hasZero = true
break
}
}
if !hasZero {
buckets = append(DurationBuckets{0}, durationBuckets...)
}
}

// Sort before iterating to create pairs
sort.Sort(buckets)

Expand All @@ -160,6 +143,7 @@ func BucketPairs(buckets Buckets) []BucketPair {
asDurationBuckets = buckets.AsDurations()
pairs = make([]BucketPair, 0, buckets.Len()+2)
)

pairs = append(pairs, bucketPair{
lowerBoundValue: -math.MaxFloat64,
upperBoundValue: asValueBuckets[0],
Expand All @@ -169,6 +153,7 @@ func BucketPairs(buckets Buckets) []BucketPair {

prevValueBucket, prevDurationBucket :=
asValueBuckets[0], asDurationBuckets[0]

for i := 1; i < buckets.Len(); i++ {
pairs = append(pairs, bucketPair{
lowerBoundValue: prevValueBucket,
Expand Down
19 changes: 0 additions & 19 deletions histogram_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,25 +86,6 @@ func TestBucketPairsSortsDurationBuckets(t *testing.T) {
assert.Equal(t, time.Duration(math.MaxInt64), pairs[3].UpperBoundDuration())
}

func TestBucketPairsDurationBucketsInsertsMissingZero(t *testing.T) {
initial := 10
buckets, err := LinearDurationBuckets(
10*time.Millisecond,
10*time.Millisecond,
initial,
)
require.NoError(t, err)
require.Equal(t, initial, len(buckets))

pairs := BucketPairs(buckets)
assert.Equal(t, initial+2, len(pairs))
assert.Equal(t, time.Duration(math.MinInt64), pairs[0].LowerBoundDuration())
assert.Equal(t, time.Duration(0), pairs[0].UpperBoundDuration())

assert.Equal(t, 100*time.Millisecond, pairs[len(pairs)-1].LowerBoundDuration())
assert.Equal(t, time.Duration(math.MaxInt64), pairs[len(pairs)-1].UpperBoundDuration())
}

func TestMustMakeLinearValueBuckets(t *testing.T) {
assert.NotPanics(t, func() {
assert.Equal(t, ValueBuckets{
Expand Down
1 change: 0 additions & 1 deletion scope_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -689,7 +689,6 @@ func TestSnapshot(t *testing.T) {

assert.EqualValues(t, map[float64]int64(nil), histograms["foo.buzz+env=test"].Values())
assert.EqualValues(t, map[time.Duration]int64{
0: 0,
time.Second * 2: 1,
time.Second * 4: 0,
math.MaxInt64: 0,
Expand Down

0 comments on commit 95078a8

Please sign in to comment.