diff --git a/cmd/prometheus/main.go b/cmd/prometheus/main.go index 3e2bee26e4..ed7aa52c8a 100644 --- a/cmd/prometheus/main.go +++ b/cmd/prometheus/main.go @@ -860,6 +860,7 @@ func main() { EnableNegativeOffset: true, EnablePerStepStats: cfg.enablePerStepStats, EnableDelayedNameRemoval: cfg.promqlEnableDelayedNameRemoval, + EnableTypeAndUnitLabels: cfg.scrape.EnableTypeAndUnitLabels, } queryEngine = promql.NewEngine(opts) diff --git a/promql/engine.go b/promql/engine.go index fec2bbf761..1e01f21db6 100644 --- a/promql/engine.go +++ b/promql/engine.go @@ -326,6 +326,8 @@ type EngineOpts struct { // This is useful in certain scenarios where the __name__ label must be preserved or where applying a // regex-matcher to the __name__ label may otherwise lead to duplicate labelset errors. EnableDelayedNameRemoval bool + // EnableTypeAndUnitLabels will allow PromQL Engine to make decisions based on the type and unit labels. + EnableTypeAndUnitLabels bool } // Engine handles the lifetime of queries from beginning to end. @@ -344,6 +346,7 @@ type Engine struct { enableNegativeOffset bool enablePerStepStats bool enableDelayedNameRemoval bool + enableTypeAndUnitLabels bool } // NewEngine returns a new engine. @@ -435,6 +438,7 @@ func NewEngine(opts EngineOpts) *Engine { enableNegativeOffset: opts.EnableNegativeOffset, enablePerStepStats: opts.EnablePerStepStats, enableDelayedNameRemoval: opts.EnableDelayedNameRemoval, + enableTypeAndUnitLabels: opts.EnableTypeAndUnitLabels, } } @@ -744,6 +748,7 @@ func (ng *Engine) execEvalStmt(ctx context.Context, query *query, s *parser.Eval samplesStats: query.sampleStats, noStepSubqueryIntervalFn: ng.noStepSubqueryIntervalFn, enableDelayedNameRemoval: ng.enableDelayedNameRemoval, + enableTypeAndUnitLabels: ng.enableTypeAndUnitLabels, querier: querier, } query.sampleStats.InitStepTracking(start, start, 1) @@ -803,6 +808,7 @@ func (ng *Engine) execEvalStmt(ctx context.Context, query *query, s *parser.Eval samplesStats: query.sampleStats, noStepSubqueryIntervalFn: ng.noStepSubqueryIntervalFn, enableDelayedNameRemoval: ng.enableDelayedNameRemoval, + enableTypeAndUnitLabels: ng.enableTypeAndUnitLabels, querier: querier, } query.sampleStats.InitStepTracking(evaluator.startTimestamp, evaluator.endTimestamp, evaluator.interval) @@ -1076,6 +1082,7 @@ type evaluator struct { samplesStats *stats.QuerySamples noStepSubqueryIntervalFn func(rangeMillis int64) int64 enableDelayedNameRemoval bool + enableTypeAndUnitLabels bool querier storage.Querier } @@ -1890,12 +1897,20 @@ func (ev *evaluator) eval(ctx context.Context, expr parser.Expr) (parser.Value, if e.Func.Name == "rate" || e.Func.Name == "increase" { 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") && - !strings.HasSuffix(metricName, "_bucket") { - warnings.Add(annotations.NewPossibleNonCounterInfo(metricName, e.Args[0].PositionRange())) + if metricName != "" && len(ss.Floats) > 0 { + if ev.enableTypeAndUnitLabels { + // When type-and-unit-labels feature is enabled, check __type__ label + typeLabel := inMatrix[0].Metric.Get("__type__") + if typeLabel != string(model.MetricTypeCounter) { + warnings.Add(annotations.NewPossibleNonCounterLabelInfo(metricName, typeLabel, e.Args[0].PositionRange())) + } + } else if !strings.HasSuffix(metricName, "_total") || + !strings.HasSuffix(metricName, "_sum") || + !strings.HasSuffix(metricName, "_count") || + !strings.HasSuffix(metricName, "_bucket") { + // Fallback to name suffix checking + warnings.Add(annotations.NewPossibleNonCounterInfo(metricName, e.Args[0].PositionRange())) + } } } } @@ -2060,6 +2075,7 @@ func (ev *evaluator) eval(ctx context.Context, expr parser.Expr) (parser.Value, samplesStats: ev.samplesStats.NewChild(), noStepSubqueryIntervalFn: ev.noStepSubqueryIntervalFn, enableDelayedNameRemoval: ev.enableDelayedNameRemoval, + enableTypeAndUnitLabels: ev.enableTypeAndUnitLabels, querier: ev.querier, } @@ -2105,6 +2121,7 @@ func (ev *evaluator) eval(ctx context.Context, expr parser.Expr) (parser.Value, samplesStats: ev.samplesStats.NewChild(), noStepSubqueryIntervalFn: ev.noStepSubqueryIntervalFn, enableDelayedNameRemoval: ev.enableDelayedNameRemoval, + enableTypeAndUnitLabels: ev.enableTypeAndUnitLabels, querier: ev.querier, } res, ws := newEv.eval(ctx, e.Expr) diff --git a/promql/engine_test.go b/promql/engine_test.go index c6136b148a..858c02d87f 100644 --- a/promql/engine_test.go +++ b/promql/engine_test.go @@ -3429,6 +3429,7 @@ func TestRateAnnotations(t *testing.T) { testCases := map[string]struct { data string expr string + typeAndUnitLabelsEnabled bool expectedWarningAnnotations []string expectedInfoAnnotations []string }{ @@ -3476,13 +3477,85 @@ func TestRateAnnotations(t *testing.T) { expectedWarningAnnotations: []string{}, expectedInfoAnnotations: []string{}, }, + "info annotation when rate() over series without _total suffix": { + data: ` + series{label="a"} 1 2 3 + `, + expr: "rate(series[1m1s])", + expectedInfoAnnotations: []string{ + `PromQL info: metric might not be a counter, name does not end in _total/_sum/_count/_bucket: "series" (1:6)`, + }, + expectedWarningAnnotations: []string{}, + }, + "info annotation when increase() over series without _total suffix": { + data: ` + series{label="a"} 1 2 3 + `, + expr: "increase(series[1m1s])", + expectedInfoAnnotations: []string{ + `PromQL info: metric might not be a counter, name does not end in _total/_sum/_count/_bucket: "series" (1:10)`, + }, + expectedWarningAnnotations: []string{}, + }, + "info annotation when rate() over series without __type__=counter label": { + data: ` + series{label="a"} 1 2 3 + `, + expr: "rate(series[1m1s])", + typeAndUnitLabelsEnabled: true, + expectedInfoAnnotations: []string{ + `PromQL info: metric might not be a counter, __type__ label is not set to "counter", got "": "series" (1:6)`, + }, + expectedWarningAnnotations: []string{}, + }, + "info annotation when increase() over series without __type__=counter label": { + data: ` + series{label="a"} 1 2 3 + `, + expr: "increase(series[1m1s])", + typeAndUnitLabelsEnabled: true, + expectedInfoAnnotations: []string{ + `PromQL info: metric might not be a counter, __type__ label is not set to "counter", got "": "series" (1:10)`, + }, + expectedWarningAnnotations: []string{}, + }, + "no info annotation when rate() over series with __type__=counter label": { + data: ` + series{label="a", __type__="counter"} 1 2 3 + `, + expr: "rate(series[1m1s])", + typeAndUnitLabelsEnabled: true, + expectedWarningAnnotations: []string{}, + expectedInfoAnnotations: []string{}, + }, + "no info annotation when increase() over series with __type__=counter label": { + data: ` + series{label="a", __type__="counter"} 1 2 3 + `, + expr: "increase(series[1m1s])", + typeAndUnitLabelsEnabled: true, + 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) + engine := promqltest.NewTestEngineWithOpts(t, promql.EngineOpts{ + Logger: nil, + Reg: nil, + MaxSamples: promqltest.DefaultMaxSamplesPerQuery, + Timeout: 100 * time.Minute, + NoStepSubqueryIntervalFn: func(int64) int64 { return int64((1 * time.Minute / time.Millisecond / time.Nanosecond)) }, + EnableAtModifier: true, + EnableNegativeOffset: true, + EnablePerStepStats: false, + LookbackDelta: 0, + EnableDelayedNameRemoval: true, + EnableTypeAndUnitLabels: testCase.typeAndUnitLabelsEnabled, + }) 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) diff --git a/util/annotations/annotations.go b/util/annotations/annotations.go index a8e1d9f900..1b3f9e2ff2 100644 --- a/util/annotations/annotations.go +++ b/util/annotations/annotations.go @@ -147,6 +147,7 @@ var ( IncompatibleBucketLayoutInBinOpWarning = fmt.Errorf("%w: incompatible bucket layout encountered for binary operator", PromQLWarning) PossibleNonCounterInfo = fmt.Errorf("%w: metric might not be a counter, name does not end in _total/_sum/_count/_bucket:", PromQLInfo) + PossibleNonCounterLabelInfo = fmt.Errorf("%w: metric might not be a counter, __type__ label is not set to %q", PromQLInfo, model.MetricTypeCounter) HistogramQuantileForcedMonotonicityInfo = fmt.Errorf("%w: input to histogram_quantile needed to be fixed for monotonicity (see https://prometheus.io/docs/prometheus/latest/querying/functions/#histogram_quantile) for metric name", PromQLInfo) IncompatibleTypesInBinOpInfo = fmt.Errorf("%w: incompatible sample types encountered for binary operator", PromQLInfo) HistogramIgnoredInAggregationInfo = fmt.Errorf("%w: ignored histogram in", PromQLInfo) @@ -270,6 +271,15 @@ func NewPossibleNonCounterInfo(metricName string, pos posrange.PositionRange) er } } +// NewPossibleNonCounterLabelInfo is used when a named counter metric with only float samples does not +// have the __type__ label set to "counter". +func NewPossibleNonCounterLabelInfo(metricName, typeLabel string, pos posrange.PositionRange) error { + return annoErr{ + PositionRange: pos, + Err: fmt.Errorf("%w, got %q: %q", PossibleNonCounterLabelInfo, typeLabel, metricName), + } +} + // NewHistogramQuantileForcedMonotonicityInfo is used when the input (classic histograms) to // histogram_quantile needs to be forced to be monotonic. func NewHistogramQuantileForcedMonotonicityInfo(metricName string, pos posrange.PositionRange) error {