mirrors/prometheus
1
0
Fork 0
mirror of https://github.com/prometheus/prometheus.git synced 2026-05-05 04:16:15 +02:00
Code Issues Projects Releases Wiki Activity

promql: Fix when to emit a HistogramCounterResetCollisionWarning

Browse Source 
So far, we emitted a `HistogramCounterResetCollisionWarning` when
encountering conflicting counter resets in the calculation of (i)rate
and friends. We even tested for that. However, in the rate
calculation, we are not interested in those collisions. They are
actually expected.

On the other hand, we did not warn about those collisions when doing a
`sum` aggregation, where such a warning would be appropriate.

This commit removes the warning in the former case and adds it in the
latter. Sadly, we cannot really test this as we still remove the
counter reset hint for the first sample in a chunk. (And that's the
only sample where we could get a `NotCounterReset` hint.)

Signed-off-by: beorn7 <beorn@grafana.com>
This commit is contained in:
beorn7 2025-08-27 18:39:33 +02:00
parent 03588328d2
commit 2628320292
4 changed files with 25 additions and 69 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
promql/engine.go
View File

@ -3081,11 +3081,14 @@ func (ev *evaluator) aggregation(e *parser.AggregateExpr, q float64, inputMatrix
if h != nil {
group.hasHistogram = true
if group.histogramValue != nil {
_, _, err := group.histogramValue.Add(h)
_, counterResetCollision, err := group.histogramValue.Add(h)
if err != nil {
handleAggregationError(err, e, inputMatrix[si].Metric.Get(model.MetricNameLabel), &annos)
group.incompatibleHistograms = true
}
if counterResetCollision {
annos.Add(annotations.NewHistogramCounterResetCollisionWarning(e.Expr.PositionRange(), annotations.HistogramAgg))
}
}
// Otherwise the aggregation contained floats
// previously and will be invalid anyway. No
@ -3134,18 +3137,24 @@ func (ev *evaluator) aggregation(e *parser.AggregateExpr, q float64, inputMatrix
if group.histogramValue != nil {
left := h.Copy().Div(group.groupCount)
right := group.histogramValue.Copy().Div(group.groupCount)
toAdd, _, err := left.Sub(right)
toAdd, counterResetCollision, err := left.Sub(right)
if err != nil {
handleAggregationError(err, e, inputMatrix[si].Metric.Get(model.MetricNameLabel), &annos)
group.incompatibleHistograms = true
continue
}
_, _, err = group.histogramValue.Add(toAdd)
if counterResetCollision {
annos.Add(annotations.NewHistogramCounterResetCollisionWarning(e.Expr.PositionRange(), annotations.HistogramAgg))
}
_, counterResetCollision, err = group.histogramValue.Add(toAdd)
if err != nil {
handleAggregationError(err, e, inputMatrix[si].Metric.Get(model.MetricNameLabel), &annos)
group.incompatibleHistograms = true
continue
}
if counterResetCollision {
annos.Add(annotations.NewHistogramCounterResetCollisionWarning(e.Expr.PositionRange(), annotations.HistogramAgg))
}
}
// Otherwise the aggregation contained floats
// previously and will be invalid anyway. No

24
promql/functions.go
View File

@ -251,7 +251,10 @@ func histogramRate(points []HPoint, isCounter bool, metricName string, pos posra
}
h := last.CopyToSchema(minSchema)
_, counterResetCollision, err := h.Sub(prev)
// This subtraction may deliberately include conflicting counter resets.
// Counter resets are treated explicitly in this function, so the
// information about conflicting counter resets is ignored here.
_, _, err := h.Sub(prev)
if err != nil {
if errors.Is(err, histogram.ErrHistogramsIncompatibleSchema) {
return nil, annotations.New().Add(annotations.NewMixedExponentialCustomHistogramsWarning(metricName, pos))
@ -260,16 +263,13 @@ func histogramRate(points []HPoint, isCounter bool, metricName string, pos posra
}
}
if counterResetCollision {
annos.Add(annotations.NewHistogramCounterResetCollisionWarning(pos, annotations.HistogramSub))
}
if isCounter {
// Second iteration to deal with counter resets.
for _, currPoint := range points[1:] {
curr := currPoint.H
if curr.DetectReset(prev) {
_, counterResetCollision, err := h.Add(prev)
// Counter reset conflict ignored here for the same reason as above.
_, _, err := h.Add(prev)
if err != nil {
if errors.Is(err, histogram.ErrHistogramsIncompatibleSchema) {
return nil, annotations.New().Add(annotations.NewMixedExponentialCustomHistogramsWarning(metricName, pos))
@ -277,9 +277,6 @@ func histogramRate(points []HPoint, isCounter bool, metricName string, pos posra
return nil, annotations.New().Add(annotations.NewIncompatibleCustomBucketsHistogramsWarning(metricName, pos))
}
}
if counterResetCollision {
annos.Add(annotations.NewHistogramCounterResetCollisionWarning(pos, annotations.HistogramAdd))
}
}
prev = curr
}
@ -393,15 +390,16 @@ func instantValue(vals Matrix, args parser.Expressions, out Vector, isRate bool)
annos.Add(annotations.NewNativeHistogramNotGaugeWarning(metricName, args.PositionRange()))
}
if !isRate || !ss[1].H.DetectReset(ss[0].H) {
_, counterResetCollision, err := resultSample.H.Sub(ss[0].H)
// This subtraction may deliberately include conflicting
// counter resets. Counter resets are treated explicitly
// in this function, so the information about
// conflicting counter resets is ignored here.
_, _, err := resultSample.H.Sub(ss[0].H)
if errors.Is(err, histogram.ErrHistogramsIncompatibleSchema) {
return out, annos.Add(annotations.NewMixedExponentialCustomHistogramsWarning(metricName, args.PositionRange()))
} else if errors.Is(err, histogram.ErrHistogramsIncompatibleBounds) {
return out, annos.Add(annotations.NewIncompatibleCustomBucketsHistogramsWarning(metricName, args.PositionRange()))
}
if counterResetCollision {
annos.Add(annotations.NewHistogramCounterResetCollisionWarning(args.PositionRange(), annotations.HistogramSub))
}
}
resultSample.H.CounterResetHint = histogram.GaugeType
resultSample.H.Compact(0)

52
promql/functions_internal_test.go
View File

@ -19,10 +19,6 @@ import (
"testing"
"github.com/stretchr/testify/require"
"github.com/prometheus/prometheus/model/histogram"
"github.com/prometheus/prometheus/promql/parser/posrange"
"github.com/prometheus/prometheus/util/annotations"
)
func TestKahanSumInc(t *testing.T) {
@ -83,51 +79,3 @@ func TestKahanSumInc(t *testing.T) {
})
}
}
func newAnnotations(errs ...error) annotations.Annotations {
var annos annotations.Annotations
for _, err := range errs {
annos.Add(err)
}
return annos
}
// TODO(juliusmh): this test ensures histogramRate sets correct annotations.
// This test can be removed in favor of a user facing promqltest when
// https://github.com/prometheus/prometheus/issues/15346 is resolved.
func TestHistogramRate_Annotations(t *testing.T) {
const metricName = "test"
pos := posrange.PositionRange{}
for _, tc := range []struct {
name string
points []HPoint
wantAnnotations annotations.Annotations
}{
{
name: "empty histograms",
points: []HPoint{
{H: &histogram.FloatHistogram{}},
{H: &histogram.FloatHistogram{}},
},
wantAnnotations: newAnnotations(
annotations.NewNativeHistogramNotGaugeWarning(metricName, pos),
),
},
{
name: "counter reset hint collision",
points: []HPoint{
{H: &histogram.FloatHistogram{CounterResetHint: histogram.NotCounterReset}},
{H: &histogram.FloatHistogram{CounterResetHint: histogram.CounterReset}},
},
wantAnnotations: newAnnotations(
annotations.NewNativeHistogramNotGaugeWarning(metricName, pos),
annotations.NewHistogramCounterResetCollisionWarning(pos, annotations.HistogramSub),
),
},
} {
t.Run(tc.name, func(t *testing.T) {
_, annos := histogramRate(tc.points, false, metricName, pos)
require.Equal(t, tc.wantAnnotations, annos)
})
}
}

3
util/annotations/annotations.go
View File

@ -362,13 +362,14 @@ type HistogramOperation string
const (
HistogramAdd HistogramOperation = "addition"
HistogramSub HistogramOperation = "subtraction"
HistogramAgg HistogramOperation = "aggregation"
)
// NewHistogramCounterResetCollisionWarning is used when two counter histograms are added or subtracted where one has
// a CounterReset hint and the other has NotCounterReset.
func NewHistogramCounterResetCollisionWarning(pos posrange.PositionRange, operation HistogramOperation) error {
switch operation {
case HistogramAdd, HistogramSub:
case HistogramAdd, HistogramSub, HistogramAgg:
default:
operation = "unknown operation"
}