diff --git a/cli/root.go b/cli/root.go index a94db39188..69c1022c6a 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, compat.NewMetrics(nil), featureConfig) + compat.InitFromFlags(logger, featureConfig) return nil } diff --git a/cmd/alertmanager/main.go b/cmd/alertmanager/main.go index cb225b2e16..b2938189d5 100644 --- a/cmd/alertmanager/main.go +++ b/cmd/alertmanager/main.go @@ -186,7 +186,7 @@ func run() int { level.Error(logger).Log("msg", "error parsing the feature flag list", "err", err) return 1 } - compat.InitFromFlags(logger, compat.RegisteredMetrics, ff) + compat.InitFromFlags(logger, ff) err = os.MkdirAll(*dataDir, 0o777) if err != nil { diff --git a/matchers/compat/metrics.go b/matchers/compat/metrics.go deleted file mode 100644 index 34b64099c8..0000000000 --- a/matchers/compat/metrics.go +++ /dev/null @@ -1,60 +0,0 @@ -// 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.CounterVec - DisagreeTotal *prometheus.CounterVec - IncompatibleTotal *prometheus.CounterVec - InvalidTotal *prometheus.CounterVec -} - -func NewMetrics(r prometheus.Registerer) *Metrics { - m := &Metrics{ - Total: promauto.With(r).NewCounterVec(prometheus.CounterOpts{ - Name: "alertmanager_matchers_parse_total", - Help: "Total number of matcher inputs parsed, including invalid inputs.", - }, []string{"origin"}), - DisagreeTotal: promauto.With(r).NewCounterVec(prometheus.CounterOpts{ - Name: "alertmanager_matchers_disagree_total", - Help: "Total number of matcher inputs which produce different parsings (disagreement).", - }, []string{"origin"}), - IncompatibleTotal: promauto.With(r).NewCounterVec(prometheus.CounterOpts{ - Name: "alertmanager_matchers_incompatible_total", - Help: "Total number of matcher inputs that are incompatible with the UTF-8 parser.", - }, []string{"origin"}), - InvalidTotal: promauto.With(r).NewCounterVec(prometheus.CounterOpts{ - Name: "alertmanager_matchers_invalid_total", - 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 e6b2758b29..7aa4e2d95b 100644 --- a/matchers/compat/parse.go +++ b/matchers/compat/parse.go @@ -21,7 +21,6 @@ import ( "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" @@ -31,8 +30,8 @@ import ( var ( isValidLabelName = isValidClassicLabelName(log.NewNopLogger()) - parseMatcher = ClassicMatcherParser(log.NewNopLogger(), RegisteredMetrics) - parseMatchers = ClassicMatchersParser(log.NewNopLogger(), RegisteredMetrics) + parseMatcher = ClassicMatcherParser(log.NewNopLogger()) + parseMatchers = ClassicMatchersParser(log.NewNopLogger()) ) // IsValidLabelName returns true if the string is a valid label name. @@ -57,33 +56,26 @@ func Matchers(input, origin string) (labels.Matchers, error) { } // InitFromFlags initializes the compat package from the flagger. -func InitFromFlags(l log.Logger, m *Metrics, f featurecontrol.Flagger) { +func InitFromFlags(l log.Logger, f featurecontrol.Flagger) { if f.ClassicMode() { isValidLabelName = isValidClassicLabelName(l) - parseMatcher = ClassicMatcherParser(l, m) - parseMatchers = ClassicMatchersParser(l, m) + parseMatcher = ClassicMatcherParser(l) + parseMatchers = ClassicMatchersParser(l) } else if f.UTF8StrictMode() { isValidLabelName = isValidUTF8LabelName(l) - parseMatcher = UTF8MatcherParser(l, m) - parseMatchers = UTF8MatchersParser(l, m) + parseMatcher = UTF8MatcherParser(l) + parseMatchers = UTF8MatchersParser(l) } else { isValidLabelName = isValidUTF8LabelName(l) - parseMatcher = FallbackMatcherParser(l, m) - parseMatchers = FallbackMatchersParser(l, m) + parseMatcher = FallbackMatcherParser(l) + parseMatchers = FallbackMatchersParser(l) } } // ClassicMatcherParser uses the pkg/labels parser to parse the matcher in // the input string. -func ClassicMatcherParser(l log.Logger, m *Metrics) ParseMatcher { +func ClassicMatcherParser(l log.Logger) 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, "origin", origin) return labels.ParseMatcher(input) } @@ -91,15 +83,8 @@ func ClassicMatcherParser(l log.Logger, m *Metrics) ParseMatcher { // 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, m *Metrics) ParseMatchers { +func ClassicMatchersParser(l log.Logger) 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, "origin", origin) return labels.ParseMatchers(input) } @@ -107,15 +92,8 @@ func ClassicMatchersParser(l log.Logger, m *Metrics) ParseMatchers { // 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 { +func UTF8MatcherParser(l log.Logger) 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, "origin", origin) if strings.HasPrefix(input, "{") || strings.HasSuffix(input, "}") { return nil, fmt.Errorf("unexpected open or close brace: %s", input) @@ -127,15 +105,8 @@ func UTF8MatcherParser(l log.Logger, m *Metrics) ParseMatcher { // 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 { +func UTF8MatchersParser(l log.Logger) 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, "origin", origin) return parse.Matchers(input) } @@ -144,15 +115,8 @@ func UTF8MatchersParser(l log.Logger, m *Metrics) ParseMatchers { // 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 { +func FallbackMatcherParser(l log.Logger) 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, "origin", origin) if strings.HasPrefix(input, "{") || strings.HasSuffix(input, "}") { return nil, fmt.Errorf("unexpected open or close brace: %s", input) @@ -168,7 +132,6 @@ func FallbackMatcherParser(l log.Logger, m *Metrics) ParseMatcher { } // 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, "origin", origin, "err", nErr, "suggestion", suggestion) return cMatcher, nil @@ -176,7 +139,6 @@ func FallbackMatcherParser(l log.Logger, m *Metrics) ParseMatcher { // 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, "origin", origin) return cMatcher, nil } @@ -187,15 +149,8 @@ func FallbackMatcherParser(l log.Logger, m *Metrics) ParseMatcher { // FallbackMatchersParser uses the new matchers/parse parser to parse the // 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 { +func FallbackMatchersParser(l log.Logger) 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, "origin", origin) // Parse the input in both parsers to look for disagreement and incompatible // inputs. @@ -208,7 +163,6 @@ func FallbackMatchersParser(l log.Logger, m *Metrics) ParseMatchers { } // 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 cMatchers { sb.WriteString(n.String()) @@ -226,7 +180,6 @@ func FallbackMatchersParser(l log.Logger, m *Metrics) ParseMatchers { // 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, "origin", origin) return cMatchers, nil } diff --git a/matchers/compat/parse_test.go b/matchers/compat/parse_test.go index d1490437bf..bb417ffbae 100644 --- a/matchers/compat/parse_test.go +++ b/matchers/compat/parse_test.go @@ -17,8 +17,6 @@ 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" @@ -27,51 +25,38 @@ import ( func TestFallbackMatcherParser(t *testing.T) { tests := []struct { - name string - input string - expected *labels.Matcher - err string - total float64 - disagreeTotal float64 - incompatibleTotal float64 - invalidTotal float64 + name string + input string + expected *labels.Matcher + err string }{{ 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: "input is accepted in neither", + input: "foo!bar", + err: "bad matcher format: foo!bar", }, { name: "input is accepted in matchers/parse but not pkg/labels", input: "foo🙂=bar", expected: mustNewMatcher(t, labels.MatchEqual, "foo🙂", "bar"), - total: 1, }, { - 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: "input is accepted in pkg/labels but not matchers/parse", + input: "foo=!bar\\n", + expected: mustNewMatcher(t, labels.MatchEqual, "foo", "!bar\n"), }, { // 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, + name: "input causes disagreement", + input: "foo=\"\\xf0\\x9f\\x99\\x82\"", + expected: mustNewMatcher(t, labels.MatchEqual, "foo", "\\xf0\\x9f\\x99\\x82"), }} for _, test := range tests { t.Run(test.name, func(t *testing.T) { - m := NewMetrics(prometheus.NewRegistry()) - f := FallbackMatcherParser(log.NewNopLogger(), m) + f := FallbackMatcherParser(log.NewNopLogger()) matcher, err := f(test.input, "test") if test.err != "" { require.EqualError(t, err, test.err) @@ -79,24 +64,16 @@ func TestFallbackMatcherParser(t *testing.T) { 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 - total float64 - disagreeTotal float64 - incompatibleTotal float64 - invalidTotal float64 + name string + input string + expected labels.Matchers + err string }{{ name: "input is accepted", input: "{foo=bar,bar=baz}", @@ -104,13 +81,10 @@ func TestFallbackMatchersParser(t *testing.T) { 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: "input is accepted in neither", + input: "{foo!bar}", + err: "bad matcher format: foo!bar", }, { name: "input is accepted in matchers/parse but not pkg/labels", input: "{foo🙂=bar,bar=baz🙂}", @@ -118,7 +92,6 @@ func TestFallbackMatchersParser(t *testing.T) { mustNewMatcher(t, labels.MatchEqual, "foo🙂", "bar"), mustNewMatcher(t, labels.MatchEqual, "bar", "baz🙂"), }, - total: 1, }, { name: "is accepted in pkg/labels but not matchers/parse", input: "{foo=!bar,bar=$baz\\n}", @@ -126,8 +99,6 @@ func TestFallbackMatchersParser(t *testing.T) { mustNewMatcher(t, labels.MatchEqual, "foo", "!bar"), mustNewMatcher(t, labels.MatchEqual, "bar", "$baz\n"), }, - total: 1, - incompatibleTotal: 1, }, { // 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, @@ -137,14 +108,11 @@ func TestFallbackMatchersParser(t *testing.T) { expected: labels.Matchers{ mustNewMatcher(t, labels.MatchEqual, "foo", "\\xf0\\x9f\\x99\\x82"), }, - total: 1, - disagreeTotal: 1, }} for _, test := range tests { t.Run(test.name, func(t *testing.T) { - m := NewMetrics(prometheus.NewRegistry()) - f := FallbackMatchersParser(log.NewNopLogger(), m) + f := FallbackMatchersParser(log.NewNopLogger()) matchers, err := f(test.input, "test") if test.err != "" { require.EqualError(t, err, test.err) @@ -152,10 +120,6 @@ func TestFallbackMatchersParser(t *testing.T) { 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) }) } } @@ -235,12 +199,3 @@ func TestIsValidUTF8LabelName(t *testing.T) { }) } } - -func requireMetric(t *testing.T, expected float64, m *prometheus.CounterVec) { - 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)) - } -} diff --git a/silence/silence_test.go b/silence/silence_test.go index da39b3a89a..2a880dd3b6 100644 --- a/silence/silence_test.go +++ b/silence/silence_test.go @@ -1334,12 +1334,12 @@ func TestValidateUTF8Matcher(t *testing.T) { // Change the mode to UTF-8 mode. ff, err := featurecontrol.NewFlags(log.NewNopLogger(), featurecontrol.FeatureUTF8StrictMode) require.NoError(t, err) - compat.InitFromFlags(log.NewNopLogger(), compat.RegisteredMetrics, ff) + compat.InitFromFlags(log.NewNopLogger(), ff) // Restore the mode to classic at the end of the test. ff, err = featurecontrol.NewFlags(log.NewNopLogger(), featurecontrol.FeatureClassicMode) require.NoError(t, err) - defer compat.InitFromFlags(log.NewNopLogger(), compat.RegisteredMetrics, ff) + defer compat.InitFromFlags(log.NewNopLogger(), ff) for _, c := range cases { checkErr(t, c.err, validateMatcher(c.m)) diff --git a/types/types_test.go b/types/types_test.go index 438f351535..ece6fb5c51 100644 --- a/types/types_test.go +++ b/types/types_test.go @@ -341,12 +341,12 @@ func TestValidateUTF8Ls(t *testing.T) { // Change the mode to UTF-8 mode. ff, err := featurecontrol.NewFlags(log.NewNopLogger(), featurecontrol.FeatureUTF8StrictMode) require.NoError(t, err) - compat.InitFromFlags(log.NewNopLogger(), compat.RegisteredMetrics, ff) + compat.InitFromFlags(log.NewNopLogger(), ff) // Restore the mode to classic at the end of the test. ff, err = featurecontrol.NewFlags(log.NewNopLogger(), featurecontrol.FeatureClassicMode) require.NoError(t, err) - defer compat.InitFromFlags(log.NewNopLogger(), compat.RegisteredMetrics, ff) + defer compat.InitFromFlags(log.NewNopLogger(), ff) for _, test := range tests { t.Run(test.name, func(t *testing.T) {