diff --git a/sdk/metric/internal/aggregate/exponential_histogram.go b/sdk/metric/internal/aggregate/exponential_histogram.go index 07d9fd6e22c..e9ba04eea5e 100644 --- a/sdk/metric/internal/aggregate/exponential_histogram.go +++ b/sdk/metric/internal/aggregate/exponential_histogram.go @@ -102,7 +102,7 @@ func (p *expoHistogramDataPoint[N]) record(v N) { return } - bin := getBin(absV, p.scale) + bin := p.getBin(absV) bucket := &p.posBuckets if v < 0 { @@ -111,7 +111,7 @@ func (p *expoHistogramDataPoint[N]) record(v N) { // If the new bin would make the counts larger than maxScale, we need to // downscale current measurements. - if scaleDelta := scaleChange(bin, bucket.startBin, len(bucket.counts), p.maxSize); scaleDelta > 0 { + if scaleDelta := p.scaleChange(bin, bucket.startBin, len(bucket.counts)); scaleDelta > 0 { if p.scale-scaleDelta < expoMinScale { // With a scale of -10 there is only two buckets for the whole range of float64 values. // This can only happen if there is a max size of 1. @@ -123,17 +123,16 @@ func (p *expoHistogramDataPoint[N]) record(v N) { p.posBuckets.downscale(scaleDelta) p.negBuckets.downscale(scaleDelta) - bin = getBin(absV, p.scale) + bin = p.getBin(absV) } bucket.record(bin) } -// getBin returns the bin of the bucket that the value v should be recorded -// into at the given scale. -func getBin(v float64, scale int) int { +// getBin returns the bin v should be recorded into. +func (p *expoHistogramDataPoint[N]) getBin(v float64) int { frac, exp := math.Frexp(v) - if scale <= 0 { + if p.scale <= 0 { // Because of the choice of fraction is always 1 power of two higher than we want. correction := 1 if frac == .5 { @@ -141,9 +140,9 @@ func getBin(v float64, scale int) int { // will be one higher than we want. correction = 2 } - return (exp - correction) >> (-scale) + return (exp - correction) >> (-p.scale) } - return exp<= maxSize { + for high-low >= p.maxSize { low = low >> 1 high = high >> 1 count++ diff --git a/sdk/metric/internal/aggregate/exponential_histogram_test.go b/sdk/metric/internal/aggregate/exponential_histogram_test.go index 4209ffd651d..040370d4729 100644 --- a/sdk/metric/internal/aggregate/exponential_histogram_test.go +++ b/sdk/metric/internal/aggregate/exponential_histogram_test.go @@ -655,7 +655,8 @@ func TestScaleChange(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := scaleChange(tt.args.bin, tt.args.startBin, tt.args.length, tt.args.maxSize) + p := newExpoHistogramDataPoint[float64](tt.args.maxSize, 20, false, false) + got := p.scaleChange(tt.args.bin, tt.args.startBin, tt.args.length) if got != tt.want { t.Errorf("scaleChange() = %v, want %v", got, tt.want) } @@ -927,15 +928,15 @@ func FuzzGetBin(f *testing.F) { t.Skip("skipping test for zero") } - // GetBin is only used with a range of -10 to 20. - scale = (scale%31+31)%31 - 10 - - got := getBin(v, scale) - if v <= lowerBound(got, scale) { - t.Errorf("v=%x scale =%d had bin %d, but was below lower bound %x", v, scale, got, lowerBound(got, scale)) + p := newExpoHistogramDataPoint[float64](4, 20, false, false) + // scale range is -10 to 20. + p.scale = (scale%31+31)%31 - 10 + got := p.getBin(v) + if v <= lowerBound(got, p.scale) { + t.Errorf("v=%x scale =%d had bin %d, but was below lower bound %x", v, p.scale, got, lowerBound(got, p.scale)) } - if v > lowerBound(got+1, scale) { - t.Errorf("v=%x scale =%d had bin %d, but was above upper bound %x", v, scale, got, lowerBound(got+1, scale)) + if v > lowerBound(got+1, p.scale) { + t.Errorf("v=%x scale =%d had bin %d, but was above upper bound %x", v, p.scale, got, lowerBound(got+1, p.scale)) } }) }