Make getBin and scaleChange methods of expoHistogramDataPoint (open-t…

…elemetry#4451)

Both functions receive parameters from an expoHistogramDataPoint and are
only ever used by other methods of an expoHistogramDataPoint. Make the
functions methods of expoHistogramDataPoint so the parameter arguments
can be dropped and the functions are scoped to the type they are used
for.

Co-authored-by: Aaron Clawson <[email protected]>

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,27 +123,26 @@ 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 {
 			// If v is an exact power of two the frac will be .5 and the exp
 			// will be one higher than we want.
 			correction = 2
 		}
-		return (exp - correction) >> (-scale)
+		return (exp - correction) >> (-p.scale)
 	}
-	return exp<<scale + int(math.Log(frac)*scaleFactors[scale]) - 1
+	return exp<<p.scale + int(math.Log(frac)*scaleFactors[p.scale]) - 1
 }
 
 // scaleFactors are constants used in calculating the logarithm index. They are
@@ -172,8 +171,9 @@ var scaleFactors = [21]float64{
 	math.Ldexp(math.Log2E, 20),
 }
 
-// scaleChange returns the magnitude of the scale change needed to fit bin in the bucket.
-func scaleChange(bin, startBin, length, maxSize int) int {
+// scaleChange returns the magnitude of the scale change needed to fit bin in
+// the bucket. If no scale change is needed 0 is returned.
+func (p *expoHistogramDataPoint[N]) scaleChange(bin, startBin, length int) int {
 	if length == 0 {
 		// No need to rescale if there are no buckets.
 		return 0
@@ -187,7 +187,7 @@ func scaleChange(bin, startBin, length, maxSize int) int {
 	}
 
 	count := 0
-	for high-low >= maxSize {
+	for high-low >= p.maxSize {
 		low = low >> 1
 		high = high >> 1
 		count++

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))
 		}
 	})
 }