Skip to content

Commit

Permalink
Address request changes
Browse files Browse the repository at this point in the history
  • Loading branch information
@codebien
codebien committed Jul 7, 2023
1 parent b7cb2b3 commit e7bee24
Show file tree
Hide file tree
Showing 4 changed files with 131 additions and 121 deletions.
40 changes: 23 additions & 17 deletions output/cloud/expv2/hdr.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,20 +67,28 @@ type histogram struct {
Count uint32
}

func newHistogram() *histogram {
return &histogram{
Buckets: make(map[uint32]uint32),
Max: -math.MaxFloat64,
Min: math.MaxFloat64,
}
}

// addToBucket increments the counter of the bucket of the provided value.
// If the value is lower or higher than the trackable limits
// then it is counted into specific buckets. All the stats are also updated accordingly.
func (h *histogram) addToBucket(v float64) {
if h.Count == 0 {
h.Max, h.Min = v, v
} else {
if v > h.Max {
h.Max = v
}
if v < h.Min {
h.Min = v
}
// if h.Count == 0 {
// h.Max, h.Min = v, v
// } else {
if v > h.Max {
h.Max = v
}
if v < h.Min {
h.Min = v
}
// }

h.Count++
h.Sum += v
Expand All @@ -99,9 +107,6 @@ func (h *histogram) addToBucket(v float64) {
h.trackBucket(ix)
}

if h.Buckets == nil {
h.Buckets = make(map[uint32]uint32)
}
h.Buckets[ix]++
}

Expand All @@ -117,10 +122,10 @@ func (h *histogram) trackBucket(index uint32) {
return
}

// insert in the middle
h.Indexes = append(h.Indexes, 0) // expand the slice
copy(h.Indexes[i+1:], h.Indexes[i:]) // make the space
h.Indexes[i] = index // set the index
// insert at specific `i`
h.Indexes = append(h.Indexes, 0)
copy(h.Indexes[i+1:], h.Indexes[i:])
h.Indexes[i] = index
}

// histogramAsProto converts the histogram into the equivalent Protobuf version.
Expand All @@ -130,7 +135,8 @@ func histogramAsProto(h *histogram, time int64) *pbcloud.TrendHdrValue {
spans []*pbcloud.BucketSpan
)

// allocate only if at least one item is available
// allocate only if at least one item is available, in the case of only
// untrackable values, then Indexes and Buckets are expected to be empty.
if len(h.Indexes) > 0 {
// init the counters
counters = make([]uint32, 1, len(h.Indexes))
Expand Down
203 changes: 101 additions & 102 deletions output/cloud/expv2/hdr_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ package expv2

import (
"math"
"strconv"
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.k6.io/k6/output/cloud/expv2/pbcloud"
"google.golang.org/protobuf/types/known/timestamppb"
)
Expand Down Expand Up @@ -42,105 +42,101 @@ func TestResolveBucketIndex(t *testing.T) {
}
}

