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,8 @@ func main() {
 
 	logger := promslog.New(&cfg.promslogConfig)
 	slog.SetDefault(logger)
+	// Hacky but temporary
+	parser.Logger = logger.With("component", "parser")
 
 	notifs := notifications.NewNotifications(cfg.maxNotificationsSubscribers, prometheus.DefaultRegisterer)
 	cfg.web.NotificationsSub = notifs.Sub

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/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,91 @@ import (
 	"github.com/prometheus/prometheus/util/strutil"
 )
 
+var (
+	Logger                   = promslog.NewNopLogger()
+	narrowSelectorOnIntegers = prometheus.NewCounter(
+		prometheus.CounterOpts{
+			Name: "prometheus_parser_narrow_selectors_on_integers_count",
+			Help: "Number of selectors that are explicitly set to only match integers on 'quantile' and 'le' labels",
+		},
+	)
+)
+
+func init() {
+	prometheus.MustRegister(narrowSelectorOnIntegers)
+}
+
+// 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 {
+					narrowSelectorOnIntegers.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
+					}
+				}
+
+				narrowSelectorOnIntegers.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 +285,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 +927,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(narrowSelectorOnIntegers)
+			err := tt.fn(tt.expr)
+			require.NoError(t, err)
+
+			if tt.shouldIncrementCounter {
+				counter++
+			}
+
+			require.Equal(t, counter, client_testutil.ToFloat64(narrowSelectorOnIntegers))
+		})
+	}
+}