Provide PromQL info annotations when rate()/increase() over series without counter label
Signed-off-by: Arthur Silva Sens <arthursens2005@gmail.com>
This commit is contained in:
parent
24057883a1
commit
f74b019505
|
@ -860,6 +860,7 @@ func main() {
|
||||||
EnableNegativeOffset: true,
|
EnableNegativeOffset: true,
|
||||||
EnablePerStepStats: cfg.enablePerStepStats,
|
EnablePerStepStats: cfg.enablePerStepStats,
|
||||||
EnableDelayedNameRemoval: cfg.promqlEnableDelayedNameRemoval,
|
EnableDelayedNameRemoval: cfg.promqlEnableDelayedNameRemoval,
|
||||||
|
EnableTypeAndUnitLabels: cfg.scrape.EnableTypeAndUnitLabels,
|
||||||
}
|
}
|
||||||
|
|
||||||
queryEngine = promql.NewEngine(opts)
|
queryEngine = promql.NewEngine(opts)
|
||||||
|
|
|
@ -326,6 +326,8 @@ type EngineOpts struct {
|
||||||
// This is useful in certain scenarios where the __name__ label must be preserved or where applying a
|
// 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.
|
// regex-matcher to the __name__ label may otherwise lead to duplicate labelset errors.
|
||||||
EnableDelayedNameRemoval bool
|
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.
|
// Engine handles the lifetime of queries from beginning to end.
|
||||||
|
@ -344,6 +346,7 @@ type Engine struct {
|
||||||
enableNegativeOffset bool
|
enableNegativeOffset bool
|
||||||
enablePerStepStats bool
|
enablePerStepStats bool
|
||||||
enableDelayedNameRemoval bool
|
enableDelayedNameRemoval bool
|
||||||
|
enableTypeAndUnitLabels bool
|
||||||
}
|
}
|
||||||
|
|
||||||
// NewEngine returns a new engine.
|
// NewEngine returns a new engine.
|
||||||
|
@ -435,6 +438,7 @@ func NewEngine(opts EngineOpts) *Engine {
|
||||||
enableNegativeOffset: opts.EnableNegativeOffset,
|
enableNegativeOffset: opts.EnableNegativeOffset,
|
||||||
enablePerStepStats: opts.EnablePerStepStats,
|
enablePerStepStats: opts.EnablePerStepStats,
|
||||||
enableDelayedNameRemoval: opts.EnableDelayedNameRemoval,
|
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,
|
samplesStats: query.sampleStats,
|
||||||
noStepSubqueryIntervalFn: ng.noStepSubqueryIntervalFn,
|
noStepSubqueryIntervalFn: ng.noStepSubqueryIntervalFn,
|
||||||
enableDelayedNameRemoval: ng.enableDelayedNameRemoval,
|
enableDelayedNameRemoval: ng.enableDelayedNameRemoval,
|
||||||
|
enableTypeAndUnitLabels: ng.enableTypeAndUnitLabels,
|
||||||
querier: querier,
|
querier: querier,
|
||||||
}
|
}
|
||||||
query.sampleStats.InitStepTracking(start, start, 1)
|
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,
|
samplesStats: query.sampleStats,
|
||||||
noStepSubqueryIntervalFn: ng.noStepSubqueryIntervalFn,
|
noStepSubqueryIntervalFn: ng.noStepSubqueryIntervalFn,
|
||||||
enableDelayedNameRemoval: ng.enableDelayedNameRemoval,
|
enableDelayedNameRemoval: ng.enableDelayedNameRemoval,
|
||||||
|
enableTypeAndUnitLabels: ng.enableTypeAndUnitLabels,
|
||||||
querier: querier,
|
querier: querier,
|
||||||
}
|
}
|
||||||
query.sampleStats.InitStepTracking(evaluator.startTimestamp, evaluator.endTimestamp, evaluator.interval)
|
query.sampleStats.InitStepTracking(evaluator.startTimestamp, evaluator.endTimestamp, evaluator.interval)
|
||||||
|
@ -1076,6 +1082,7 @@ type evaluator struct {
|
||||||
samplesStats *stats.QuerySamples
|
samplesStats *stats.QuerySamples
|
||||||
noStepSubqueryIntervalFn func(rangeMillis int64) int64
|
noStepSubqueryIntervalFn func(rangeMillis int64) int64
|
||||||
enableDelayedNameRemoval bool
|
enableDelayedNameRemoval bool
|
||||||
|
enableTypeAndUnitLabels bool
|
||||||
querier storage.Querier
|
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" {
|
if e.Func.Name == "rate" || e.Func.Name == "increase" {
|
||||||
metricName := inMatrix[0].Metric.Get(labels.MetricName)
|
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__")
|
||||||
!strings.HasSuffix(metricName, "_bucket") {
|
if typeLabel != "counter" {
|
||||||
warnings.Add(annotations.NewPossibleNonCounterInfo(metricName, e.Args[0].PositionRange()))
|
warnings.Add(annotations.NewPossibleNonCounterLabelInfo(metricName, 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(),
|
samplesStats: ev.samplesStats.NewChild(),
|
||||||
noStepSubqueryIntervalFn: ev.noStepSubqueryIntervalFn,
|
noStepSubqueryIntervalFn: ev.noStepSubqueryIntervalFn,
|
||||||
enableDelayedNameRemoval: ev.enableDelayedNameRemoval,
|
enableDelayedNameRemoval: ev.enableDelayedNameRemoval,
|
||||||
|
enableTypeAndUnitLabels: ev.enableTypeAndUnitLabels,
|
||||||
querier: ev.querier,
|
querier: ev.querier,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -2105,6 +2121,7 @@ func (ev *evaluator) eval(ctx context.Context, expr parser.Expr) (parser.Value,
|
||||||
samplesStats: ev.samplesStats.NewChild(),
|
samplesStats: ev.samplesStats.NewChild(),
|
||||||
noStepSubqueryIntervalFn: ev.noStepSubqueryIntervalFn,
|
noStepSubqueryIntervalFn: ev.noStepSubqueryIntervalFn,
|
||||||
enableDelayedNameRemoval: ev.enableDelayedNameRemoval,
|
enableDelayedNameRemoval: ev.enableDelayedNameRemoval,
|
||||||
|
enableTypeAndUnitLabels: ev.enableTypeAndUnitLabels,
|
||||||
querier: ev.querier,
|
querier: ev.querier,
|
||||||
}
|
}
|
||||||
res, ws := newEv.eval(ctx, e.Expr)
|
res, ws := newEv.eval(ctx, e.Expr)
|
||||||
|
|
|
@ -3428,6 +3428,7 @@ func TestRateAnnotations(t *testing.T) {
|
||||||
testCases := map[string]struct {
|
testCases := map[string]struct {
|
||||||
data string
|
data string
|
||||||
expr string
|
expr string
|
||||||
|
typeAndUnitLabelsEnabled bool
|
||||||
expectedWarningAnnotations []string
|
expectedWarningAnnotations []string
|
||||||
expectedInfoAnnotations []string
|
expectedInfoAnnotations []string
|
||||||
}{
|
}{
|
||||||
|
@ -3475,13 +3476,85 @@ func TestRateAnnotations(t *testing.T) {
|
||||||
expectedWarningAnnotations: []string{},
|
expectedWarningAnnotations: []string{},
|
||||||
expectedInfoAnnotations: []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": "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": "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 {
|
for name, testCase := range testCases {
|
||||||
t.Run(name, func(t *testing.T) {
|
t.Run(name, func(t *testing.T) {
|
||||||
store := promqltest.LoadedStorage(t, "load 1m\n"+strings.TrimSpace(testCase.data))
|
store := promqltest.LoadedStorage(t, "load 1m\n"+strings.TrimSpace(testCase.data))
|
||||||
t.Cleanup(func() { _ = store.Close() })
|
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))
|
query, err := engine.NewInstantQuery(context.Background(), store, nil, testCase.expr, timestamp.Time(0).Add(1*time.Minute))
|
||||||
require.NoError(t, err)
|
require.NoError(t, err)
|
||||||
t.Cleanup(query.Close)
|
t.Cleanup(query.Close)
|
||||||
|
|
|
@ -147,6 +147,7 @@ var (
|
||||||
IncompatibleBucketLayoutInBinOpWarning = fmt.Errorf("%w: incompatible bucket layout encountered for binary operator", PromQLWarning)
|
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)
|
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, "counter")
|
||||||
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)
|
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)
|
IncompatibleTypesInBinOpInfo = fmt.Errorf("%w: incompatible sample types encountered for binary operator", PromQLInfo)
|
||||||
HistogramIgnoredInAggregationInfo = fmt.Errorf("%w: ignored histogram in", 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 string, pos posrange.PositionRange) error {
|
||||||
|
return annoErr{
|
||||||
|
PositionRange: pos,
|
||||||
|
Err: fmt.Errorf("%w %q", PossibleNonCounterLabelInfo, metricName),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// NewHistogramQuantileForcedMonotonicityInfo is used when the input (classic histograms) to
|
// NewHistogramQuantileForcedMonotonicityInfo is used when the input (classic histograms) to
|
||||||
// histogram_quantile needs to be forced to be monotonic.
|
// histogram_quantile needs to be forced to be monotonic.
|
||||||
func NewHistogramQuantileForcedMonotonicityInfo(metricName string, pos posrange.PositionRange) error {
|
func NewHistogramQuantileForcedMonotonicityInfo(metricName string, pos posrange.PositionRange) error {
|
||||||
|
|
Loading…
Reference in New Issue