From 0a4c1cf231b47d0e99c82c91d439fb925ac316de Mon Sep 17 00:00:00 2001 From: George Robinson Date: Fri, 5 Jan 2024 10:21:20 +0000 Subject: [PATCH] Support UTF-8 label matchers: Add metrics to matchers compat package (#3658) * Add metrics to matchers compat package This commit adds the following metrics to the compat package: alertmanager_matchers_parse alertmanager_matchers_disagree alertmanager_matchers_incompatible alertmanager_matchers_invalid With a label called origin to differentiate the different sources of inputs: the configuration file, the API, and amtool. The disagree_total metric is incremented when an input is invalid in both parsers, but results in different parsed representations, then there is disagreement. This should not happen, and suggests their is either a bug in one of the parsers or a mistake in the backwards compatible guarantees of the matchers/parse parser. The incompatible_total metric is incremented when an input is valid in pkg/labels, but not the UTF-8 parser in matchers/parse. In such case, the matcher should be updated to be compatible. This often means adding double quotes around the right hand side of the matcher. For example, foo="bar". The invalid_total metric is incremented when an input is invalid in both parsers. This was never a valid input. The tests have been updated to check the metrics are incremented as expected. Signed-off-by: George Robinson --------- Signed-off-by: George Robinson Signed-off-by: Gokhan Sari --- api/v2/api.go | 2 +- cli/alert_add.go | 6 +- cli/alert_query.go | 2 +- cli/root.go | 2 +- cli/silence_add.go | 4 +- cli/silence_query.go | 2 +- cli/test_routing.go | 2 +- cmd/alertmanager/main.go | 2 +- config/config.go | 4 +- matchers/compat/metrics.go | 60 +++++++++ matchers/compat/parse.go | 232 +++++++++++++++++++++------------- matchers/compat/parse_test.go | 115 +++++++++++++---- 12 files changed, 306 insertions(+), 127 deletions(-) create mode 100644 matchers/compat/metrics.go diff --git a/api/v2/api.go b/api/v2/api.go index 3e0bc0fd9b..9d70625cd8 100644 --- a/api/v2/api.go +++ b/api/v2/api.go @@ -684,7 +684,7 @@ func (api *API) postSilencesHandler(params silence_ops.PostSilencesParams) middl func parseFilter(filter []string) ([]*labels.Matcher, error) { matchers := make([]*labels.Matcher, 0, len(filter)) for _, matcherString := range filter { - matcher, err := compat.Matcher(matcherString) + matcher, err := compat.Matcher(matcherString, "api") if err != nil { return nil, err } diff --git a/cli/alert_add.go b/cli/alert_add.go index 6018b95654..433afc0765 100644 --- a/cli/alert_add.go +++ b/cli/alert_add.go @@ -77,14 +77,14 @@ func (a *alertAddCmd) addAlert(ctx context.Context, _ *kingpin.ParseContext) err if len(a.labels) > 0 { // Allow the alertname label to be defined implicitly as the first argument rather // than explicitly as a key=value pair. - if _, err := compat.Matcher(a.labels[0]); err != nil { + if _, err := compat.Matcher(a.labels[0], "cli"); err != nil { a.labels[0] = fmt.Sprintf("alertname=%s", strconv.Quote(a.labels[0])) } } ls := make(models.LabelSet, len(a.labels)) for _, l := range a.labels { - matcher, err := compat.Matcher(l) + matcher, err := compat.Matcher(l, "cli") if err != nil { return err } @@ -96,7 +96,7 @@ func (a *alertAddCmd) addAlert(ctx context.Context, _ *kingpin.ParseContext) err annotations := make(models.LabelSet, len(a.annotations)) for _, a := range a.annotations { - matcher, err := compat.Matcher(a) + matcher, err := compat.Matcher(a, "cli") if err != nil { return err } diff --git a/cli/alert_query.go b/cli/alert_query.go index e5f5ba6ac8..e4bddaa651 100644 --- a/cli/alert_query.go +++ b/cli/alert_query.go @@ -81,7 +81,7 @@ func (a *alertQueryCmd) queryAlerts(ctx context.Context, _ *kingpin.ParseContext // the user wants alertname= and prepend `alertname=` to // the front. m := a.matcherGroups[0] - _, err := compat.Matcher(m) + _, err := compat.Matcher(m, "cli") if err != nil { a.matcherGroups[0] = fmt.Sprintf("alertname=%s", strconv.Quote(m)) } diff --git a/cli/root.go b/cli/root.go index 69c1022c6a..e1e5ac59aa 100644 --- a/cli/root.go +++ b/cli/root.go @@ -61,7 +61,7 @@ func initMatchersCompat(_ *kingpin.ParseContext) error { if err != nil { kingpin.Fatalf("error parsing the feature flag list: %v\n", err) } - compat.InitFromFlags(logger, featureConfig) + compat.InitFromFlags(logger, compat.RegisteredMetrics, featureConfig) return nil } diff --git a/cli/silence_add.go b/cli/silence_add.go index d30a523431..4456ddec9a 100644 --- a/cli/silence_add.go +++ b/cli/silence_add.go @@ -95,7 +95,7 @@ func (c *silenceAddCmd) add(ctx context.Context, _ *kingpin.ParseContext) error // If the parser fails then we likely don't have a (=|=~|!=|!~) so lets // assume that the user wants alertname= and prepend `alertname=` // to the front. - _, err := compat.Matcher(c.matchers[0]) + _, err := compat.Matcher(c.matchers[0], "cli") if err != nil { c.matchers[0] = fmt.Sprintf("alertname=%s", strconv.Quote(c.matchers[0])) } @@ -103,7 +103,7 @@ func (c *silenceAddCmd) add(ctx context.Context, _ *kingpin.ParseContext) error matchers := make([]labels.Matcher, 0, len(c.matchers)) for _, s := range c.matchers { - m, err := compat.Matcher(s) + m, err := compat.Matcher(s, "cli") if err != nil { return err } diff --git a/cli/silence_query.go b/cli/silence_query.go index b25569f818..5eb33a27d7 100644 --- a/cli/silence_query.go +++ b/cli/silence_query.go @@ -99,7 +99,7 @@ func (c *silenceQueryCmd) query(ctx context.Context, _ *kingpin.ParseContext) er // If the parser fails then we likely don't have a (=|=~|!=|!~) so lets // assume that the user wants alertname= and prepend `alertname=` // to the front. - _, err := compat.Matcher(c.matchers[0]) + _, err := compat.Matcher(c.matchers[0], "cli") if err != nil { c.matchers[0] = fmt.Sprintf("alertname=%s", strconv.Quote(c.matchers[0])) } diff --git a/cli/test_routing.go b/cli/test_routing.go index 85c29a7e2f..589a2e8cf8 100644 --- a/cli/test_routing.go +++ b/cli/test_routing.go @@ -84,7 +84,7 @@ func (c *routingShow) routingTestAction(ctx context.Context, _ *kingpin.ParseCon // Parse labels to LabelSet. ls := make(models.LabelSet, len(c.labels)) for _, l := range c.labels { - matcher, err := compat.Matcher(l) + matcher, err := compat.Matcher(l, "cli") if err != nil { kingpin.Fatalf("Failed to parse labels: %v\n", err) } diff --git a/cmd/alertmanager/main.go b/cmd/alertmanager/main.go index 15716c985c..d44a69d154 100644 --- a/cmd/alertmanager/main.go +++ b/cmd/alertmanager/main.go @@ -180,7 +180,7 @@ func run() int { level.Error(logger).Log("msg", "error parsing the feature flag list", "err", err) return 1 } - compat.InitFromFlags(logger, ff) + compat.InitFromFlags(logger, compat.RegisteredMetrics, ff) err = os.MkdirAll(*dataDir, 0o777) if err != nil { diff --git a/config/config.go b/config/config.go index 355209b467..7f3602e066 100644 --- a/config/config.go +++ b/config/config.go @@ -1006,7 +1006,7 @@ func (m *Matchers) UnmarshalYAML(unmarshal func(interface{}) error) error { return err } for _, line := range lines { - pm, err := compat.Matchers(line) + pm, err := compat.Matchers(line, "config") if err != nil { return err } @@ -1032,7 +1032,7 @@ func (m *Matchers) UnmarshalJSON(data []byte) error { return err } for _, line := range lines { - pm, err := compat.Matchers(line) + pm, err := compat.Matchers(line, "config") if err != nil { return err } diff --git a/matchers/compat/metrics.go b/matchers/compat/metrics.go new file mode 100644 index 0000000000..4741cf182b --- /dev/null +++ b/matchers/compat/metrics.go @@ -0,0 +1,60 @@ +// Copyright 2023 The Prometheus Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package compat + +import ( + "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/promauto" +) + +const ( + OriginAPI = "api" + OriginConfig = "config" +) + +var DefaultOrigins = []string{ + OriginAPI, + OriginConfig, +} + +var RegisteredMetrics = NewMetrics(prometheus.DefaultRegisterer) + +type Metrics struct { + Total *prometheus.GaugeVec + DisagreeTotal *prometheus.GaugeVec + IncompatibleTotal *prometheus.GaugeVec + InvalidTotal *prometheus.GaugeVec +} + +func NewMetrics(r prometheus.Registerer) *Metrics { + m := &Metrics{ + Total: promauto.With(r).NewGaugeVec(prometheus.GaugeOpts{ + Name: "alertmanager_matchers_parse", + Help: "Total number of matcher inputs parsed, including invalid inputs.", + }, []string{"origin"}), + DisagreeTotal: promauto.With(r).NewGaugeVec(prometheus.GaugeOpts{ + Name: "alertmanager_matchers_disagree", + Help: "Total number of matcher inputs which produce different parsings (disagreement).", + }, []string{"origin"}), + IncompatibleTotal: promauto.With(r).NewGaugeVec(prometheus.GaugeOpts{ + Name: "alertmanager_matchers_incompatible", + Help: "Total number of matcher inputs that are incompatible with the UTF-8 parser.", + }, []string{"origin"}), + InvalidTotal: promauto.With(r).NewGaugeVec(prometheus.GaugeOpts{ + Name: "alertmanager_matchers_invalid", + Help: "Total number of matcher inputs that could not be parsed.", + }, []string{"origin"}), + } + return m +} diff --git a/matchers/compat/parse.go b/matchers/compat/parse.go index f2eab755c8..9eb09f836b 100644 --- a/matchers/compat/parse.go +++ b/matchers/compat/parse.go @@ -15,11 +15,13 @@ package compat import ( "fmt" + "reflect" "strings" "unicode/utf8" "github.com/go-kit/log" "github.com/go-kit/log/level" + "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/common/model" "github.com/prometheus/alertmanager/featurecontrol" @@ -29,8 +31,8 @@ import ( var ( isValidLabelName = isValidClassicLabelName(log.NewNopLogger()) - parseMatcher = ClassicMatcherParser(log.NewNopLogger()) - parseMatchers = ClassicMatchersParser(log.NewNopLogger()) + parseMatcher = ClassicMatcherParser(log.NewNopLogger(), RegisteredMetrics) + parseMatchers = ClassicMatchersParser(log.NewNopLogger(), RegisteredMetrics) ) // IsValidLabelName returns true if the string is a valid label name. @@ -38,143 +40,197 @@ func IsValidLabelName(name model.LabelName) bool { return isValidLabelName(name) } -type ParseMatcher func(s string) (*labels.Matcher, error) +type ParseMatcher func(input, origin string) (*labels.Matcher, error) -type ParseMatchers func(s string) (labels.Matchers, error) +type ParseMatchers func(input, origin string) (labels.Matchers, error) // Matcher parses the matcher in the input string. It returns an error // if the input is invalid or contains two or more matchers. -func Matcher(s string) (*labels.Matcher, error) { - return parseMatcher(s) +func Matcher(input, origin string) (*labels.Matcher, error) { + return parseMatcher(input, origin) } // Matchers parses one or more matchers in the input string. It returns // an error if the input is invalid. -func Matchers(s string) (labels.Matchers, error) { - return parseMatchers(s) +func Matchers(input, origin string) (labels.Matchers, error) { + return parseMatchers(input, origin) } // InitFromFlags initializes the compat package from the flagger. -func InitFromFlags(l log.Logger, f featurecontrol.Flagger) { +func InitFromFlags(l log.Logger, m *Metrics, f featurecontrol.Flagger) { if f.ClassicMode() { isValidLabelName = isValidClassicLabelName(l) - parseMatcher = ClassicMatcherParser(l) - parseMatchers = ClassicMatchersParser(l) + parseMatcher = ClassicMatcherParser(l, m) + parseMatchers = ClassicMatchersParser(l, m) } else if f.UTF8StrictMode() { isValidLabelName = isValidUTF8LabelName(l) - parseMatcher = UTF8MatcherParser(l) - parseMatchers = UTF8MatchersParser(l) + parseMatcher = UTF8MatcherParser(l, m) + parseMatchers = UTF8MatchersParser(l, m) } else { isValidLabelName = isValidUTF8LabelName(l) - parseMatcher = FallbackMatcherParser(l) - parseMatchers = FallbackMatchersParser(l) + parseMatcher = FallbackMatcherParser(l, m) + parseMatchers = FallbackMatchersParser(l, m) } } -// ClassicMatcherParser uses the old pkg/labels parser to parse the matcher in +// ClassicMatcherParser uses the pkg/labels parser to parse the matcher in // the input string. -func ClassicMatcherParser(l log.Logger) ParseMatcher { - return func(s string) (*labels.Matcher, error) { - level.Debug(l).Log("msg", "Parsing with classic matchers parser", "input", s) - return labels.ParseMatcher(s) +func ClassicMatcherParser(l log.Logger, m *Metrics) ParseMatcher { + return func(input, origin string) (matcher *labels.Matcher, err error) { + defer func() { + lbs := prometheus.Labels{"origin": origin} + m.Total.With(lbs).Inc() + if err != nil { + m.InvalidTotal.With(lbs).Inc() + } + }() + level.Debug(l).Log("msg", "Parsing with classic matchers parser", "input", input) + return labels.ParseMatcher(input) } } -// ClassicMatchersParser uses the old pkg/labels parser to parse zero or more +// ClassicMatchersParser uses the pkg/labels parser to parse zero or more // matchers in the input string. It returns an error if the input is invalid. -func ClassicMatchersParser(l log.Logger) ParseMatchers { - return func(s string) (labels.Matchers, error) { - level.Debug(l).Log("msg", "Parsing with classic matchers parser", "input", s) - return labels.ParseMatchers(s) +func ClassicMatchersParser(l log.Logger, m *Metrics) ParseMatchers { + return func(input, origin string) (matchers labels.Matchers, err error) { + defer func() { + lbs := prometheus.Labels{"origin": origin} + m.Total.With(lbs).Inc() + if err != nil { + m.InvalidTotal.With(lbs).Inc() + } + }() + level.Debug(l).Log("msg", "Parsing with classic matchers parser", "input", input) + return labels.ParseMatchers(input) } } -// UTF8MatcherParser uses the new matchers/parse parser to parse -// the matcher in the input string. If this fails it does not fallback -// to the old pkg/labels parser. -func UTF8MatcherParser(l log.Logger) ParseMatcher { - return func(s string) (*labels.Matcher, error) { - level.Debug(l).Log("msg", "Parsing with UTF-8 matchers parser", "input", s) - if strings.HasPrefix(s, "{") || strings.HasSuffix(s, "}") { - return nil, fmt.Errorf("unexpected open or close brace: %s", s) +// UTF8MatcherParser uses the new matchers/parse parser to parse the matcher +// in the input string. If this fails it does not revert to the pkg/labels parser. +func UTF8MatcherParser(l log.Logger, m *Metrics) ParseMatcher { + return func(input, origin string) (matcher *labels.Matcher, err error) { + defer func() { + lbs := prometheus.Labels{"origin": origin} + m.Total.With(lbs).Inc() + if err != nil { + m.InvalidTotal.With(lbs).Inc() + } + }() + level.Debug(l).Log("msg", "Parsing with UTF-8 matchers parser", "input", input) + if strings.HasPrefix(input, "{") || strings.HasSuffix(input, "}") { + return nil, fmt.Errorf("unexpected open or close brace: %s", input) } - return parse.Matcher(s) + return parse.Matcher(input) } } -// UTF8MatchersParser uses the new matchers/parse parser to parse -// zero or more matchers in the input string. If this fails it -// does not fallback to the old pkg/labels parser. -func UTF8MatchersParser(l log.Logger) ParseMatchers { - return func(s string) (labels.Matchers, error) { - level.Debug(l).Log("msg", "Parsing with UTF-8 matchers parser", "input", s) - return parse.Matchers(s) +// UTF8MatchersParser uses the new matchers/parse parser to parse zero or more +// matchers in the input string. If this fails it does not revert to the +// pkg/labels parser. +func UTF8MatchersParser(l log.Logger, m *Metrics) ParseMatchers { + return func(input, origin string) (matchers labels.Matchers, err error) { + defer func() { + lbs := prometheus.Labels{"origin": origin} + m.Total.With(lbs).Inc() + if err != nil { + m.InvalidTotal.With(lbs).Inc() + } + }() + level.Debug(l).Log("msg", "Parsing with UTF-8 matchers parser", "input", input) + return parse.Matchers(input) } } -// FallbackMatcherParser uses the new matchers/parse parser to parse -// zero or more matchers in the string. If this fails it falls back to -// the old pkg/labels parser and emits a warning log line. -func FallbackMatcherParser(l log.Logger) ParseMatcher { - return func(s string) (*labels.Matcher, error) { - var ( - m *labels.Matcher - err error - invalidErr error - ) - level.Debug(l).Log("msg", "Parsing with UTF-8 matchers parser, with fallback to classic matchers parser", "input", s) - if strings.HasPrefix(s, "{") || strings.HasSuffix(s, "}") { - return nil, fmt.Errorf("unexpected open or close brace: %s", s) +// FallbackMatcherParser uses the new matchers/parse parser to parse zero or more +// matchers in the string. If this fails it reverts to the pkg/labels parser and +// emits a warning log line. +func FallbackMatcherParser(l log.Logger, m *Metrics) ParseMatcher { + return func(input, origin string) (matcher *labels.Matcher, err error) { + lbs := prometheus.Labels{"origin": origin} + defer func() { + m.Total.With(lbs).Inc() + if err != nil { + m.InvalidTotal.With(lbs).Inc() + } + }() + level.Debug(l).Log("msg", "Parsing with UTF-8 matchers parser, with fallback to classic matchers parser", "input", input) + if strings.HasPrefix(input, "{") || strings.HasSuffix(input, "}") { + return nil, fmt.Errorf("unexpected open or close brace: %s", input) } - m, err = parse.Matcher(s) - if err != nil { - m, invalidErr = labels.ParseMatcher(s) - if invalidErr != nil { - // The input is not valid in the old pkg/labels parser either, - // it cannot be valid input. - return nil, invalidErr + // Parse the input in both parsers to look for disagreement and incompatible + // inputs. + nMatcher, nErr := parse.Matcher(input) + cMatcher, cErr := labels.ParseMatcher(input) + if nErr != nil { + // If the input is invalid in both parsers, return the error. + if cErr != nil { + return nil, cErr } - // The input is valid in the old pkg/labels parser, but not the - // new matchers/parse parser. - suggestion := m.String() - level.Warn(l).Log("msg", "Alertmanager is moving to a new parser for labels and matchers, and this input is incompatible. Alertmanager has instead parsed the input using the old matchers parser as a fallback. To make this input compatible with the new parser please make sure all regular expressions and values are double-quoted. If you are still seeing this message please open an issue.", "input", s, "err", err, "suggestion", suggestion) + // The input is valid in the pkg/labels parser, but not the matchers/parse + // parser. This means the input is not forwards compatible. + m.IncompatibleTotal.With(lbs).Inc() + suggestion := cMatcher.String() + level.Warn(l).Log("msg", "Alertmanager is moving to a new parser for labels and matchers, and this input is incompatible. Alertmanager has instead parsed the input using the old matchers parser as a fallback. To make this input compatible with the new parser please make sure all regular expressions and values are double-quoted. If you are still seeing this message please open an issue.", "input", input, "err", err, "suggestion", suggestion) + return cMatcher, nil + } + // If the input is valid in both parsers, but produces different results, + // then there is disagreement. + if nErr == nil && cErr == nil && !reflect.DeepEqual(nMatcher, cMatcher) { + m.DisagreeTotal.With(lbs).Inc() + level.Warn(l).Log("msg", "Matchers input has disagreement", "input", input) + return cMatcher, nil } - return m, nil + return nMatcher, nil } } // FallbackMatchersParser uses the new matchers/parse parser to parse the -// matcher in the input string. If this fails it falls back to the old -// pkg/labels parser and emits a warning log line. -func FallbackMatchersParser(l log.Logger) ParseMatchers { - return func(s string) (labels.Matchers, error) { - var ( - m []*labels.Matcher - err error - invalidErr error - ) - level.Debug(l).Log("msg", "Parsing with UTF-8 matchers parser, with fallback to classic matchers parser", "input", s) - m, err = parse.Matchers(s) - if err != nil { - m, invalidErr = labels.ParseMatchers(s) - if invalidErr != nil { - // The input is not valid in the old pkg/labels parser either, - // it cannot be valid input. - return nil, invalidErr +// matcher in the input string. If this fails it falls back to the pkg/labels +// parser and emits a warning log line. +func FallbackMatchersParser(l log.Logger, m *Metrics) ParseMatchers { + return func(input, origin string) (matchers labels.Matchers, err error) { + lbs := prometheus.Labels{"origin": origin} + defer func() { + m.Total.With(lbs).Inc() + if err != nil { + m.InvalidTotal.With(lbs).Inc() } + }() + level.Debug(l).Log("msg", "Parsing with UTF-8 matchers parser, with fallback to classic matchers parser", "input", input) + // Parse the input in both parsers to look for disagreement and incompatible + // inputs. + nMatchers, nErr := parse.Matchers(input) + cMatchers, cErr := labels.ParseMatchers(input) + if nErr != nil { + // If the input is invalid in both parsers, return the error. + if cErr != nil { + return nil, cErr + } + // The input is valid in the pkg/labels parser, but not the matchers/parse + // parser. This means the input is not forwards compatible. + m.IncompatibleTotal.With(lbs).Inc() var sb strings.Builder - for i, n := range m { + for i, n := range cMatchers { sb.WriteString(n.String()) - if i < len(m)-1 { + if i < len(cMatchers)-1 { sb.WriteRune(',') } } suggestion := sb.String() - // The input is valid in the old pkg/labels parser, but not the + // The input is valid in the pkg/labels parser, but not the // new matchers/parse parser. - level.Warn(l).Log("msg", "Alertmanager is moving to a new parser for labels and matchers, and this input is incompatible. Alertmanager has instead parsed the input using the old matchers parser as a fallback. To make this input compatible with the new parser please make sure all regular expressions and values are double-quoted. If you are still seeing this message please open an issue.", "input", s, "err", err, "suggestion", suggestion) + level.Warn(l).Log("msg", "Alertmanager is moving to a new parser for labels and matchers, and this input is incompatible. Alertmanager has instead parsed the input using the old matchers parser as a fallback. To make this input compatible with the new parser please make sure all regular expressions and values are double-quoted. If you are still seeing this message please open an issue.", "input", input, "err", err, "suggestion", suggestion) + return cMatchers, nil + } + // If the input is valid in both parsers, but produces different results, + // then there is disagreement. We need to compare to labels.Matchers(cMatchers) + // as cMatchers is a []*labels.Matcher not labels.Matchers. + if nErr == nil && cErr == nil && !reflect.DeepEqual(nMatchers, labels.Matchers(cMatchers)) { + m.DisagreeTotal.With(lbs).Inc() + level.Warn(l).Log("msg", "Matchers input has disagreement", "input", input) + return cMatchers, nil } - return m, nil + return nMatchers, nil } } diff --git a/matchers/compat/parse_test.go b/matchers/compat/parse_test.go index 8343ea17f3..a026b8f570 100644 --- a/matchers/compat/parse_test.go +++ b/matchers/compat/parse_test.go @@ -17,6 +17,8 @@ import ( "testing" "github.com/go-kit/log" + "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/testutil" "github.com/prometheus/common/model" "github.com/stretchr/testify/require" @@ -25,83 +27,135 @@ import ( func TestFallbackMatcherParser(t *testing.T) { tests := []struct { - name string - input string - expected *labels.Matcher - err string + name string + input string + expected *labels.Matcher + err string + total float64 + disagreeTotal float64 + incompatibleTotal float64 + invalidTotal float64 }{{ - name: "is accepted in both", + name: "input is accepted", input: "foo=bar", expected: mustNewMatcher(t, labels.MatchEqual, "foo", "bar"), + total: 1, + }, { + name: "input is accepted in neither", + input: "foo!bar", + err: "bad matcher format: foo!bar", + total: 1, + invalidTotal: 1, }, { - name: "is accepted in new parser but not old", + name: "input is accepted in matchers/parse but not pkg/labels", input: "foo🙂=bar", expected: mustNewMatcher(t, labels.MatchEqual, "foo🙂", "bar"), + total: 1, }, { - name: "is accepted in old parser but not new", - input: "foo=!bar\\n", - expected: mustNewMatcher(t, labels.MatchEqual, "foo", "!bar\n"), + name: "input is accepted in pkg/labels but not matchers/parse", + input: "foo=!bar\\n", + expected: mustNewMatcher(t, labels.MatchEqual, "foo", "!bar\n"), + total: 1, + incompatibleTotal: 1, }, { - name: "is accepted in neither", - input: "foo!bar", - err: "bad matcher format: foo!bar", + // This input causes disagreement because \xf0\x9f\x99\x82 is the byte sequence for 🙂, + // which is not understood by pkg/labels but is understood by matchers/parse. In such cases, + // the fallback parser returns the result from pkg/labels. + name: "input causes disagreement", + input: "foo=\"\\xf0\\x9f\\x99\\x82\"", + expected: mustNewMatcher(t, labels.MatchEqual, "foo", "\\xf0\\x9f\\x99\\x82"), + total: 1, + disagreeTotal: 1, }} - f := FallbackMatcherParser(log.NewNopLogger()) + for _, test := range tests { t.Run(test.name, func(t *testing.T) { - matcher, err := f(test.input) + m := NewMetrics(prometheus.NewRegistry()) + f := FallbackMatcherParser(log.NewNopLogger(), m) + matcher, err := f(test.input, "test") if test.err != "" { require.EqualError(t, err, test.err) } else { require.NoError(t, err) require.EqualValues(t, test.expected, matcher) } + requireMetric(t, test.total, m.Total) + requireMetric(t, test.disagreeTotal, m.DisagreeTotal) + requireMetric(t, test.incompatibleTotal, m.IncompatibleTotal) + requireMetric(t, test.invalidTotal, m.InvalidTotal) }) } } func TestFallbackMatchersParser(t *testing.T) { tests := []struct { - name string - input string - expected labels.Matchers - err string + name string + input string + expected labels.Matchers + err string + total float64 + disagreeTotal float64 + incompatibleTotal float64 + invalidTotal float64 }{{ - name: "is accepted in both", + name: "input is accepted", input: "{foo=bar,bar=baz}", expected: labels.Matchers{ mustNewMatcher(t, labels.MatchEqual, "foo", "bar"), mustNewMatcher(t, labels.MatchEqual, "bar", "baz"), }, + total: 1, + }, { + name: "input is accepted in neither", + input: "{foo!bar}", + err: "bad matcher format: foo!bar", + total: 1, + invalidTotal: 1, }, { - name: "is accepted in new parser but not old", + name: "input is accepted in matchers/parse but not pkg/labels", input: "{foo🙂=bar,bar=baz🙂}", expected: labels.Matchers{ mustNewMatcher(t, labels.MatchEqual, "foo🙂", "bar"), mustNewMatcher(t, labels.MatchEqual, "bar", "baz🙂"), }, + total: 1, }, { - name: "is accepted in old parser but not new", + name: "is accepted in pkg/labels but not matchers/parse", input: "{foo=!bar,bar=$baz\\n}", expected: labels.Matchers{ mustNewMatcher(t, labels.MatchEqual, "foo", "!bar"), mustNewMatcher(t, labels.MatchEqual, "bar", "$baz\n"), }, + total: 1, + incompatibleTotal: 1, }, { - name: "is accepted in neither", - input: "{foo!bar}", - err: "bad matcher format: foo!bar", + // This input causes disagreement because \xf0\x9f\x99\x82 is the byte sequence for 🙂, + // which is not understood by pkg/labels but is understood by matchers/parse. In such cases, + // the fallback parser returns the result from pkg/labels. + name: "input causes disagreement", + input: "{foo=\"\\xf0\\x9f\\x99\\x82\"}", + expected: labels.Matchers{ + mustNewMatcher(t, labels.MatchEqual, "foo", "\\xf0\\x9f\\x99\\x82"), + }, + total: 1, + disagreeTotal: 1, }} - f := FallbackMatchersParser(log.NewNopLogger()) + for _, test := range tests { t.Run(test.name, func(t *testing.T) { - matchers, err := f(test.input) + m := NewMetrics(prometheus.NewRegistry()) + f := FallbackMatchersParser(log.NewNopLogger(), m) + matchers, err := f(test.input, "test") if test.err != "" { require.EqualError(t, err, test.err) } else { require.NoError(t, err) require.EqualValues(t, test.expected, matchers) } + requireMetric(t, test.total, m.Total) + requireMetric(t, test.disagreeTotal, m.DisagreeTotal) + requireMetric(t, test.incompatibleTotal, m.IncompatibleTotal) + requireMetric(t, test.invalidTotal, m.InvalidTotal) }) } } @@ -173,3 +227,12 @@ func TestIsValidUTF8LabelName(t *testing.T) { }) } } + +func requireMetric(t *testing.T, expected float64, m *prometheus.GaugeVec) { + if expected == 0 { + require.Equal(t, 0, testutil.CollectAndCount(m)) + } else { + require.Equal(t, 1, testutil.CollectAndCount(m)) + require.Equal(t, expected, testutil.ToFloat64(m)) + } +}