func TestHistogramAddWithSimpleValue(t *testing.T) {
func TestHistogramAddWithSimpleValues(t *testing.T) {
t.Parallel()

// Zero as value
res := histogram{}
res.Add(0)
exp := histogram{
Buckets: map[uint32]uint32{0: 1},
Indexes: []uint32{0},
ExtraLowBucket: 0,
ExtraHighBucket: 0,
Max: 0,
Min: 0,
Sum: 0,
Count: 1,
}
require.Equal(t, exp, res)

// Add a lower bucket index within slice capacity
res = histogram{}
res.Add(8)
res.Add(5)

exp = histogram{
Buckets: map[uint32]uint32{5: 1, 8: 1},
Indexes: []uint32{5, 8},
ExtraLowBucket: 0,
ExtraHighBucket: 0,
Max: 8,
Min: 5,
Sum: 13,
Count: 2,
}
require.Equal(t, exp, res)

// Add a higher bucket index within slice capacity
res = histogram{}
res.Add(100)
res.Add(101)

exp = histogram{
Buckets: map[uint32]uint32{100: 1, 101: 1},
Indexes: []uint32{100, 101},
ExtraLowBucket: 0,
ExtraHighBucket: 0,
Max: 101,
Min: 100,
Sum: 201,
Count: 2,
}
require.Equal(t, exp, res)

// Same case but reversed test check
res = histogram{}
res.Add(101)
res.Add(100)

exp = histogram{
Buckets: map[uint32]uint32{100: 1, 101: 1},
Indexes: []uint32{100, 101},
ExtraLowBucket: 0,
ExtraHighBucket: 0,
Max: 101,
Min: 100,
Sum: 201,
Count: 2,
cases := []struct {
vals []float64
exp histogram
}{
{
vals: []float64{0},
exp: histogram{
Buckets: map[uint32]uint32{0: 1},
Indexes: []uint32{0},
ExtraLowBucket: 0,
ExtraHighBucket: 0,
Max: 0,
Min: 0,
Sum: 0,
Count: 1,
},
},
{
vals: []float64{8, 5},
exp: histogram{
Buckets: map[uint32]uint32{5: 1, 8: 1},
Indexes: []uint32{5, 8},
ExtraLowBucket: 0,
ExtraHighBucket: 0,
Max: 8,
Min: 5,
Sum: 13,
Count: 2,
},
},
{
vals: []float64{8, 9, 10, 5},
exp: histogram{
Buckets: map[uint32]uint32{8: 1, 9: 1, 10: 1, 5: 1},
Indexes: []uint32{5, 8, 9, 10},
ExtraLowBucket: 0,
ExtraHighBucket: 0,
Max: 10,
Min: 5,
Sum: 32,
Count: 4,
},
},
{
vals: []float64{100, 101},
exp: histogram{
Buckets: map[uint32]uint32{100: 1, 101: 1},
Indexes: []uint32{100, 101},
ExtraLowBucket: 0,
ExtraHighBucket: 0,
Max: 101,
Min: 100,
Sum: 201,
Count: 2,
},
},
{
vals: []float64{101, 100},
exp: histogram{
Buckets: map[uint32]uint32{100: 1, 101: 1},
Indexes: []uint32{100, 101},
ExtraLowBucket: 0,
ExtraHighBucket: 0,
Max: 101,
Min: 100,
Sum: 201,
Count: 2,
},
},
}
assert.Equal(t, exp, res)

// One more complex case with lower index and more than two indexes
res = histogram{}
res.Add(8)
res.Add(9)
res.Add(10)
res.Add(5)

exp = histogram{
Buckets: map[uint32]uint32{8: 1, 9: 1, 10: 1, 5: 1},
Indexes: []uint32{5, 8, 9, 10},
ExtraLowBucket: 0,
ExtraHighBucket: 0,
Max: 10,
Min: 5,
Sum: 32,
Count: 4,
for i, tc := range cases {
tc := tc
t.Run(strconv.Itoa(i), func(t *testing.T) {
h := newHistogram()
for _, v := range tc.vals {
h.Add(v)
}
assert.Equal(t, &tc.exp, h)
})
}

assert.Equal(t, exp, res)
}

func TestHistogramAddWithUntrackables(t *testing.T) {
t.Parallel()

res := histogram{}
res := newHistogram()
for _, v := range []float64{5, -3.14, 2 * 1e9, 1} {
res.Add(v)
}

exp := histogram{
exp := &histogram{
Buckets: map[uint32]uint32{1: 1, 5: 1},
Indexes: []uint32{1, 5},
ExtraLowBucket: 1,
Expand All @@ -156,12 +152,12 @@ func TestHistogramAddWithUntrackables(t *testing.T) {
func TestHistogramAddWithMultipleOccurances(t *testing.T) {
t.Parallel()

res := histogram{}
res := newHistogram()
for _, v := range []float64{51.8, 103.6, 103.6, 103.6, 103.6} {
res.Add(v)
}

exp := histogram{
exp := &histogram{
Buckets: map[uint32]uint32{52: 1, 104: 4},
Indexes: []uint32{52, 104},
Max: 103.6,
Expand All @@ -177,13 +173,13 @@ func TestHistogramAddWithMultipleOccurances(t *testing.T) {
func TestHistogramAddWithNegativeNum(t *testing.T) {
t.Parallel()

res := histogram{}
res := newHistogram()
res.Add(-2.42314)

exp := histogram{
exp := &histogram{
Max: -2.42314,
Min: -2.42314,
Buckets: nil,
Buckets: map[uint32]uint32{},
ExtraLowBucket: 1,
ExtraHighBucket: 0,
Sum: -2.42314,
Expand All @@ -194,13 +190,13 @@ func TestHistogramAddWithNegativeNum(t *testing.T) {

func TestHistogramAddWithMultipleNegativeNums(t *testing.T) {
t.Parallel()
res := histogram{}
res := newHistogram()
for _, v := range []float64{-0.001, -0.001, -0.001} {
res.Add(v)
}

exp := histogram{
Buckets: nil,
exp := &histogram{
Buckets: map[uint32]uint32{},
ExtraLowBucket: 3,
ExtraHighBucket: 0,
Max: -0.001,
Expand All @@ -214,13 +210,13 @@ func TestHistogramAddWithMultipleNegativeNums(t *testing.T) {
func TestNewHistoramWithNoVals(t *testing.T) {
t.Parallel()

res := histogram{}
exp := histogram{
Buckets: nil,
res := newHistogram()
exp := &histogram{
Buckets: map[uint32]uint32{},
ExtraLowBucket: 0,
ExtraHighBucket: 0,
Max: 0,
Min: 0,
Max: -math.MaxFloat64,
Min: math.MaxFloat64,
Sum: 0,
}
assert.Equal(t, exp, res)
Expand All @@ -240,7 +236,10 @@ func TestHistogramAsProto(t *testing.T) {
}{
{
name: "empty histogram",
exp: &pbcloud.TrendHdrValue{},
exp: &pbcloud.TrendHdrValue{
MaxValue: -math.MaxFloat64,
MinValue: math.MaxFloat64,
},
},
{
name: "not trackable values",
Expand Down Expand Up @@ -317,11 +316,11 @@ func TestHistogramAsProto(t *testing.T) {
}

for _, tc := range cases {
h := histogram{}
h := newHistogram()
for _, v := range tc.vals {
h.Add(v)
}
tc.exp.Time = &timestamppb.Timestamp{Seconds: 1}
assert.Equal(t, tc.exp, histogramAsProto(&h, time.Unix(1, 0).UnixNano()), tc.name)
assert.Equal(t, tc.exp, histogramAsProto(h, time.Unix(1, 0).UnixNano()), tc.name)
}
}
2 changes: 1 addition & 1 deletion output/cloud/expv2/sink.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func newMetricValue(mt metrics.MetricType) metricValue {
case metrics.Rate:
am = &rate{}
case metrics.Trend:
am = &histogram{}
am = newHistogram()
default:
// Should not be possible to create
// an invalid metric type except for specific
Expand Down
7 changes: 6 additions & 1 deletion output/cloud/expv2/sink_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package expv2

import (
"math"
"testing"

"github.com/stretchr/testify/assert"
Expand All @@ -16,7 +17,11 @@ func TestNewSink(t *testing.T) {
{metrics.Counter, &counter{}},
{metrics.Gauge, &gauge{}},
{metrics.Rate, &rate{}},
{metrics.Trend, &histogram{}},
{metrics.Trend, &histogram{
Buckets: map[uint32]uint32{},
Max: -math.MaxFloat64,
Min: math.MaxFloat64},
},
}
for _, tc := range tests {
assert.Equal(t, tc.exp, newMetricValue(tc.mt))
Expand Down

0 comments on commit e7bee24

Please sign in to comment.