Skip to content

Commit

Permalink
Merge pull request #753 from grafana/arve/sync-upstream
Browse files Browse the repository at this point in the history 
Cherry-pick native histograms fixes
  • Loading branch information
@aknuds1
aknuds1 authored Nov 19, 2024
2 parents f1021fc + a9b56d6 commit a3ced82
Show file tree
Hide file tree
Showing 5 changed files with 476 additions and 67 deletions.
176 changes: 111 additions & 65 deletions promql/engine.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2608,9 +2608,11 @@ func (ev *evaluator) VectorBinop(op parser.ItemType, lhs, rhs Vector, matching *
floatValue, histogramValue, keep, err := vectorElemBinop(op, fl, fr, hl, hr, pos)
if err != nil {
lastErr = err
continue
}
switch {
case returnBool:
histogramValue = nil
if keep {
floatValue = 1.0
} else {
Expand Down Expand Up @@ -2729,6 +2731,7 @@ func (ev *evaluator) VectorscalarBinop(op parser.ItemType, lhs Vector, rhs Scala
float, histogram, keep, err := vectorElemBinop(op, lf, rf, lh, rh, pos)
if err != nil {
lastErr = err
continue
}
// Catch cases where the scalar is the LHS in a scalar-vector comparison operation.
// We want to always keep the vector element value as the output value, even if it's on the RHS.
Expand Down Expand Up @@ -2794,77 +2797,84 @@ func scalarBinop(op parser.ItemType, lhs, rhs float64) float64 {

// vectorElemBinop evaluates a binary operation between two Vector elements.
func vectorElemBinop(op parser.ItemType, lhs, rhs float64, hlhs, hrhs *histogram.FloatHistogram, pos posrange.PositionRange) (float64, *histogram.FloatHistogram, bool, error) {
switch op {
case parser.ADD:
if hlhs != nil && hrhs != nil {
res, err := hlhs.Copy().Add(hrhs)
if err != nil {
return 0, nil, false, err
opName := parser.ItemTypeStr[op]
switch {
case hlhs == nil && hrhs == nil:
{
switch op {
case parser.ADD:
return lhs + rhs, nil, true, nil
case parser.SUB:
return lhs - rhs, nil, true, nil
case parser.MUL:
return lhs * rhs, nil, true, nil
case parser.DIV:
return lhs / rhs, nil, true, nil
case parser.POW:
return math.Pow(lhs, rhs), nil, true, nil
case parser.MOD:
return math.Mod(lhs, rhs), nil, true, nil
case parser.EQLC:
return lhs, nil, lhs == rhs, nil
case parser.NEQ:
return lhs, nil, lhs != rhs, nil
case parser.GTR:
return lhs, nil, lhs > rhs, nil
case parser.LSS:
return lhs, nil, lhs < rhs, nil
case parser.GTE:
return lhs, nil, lhs >= rhs, nil
case parser.LTE:
return lhs, nil, lhs <= rhs, nil
case parser.ATAN2:
return math.Atan2(lhs, rhs), nil, true, nil
}
}
case hlhs == nil && hrhs != nil:
{
switch op {
case parser.MUL:
return 0, hrhs.Copy().Mul(lhs).Compact(0), true, nil
case parser.ADD, parser.SUB, parser.DIV, parser.POW, parser.MOD, parser.EQLC, parser.NEQ, parser.GTR, parser.LSS, parser.GTE, parser.LTE, parser.ATAN2:
return 0, nil, false, annotations.NewIncompatibleTypesInBinOpInfo("float", opName, "histogram", pos)
}
return 0, res.Compact(0), true, nil
}
if hlhs == nil && hrhs == nil {
return lhs + rhs, nil, true, nil
}
if hlhs != nil {
return 0, nil, false, annotations.NewIncompatibleTypesInBinOpInfo("histogram", "+", "float", pos)
}
return 0, nil, false, annotations.NewIncompatibleTypesInBinOpInfo("float", "+", "histogram", pos)
case parser.SUB:
if hlhs != nil && hrhs != nil {
res, err := hlhs.Copy().Sub(hrhs)
if err != nil {
return 0, nil, false, err
case hlhs != nil && hrhs == nil:
{
switch op {
case parser.MUL:
return 0, hlhs.Copy().Mul(rhs).Compact(0), true, nil
case parser.DIV:
return 0, hlhs.Copy().Div(rhs).Compact(0), true, nil
case parser.ADD, parser.SUB, parser.POW, parser.MOD, parser.EQLC, parser.NEQ, parser.GTR, parser.LSS, parser.GTE, parser.LTE, parser.ATAN2:
return 0, nil, false, annotations.NewIncompatibleTypesInBinOpInfo("histogram", opName, "float", pos)
}
return 0, res.Compact(0), true, nil
}
if hlhs == nil && hrhs == nil {
return lhs - rhs, nil, true, nil
}
if hlhs != nil {
return 0, nil, false, annotations.NewIncompatibleTypesInBinOpInfo("histogram", "-", "float", pos)
}
return 0, nil, false, annotations.NewIncompatibleTypesInBinOpInfo("float", "-", "histogram", pos)
case parser.MUL:
if hlhs != nil && hrhs == nil {
return 0, hlhs.Copy().Mul(rhs), true, nil
}
if hlhs == nil && hrhs != nil {
return 0, hrhs.Copy().Mul(lhs), true, nil
}
if hlhs != nil && hrhs != nil {
return 0, nil, false, annotations.NewIncompatibleTypesInBinOpInfo("histogram", "*", "histogram", pos)
}
return lhs * rhs, nil, true, nil
case parser.DIV:
if hlhs != nil && hrhs == nil {
return 0, hlhs.Copy().Div(rhs), true, nil
}
if hrhs != nil {
if hlhs != nil {
return 0, nil, false, annotations.NewIncompatibleTypesInBinOpInfo("histogram", "/", "histogram", pos)
case hlhs != nil && hrhs != nil:
{
switch op {
case parser.ADD:
res, err := hlhs.Copy().Add(hrhs)
if err != nil {
return 0, nil, false, err
}
return 0, res.Compact(0), true, nil
case parser.SUB:
res, err := hlhs.Copy().Sub(hrhs)
if err != nil {
return 0, nil, false, err
}
return 0, res.Compact(0), true, nil
case parser.EQLC:
// This operation expects that both histograms are compacted.
return 0, hlhs, hlhs.Equals(hrhs), nil
case parser.NEQ:
// This operation expects that both histograms are compacted.
return 0, hlhs, !hlhs.Equals(hrhs), nil
case parser.MUL, parser.DIV, parser.POW, parser.MOD, parser.GTR, parser.LSS, parser.GTE, parser.LTE, parser.ATAN2:
return 0, nil, false, annotations.NewIncompatibleTypesInBinOpInfo("histogram", opName, "histogram", pos)
}
return 0, nil, false, annotations.NewIncompatibleTypesInBinOpInfo("float", "/", "histogram", pos)
}
return lhs / rhs, nil, true, nil
case parser.POW:
return math.Pow(lhs, rhs), nil, true, nil
case parser.MOD:
return math.Mod(lhs, rhs), nil, true, nil
case parser.EQLC:
return lhs, nil, lhs == rhs, nil
case parser.NEQ:
return lhs, nil, lhs != rhs, nil
case parser.GTR:
return lhs, nil, lhs > rhs, nil
case parser.LSS:
return lhs, nil, lhs < rhs, nil
case parser.GTE:
return lhs, nil, lhs >= rhs, nil
case parser.LTE:
return lhs, nil, lhs <= rhs, nil
case parser.ATAN2:
return math.Atan2(lhs, rhs), nil, true, nil
}
panic(fmt.Errorf("operator %q not allowed for operations between Vectors", op))
}
Expand Down Expand Up @@ -2926,16 +2936,34 @@ func (ev *evaluator) aggregation(e *parser.AggregateExpr, q float64, inputMatrix
case h != nil:
// Ignore histograms for STDVAR and STDDEV.
group.seen = false
if op == parser.STDVAR {
annos.Add(annotations.NewHistogramIgnoredInAggregationInfo("stdvar", e.Expr.PositionRange()))
} else {
annos.Add(annotations.NewHistogramIgnoredInAggregationInfo("stddev", e.Expr.PositionRange()))
}
case math.IsNaN(f), math.IsInf(f, 0):
group.floatValue = math.NaN()
default:
group.floatValue = 0
}
case parser.QUANTILE:
if h != nil {
group.seen = false
annos.Add(annotations.NewHistogramIgnoredInAggregationInfo("quantile", e.Expr.PositionRange()))
}
group.heap = make(vectorByValueHeap, 1)
group.heap[0] = Sample{F: f}
case parser.GROUP:
group.floatValue = 1
case parser.MIN, parser.MAX:
if h != nil {
group.seen = false
if op == parser.MIN {
annos.Add(annotations.NewHistogramIgnoredInAggregationInfo("min", e.Expr.PositionRange()))
} else {
annos.Add(annotations.NewHistogramIgnoredInAggregationInfo("max", e.Expr.PositionRange()))
}
}
}
continue
}
Expand Down Expand Up @@ -3034,11 +3062,19 @@ func (ev *evaluator) aggregation(e *parser.AggregateExpr, q float64, inputMatrix
// Do nothing. Required to avoid the panic in `default:` below.

case parser.MAX:
if h != nil {
annos.Add(annotations.NewHistogramIgnoredInAggregationInfo("max", e.Expr.PositionRange()))
continue
}
if group.floatValue < f || math.IsNaN(group.floatValue) {
group.floatValue = f
}

case parser.MIN:
if h != nil {
annos.Add(annotations.NewHistogramIgnoredInAggregationInfo("min", e.Expr.PositionRange()))
continue
}
if group.floatValue > f || math.IsNaN(group.floatValue) {
group.floatValue = f
}
Expand All @@ -3052,9 +3088,19 @@ func (ev *evaluator) aggregation(e *parser.AggregateExpr, q float64, inputMatrix
delta := f - group.floatMean
group.floatMean += delta / group.groupCount
group.floatValue += delta * (f - group.floatMean)
} else {
if op == parser.STDVAR {
annos.Add(annotations.NewHistogramIgnoredInAggregationInfo("stdvar", e.Expr.PositionRange()))
} else {
annos.Add(annotations.NewHistogramIgnoredInAggregationInfo("stddev", e.Expr.PositionRange()))
}
}

case parser.QUANTILE:
if h != nil {
annos.Add(annotations.NewHistogramIgnoredInAggregationInfo("quantile", e.Expr.PositionRange()))
continue
}
group.heap = append(group.heap, Sample{F: f})

default:
Expand Down
35 changes: 33 additions & 2 deletions promql/promqltest/testdata/aggregators.test
View file
Original file line number Diff line number Diff line change
Expand Up @@ -229,13 +229,28 @@ load 5m
http_requests{job="api-server", instance="0", group="canary"} NaN
http_requests{job="api-server", instance="1", group="canary"} 3
http_requests{job="api-server", instance="2", group="canary"} 4
http_requests_histogram{job="api-server", instance="3", group="canary"} {{schema:2 count:4 sum:10 buckets:[1 0 0 0 1 0 0 1 1]}}

eval instant at 0m max(http_requests)
{} 4

# The histogram is ignored here so the result doesn't change but it has an info annotation now.
eval_info instant at 0m max({job="api-server"})
{} 4

# The histogram is ignored here so there is no result but it has an info annotation now.
eval_info instant at 0m max(http_requests_histogram)

eval instant at 0m min(http_requests)
{} 1

# The histogram is ignored here so the result doesn't change but it has an info annotation now.
eval_info instant at 0m min({job="api-server"})
{} 1

# The histogram is ignored here so there is no result but it has an info annotation now.
eval_info instant at 0m min(http_requests_histogram)

eval instant at 0m max by (group) (http_requests)
{group="production"} 2
{group="canary"} 4
Expand Down Expand Up @@ -380,13 +395,23 @@ load 10s
data{test="uneven samples",point="a"} 0
data{test="uneven samples",point="b"} 1
data{test="uneven samples",point="c"} 4
data_histogram{test="histogram sample", point="c"} {{schema:2 count:4 sum:10 buckets:[1 0 0 0 1 0 0 1 1]}}
foo .8

eval instant at 1m quantile without(point)(0.8, data)
{test="two samples"} 0.8
{test="three samples"} 1.6
{test="uneven samples"} 2.8

# The histogram is ignored here so the result doesn't change but it has an info annotation now.
eval_info instant at 1m quantile without(point)(0.8, {__name__=~"data(_histogram)?"})
{test="two samples"} 0.8
{test="three samples"} 1.6
{test="uneven samples"} 2.8

# The histogram is ignored here so there is no result but it has an info annotation now.
eval_info instant at 1m quantile(0.8, data_histogram)

# Bug #5276.
eval instant at 1m quantile without(point)(scalar(foo), data)
{test="two samples"} 0.8
Expand Down Expand Up @@ -581,12 +606,18 @@ load 5m
series{label="b"} 2
series{label="c"} {{schema:1 sum:15 count:10 buckets:[3 2 5 7 9]}}

eval instant at 0m stddev(series)
# The histogram is ignored here so the result doesn't change but it has an info annotation now.
eval_info instant at 0m stddev(series)
{} 0.5

eval instant at 0m stdvar(series)
eval_info instant at 0m stdvar(series)
{} 0.25

# The histogram is ignored here so there is no result but it has an info annotation now.
eval_info instant at 0m stddev({label="c"})

eval_info instant at 0m stdvar({label="c"})

eval instant at 0m stddev by (label) (series)
{label="a"} 0
{label="b"} 0
Expand Down
2 changes: 2 additions & 0 deletions promql/promqltest/testdata/native_histograms.test
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1184,3 +1184,5 @@ eval instant at 3m sum_over_time(histogram_sum_over_time[4m:1m])

eval instant at 3m avg_over_time(histogram_sum_over_time[4m:1m])
{} {{schema:0 count:26.75 sum:1172.8 z_bucket:3.5 z_bucket_w:0.001 buckets:[0.75 2 0.5 1.25 0.75 0.5 0.5] n_buckets:[0.5 1.5 2 1 3.75 2.25 0 0 0 2.5 2.5 1]}}

clear
Loading

0 comments on commit a3ced82

Please sign in to comment.