histogram: Fix bounds of buckets returned by floatBucketIterator

The bounds weren't really used so far, so no actual bug in the code so
far. But it's obviously confusing if the bounds returned by a
floatBucketIterator with a target schema different from the original
schema are wrong.

Signed-off-by: beorn7 <beorn@grafana.com>
This commit is contained in:
beorn7 2023-07-18 18:17:47 +02:00
parent 0a48f93111
commit 9aadd54786
3 changed files with 62 additions and 4 deletions

View file

@ -818,6 +818,11 @@ type floatBucketIterator struct {
absoluteStartValue float64 // Never return buckets with an upper bound ≤ this value. absoluteStartValue float64 // Never return buckets with an upper bound ≤ this value.
} }
func (i *floatBucketIterator) At() Bucket[float64] {
// Need to use i.targetSchema rather than i.baseBucketIterator.schema.
return i.baseBucketIterator.at(i.targetSchema)
}
func (i *floatBucketIterator) Next() bool { func (i *floatBucketIterator) Next() bool {
if i.spansIdx >= len(i.spans) { if i.spansIdx >= len(i.spans) {
return false return false

View file

@ -2205,3 +2205,50 @@ func TestAllReverseFloatBucketIterator(t *testing.T) {
}) })
} }
} }
func TestFloatBucketIteratorTargetSchema(t *testing.T) {
h := FloatHistogram{
Count: 405,
Sum: 1008.4,
Schema: 1,
PositiveSpans: []Span{
{Offset: 0, Length: 4},
{Offset: 1, Length: 3},
{Offset: 2, Length: 3},
},
PositiveBuckets: []float64{100, 344, 123, 55, 3, 63, 2, 54, 235, 33},
NegativeSpans: []Span{
{Offset: 0, Length: 3},
{Offset: 7, Length: 4},
{Offset: 1, Length: 3},
},
NegativeBuckets: []float64{10, 34, 1230, 54, 67, 63, 2, 554, 235, 33},
}
expPositiveBuckets := []Bucket[float64]{
{Lower: 0.25, Upper: 1, LowerInclusive: false, UpperInclusive: true, Count: 100, Index: 0},
{Lower: 1, Upper: 4, LowerInclusive: false, UpperInclusive: true, Count: 522, Index: 1},
{Lower: 4, Upper: 16, LowerInclusive: false, UpperInclusive: true, Count: 68, Index: 2},
{Lower: 16, Upper: 64, LowerInclusive: false, UpperInclusive: true, Count: 322, Index: 3},
}
expNegativeBuckets := []Bucket[float64]{
{Lower: -1, Upper: -0.25, LowerInclusive: true, UpperInclusive: false, Count: 10, Index: 0},
{Lower: -4, Upper: -1, LowerInclusive: true, UpperInclusive: false, Count: 1264, Index: 1},
{Lower: -64, Upper: -16, LowerInclusive: true, UpperInclusive: false, Count: 184, Index: 3},
{Lower: -256, Upper: -64, LowerInclusive: true, UpperInclusive: false, Count: 791, Index: 4},
{Lower: -1024, Upper: -256, LowerInclusive: true, UpperInclusive: false, Count: 33, Index: 5},
}
it := h.floatBucketIterator(true, 0, -1)
for i, b := range expPositiveBuckets {
require.True(t, it.Next(), "positive iterator exhausted too early")
require.Equal(t, b, it.At(), "bucket %d", i)
}
require.False(t, it.Next(), "positive iterator not exhausted")
it = h.floatBucketIterator(false, 0, -1)
for i, b := range expNegativeBuckets {
require.True(t, it.Next(), "negative iterator exhausted too early")
require.Equal(t, b, it.At(), "bucket %d", i)
}
require.False(t, it.Next(), "negative iterator not exhausted")
}

View file

@ -102,16 +102,22 @@ type baseBucketIterator[BC BucketCount, IBC InternalBucketCount] struct {
} }
func (b baseBucketIterator[BC, IBC]) At() Bucket[BC] { func (b baseBucketIterator[BC, IBC]) At() Bucket[BC] {
return b.at(b.schema)
}
// at is an internal version of the exported At to enable using a different
// schema.
func (b baseBucketIterator[BC, IBC]) at(schema int32) Bucket[BC] {
bucket := Bucket[BC]{ bucket := Bucket[BC]{
Count: BC(b.currCount), Count: BC(b.currCount),
Index: b.currIdx, Index: b.currIdx,
} }
if b.positive { if b.positive {
bucket.Upper = getBound(b.currIdx, b.schema) bucket.Upper = getBound(b.currIdx, schema)
bucket.Lower = getBound(b.currIdx-1, b.schema) bucket.Lower = getBound(b.currIdx-1, schema)
} else { } else {
bucket.Lower = -getBound(b.currIdx, b.schema) bucket.Lower = -getBound(b.currIdx, schema)
bucket.Upper = -getBound(b.currIdx-1, b.schema) bucket.Upper = -getBound(b.currIdx-1, schema)
} }
bucket.LowerInclusive = bucket.Lower < 0 bucket.LowerInclusive = bucket.Lower < 0
bucket.UpperInclusive = bucket.Upper > 0 bucket.UpperInclusive = bucket.Upper > 0