promql: Refine zero bucket treatment in histogramQuantile

Essentially, this mirrors the existing behavior for negative buckets:
If a histogram has only negative buckets, the upper bound of the zero
bucket is assumed to be zero.

Furthermore, it makes sure that the zero bucket boundaries are not
modified if a histogram that has no buckets at all but samples in the
zero bucket.

Also, add an TODO to vet if we really want this behavior.

Signed-off-by: beorn7 <beorn@grafana.com>
This commit is contained in:
beorn7 2022-06-16 18:54:28 +02:00
parent 095b6c93dd
commit ffaabea91a
2 changed files with 17 additions and 8 deletions

View file

@ -3244,15 +3244,15 @@ func TestSparseHistogram_HistogramQuantile(t *testing.T) {
},
{ // Zero bucket.
quantile: "1",
value: 0.001,
value: 0,
},
{ // Zero bucket.
quantile: "0.99",
value: 0.0008799999999999991,
value: -6.000000000000048e-05,
},
{ // Zero bucket.
quantile: "0.9",
value: -0.00019999999999999933,
value: -0.0005999999999999996,
},
{
quantile: "0.5",

View file

@ -127,7 +127,10 @@ func bucketQuantile(q float64, buckets buckets) float64 {
// TODO(beorn7): Find an interpolation method that is a better fit for
// exponential buckets (and think about configurable interpolation).
//
// A natural lower bound of 0 is assumed if the histogram has no negative buckets.
// A natural lower bound of 0 is assumed if the histogram has only positive
// buckets. Likewise, a natural upper bound of 0 is assumed if the histogram has
// only negative buckets.
// TODO(beorn7): Come to terms if we want that.
//
// There are a number of special cases (once we have a way to report errors
// happening during evaluations of AST functions, we should report those
@ -163,10 +166,16 @@ func histogramQuantile(q float64, h *histogram.FloatHistogram) float64 {
break
}
}
if bucket.Lower < 0 && bucket.Upper > 0 && len(h.NegativeBuckets) == 0 {
// The result is in the zero bucket and the histogram has no
// negative buckets. So we consider 0 to be the lower bound.
bucket.Lower = 0
if bucket.Lower < 0 && bucket.Upper > 0 {
if len(h.NegativeBuckets) == 0 && len(h.PositiveBuckets) > 0 {
// The result is in the zero bucket and the histogram has only
// positive buckets. So we consider 0 to be the lower bound.
bucket.Lower = 0
} else if len(h.PositiveBuckets) == 0 && len(h.NegativeBuckets) > 0 {
// The result is in the zero bucket and the histogram has only
// negative buckets. So we consider 0 to be the upper bound.
bucket.Upper = 0
}
}
// Due to numerical inaccuracies, we could end up with a higher count
// than h.Count. Thus, make sure count is never higher than h.Count.