diff --git a/promql/parser/generated_parser.y b/promql/parser/generated_parser.y index 71ab6ed4b3..6e336e230b 100644 --- a/promql/parser/generated_parser.y +++ b/promql/parser/generated_parser.y @@ -850,14 +850,15 @@ series_item : BLANK // Histogram descriptions (part of unit testing). | histogram_series_value { - $$ = []SequenceValue{{Histogram:$1}} + $$ = []SequenceValue{yylex.(*parser).newHistogramSequenceValue($1)} } | histogram_series_value TIMES uint { $$ = []SequenceValue{} // Add an additional value for time 0, which we ignore in tests. + sv := yylex.(*parser).newHistogramSequenceValue($1) for i:=uint64(0); i <= $3; i++{ - $$ = append($$, SequenceValue{Histogram:$1}) + $$ = append($$, sv) //$1 += $2 } } diff --git a/promql/parser/generated_parser.y.go b/promql/parser/generated_parser.y.go index d20460ed5b..4b90d757cf 100644 --- a/promql/parser/generated_parser.y.go +++ b/promql/parser/generated_parser.y.go @@ -1931,15 +1931,16 @@ yydefault: case 170: yyDollar = yyS[yypt-1 : yypt+1] { - yyVAL.series = []SequenceValue{{Histogram: yyDollar[1].histogram}} + yyVAL.series = []SequenceValue{yylex.(*parser).newHistogramSequenceValue(yyDollar[1].histogram)} } case 171: yyDollar = yyS[yypt-3 : yypt+1] { yyVAL.series = []SequenceValue{} // Add an additional value for time 0, which we ignore in tests. + sv := yylex.(*parser).newHistogramSequenceValue(yyDollar[1].histogram) for i := uint64(0); i <= yyDollar[3].uint; i++ { - yyVAL.series = append(yyVAL.series, SequenceValue{Histogram: yyDollar[1].histogram}) + yyVAL.series = append(yyVAL.series, sv) //$1 += $2 } } diff --git a/promql/parser/parse.go b/promql/parser/parse.go index 9b284c541e..cefc627fda 100644 --- a/promql/parser/parse.go +++ b/promql/parser/parse.go @@ -70,6 +70,11 @@ type parser struct { generatedParserResult any parseErrors ParseErrors + + // lastHistogramCounterResetHintSet is set to true when the most recently + // built histogram had a counter_reset_hint explicitly specified. + // This is used to populate CounterResetHintSet in SequenceValue. + lastHistogramCounterResetHintSet bool } type Opt func(p *parser) @@ -237,6 +242,11 @@ type SequenceValue struct { Value float64 Omitted bool Histogram *histogram.FloatHistogram + // CounterResetHintSet is true if the counter reset hint was explicitly + // specified in the test file using counter_reset_hint:... syntax. + // This allows distinguishing between "no hint specified" (don't care) + // vs "counter_reset_hint:unknown" (verify it's unknown). + CounterResetHintSet bool } func (v SequenceValue) String() string { @@ -504,25 +514,30 @@ func (p *parser) mergeMaps(left, right *map[string]any) (ret *map[string]any) { } func (p *parser) histogramsIncreaseSeries(base, inc *histogram.FloatHistogram, times uint64) ([]SequenceValue, error) { - return p.histogramsSeries(base, inc, times, func(a, b *histogram.FloatHistogram) (*histogram.FloatHistogram, error) { + // Capture the hint set flag immediately after inc histogram is built. + // The base histogram's hint set flag was already captured. + hintSet := p.lastHistogramCounterResetHintSet + return p.histogramsSeries(base, inc, times, hintSet, func(a, b *histogram.FloatHistogram) (*histogram.FloatHistogram, error) { res, _, _, err := a.Add(b) return res, err }) } func (p *parser) histogramsDecreaseSeries(base, inc *histogram.FloatHistogram, times uint64) ([]SequenceValue, error) { - return p.histogramsSeries(base, inc, times, func(a, b *histogram.FloatHistogram) (*histogram.FloatHistogram, error) { + // Capture the hint set flag immediately after inc histogram is built. + hintSet := p.lastHistogramCounterResetHintSet + return p.histogramsSeries(base, inc, times, hintSet, func(a, b *histogram.FloatHistogram) (*histogram.FloatHistogram, error) { res, _, _, err := a.Sub(b) return res, err }) } -func (*parser) histogramsSeries(base, inc *histogram.FloatHistogram, times uint64, +func (*parser) histogramsSeries(base, inc *histogram.FloatHistogram, times uint64, counterResetHintSet bool, combine func(*histogram.FloatHistogram, *histogram.FloatHistogram) (*histogram.FloatHistogram, error), ) ([]SequenceValue, error) { ret := make([]SequenceValue, times+1) // Add an additional value (the base) for time 0, which we ignore in tests. - ret[0] = SequenceValue{Histogram: base} + ret[0] = SequenceValue{Histogram: base, CounterResetHintSet: counterResetHintSet} cur := base for i := uint64(1); i <= times; i++ { if cur.Schema > inc.Schema { @@ -534,7 +549,7 @@ func (*parser) histogramsSeries(base, inc *histogram.FloatHistogram, times uint6 if err != nil { return ret, err } - ret[i] = SequenceValue{Histogram: cur} + ret[i] = SequenceValue{Histogram: cur, CounterResetHintSet: counterResetHintSet} } return ret, nil @@ -543,6 +558,8 @@ func (*parser) histogramsSeries(base, inc *histogram.FloatHistogram, times uint6 // buildHistogramFromMap is used in the grammar to take then individual parts of the histogram and complete it. func (p *parser) buildHistogramFromMap(desc *map[string]any) *histogram.FloatHistogram { output := &histogram.FloatHistogram{} + // Reset the flag for each new histogram being built. + p.lastHistogramCounterResetHintSet = false val, ok := (*desc)["schema"] if ok { @@ -603,6 +620,8 @@ func (p *parser) buildHistogramFromMap(desc *map[string]any) *histogram.FloatHis val, ok = (*desc)["counter_reset_hint"] if ok { + // Mark that the counter reset hint was explicitly specified. + p.lastHistogramCounterResetHintSet = true resetHint, ok := val.(Item) if ok { @@ -634,6 +653,16 @@ func (p *parser) buildHistogramFromMap(desc *map[string]any) *histogram.FloatHis return output } +// newHistogramSequenceValue creates a SequenceValue for a histogram, +// setting CounterResetHintSet based on whether counter_reset_hint was +// explicitly specified in the histogram description. +func (p *parser) newHistogramSequenceValue(h *histogram.FloatHistogram) SequenceValue { + return SequenceValue{ + Histogram: h, + CounterResetHintSet: p.lastHistogramCounterResetHintSet, + } +} + func (p *parser) buildHistogramBucketsAndSpans(desc *map[string]any, bucketsKey, offsetKey string, ) (buckets []float64, spans []histogram.Span) { bucketCount := 0 diff --git a/promql/promqltest/test.go b/promql/promqltest/test.go index 10584d0ed1..7d48abb606 100644 --- a/promql/promqltest/test.go +++ b/promql/promqltest/test.go @@ -1047,7 +1047,12 @@ func (ev *evalCmd) compareResult(result parser.Value) error { exp := ev.expected[hash] var expectedFloats []promql.FPoint - var expectedHistograms []promql.HPoint + // expectedHPoint wraps HPoint with CounterResetHintSet flag from SequenceValue. + type expectedHPoint struct { + promql.HPoint + CounterResetHintSet bool + } + var expectedHistograms []expectedHPoint for i, e := range exp.vals { ts := ev.start.Add(time.Duration(i) * ev.step) @@ -1059,7 +1064,10 @@ func (ev *evalCmd) compareResult(result parser.Value) error { t := ts.UnixNano() / int64(time.Millisecond/time.Nanosecond) if e.Histogram != nil { - expectedHistograms = append(expectedHistograms, promql.HPoint{T: t, H: e.Histogram}) + expectedHistograms = append(expectedHistograms, expectedHPoint{ + HPoint: promql.HPoint{T: t, H: e.Histogram}, + CounterResetHintSet: e.CounterResetHintSet, + }) } else if !e.Omitted { expectedFloats = append(expectedFloats, promql.FPoint{T: t, F: e.Value}) } @@ -1088,7 +1096,7 @@ func (ev *evalCmd) compareResult(result parser.Value) error { return fmt.Errorf("expected histogram value at index %v for %s to have timestamp %v, but it had timestamp %v (result has %s)", i, ev.metrics[hash], expected.T, actual.T, formatSeriesResult(s)) } - if !compareNativeHistogram(expected.H.Compact(0), actual.H.Compact(0)) { + if !compareNativeHistogram(expected.H.Compact(0), actual.H.Compact(0), expected.CounterResetHintSet) { return fmt.Errorf("expected histogram value at index %v (t=%v) for %s to be %v, but got %v (result has %s)", i, actual.T, ev.metrics[hash], expected.H.TestExpression(), actual.H.TestExpression(), formatSeriesResult(s)) } } @@ -1127,7 +1135,7 @@ func (ev *evalCmd) compareResult(result parser.Value) error { if expH != nil && v.H == nil { return fmt.Errorf("expected histogram %s for %s but got float value %v", HistogramTestExpression(expH), v.Metric, v.F) } - if expH != nil && !compareNativeHistogram(expH.Compact(0), v.H.Compact(0)) { + if expH != nil && !compareNativeHistogram(expH.Compact(0), v.H.Compact(0), exp0.CounterResetHintSet) { return fmt.Errorf("expected %v for %s but got %s", HistogramTestExpression(expH), v.Metric, HistogramTestExpression(v.H)) } if !almost.Equal(exp0.Value, v.F, defaultEpsilon) { @@ -1165,7 +1173,9 @@ func (ev *evalCmd) compareResult(result parser.Value) error { // compareNativeHistogram is helper function to compare two native histograms // which can tolerate some differ in the field of float type, such as Count, Sum. -func compareNativeHistogram(exp, cur *histogram.FloatHistogram) bool { +// The counterResetHintSet parameter indicates whether the counter reset hint was +// explicitly specified in the expected histogram (from the test file). +func compareNativeHistogram(exp, cur *histogram.FloatHistogram, counterResetHintSet bool) bool { if exp == nil || cur == nil { return false } @@ -1201,6 +1211,15 @@ func compareNativeHistogram(exp, cur *histogram.FloatHistogram) bool { return false } + // Compare CounterResetHint only if explicitly specified in expected histogram. + // When counterResetHintSet is false, no hint was specified, meaning "don't care". + // When counterResetHintSet is true, the hint was explicitly specified and must match. + if counterResetHintSet { + if exp.CounterResetHint != cur.CounterResetHint { + return false + } + } + return true } diff --git a/promql/promqltest/testdata/native_histograms.test b/promql/promqltest/testdata/native_histograms.test index fd4b1f4178..d66400f787 100644 --- a/promql/promqltest/testdata/native_histograms.test +++ b/promql/promqltest/testdata/native_histograms.test @@ -1283,7 +1283,7 @@ eval instant at 12m sum_over_time(nhcb_metric[13m]) eval instant at 12m avg_over_time(nhcb_metric[13m]) expect no_warn expect info msg: PromQL info: mismatched custom buckets were reconciled during aggregation - {} {{schema:-53 count:1 sum:1 custom_values:[5] counter_reset_hint:gauge buckets:[1]}} + {} {{schema:-53 count:1 sum:1 custom_values:[5] buckets:[1]}} eval instant at 12m last_over_time(nhcb_metric[13m]) expect no_warn