Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement min/max for the expo-histogram #215

Merged
merged 2 commits into from
Jul 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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