PROM-39: Provide PromQL info annotations when rate()/increase() over series without `__type__`="counter" label (#16632)

* Provide PromQL info annotations when rate()/increase() over series without counter label

Signed-off-by: Arthur Silva Sens <arthursens2005@gmail.com>

* Address comments

Signed-off-by: Arthur Silva Sens <arthursens2005@gmail.com>

---------

Signed-off-by: Arthur Silva Sens <arthursens2005@gmail.com>

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)

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,15 +1897,23 @@ 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 &&
+				if metricName != "" && len(ss.Floats) > 0 {
-					!strings.HasSuffix(metricName, "_total") &&
+					if ev.enableTypeAndUnitLabels {
-					!strings.HasSuffix(metricName, "_sum") &&
+						// When type-and-unit-labels feature is enabled, check __type__ label
-					!strings.HasSuffix(metricName, "_count") &&
+						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()))
 					}
 				}
 			}
+		}
 		ev.samplesStats.UpdatePeak(ev.currentSamples)
 
 		ev.currentSamples -= len(floats) + totalHPointSize(histograms)
@@ -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)

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)

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 {