Skip to content

Commit

Permalink
telemetry: fix the integer quantization
Browse files Browse the repository at this point in the history
The code was previously quantizing 100 to 10, and only
101 and higher to 100. This patch fixes it.

Additionally, this code ensures that negative values also get
quantized. This change does not impact any caller for now (there is no
caller that expects negative value) but will be used in an upcoming
setting telemetry change.

Release justification: low risk, high benefit changes to existing functionality

Release note: None
  • Loading branch information
knz committed Aug 28, 2020
1 parent d09d0c8 commit a677e80
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 5 deletions.
17 changes: 12 additions & 5 deletions pkg/server/telemetry/features.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ package telemetry

import (
"fmt"
"math"
"sync/atomic"

"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
Expand All @@ -27,16 +28,22 @@ import (
// raw numbers.
// The numbers 0-10 are reported unchanged.
func Bucket10(num int64) int64 {
if num <= 0 {
return 0
if num == math.MinInt64 {
// This is needed to prevent overflow in the negation below.
return -1000000000000000000
}
sign := int64(1)
if num < 0 {
sign = -1
num = -num
}
if num < 10 {
return num
return num * sign
}
res := int64(10)
for ; res < 1000000000000000000 && res*10 < num; res *= 10 {
for ; res < 1000000000000000000 && res*10 <= num; res *= 10 {
}
return res
return res * sign
}

// CountBucketed counts the feature identified by prefix and the value, using
Expand Down
51 changes: 51 additions & 0 deletions pkg/server/telemetry/features_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
package telemetry_test

import (
"math"
"sync"
"testing"

Expand Down Expand Up @@ -40,3 +41,53 @@ func TestGetCounterDoesNotRace(t *testing.T) {
}
require.Len(t, counterSet, 1)
}

// TestBucket checks integer quantization.
func TestBucket(t *testing.T) {
testData := []struct {
input int64
expected int64
}{
{0, 0},
{1, 1},
{2, 2},
{3, 3},
{4, 4},
{5, 5},
{6, 6},
{7, 7},
{8, 8},
{9, 9},
{10, 10},
{11, 10},
{20, 10},
{99, 10},
{100, 100},
{101, 100},
{200, 100},
{999, 100},
{1000, 1000},
{math.MaxInt64, 1000000000000000000},
{-1, -1},
{-2, -2},
{-3, -3},
{-4, -4},
{-5, -5},
{-6, -6},
{-7, -7},
{-8, -8},
{-9, -9},
{-10, -10},
{-11, -10},
{-20, -10},
{-100, -100},
{-200, -100},
{math.MinInt64, -1000000000000000000},
}

for _, tc := range testData {
if actual, expected := telemetry.Bucket10(tc.input), tc.expected; actual != expected {
t.Errorf("%d: expected %d, got %d", tc.input, expected, actual)
}
}
}

0 comments on commit a677e80

Please sign in to comment.