Skip to content

Commit

Permalink
Implement min/max for the expo-histogram (#215)
Browse files Browse the repository at this point in the history
  • Loading branch information
jmacd authored Jul 11, 2022
1 parent 791a400 commit f391a22
Show file tree
Hide file tree
Showing 7 changed files with 71 additions and 2 deletions.
2 changes: 2 additions & 0 deletions lightstep/sdk/metric/aggregator/aggregation/aggregation.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ type (
ZeroCount() uint64
Positive() Buckets
Negative() Buckets
Min() number.Number
Max() number.Number
}

// Buckets describes a range of consecutive buckets, starting
Expand Down
25 changes: 25 additions & 0 deletions lightstep/sdk/metric/aggregator/histogram/exponential.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,19 @@ func (s *State[N, Traits]) UpdateByIncr(number N, incr uint64) {

value := float64(number)

// Maintain min and max
if s.count == 0 {
s.min = number
s.max = number
} else {
if number < s.min {
s.min = number
}
if number > s.max {
s.max = number
}
}

// Note: Not checking for overflow here. TODO.
s.count += incr

Expand Down Expand Up @@ -338,6 +351,18 @@ func (s *State[N, Traits]) Merge(o *State[N, Traits]) {
s.lock.Lock()
defer s.lock.Unlock()

if s.count == 0 {
s.min = o.min
s.max = o.max
} else if o.count != 0 {
if o.min < s.min {
s.min = o.min
}
if o.max > s.max {
s.max = o.max
}
}

// Note: Not checking for overflow here. TODO.
s.sum += o.sum
s.count += o.count
Expand Down
6 changes: 6 additions & 0 deletions lightstep/sdk/metric/aggregator/histogram/exponential_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -765,6 +765,12 @@ func TestAggregatorCopyMove(t *testing.T) {
requireEqualValues(t, h2, h3)
}

func TestAggregatorMinMax(t *testing.T) {
h := NewFloat64(aggregator.HistogramConfig{}, 1, 3, 5, 7, 9)
require.Equal(t, 1.0, number.ToFloat64(h.Min()))
require.Equal(t, 9.0, number.ToFloat64(h.Max()))
}

// Benchmarks the Update() function for values in the range [1,2)
func BenchmarkLinear(b *testing.B) {
src := rand.NewSource(77777677777)
Expand Down
18 changes: 18 additions & 0 deletions lightstep/sdk/metric/aggregator/histogram/histogram.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@ type (
// zeroCount is incremented by 1 when the measured
// value is exactly 0.
zeroCount uint64
// min is set when count > 0
min N
// max is set when count > 0
max N
// positive holds the positive values
positive buckets
// negative holds the negative values in these buckets
Expand Down Expand Up @@ -147,6 +151,18 @@ func (h *State[N, Traits]) Sum() number.Number {
return t.ToNumber(h.sum)
}

// Min implements aggregation.Histogram.
func (h *State[N, Traits]) Min() number.Number {
var t Traits
return t.ToNumber(h.min)
}

// Max implements aggregation.Histogram.
func (h *State[N, Traits]) Max() number.Number {
var t Traits
return t.ToNumber(h.max)
}

// Count implements aggregation.Histogram.
func (h *State[N, Traits]) Count() uint64 {
return h.count
Expand Down Expand Up @@ -214,6 +230,8 @@ func (h *State[N, Traits]) clearState() {
h.sum = 0
h.count = 0
h.zeroCount = 0
h.min = 0
h.max = 0
h.mapping, _ = newMapping(logarithm.MaxScale)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,13 @@ func HistogramPoints(desc *sdkinstrument.Descriptor, points []data.Point) []*met
// supposed to drop the sum.
sum := hist.Sum().CoerceToFloat64(desc.NumberKind)

var minp, maxp *float64

if hist.Count() != 0 {
minp = float64Ptr(hist.Min().CoerceToFloat64(desc.NumberKind))
maxp = float64Ptr(hist.Max().CoerceToFloat64(desc.NumberKind))
}

results[i] = &metricspb.ExponentialHistogramDataPoint{
Attributes: Attributes(pt.Attributes),
StartTimeUnixNano: toNanos(pt.Start),
Expand All @@ -166,6 +173,8 @@ func HistogramPoints(desc *sdkinstrument.Descriptor, points []data.Point) []*met
Sum: &sum,
ZeroCount: hist.ZeroCount(),
Scale: hist.Scale(),
Min: minp,
Max: maxp,
Positive: HistogramBuckets(hist.Positive()),
Negative: HistogramBuckets(hist.Negative()),
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ func TestMetricTransform(t *testing.T) {
testUnit,
expectCumulative,
otlptest.HistogramDataPoint(
expectAttrs1, startTime, endTime, 7, 3, 0, 0, 0, []uint64{1, 1, 1}, 0, nil,
expectAttrs1, startTime, endTime, 7, 3, 0, 1, 4, 0, 0, []uint64{1, 1, 1}, 0, nil,
),
),
),
Expand Down Expand Up @@ -247,6 +247,8 @@ func TestMetricTransform(t *testing.T) {
10.5,
8,
2,
-2, // min
8, // max
0,
// positive offset by 1
1,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ func Gauge(name, desc, unit string, idps ...*metricspb.NumberDataPoint) *metrics
}
}

func HistogramDataPoint(attributes []*commonpb.KeyValue, start, end time.Time, sum float64, count, zeroCount uint64, scale, posOffset int32, posBucketCounts []uint64, negOffset int32, negBucketCounts []uint64) *metricspb.ExponentialHistogramDataPoint {
func HistogramDataPoint(attributes []*commonpb.KeyValue, start, end time.Time, sum float64, count, zeroCount uint64, min, max float64, scale, posOffset int32, posBucketCounts []uint64, negOffset int32, negBucketCounts []uint64) *metricspb.ExponentialHistogramDataPoint {
dp := &metricspb.ExponentialHistogramDataPoint{
Attributes: attributes,
StartTimeUnixNano: toNanos(start),
Expand All @@ -149,6 +149,13 @@ func HistogramDataPoint(attributes []*commonpb.KeyValue, start, end time.Time, s
ZeroCount: zeroCount,
Scale: scale,
}
if !math.IsNaN(min) {
dp.Min = &min
}
if !math.IsNaN(max) {
dp.Max = &max
}

if posBucketCounts != nil {
dp.Positive = &metricspb.ExponentialHistogramDataPoint_Buckets{
Offset: posOffset,
Expand Down

0 comments on commit f391a22

Please sign in to comment.