From 95078a8f10668bd1fa73ae46761cdc58d25436b8 Mon Sep 17 00:00:00 2001 From: Katie Tezapsidis Date: Fri, 29 Sep 2017 22:52:23 -0400 Subject: [PATCH] Change default histogram buckets from {-infinity,0} to {-infinity,first_defined} (#61) --- histogram.go | 21 +++------------------ histogram_test.go | 19 ------------------- scope_test.go | 1 - 3 files changed, 3 insertions(+), 38 deletions(-) diff --git a/histogram.go b/histogram.go index 8e531d8a..1ad45ee1 100644 --- a/histogram.go +++ b/histogram.go @@ -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, @@ -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) @@ -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], @@ -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, diff --git a/histogram_test.go b/histogram_test.go index 1c061e17..2a4a4b35 100644 --- a/histogram_test.go +++ b/histogram_test.go @@ -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{ diff --git a/scope_test.go b/scope_test.go index 2d800331..a4a19f3e 100644 --- a/scope_test.go +++ b/scope_test.go @@ -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,