Skip to content

Commit

Permalink
Make rate possible non-counter annotation consistent (prometheus#14910)
Browse files Browse the repository at this point in the history
* Make rate possible non-counter annotation consistent

Previously a PossibleNonCounterInfo annotation would be left in cases
where a range-vector selects 1 float data point, even if no more points
are selected in order to calculate a rate.

This change ensures an output float exists before emitting such an
annotation.

This fixes an inconsistency where a series with mixed data (ie, a float
and a native histogram) would emit an annotation without any points.

For example,

```

load 1m
series{label="a"} 1 {{schema:1 sum:10 count:5 buckets:[1 2 3]}}

eval instant at 1m rate(series[1m1s])

```

Would have a PossibleNonCounterInfo annotation.

Wheras

```

load 1m
series{label="a"} {{schema:1 sum:10 count:5 buckets:[1 2 3]}} {{schema:1 sum:15 count:10 buckets:[1 2 3]}}

eval instant at 1m rate(series[1m1s])

```

Would not. 

---------

Signed-off-by: Joshua Hesketh <[email protected]>
  • Loading branch information
jhesketh authored Sep 18, 2024
1 parent bb47f78 commit b6107cc
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 3 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Changelog

## unreleased

* [BUGFIX] PromQL: Only return "possible non-counter" annotation when `rate` returns points. #14910

## 3.0.0-beta.0 / 2024-09-05

Release 3.0.0-beta.0 includes new features such as a brand new UI and UTF-8 support enabled by default. As a new major version, several breaking changes are introduced. The breaking changes are mainly around the removal of deprecated feature flags and CLI arguments, and the full list can be found below. Most users should be able to try this release out of the box without any configuration changes.
Expand Down
5 changes: 2 additions & 3 deletions promql/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -1742,9 +1742,8 @@ func (ev *evaluator) eval(ctx context.Context, expr parser.Expr) (parser.Value,
ev.samplesStats.UpdatePeak(ev.currentSamples)

if e.Func.Name == "rate" || e.Func.Name == "increase" {
samples := inMatrix[0]
metricName := samples.Metric.Get(labels.MetricName)
if metricName != "" && len(samples.Floats) > 0 &&
metricName := inMatrix[0].Metric.Get(labels.MetricName)
if metricName != "" && len(ss.Floats) > 0 &&
!strings.HasSuffix(metricName, "_total") &&
!strings.HasSuffix(metricName, "_sum") &&
!strings.HasSuffix(metricName, "_count") &&
Expand Down
73 changes: 73 additions & 0 deletions promql/engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"fmt"
"sort"
"strconv"
"strings"
"sync"
"testing"
"time"
Expand Down Expand Up @@ -3708,3 +3709,75 @@ histogram {{sum:4 count:4 buckets:[2 2]}} {{sum:6 count:6 buckets:[3 3]}} {{sum:
},
})
}

func TestRateAnnotations(t *testing.T) {
testCases := map[string]struct {
data string
expr string
expectedWarningAnnotations []string
expectedInfoAnnotations []string
}{
"info annotation when two samples are selected": {
data: `
series 1 2
`,
expr: "rate(series[1m1s])",
expectedWarningAnnotations: []string{},
expectedInfoAnnotations: []string{
`PromQL info: metric might not be a counter, name does not end in _total/_sum/_count/_bucket: "series" (1:6)`,
},
},
"no info annotations when no samples": {
data: `
series
`,
expr: "rate(series[1m1s])",
expectedWarningAnnotations: []string{},
expectedInfoAnnotations: []string{},
},
"no info annotations when selecting one sample": {
data: `
series 1 2
`,
expr: "rate(series[10s])",
expectedWarningAnnotations: []string{},
expectedInfoAnnotations: []string{},
},
"no info annotations when no samples due to mixed data types": {
data: `
series{label="a"} 1 {{schema:1 sum:15 count:10 buckets:[1 2 3]}}
`,
expr: "rate(series[1m1s])",
expectedWarningAnnotations: []string{
`PromQL warning: encountered a mix of histograms and floats for metric name "series" (1:6)`,
},
expectedInfoAnnotations: []string{},
},
"no info annotations when selecting two native histograms": {
data: `
series{label="a"} {{schema:1 sum:10 count:5 buckets:[1 2 3]}} {{schema:1 sum:15 count:10 buckets:[1 2 3]}}
`,
expr: "rate(series[1m1s])",
expectedWarningAnnotations: []string{},
expectedInfoAnnotations: []string{},
},
}
for name, testCase := range testCases {
t.Run(name, func(t *testing.T) {
store := promqltest.LoadedStorage(t, "load 1m\n"+strings.TrimSpace(testCase.data))
t.Cleanup(func() { _ = store.Close() })

engine := newTestEngine(t)
query, err := engine.NewInstantQuery(context.Background(), store, nil, testCase.expr, timestamp.Time(0).Add(1*time.Minute))
require.NoError(t, err)
t.Cleanup(query.Close)

res := query.Exec(context.Background())
require.NoError(t, res.Err)

warnings, infos := res.Warnings.AsStrings(testCase.expr, 0, 0)
testutil.RequireEqual(t, testCase.expectedWarningAnnotations, warnings)
testutil.RequireEqual(t, testCase.expectedInfoAnnotations, infos)
})
}
}

0 comments on commit b6107cc

Please sign in to comment.