Skip to content

Commit

Permalink
Use uint64 Count consistently in metric aggregation (#1430)
Browse files Browse the repository at this point in the history
* Use uint64 Count consistently

* Number
  • Loading branch information
jmacd authored Jan 6, 2021
1 parent 3a337d0 commit fe9d1f7
Show file tree
Hide file tree
Showing 16 changed files with 37 additions and 43 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- `NewExporter` from `exporters/otlp` now takes a `ProtocolDriver` as a parameter. (#1369)
- Many OTLP Exporter options became gRPC ProtocolDriver options. (#1369)
- Unify endpoint API that related to OTel exporter. (#1401)
- Metric aggregator Count() and histogram Bucket.Counts are consistently `uint64`. (1430)

### Removed

Expand Down
12 changes: 4 additions & 8 deletions exporters/otlp/internal/transform/metric.go
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,7 @@ func sumPoint(record export.Record, num number.Number, start, end time.Time, ek

// minMaxSumCountValue returns the values of the MinMaxSumCount Aggregator
// as discrete values.
func minMaxSumCountValues(a aggregation.MinMaxSumCount) (min, max, sum number.Number, count int64, err error) {
func minMaxSumCountValues(a aggregation.MinMaxSumCount) (min, max, sum number.Number, count uint64, err error) {
if min, err = a.Min(); err != nil {
return
}
Expand Down Expand Up @@ -531,7 +531,7 @@ func minMaxSumCount(record export.Record, a aggregation.MinMaxSumCount) (*metric
return m, nil
}

func histogramValues(a aggregation.Histogram) (boundaries []float64, counts []float64, err error) {
func histogramValues(a aggregation.Histogram) (boundaries []float64, counts []uint64, err error) {
var buckets aggregation.Buckets
if buckets, err = a.Histogram(); err != nil {
return
Expand Down Expand Up @@ -563,10 +563,6 @@ func histogramPoint(record export.Record, ek export.ExportKind, a aggregation.Hi
return nil, err
}

buckets := make([]uint64, len(counts))
for i := 0; i < len(counts); i++ {
buckets[i] = uint64(counts[i])
}
m := &metricpb.Metric{
Name: desc.Name(),
Description: desc.Description(),
Expand All @@ -584,7 +580,7 @@ func histogramPoint(record export.Record, ek export.ExportKind, a aggregation.Hi
StartTimeUnixNano: toNanos(record.StartTime()),
TimeUnixNano: toNanos(record.EndTime()),
Count: uint64(count),
BucketCounts: buckets,
BucketCounts: counts,
ExplicitBounds: boundaries,
},
},
Expand All @@ -601,7 +597,7 @@ func histogramPoint(record export.Record, ek export.ExportKind, a aggregation.Hi
StartTimeUnixNano: toNanos(record.StartTime()),
TimeUnixNano: toNanos(record.EndTime()),
Count: uint64(count),
BucketCounts: buckets,
BucketCounts: counts,
ExplicitBounds: boundaries,
},
},
Expand Down
4 changes: 2 additions & 2 deletions exporters/otlp/internal/transform/metric_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ func TestMinMaxSumCountValue(t *testing.T) {
assert.Equal(t, min, number.NewInt64Number(1))
assert.Equal(t, max, number.NewInt64Number(10))
assert.Equal(t, sum, number.NewInt64Number(11))
assert.Equal(t, count, int64(2))
assert.Equal(t, count, uint64(2))
}
}

Expand Down Expand Up @@ -369,7 +369,7 @@ func (te *testErrMinMaxSumCount) Max() (number.Number, error) {
return 0, te.err
}

func (te *testErrMinMaxSumCount) Count() (int64, error) {
func (te *testErrMinMaxSumCount) Count() (uint64, error) {
return 0, te.err
}

Expand Down
14 changes: 6 additions & 8 deletions sdk/export/metric/aggregation/aggregation.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ type (
// Count returns the number of values that were aggregated.
Count interface {
Aggregation
Count() (int64, error)
Count() (uint64, error)
}

// Min returns the minimum value over the set of values that were aggregated.
Expand Down Expand Up @@ -86,16 +86,14 @@ type (
// aggregating integers.
Boundaries []float64

// Counts are floating point numbers to account for
// the possibility of sampling which allows for
// non-integer count values.
Counts []float64
// Counts holds the count in each bucket.
Counts []uint64
}

// Histogram returns the count of events in pre-determined buckets.
Histogram interface {
Aggregation
Count() (int64, error)
Count() (uint64, error)
Sum() (number.Number, error)
Histogram() (Buckets, error)
}
Expand All @@ -106,7 +104,7 @@ type (
Min() (number.Number, error)
Max() (number.Number, error)
Sum() (number.Number, error)
Count() (int64, error)
Count() (uint64, error)
}

// Distribution supports the Min, Max, Sum, Count, and Quantile
Expand All @@ -116,7 +114,7 @@ type (
Min() (number.Number, error)
Max() (number.Number, error)
Sum() (number.Number, error)
Count() (int64, error)
Count() (uint64, error)
Quantile(float64) (number.Number, error)
}
)
Expand Down
8 changes: 4 additions & 4 deletions sdk/metric/aggregator/aggregatortest/test.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,8 @@ func (n *Numbers) Sum() number.Number {
return sum
}

func (n *Numbers) Count() int64 {
return int64(len(n.numbers))
func (n *Numbers) Count() uint64 {
return uint64(len(n.numbers))
}

func (n *Numbers) Min() number.Number {
Expand Down Expand Up @@ -223,7 +223,7 @@ func SynchronizedMoveResetTest(t *testing.T, mkind metric.InstrumentKind, nf fun

if count, ok := agg.(aggregation.Count); ok {
c, err := count.Count()
require.Equal(t, int64(0), c)
require.Equal(t, uint64(0), c)
require.NoError(t, err)
}

Expand Down Expand Up @@ -270,7 +270,7 @@ func SynchronizedMoveResetTest(t *testing.T, mkind metric.InstrumentKind, nf fun

if count, ok := agg.(aggregation.Count); ok {
c, err := count.Count()
require.Equal(t, int64(1), c)
require.Equal(t, uint64(1), c)
require.NoError(t, err)
}

Expand Down
4 changes: 2 additions & 2 deletions sdk/metric/aggregator/array/array.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ func (c *Aggregator) Sum() (number.Number, error) {
}

// Count returns the number of values in the checkpoint.
func (c *Aggregator) Count() (int64, error) {
return int64(len(c.points)), nil
func (c *Aggregator) Count() (uint64, error) {
return uint64(len(c.points)), nil
}

// Max returns the maximum value in the checkpoint.
Expand Down
4 changes: 2 additions & 2 deletions sdk/metric/aggregator/array/array_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func checkZero(t *testing.T, agg *Aggregator, desc *metric.Descriptor) {

count, err := agg.Count()
require.NoError(t, err)
require.Equal(t, int64(0), count)
require.Equal(t, uint64(0), count)

max, err := agg.Max()
require.True(t, errors.Is(err, aggregation.ErrNoData))
Expand Down Expand Up @@ -237,7 +237,7 @@ func TestArrayErrors(t *testing.T) {
require.NoError(t, agg.SynchronizedMove(ckpt, descriptor))

count, err := ckpt.Count()
require.Equal(t, int64(1), count, "NaN value was not counted")
require.Equal(t, uint64(1), count, "NaN value was not counted")
require.Nil(t, err)

num, err := ckpt.Quantile(0)
Expand Down
4 changes: 2 additions & 2 deletions sdk/metric/aggregator/ddsketch/ddsketch.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,8 @@ func (c *Aggregator) Sum() (number.Number, error) {
}

// Count returns the number of values in the checkpoint.
func (c *Aggregator) Count() (int64, error) {
return c.sketch.Count(), nil
func (c *Aggregator) Count() (uint64, error) {
return uint64(c.sketch.Count()), nil
}

// Max returns the maximum value in the checkpoint.
Expand Down
2 changes: 1 addition & 1 deletion sdk/metric/aggregator/ddsketch/ddsketch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func checkZero(t *testing.T, agg *Aggregator, desc *metric.Descriptor) {

count, err := agg.Count()
require.NoError(t, err)
require.Equal(t, int64(0), count)
require.Equal(t, uint64(0), count)

max, err := agg.Max()
require.True(t, errors.Is(err, aggregation.ErrNoData))
Expand Down
8 changes: 4 additions & 4 deletions sdk/metric/aggregator/histogram/histogram.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@ type (
// the sum and counts for all observed values and
// the less than equal bucket count for the pre-determined boundaries.
state struct {
bucketCounts []float64
bucketCounts []uint64
sum number.Number
count int64
count uint64
}
)

Expand Down Expand Up @@ -100,7 +100,7 @@ func (c *Aggregator) Sum() (number.Number, error) {
}

// Count returns the number of values in the checkpoint.
func (c *Aggregator) Count() (int64, error) {
func (c *Aggregator) Count() (uint64, error) {
return c.state.count, nil
}

Expand Down Expand Up @@ -135,7 +135,7 @@ func (c *Aggregator) SynchronizedMove(oa export.Aggregator, desc *metric.Descrip

func emptyState(boundaries []float64) state {
return state{
bucketCounts: make([]float64, len(boundaries)+1),
bucketCounts: make([]uint64, len(boundaries)+1),
}
}

Expand Down
2 changes: 1 addition & 1 deletion sdk/metric/aggregator/histogram/histogram_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func checkZero(t *testing.T, agg *histogram.Aggregator, desc *metric.Descriptor)
require.NoError(t, err)

count, err := agg.Count()
require.Equal(t, int64(0), count, "Empty checkpoint count = 0")
require.Equal(t, uint64(0), count, "Empty checkpoint count = 0")
require.NoError(t, err)

buckets, err := agg.Histogram()
Expand Down
4 changes: 2 additions & 2 deletions sdk/metric/aggregator/minmaxsumcount/mmsc.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ type (
sum number.Number
min number.Number
max number.Number
count int64
count uint64
}
)

Expand Down Expand Up @@ -78,7 +78,7 @@ func (c *Aggregator) Sum() (number.Number, error) {
}

// Count returns the number of values in the checkpoint.
func (c *Aggregator) Count() (int64, error) {
func (c *Aggregator) Count() (uint64, error) {
return c.count, nil
}

Expand Down
4 changes: 2 additions & 2 deletions sdk/metric/aggregator/minmaxsumcount/mmsc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ func checkZero(t *testing.T, agg *Aggregator, desc *metric.Descriptor) {

count, err := agg.Count()
require.NoError(t, err)
require.Equal(t, int64(0), count)
require.Equal(t, uint64(0), count)

max, err := agg.Max()
require.True(t, errors.Is(err, aggregation.ErrNoData))
Expand Down Expand Up @@ -228,7 +228,7 @@ func TestMaxSumCountNotSet(t *testing.T) {
require.Nil(t, err)

count, err := ckpt.Count()
require.Equal(t, int64(0), count, "Empty checkpoint count = 0")
require.Equal(t, uint64(0), count, "Empty checkpoint count = 0")
require.Nil(t, err)

max, err := ckpt.Max()
Expand Down
2 changes: 1 addition & 1 deletion sdk/metric/correct_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ func TestInputRangeValueRecorder(t *testing.T) {
checkpointed = sdk.Collect(ctx)

count, err := processor.accumulations[0].Aggregator().(aggregation.Distribution).Count()
require.Equal(t, int64(2), count)
require.Equal(t, uint64(2), count)
require.Equal(t, 1, checkpointed)
require.Nil(t, testHandler.Flush())
require.Nil(t, err)
Expand Down
5 changes: 2 additions & 3 deletions sdk/metric/histogram_stress_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,9 @@ func TestStressInt64Histogram(t *testing.T) {
b, _ := ckpt.Histogram()
c, _ := ckpt.Count()

var realCount int64
var realCount uint64
for _, c := range b.Counts {
v := int64(c)
realCount += v
realCount += c
}

if realCount != c {
Expand Down
2 changes: 1 addition & 1 deletion sdk/metric/minmaxsumcount_stress_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func TestStressInt64MinMaxSumCount(t *testing.T) {
}
lo, hi, sum := min.AsInt64(), max.AsInt64(), s.AsInt64()

if hi-lo+1 != c {
if uint64(hi-lo)+1 != c {
t.Fail()
}
if c == 1 {
Expand Down

0 comments on commit fe9d1f7

Please sign in to comment.