feat(parser): offer assistance for a smooth Prom 3 upgrade after the …

…"quantile" and "le" normalization breaking change

See https://prometheus.io/docs/prometheus/latest/migration/#le-and-quantile-label-values

cmd/prometheus/main.go

@@ -539,6 +539,9 @@ func main() {
 
 	logger := promslog.New(&cfg.promslogConfig)
 	slog.SetDefault(logger)
+	// Hacky but temporary
+	parser.Logger = logger.With("component", "parser")
+	relabel.Logger = logger.With("component", "relabel")
 
 	notifs := notifications.NewNotifications(cfg.maxNotificationsSubscribers, prometheus.DefaultRegisterer)
 	cfg.web.NotificationsSub = notifs.Sub
@@ -994,18 +997,12 @@ func main() {
 	listeners, err := webHandler.Listeners()
 	if err != nil {
 		logger.Error("Unable to start web listener", "err", err)
-		if err := queryEngine.Close(); err != nil {
-			logger.Warn("Closing query engine failed", "err", err)
-		}
 		os.Exit(1)
 	}
 
 	err = toolkit_web.Validate(*webConfig)
 	if err != nil {
 		logger.Error("Unable to validate web configuration file", "err", err)
-		if err := queryEngine.Close(); err != nil {
-			logger.Warn("Closing query engine failed", "err", err)
-		}
 		os.Exit(1)
 	}
 
@@ -1027,9 +1024,6 @@ func main() {
 				case <-cancel:
 					reloadReady.Close()
 				}
-				if err := queryEngine.Close(); err != nil {
-					logger.Warn("Closing query engine failed", "err", err)
-				}
 				return nil
 			},
 			func(err error) {

model/relabel/relabel.go

@@ -22,11 +22,15 @@ import (
 
 	"github.com/grafana/regexp"
 	"github.com/prometheus/common/model"
+	"github.com/prometheus/common/promslog"
 
 	"github.com/prometheus/prometheus/model/labels"
+	"github.com/prometheus/prometheus/promql/parser"
 )
 
 var (
+	Logger = promslog.NewNopLogger()
+
 	relabelTarget = regexp.MustCompile(`^(?:(?:[a-zA-Z_]|\$(?:\{\w+\}|\w+))+\w*)+$`)
 
 	DefaultRelabelConfig = Config{
@@ -254,7 +258,18 @@ func relabel(cfg *Config, lb *labels.Builder) (keep bool) {
 	if len(cfg.SourceLabels) > cap(values) {
 		values = make([]string, 0, len(cfg.SourceLabels))
 	}
-	for _, ln := range cfg.SourceLabels {
+	for i, ln := range cfg.SourceLabels {
+		if ln == model.BucketLabel || ln == model.QuantileLabel {
+			vals := strings.Split(cfg.Regex.String(), cfg.Separator)
+			if len(vals) <= i {
+				continue
+			}
+			// Supposing regexes ending with (\.0)? to be already adjusted, excluding them to avoid noise.
+			if !strings.HasSuffix(vals[i], `(\.0)?`) {
+				parser.NarrowSelectors.WithLabelValues("relabel").Inc()
+				Logger.Debug("relabel_config concerns 'le' or 'quantile' labels, it may need to be adjusted to count for float values", "source_labels", cfg.SourceLabels, "regex", cfg.Regex)
+			}
+		}
 		values = append(values, lb.Get(string(ln)))
 	}
 	val := strings.Join(values, cfg.Separator)

promql/bench_test.go

@@ -539,6 +539,10 @@ func generateNativeHistogramSeries(app storage.Appender, numSeries int) error {
 
 func BenchmarkParser(b *testing.B) {
 	cases := []string{
+		`foo_bucket{a="b",c="d",le=~"1.0|2.0|3.0|4"}`,
+		`bar_bucket{a="b",c="d",le="1.0"}`,
+		`foo{a="b",c="d",quantile="0"}`,
+		`bar{a="b",c="d",quantile="0.99"}`,
 		"a",
 		"metric",
 		"1",

promql/engine.go

@@ -436,6 +436,8 @@ func NewEngine(opts EngineOpts) *Engine {
 }
 
 // Close closes ng.
+// Callers must ensure the engine is really no longer in use before calling this to avoid
+// issues failures like in https://github.com/prometheus/prometheus/issues/15232
 func (ng *Engine) Close() error {
 	if ng == nil {
 		return nil

promql/parser/parse.go

@@ -24,7 +24,9 @@ import (
 	"sync"
 	"time"
 
+	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/common/model"
+	"github.com/prometheus/common/promslog"
 
 	"github.com/prometheus/prometheus/model/histogram"
 	"github.com/prometheus/prometheus/model/labels"
@@ -33,6 +35,92 @@ import (
 	"github.com/prometheus/prometheus/util/strutil"
 )
 
+var (
+	Logger          = promslog.NewNopLogger()
+	NarrowSelectors = prometheus.NewCounterVec(
+		prometheus.CounterOpts{
+			Name: "prometheus_narrow_selectors_count",
+			Help: "Number of selectors that may unintentionally only match integers on 'quantile' and 'le' labels",
+		},
+		[]string{"component"},
+	)
+)
+
+func init() {
+	prometheus.MustRegister(NarrowSelectors)
+}
+
+// isInteger is a light strconv.Atoi that avoids allocations due to Atoi's returned error and doesn't
+// consider as integers invalid, big, or underscored integers.
+func isInteger(s string) (int, bool) {
+	sLen := len(s)
+	if strconv.IntSize == 32 && (0 < sLen && sLen < 10) ||
+		strconv.IntSize == 64 && (0 < sLen && sLen < 19) {
+		// Fast path for small integers that fit int type.
+		s0 := s
+		if s[0] == '-' || s[0] == '+' {
+			s = s[1:]
+			if len(s) < 1 {
+				return 0, false
+			}
+		}
+
+		n := 0
+		for _, ch := range []byte(s) {
+			ch -= '0'
+			if ch > 9 {
+				return 0, false
+			}
+			n = n*10 + int(ch)
+		}
+		if s0[0] == '-' {
+			n = -n
+		}
+		return n, true
+	}
+
+	// Slow path for invalid, big, or underscored integers.
+	i64, err := strconv.ParseInt(s, 10, 0)
+	return int(i64), err == nil
+}
+
+// checkLabelMatchers helps to detect unintentional misuses of matchers on "quantile" and "le" labels after normalization during scraping
+// introduced in Prometheus3. For more details, see https://prometheus.io/docs/prometheus/latest/migration/#le-and-quantile-label-values.
+// It specifically detects integers-only label matchers formatted as "integer" and "integer|integer|integer" (as they're the most likely to contain them).
+// Refer to Test_checkLabelMatchers for more details.
+func checkLabelMatchers(vs *VectorSelector) {
+Outer:
+	for _, lm := range vs.LabelMatchers {
+		if lm == nil {
+			continue
+		}
+		if lm.Name == model.QuantileLabel || (strings.HasSuffix(vs.Name, "_bucket") && lm.Name == model.BucketLabel) {
+			switch lm.Type {
+			case labels.MatchEqual, labels.MatchNotEqual:
+				if n, ok := isInteger(lm.Value); ok {
+					NarrowSelectors.WithLabelValues("parser").Inc()
+					Logger.Debug("selector set to explicitly match an integer, but values could be floats", "narrow_matcher_label", lm.Name, "integer", n, "matchers", vs.LabelMatchers)
+				}
+				break Outer
+			case labels.MatchRegexp, labels.MatchNotRegexp:
+				if len(lm.Value) == 0 {
+					break Outer
+				}
+				vals := strings.Split(lm.Value, "|")
+				for _, val := range vals {
+					if _, ok := isInteger(val); !ok {
+						break Outer
+					}
+				}
+
+				NarrowSelectors.WithLabelValues("parser").Inc()
+				Logger.Debug("selector set to explicitly match integers only, but values could be floats", "narrow_matcher_label", lm.Name, "matchers", vs.LabelMatchers)
+				break Outer
+			}
+		}
+	}
+}
+
 var parserPool = sync.Pool{
 	New: func() interface{} {
 		return &parser{}
@@ -198,6 +286,7 @@ func ParseMetricSelector(input string) (m []*labels.Matcher, err error) {
 
 	parseResult := p.parseGenerated(START_METRIC_SELECTOR)
 	if parseResult != nil {
+		checkLabelMatchers(parseResult.(*VectorSelector))
 		m = parseResult.(*VectorSelector).LabelMatchers
 	}
 
@@ -839,6 +928,8 @@ func (p *parser) checkAST(node Node) (typ ValueType) {
 				}
 			}
 
+			checkLabelMatchers(n)
+
 			// Skip the check for non-empty matchers because an explicit
 			// metric name is a non-empty matcher.
 			break

promql/parser/parse_test.go

@@ -28,6 +28,8 @@ import (
 	"github.com/prometheus/prometheus/model/labels"
 	"github.com/prometheus/prometheus/util/testutil"
 
+	client_testutil "github.com/prometheus/client_golang/prometheus/testutil"
+
 	"github.com/prometheus/prometheus/promql/parser/posrange"
 )
 
@@ -4607,3 +4609,64 @@ func TestParseCustomFunctions(t *testing.T) {
 	require.True(t, ok)
 	require.Equal(t, "custom_func", call.Func.Name)
 }
+
+func Test_checkLabelMatchers(t *testing.T) {
+	parseExpr := func(s string) error {
+		_, err := ParseExpr(s)
+		return err
+	}
+
+	parseMetricSelector := func(s string) error {
+		_, err := ParseMetricSelector(s)
+		return err
+	}
+
+	cases := []struct {
+		expr                   string
+		shouldIncrementCounter bool
+		fn                     func(string) error
+	}{
+		{`rate(foo_bucket{le="1"}[1d])`, true, parseExpr},
+		{`sum(baz) - sum(foo_bucket{a="b",le=~"1|2|3"})`, true, parseExpr},
+
+		{`foo_bucket{le="1"}`, true, parseMetricSelector},
+		{`foo_bucket{le="-1"}`, true, parseMetricSelector},
+		{`foo_bucket{a="b",le="1"}`, true, parseMetricSelector},
+		{`foo_bucket{le=~"5|1|3"}`, true, parseMetricSelector},
+		{`foo_bucket{bar="1"}`, false, parseMetricSelector},
+		{`foo_bucket{le=~"0.5|1.5"}`, false, parseMetricSelector},
+		{`foo_bucket{le="0.5"}`, false, parseMetricSelector},
+		{`foo{le="0"}`, false, parseMetricSelector},
+		{`foo_bucket{le=~"0.5|1"}`, false, parseMetricSelector},
+		{`foo_bucket{le=~"1|0.6"}`, false, parseMetricSelector},
+		{`foo_bucket{le=""}`, false, parseMetricSelector},
+		// name inside the braces is not supported
+		{`{le="1",__name__="foo_bucket"}`, false, parseMetricSelector},
+
+		{`bar{quantile="0"}`, true, parseMetricSelector},
+		{`bar{quantile="-0"}`, true, parseMetricSelector},
+		{`bar{a="b",quantile="0"}`, true, parseMetricSelector},
+		{`bar{quantile=~"0|1"}`, true, parseMetricSelector},
+		{`{quantile="1",__name__="bar"}`, true, parseMetricSelector},
+		{`foo_bucket{bar="0"}`, false, parseMetricSelector},
+		{`bar{quantile="0.95"}`, false, parseMetricSelector},
+		{`bar{quantile=~"0.5|0.95"}`, false, parseMetricSelector},
+		{`bar{quantile=~"0.5|0"}`, false, parseMetricSelector},
+		{`bar{quantile=~"0|0.6"}`, false, parseMetricSelector},
+		{`bar{quantile=""}`, false, parseMetricSelector},
+	}
+
+	for _, tt := range cases {
+		t.Run(tt.expr, func(t *testing.T) {
+			counter := client_testutil.ToFloat64(NarrowSelectors.WithLabelValues("parser"))
+			err := tt.fn(tt.expr)
+			require.NoError(t, err)
+
+			if tt.shouldIncrementCounter {
+				counter++
+			}
+
+			require.Equal(t, counter, client_testutil.ToFloat64(NarrowSelectors.WithLabelValues("parser")))
+		})
+	}
+}