From ed5998db364a0d3a9137eafc39fdc56db08cec9c Mon Sep 17 00:00:00 2001 From: George Robinson Date: Thu, 24 Aug 2023 07:03:52 +0100 Subject: [PATCH 01/27] Add adapter package for parser feature flag This commit adds the adapter package that will allow users to switch between the new matchers/parse parser and the old pkg/labels parser. The new matchers/parse parser uses a fallback mechanism where if the input cannot be parsed in the new parser it then attempts to use the old parser. If an input is parsed in the old parser but not the new parser then a warning log is emitted. Signed-off-by: George Robinson --- matchers/adapter/parse.go | 141 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 141 insertions(+) create mode 100644 matchers/adapter/parse.go diff --git a/matchers/adapter/parse.go b/matchers/adapter/parse.go new file mode 100644 index 0000000000..208a1cbf46 --- /dev/null +++ b/matchers/adapter/parse.go @@ -0,0 +1,141 @@ +// 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 adapter + +import ( + "github.com/go-kit/log" + "github.com/go-kit/log/level" + "github.com/prometheus/alertmanager/matchers/parse" + "github.com/prometheus/alertmanager/pkg/labels" +) + +var ( + // ParseMatcher is the default parser for parsing individual matchers. + ParseMatcher = FallbackMatcherParser(log.NewNopLogger()) + + // ParseMatchers is the default parser for parsing a series of zero or + // more matchers. + ParseMatchers = FallbackMatchersParser(log.NewNopLogger()) +) + +// MatcherParser is an interface for parsing individual matchers. +type MatcherParser func(s string) (*labels.Matcher, error) + +// MatchersParser is an interface for parsing a series of zero or more matchers. +type MatchersParser func(s string) (labels.Matchers, error) + +// OldMatcherParser uses the old pkg/labels parser to parse the matcher in +// the input string. +func OldMatcherParser(l log.Logger) MatcherParser { + return func(s string) (*labels.Matcher, error) { + level.Debug(l).Log( + "msg", + "Parsing matcher with old regular expressions parser", + "matcher", + s, + ) + return labels.ParseMatcher(s) + } +} + +// OldMatchersParser uses the old pkg/labels parser to parse zero or more +// matchers in the string. It returns an error if the input is invalid. +func OldMatchersParser(l log.Logger) MatchersParser { + return func(s string) (labels.Matchers, error) { + level.Debug(l).Log( + "msg", + "Parsing matchers with old regular expressions parser", + "matchers", + s) + return labels.ParseMatchers(s) + } +} + +// 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) MatchersParser { + return func(s string) (labels.Matchers, error) { + var ( + m []*labels.Matcher + err error + invalidErr error + ) + level.Debug(l).Log( + "msg", + "Parsing matchers with new parser", + "matchers", + s, + ) + m, err = parse.Parse(s) + if err != nil { + // The input is not valid in the old pkg/labels parser either, + // it cannot be valid input. + m, invalidErr = labels.ParseMatchers(s) + if invalidErr != nil { + return nil, invalidErr + } + // The input is valid in the old pkg/labels parser, but not the + // new matchers/parse parser. + level.Warn(l).Log( + "msg", + "Failed to parse input with matchers/parse, falling back to pkg/labels parser", + "matchers", + s, + "err", + err, + ) + } + return m, nil + } +} + +// 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) MatcherParser { + return func(s string) (*labels.Matcher, error) { + var ( + m *labels.Matcher + err error + invalidErr error + ) + level.Debug(l).Log( + "msg", + "Parsing matcher with new parser", + "matcher", + s, + ) + m, err = parse.ParseMatcher(s) + if err != nil { + // The input is not valid in the old pkg/labels parser either, + // it cannot be valid input. + m, invalidErr = labels.ParseMatcher(s) + if invalidErr != nil { + return nil, invalidErr + } + // The input is valid in the old pkg/labels parser, but not the + // new matchers/parse parser. + level.Warn(l).Log( + "msg", + "Failed to parse input with matchers/parse, falling back to pkg/labels parser", + "matcher", + s, + "err", + err, + ) + } + return m, nil + } +} From 2c311dbd02ff25bb9bc05b4696516b6c03d32975 Mon Sep 17 00:00:00 2001 From: George Robinson Date: Thu, 24 Aug 2023 07:18:38 +0100 Subject: [PATCH 02/27] Fix lint Signed-off-by: George Robinson --- matchers/adapter/parse.go | 1 + 1 file changed, 1 insertion(+) diff --git a/matchers/adapter/parse.go b/matchers/adapter/parse.go index 208a1cbf46..22dcc5b34b 100644 --- a/matchers/adapter/parse.go +++ b/matchers/adapter/parse.go @@ -16,6 +16,7 @@ package adapter import ( "github.com/go-kit/log" "github.com/go-kit/log/level" + "github.com/prometheus/alertmanager/matchers/parse" "github.com/prometheus/alertmanager/pkg/labels" ) From 5fde3a0009f5aa3c71c476ede84ccf52e9a543a8 Mon Sep 17 00:00:00 2001 From: George Robinson Date: Wed, 6 Sep 2023 21:18:01 +0100 Subject: [PATCH 03/27] Add tests Signed-off-by: George Robinson --- matchers/adapter/parse.go | 50 ++++++++--------- matchers/adapter/parse_test.go | 98 ++++++++++++++++++++++++++++++++++ 2 files changed, 123 insertions(+), 25 deletions(-) create mode 100644 matchers/adapter/parse_test.go diff --git a/matchers/adapter/parse.go b/matchers/adapter/parse.go index 22dcc5b34b..ead6848056 100644 --- a/matchers/adapter/parse.go +++ b/matchers/adapter/parse.go @@ -30,10 +30,10 @@ var ( ParseMatchers = FallbackMatchersParser(log.NewNopLogger()) ) -// MatcherParser is an interface for parsing individual matchers. +// MatcherParser is an interface for parsing a single matcher in the input string. type MatcherParser func(s string) (*labels.Matcher, error) -// MatchersParser is an interface for parsing a series of zero or more matchers. +// MatchersParser is an interface for parsing one or more matchers in the input string. type MatchersParser func(s string) (labels.Matchers, error) // OldMatcherParser uses the old pkg/labels parser to parse the matcher in @@ -51,7 +51,7 @@ func OldMatcherParser(l log.Logger) MatcherParser { } // OldMatchersParser uses the old pkg/labels parser to parse zero or more -// matchers in the string. It returns an error if the input is invalid. +// matchers in the string. It returns an error if the input is invalid. func OldMatchersParser(l log.Logger) MatchersParser { return func(s string) (labels.Matchers, error) { level.Debug(l).Log( @@ -63,27 +63,27 @@ func OldMatchersParser(l log.Logger) MatchersParser { } } -// 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) MatchersParser { - return func(s string) (labels.Matchers, error) { +// 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) MatcherParser { + return func(s string) (*labels.Matcher, error) { var ( - m []*labels.Matcher + m *labels.Matcher err error invalidErr error ) level.Debug(l).Log( "msg", - "Parsing matchers with new parser", - "matchers", + "Parsing matcher with new parser", + "matcher", s, ) - m, err = parse.Parse(s) + m, err = parse.Matcher(s) if err != nil { // The input is not valid in the old pkg/labels parser either, // it cannot be valid input. - m, invalidErr = labels.ParseMatchers(s) + m, invalidErr = labels.ParseMatcher(s) if invalidErr != nil { return nil, invalidErr } @@ -92,7 +92,7 @@ func FallbackMatchersParser(l log.Logger) MatchersParser { level.Warn(l).Log( "msg", "Failed to parse input with matchers/parse, falling back to pkg/labels parser", - "matchers", + "matcher", s, "err", err, @@ -102,27 +102,27 @@ func FallbackMatchersParser(l log.Logger) MatchersParser { } } -// 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) MatcherParser { - return func(s string) (*labels.Matcher, error) { +// 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) MatchersParser { + return func(s string) (labels.Matchers, error) { var ( - m *labels.Matcher + m []*labels.Matcher err error invalidErr error ) level.Debug(l).Log( "msg", - "Parsing matcher with new parser", - "matcher", + "Parsing matchers with new parser", + "matchers", s, ) - m, err = parse.ParseMatcher(s) + m, err = parse.Matchers(s) if err != nil { // The input is not valid in the old pkg/labels parser either, // it cannot be valid input. - m, invalidErr = labels.ParseMatcher(s) + m, invalidErr = labels.ParseMatchers(s) if invalidErr != nil { return nil, invalidErr } @@ -131,7 +131,7 @@ func FallbackMatcherParser(l log.Logger) MatcherParser { level.Warn(l).Log( "msg", "Failed to parse input with matchers/parse, falling back to pkg/labels parser", - "matcher", + "matchers", s, "err", err, diff --git a/matchers/adapter/parse_test.go b/matchers/adapter/parse_test.go new file mode 100644 index 0000000000..8b37f63a44 --- /dev/null +++ b/matchers/adapter/parse_test.go @@ -0,0 +1,98 @@ +package adapter + +import ( + "testing" + + "github.com/go-kit/log" + "github.com/prometheus/alertmanager/pkg/labels" + "github.com/stretchr/testify/require" +) + +func TestFallbackMatcherParser(t *testing.T) { + tests := []struct { + name string + input string + expected *labels.Matcher + err string + }{{ + name: "is accepted in both", + input: "foo=bar", + expected: mustNewMatcher(t, labels.MatchEqual, "foo", "bar"), + }, { + name: "is accepted in new parser but not old", + input: "foo🙂=bar", + expected: mustNewMatcher(t, labels.MatchEqual, "foo🙂", "bar"), + }, { + name: "is accepted in old parser but not new", + input: "foo=!bar", + expected: mustNewMatcher(t, labels.MatchEqual, "foo", "!bar"), + }, { + name: "is accepted in neither", + input: "foo!bar", + err: "bad matcher format: foo!bar", + }} + f := FallbackMatcherParser(log.NewNopLogger()) + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + matcher, err := f(test.input) + if test.err != "" { + require.EqualError(t, err, test.err) + } else { + require.Nil(t, err) + require.EqualValues(t, test.expected, matcher) + } + }) + } +} + +func TestFallbackMatchersParser(t *testing.T) { + tests := []struct { + name string + input string + expected labels.Matchers + err string + }{{ + name: "is accepted in both", + input: "{foo=bar,bar=baz}", + expected: labels.Matchers{ + mustNewMatcher(t, labels.MatchEqual, "foo", "bar"), + mustNewMatcher(t, labels.MatchEqual, "bar", "baz"), + }, + }, { + name: "is accepted in new parser but not old", + input: "{foo🙂=bar,bar=baz🙂}", + expected: labels.Matchers{ + mustNewMatcher(t, labels.MatchEqual, "foo🙂", "bar"), + mustNewMatcher(t, labels.MatchEqual, "bar", "baz🙂"), + }, + }, { + name: "is accepted in old parser but not new", + input: "{foo=!bar,bar=$baz}", + expected: labels.Matchers{ + mustNewMatcher(t, labels.MatchEqual, "foo", "!bar"), + mustNewMatcher(t, labels.MatchEqual, "bar", "$baz"), + }, + }, { + name: "is accepted in neither", + input: "{foo!bar}", + err: "bad matcher format: foo!bar", + }} + f := FallbackMatchersParser(log.NewNopLogger()) + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + matchers, err := f(test.input) + if test.err != "" { + require.EqualError(t, err, test.err) + } else { + require.Nil(t, err) + require.EqualValues(t, test.expected, matchers) + } + }) + } +} + +func mustNewMatcher(t *testing.T, op labels.MatchType, name, value string) *labels.Matcher { + m, err := labels.NewMatcher(op, name, value) + require.NoError(t, err) + return m +} From 7516580d3c689ded23e94bfd03b1175021aa44dd Mon Sep 17 00:00:00 2001 From: George Robinson Date: Thu, 7 Sep 2023 00:41:13 +0100 Subject: [PATCH 04/27] Fix missing license Signed-off-by: George Robinson --- matchers/adapter/parse_test.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/matchers/adapter/parse_test.go b/matchers/adapter/parse_test.go index 8b37f63a44..efa0d6efb4 100644 --- a/matchers/adapter/parse_test.go +++ b/matchers/adapter/parse_test.go @@ -1,3 +1,16 @@ +// 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 adapter import ( From 075bd069b447c93d7c1f4c12407b4263e71275c7 Mon Sep 17 00:00:00 2001 From: George Robinson Date: Thu, 7 Sep 2023 08:46:15 +0100 Subject: [PATCH 05/27] Fix lint Signed-off-by: George Robinson --- matchers/adapter/parse_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/matchers/adapter/parse_test.go b/matchers/adapter/parse_test.go index efa0d6efb4..0bfa245811 100644 --- a/matchers/adapter/parse_test.go +++ b/matchers/adapter/parse_test.go @@ -17,8 +17,9 @@ import ( "testing" "github.com/go-kit/log" - "github.com/prometheus/alertmanager/pkg/labels" "github.com/stretchr/testify/require" + + "github.com/prometheus/alertmanager/pkg/labels" ) func TestFallbackMatcherParser(t *testing.T) { From f9586758e0fd75328f2b8d4a2f53d28c0f33bc30 Mon Sep 17 00:00:00 2001 From: George Robinson Date: Thu, 7 Sep 2023 19:08:08 +0100 Subject: [PATCH 06/27] Use adapter in amtool Signed-off-by: George Robinson --- cli/alert_query.go | 4 ++-- cli/silence_query.go | 4 ++-- cli/utils.go | 5 +++-- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/cli/alert_query.go b/cli/alert_query.go index ff06b439b7..383d245cf9 100644 --- a/cli/alert_query.go +++ b/cli/alert_query.go @@ -22,7 +22,7 @@ import ( "github.com/prometheus/alertmanager/api/v2/client/alert" "github.com/prometheus/alertmanager/cli/format" - "github.com/prometheus/alertmanager/pkg/labels" + "github.com/prometheus/alertmanager/matchers/adapter" ) type alertQueryCmd struct { @@ -80,7 +80,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 := labels.ParseMatcher(m) + _, err := adapter.ParseMatcher(m) if err != nil { a.matcherGroups[0] = fmt.Sprintf("alertname=%s", m) } diff --git a/cli/silence_query.go b/cli/silence_query.go index 89a2722be0..a80b0f0c2b 100644 --- a/cli/silence_query.go +++ b/cli/silence_query.go @@ -24,7 +24,7 @@ import ( "github.com/prometheus/alertmanager/api/v2/client/silence" "github.com/prometheus/alertmanager/api/v2/models" "github.com/prometheus/alertmanager/cli/format" - "github.com/prometheus/alertmanager/pkg/labels" + "github.com/prometheus/alertmanager/matchers/adapter" ) type silenceQueryCmd struct { @@ -98,7 +98,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 := labels.ParseMatcher(c.matchers[0]) + _, err := adapter.ParseMatcher(c.matchers[0]) if err != nil { c.matchers[0] = fmt.Sprintf("alertname=%s", c.matchers[0]) } diff --git a/cli/utils.go b/cli/utils.go index 60221e239a..31cdf645da 100644 --- a/cli/utils.go +++ b/cli/utils.go @@ -26,6 +26,7 @@ import ( "github.com/prometheus/alertmanager/api/v2/client/general" "github.com/prometheus/alertmanager/api/v2/models" "github.com/prometheus/alertmanager/config" + "github.com/prometheus/alertmanager/matchers/adapter" "github.com/prometheus/alertmanager/pkg/labels" ) @@ -34,7 +35,7 @@ func parseMatchers(inputMatchers []string) ([]labels.Matcher, error) { matchers := make([]labels.Matcher, 0, len(inputMatchers)) for _, v := range inputMatchers { - matcher, err := labels.ParseMatcher(v) + matcher, err := adapter.ParseMatcher(v) if err != nil { return []labels.Matcher{}, err } @@ -99,7 +100,7 @@ func parseLabels(inputLabels []string) (models.LabelSet, error) { labelSet := make(models.LabelSet, len(inputLabels)) for _, l := range inputLabels { - matcher, err := labels.ParseMatcher(l) + matcher, err := adapter.ParseMatcher(l) if err != nil { return models.LabelSet{}, err } From 9282da467e3bade26ec6a7a90ef49b80cc71dc56 Mon Sep 17 00:00:00 2001 From: George Robinson Date: Fri, 8 Sep 2023 09:40:02 +0100 Subject: [PATCH 07/27] Create feature flag to disable new label matchers Signed-off-by: George Robinson --- cli/root.go | 16 ++++++++++++++++ featurecontrol/featurecontrol.go | 20 +++++++++++++++++++- 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/cli/root.go b/cli/root.go index 09043159db..cf654b25a9 100644 --- a/cli/root.go +++ b/cli/root.go @@ -18,9 +18,11 @@ import ( "net/url" "os" "path" + "strings" "time" "github.com/alecthomas/kingpin/v2" + "github.com/go-kit/log" "github.com/go-openapi/strfmt" promconfig "github.com/prometheus/common/config" "github.com/prometheus/common/version" @@ -29,6 +31,8 @@ import ( "github.com/prometheus/alertmanager/api/v2/client" "github.com/prometheus/alertmanager/cli/config" "github.com/prometheus/alertmanager/cli/format" + "github.com/prometheus/alertmanager/featurecontrol" + "github.com/prometheus/alertmanager/matchers/adapter" clientruntime "github.com/go-openapi/runtime/client" ) @@ -40,6 +44,7 @@ var ( timeout time.Duration httpConfigFile string versionCheck bool + featureFlags string configFiles = []string{os.ExpandEnv("$HOME/.config/amtool/config.yml"), "/etc/amtool/config.yml"} legacyFlags = map[string]string{"comment_required": "require-comment"} @@ -137,6 +142,17 @@ func Execute() { app.Flag("timeout", "Timeout for the executed command").Default("30s").DurationVar(&timeout) app.Flag("http.config.file", "HTTP client configuration file for amtool to connect to Alertmanager.").PlaceHolder("").ExistingFileVar(&httpConfigFile) app.Flag("version-check", "Check alertmanager version. Use --no-version-check to disable.").Default("true").BoolVar(&versionCheck) + app.Flag("enable-feature", fmt.Sprintf("Experimental features to enable. The flag can be repeated to enable multiple features. Valid options: %s", strings.Join(featurecontrol.AllowedFlags, ", "))).Default("").StringVar(&featureFlags) + + logger := log.NewLogfmtLogger(os.Stdout) + featureConfig, err := featurecontrol.NewFlags(logger, featureFlags) + if err != nil { + kingpin.Fatalf(":error parsing the feature flag list: %v\n", err) + } + if featureConfig.DisableNewLabelMatchers() { + adapter.ParseMatcher = adapter.OldMatcherParser(logger) + adapter.ParseMatchers = adapter.OldMatchersParser(logger) + } app.Version(version.Print("amtool")) app.GetFlag("help").Short('h') diff --git a/featurecontrol/featurecontrol.go b/featurecontrol/featurecontrol.go index 7a04499545..6eac1927fa 100644 --- a/featurecontrol/featurecontrol.go +++ b/featurecontrol/featurecontrol.go @@ -22,24 +22,31 @@ import ( ) const ( - fcReceiverNameInMetrics = "receiver-name-in-metrics" + fcReceiverNameInMetrics = "receiver-name-in-metrics" + fcDisabledNewLabelMatchers = "disable-new-label-matchers" ) var AllowedFlags = []string{fcReceiverNameInMetrics} type Flagger interface { EnableReceiverNamesInMetrics() bool + DisableNewLabelMatchers() bool } type Flags struct { logger log.Logger enableReceiverNamesInMetrics bool + disableNewLabelMatchers bool } func (f *Flags) EnableReceiverNamesInMetrics() bool { return f.enableReceiverNamesInMetrics } +func (f *Flags) DisableNewLabelMatchers() bool { + return f.disableNewLabelMatchers +} + type flagOption func(flags *Flags) func enableReceiverNameInMetrics() flagOption { @@ -48,6 +55,12 @@ func enableReceiverNameInMetrics() flagOption { } } +func disableNewLabelMatchers() flagOption { + return func(configs *Flags) { + configs.disableNewLabelMatchers = true + } +} + func NewFlags(logger log.Logger, features string) (Flagger, error) { fc := &Flags{logger: logger} opts := []flagOption{} @@ -61,6 +74,9 @@ func NewFlags(logger log.Logger, features string) (Flagger, error) { case fcReceiverNameInMetrics: opts = append(opts, enableReceiverNameInMetrics()) level.Warn(logger).Log("msg", "Experimental receiver name in metrics enabled") + case fcDisabledNewLabelMatchers: + opts = append(opts, disableNewLabelMatchers()) + level.Warn(logger).Log("msg", "Disabled new label matchers") default: return nil, fmt.Errorf("Unknown option '%s' for --enable-feature", feature) } @@ -76,3 +92,5 @@ func NewFlags(logger log.Logger, features string) (Flagger, error) { type NoopFlags struct{} func (n NoopFlags) EnableReceiverNamesInMetrics() bool { return false } + +func (n NoopFlags) DisableNewLabelMatchers() bool { return false } From 87446822472fa33228c167b816030e1c3576a4d0 Mon Sep 17 00:00:00 2001 From: George Robinson Date: Fri, 8 Sep 2023 09:43:48 +0100 Subject: [PATCH 08/27] Fix missing feature in AllowedFlags Signed-off-by: George Robinson --- featurecontrol/featurecontrol.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/featurecontrol/featurecontrol.go b/featurecontrol/featurecontrol.go index 6eac1927fa..462bf33cb8 100644 --- a/featurecontrol/featurecontrol.go +++ b/featurecontrol/featurecontrol.go @@ -26,7 +26,7 @@ const ( fcDisabledNewLabelMatchers = "disable-new-label-matchers" ) -var AllowedFlags = []string{fcReceiverNameInMetrics} +var AllowedFlags = []string{fcReceiverNameInMetrics, fcDisabledNewLabelMatchers} type Flagger interface { EnableReceiverNamesInMetrics() bool From cdab2c475fa86d3a24c3329c63f83f470a6f579e Mon Sep 17 00:00:00 2001 From: George Robinson Date: Mon, 11 Sep 2023 11:29:41 +0100 Subject: [PATCH 09/27] Rename adapter to compat Signed-off-by: George Robinson --- cli/alert_query.go | 4 ++-- cli/root.go | 9 ++++----- cli/silence_query.go | 4 ++-- cli/utils.go | 6 +++--- matchers/{adapter => compat}/parse.go | 2 +- matchers/{adapter => compat}/parse_test.go | 2 +- 6 files changed, 13 insertions(+), 14 deletions(-) rename matchers/{adapter => compat}/parse.go (99%) rename matchers/{adapter => compat}/parse_test.go (99%) diff --git a/cli/alert_query.go b/cli/alert_query.go index 383d245cf9..aed220cb8f 100644 --- a/cli/alert_query.go +++ b/cli/alert_query.go @@ -22,7 +22,7 @@ import ( "github.com/prometheus/alertmanager/api/v2/client/alert" "github.com/prometheus/alertmanager/cli/format" - "github.com/prometheus/alertmanager/matchers/adapter" + "github.com/prometheus/alertmanager/matchers/compat" ) type alertQueryCmd struct { @@ -80,7 +80,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 := adapter.ParseMatcher(m) + _, err := compat.ParseMatcher(m) if err != nil { a.matcherGroups[0] = fmt.Sprintf("alertname=%s", m) } diff --git a/cli/root.go b/cli/root.go index cf654b25a9..af454ad08c 100644 --- a/cli/root.go +++ b/cli/root.go @@ -28,13 +28,12 @@ import ( "github.com/prometheus/common/version" "golang.org/x/mod/semver" + clientruntime "github.com/go-openapi/runtime/client" "github.com/prometheus/alertmanager/api/v2/client" "github.com/prometheus/alertmanager/cli/config" "github.com/prometheus/alertmanager/cli/format" "github.com/prometheus/alertmanager/featurecontrol" - "github.com/prometheus/alertmanager/matchers/adapter" - - clientruntime "github.com/go-openapi/runtime/client" + "github.com/prometheus/alertmanager/matchers/compat" ) var ( @@ -150,8 +149,8 @@ func Execute() { kingpin.Fatalf(":error parsing the feature flag list: %v\n", err) } if featureConfig.DisableNewLabelMatchers() { - adapter.ParseMatcher = adapter.OldMatcherParser(logger) - adapter.ParseMatchers = adapter.OldMatchersParser(logger) + compat.ParseMatcher = compat.OldMatcherParser(logger) + compat.ParseMatchers = compat.OldMatchersParser(logger) } app.Version(version.Print("amtool")) diff --git a/cli/silence_query.go b/cli/silence_query.go index a80b0f0c2b..c743dba44b 100644 --- a/cli/silence_query.go +++ b/cli/silence_query.go @@ -24,7 +24,7 @@ import ( "github.com/prometheus/alertmanager/api/v2/client/silence" "github.com/prometheus/alertmanager/api/v2/models" "github.com/prometheus/alertmanager/cli/format" - "github.com/prometheus/alertmanager/matchers/adapter" + "github.com/prometheus/alertmanager/matchers/compat" ) type silenceQueryCmd struct { @@ -98,7 +98,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 := adapter.ParseMatcher(c.matchers[0]) + _, err := compat.ParseMatcher(c.matchers[0]) if err != nil { c.matchers[0] = fmt.Sprintf("alertname=%s", c.matchers[0]) } diff --git a/cli/utils.go b/cli/utils.go index 31cdf645da..6be27261dd 100644 --- a/cli/utils.go +++ b/cli/utils.go @@ -26,7 +26,7 @@ import ( "github.com/prometheus/alertmanager/api/v2/client/general" "github.com/prometheus/alertmanager/api/v2/models" "github.com/prometheus/alertmanager/config" - "github.com/prometheus/alertmanager/matchers/adapter" + "github.com/prometheus/alertmanager/matchers/compat" "github.com/prometheus/alertmanager/pkg/labels" ) @@ -35,7 +35,7 @@ func parseMatchers(inputMatchers []string) ([]labels.Matcher, error) { matchers := make([]labels.Matcher, 0, len(inputMatchers)) for _, v := range inputMatchers { - matcher, err := adapter.ParseMatcher(v) + matcher, err := compat.ParseMatcher(v) if err != nil { return []labels.Matcher{}, err } @@ -100,7 +100,7 @@ func parseLabels(inputLabels []string) (models.LabelSet, error) { labelSet := make(models.LabelSet, len(inputLabels)) for _, l := range inputLabels { - matcher, err := adapter.ParseMatcher(l) + matcher, err := compat.ParseMatcher(l) if err != nil { return models.LabelSet{}, err } diff --git a/matchers/adapter/parse.go b/matchers/compat/parse.go similarity index 99% rename from matchers/adapter/parse.go rename to matchers/compat/parse.go index ead6848056..930da9b071 100644 --- a/matchers/adapter/parse.go +++ b/matchers/compat/parse.go @@ -11,7 +11,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package adapter +package compat import ( "github.com/go-kit/log" diff --git a/matchers/adapter/parse_test.go b/matchers/compat/parse_test.go similarity index 99% rename from matchers/adapter/parse_test.go rename to matchers/compat/parse_test.go index 0bfa245811..b92b1d421b 100644 --- a/matchers/adapter/parse_test.go +++ b/matchers/compat/parse_test.go @@ -11,7 +11,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package adapter +package compat import ( "testing" From 4ec38dacdc606d2d456ad81cc8f7579980a5ccba Mon Sep 17 00:00:00 2001 From: George Robinson Date: Mon, 11 Sep 2023 11:33:20 +0100 Subject: [PATCH 10/27] Fix lint Signed-off-by: George Robinson --- cli/root.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/root.go b/cli/root.go index af454ad08c..4f3be8ecff 100644 --- a/cli/root.go +++ b/cli/root.go @@ -23,12 +23,12 @@ import ( "github.com/alecthomas/kingpin/v2" "github.com/go-kit/log" + clientruntime "github.com/go-openapi/runtime/client" "github.com/go-openapi/strfmt" promconfig "github.com/prometheus/common/config" "github.com/prometheus/common/version" "golang.org/x/mod/semver" - clientruntime "github.com/go-openapi/runtime/client" "github.com/prometheus/alertmanager/api/v2/client" "github.com/prometheus/alertmanager/cli/config" "github.com/prometheus/alertmanager/cli/format" From 86f08515fd53a92627b48606189e2cf1c772b555 Mon Sep 17 00:00:00 2001 From: George Robinson Date: Tue, 12 Sep 2023 11:16:42 +0100 Subject: [PATCH 11/27] Add two option flag Signed-off-by: George Robinson --- cli/alert_query.go | 2 +- cli/root.go | 5 +- cli/silence_query.go | 2 +- cli/utils.go | 4 +- featurecontrol/featurecontrol.go | 53 ++++++++++++----- matchers/compat/parse.go | 99 ++++++++++++++++++-------------- matchers/compat/parse_test.go | 4 +- 7 files changed, 104 insertions(+), 65 deletions(-) diff --git a/cli/alert_query.go b/cli/alert_query.go index aed220cb8f..219d76fdb9 100644 --- a/cli/alert_query.go +++ b/cli/alert_query.go @@ -80,7 +80,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.ParseMatcher(m) + _, err := compat.Matcher(m) if err != nil { a.matcherGroups[0] = fmt.Sprintf("alertname=%s", m) } diff --git a/cli/root.go b/cli/root.go index 4f3be8ecff..bff9dd747c 100644 --- a/cli/root.go +++ b/cli/root.go @@ -148,10 +148,7 @@ func Execute() { if err != nil { kingpin.Fatalf(":error parsing the feature flag list: %v\n", err) } - if featureConfig.DisableNewLabelMatchers() { - compat.ParseMatcher = compat.OldMatcherParser(logger) - compat.ParseMatchers = compat.OldMatchersParser(logger) - } + compat.InitFromFlags(logger, featureConfig) app.Version(version.Print("amtool")) app.GetFlag("help").Short('h') diff --git a/cli/silence_query.go b/cli/silence_query.go index c743dba44b..5502fed2c1 100644 --- a/cli/silence_query.go +++ b/cli/silence_query.go @@ -98,7 +98,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.ParseMatcher(c.matchers[0]) + _, err := compat.Matcher(c.matchers[0]) if err != nil { c.matchers[0] = fmt.Sprintf("alertname=%s", c.matchers[0]) } diff --git a/cli/utils.go b/cli/utils.go index 6be27261dd..22e18c3dd8 100644 --- a/cli/utils.go +++ b/cli/utils.go @@ -35,7 +35,7 @@ func parseMatchers(inputMatchers []string) ([]labels.Matcher, error) { matchers := make([]labels.Matcher, 0, len(inputMatchers)) for _, v := range inputMatchers { - matcher, err := compat.ParseMatcher(v) + matcher, err := compat.Matcher(v) if err != nil { return []labels.Matcher{}, err } @@ -100,7 +100,7 @@ func parseLabels(inputLabels []string) (models.LabelSet, error) { labelSet := make(models.LabelSet, len(inputLabels)) for _, l := range inputLabels { - matcher, err := compat.ParseMatcher(l) + matcher, err := compat.Matcher(l) if err != nil { return models.LabelSet{}, err } diff --git a/featurecontrol/featurecontrol.go b/featurecontrol/featurecontrol.go index 462bf33cb8..0b61a77386 100644 --- a/featurecontrol/featurecontrol.go +++ b/featurecontrol/featurecontrol.go @@ -14,6 +14,7 @@ package featurecontrol import ( + "errors" "fmt" "strings" @@ -22,29 +23,40 @@ import ( ) const ( - fcReceiverNameInMetrics = "receiver-name-in-metrics" - fcDisabledNewLabelMatchers = "disable-new-label-matchers" + fcReceiverNameInMetrics = "receiver-name-in-metrics" + fcStableMatchersParsing = "stable-matchers-parsing" + fcUTF8MatchersParsing = "utf8-matchers-parsing" ) -var AllowedFlags = []string{fcReceiverNameInMetrics, fcDisabledNewLabelMatchers} +var AllowedFlags = []string{ + fcReceiverNameInMetrics, + fcStableMatchersParsing, + fcUTF8MatchersParsing, +} type Flagger interface { EnableReceiverNamesInMetrics() bool - DisableNewLabelMatchers() bool + StableMatchersParsing() bool + UTF8MatchersParsing() bool } type Flags struct { logger log.Logger enableReceiverNamesInMetrics bool - disableNewLabelMatchers bool + utf8MatchersParsing bool + stableMatchersParsing bool } func (f *Flags) EnableReceiverNamesInMetrics() bool { return f.enableReceiverNamesInMetrics } -func (f *Flags) DisableNewLabelMatchers() bool { - return f.disableNewLabelMatchers +func (f *Flags) StableMatchersParsing() bool { + return f.stableMatchersParsing +} + +func (f *Flags) UTF8MatchersParsing() bool { + return f.utf8MatchersParsing } type flagOption func(flags *Flags) @@ -55,9 +67,15 @@ func enableReceiverNameInMetrics() flagOption { } } -func disableNewLabelMatchers() flagOption { +func enableStableMatchersParsing() flagOption { + return func(configs *Flags) { + configs.stableMatchersParsing = true + } +} + +func enableUTF8MatchersParsing() flagOption { return func(configs *Flags) { - configs.disableNewLabelMatchers = true + configs.utf8MatchersParsing = true } } @@ -74,9 +92,12 @@ func NewFlags(logger log.Logger, features string) (Flagger, error) { case fcReceiverNameInMetrics: opts = append(opts, enableReceiverNameInMetrics()) level.Warn(logger).Log("msg", "Experimental receiver name in metrics enabled") - case fcDisabledNewLabelMatchers: - opts = append(opts, disableNewLabelMatchers()) - level.Warn(logger).Log("msg", "Disabled new label matchers") + case fcStableMatchersParsing: + opts = append(opts, enableStableMatchersParsing()) + level.Warn(logger).Log("msg", "Enabled stable matchers parsing") + case fcUTF8MatchersParsing: + opts = append(opts, enableUTF8MatchersParsing()) + level.Warn(logger).Log("msg", "Enabled UTF-8 matchers parsing") default: return nil, fmt.Errorf("Unknown option '%s' for --enable-feature", feature) } @@ -86,6 +107,10 @@ func NewFlags(logger log.Logger, features string) (Flagger, error) { opt(fc) } + if fc.stableMatchersParsing && fc.utf8MatchersParsing { + return nil, errors.New("Both stable and UTF-8 matchers parsing is enabled, please choose one or remove the flag for both") + } + return fc, nil } @@ -93,4 +118,6 @@ type NoopFlags struct{} func (n NoopFlags) EnableReceiverNamesInMetrics() bool { return false } -func (n NoopFlags) DisableNewLabelMatchers() bool { return false } +func (n NoopFlags) StableMatchersParsing() bool { return false } + +func (n NoopFlags) UTF8MatchersParsing() bool { return false } diff --git a/matchers/compat/parse.go b/matchers/compat/parse.go index 930da9b071..7e6814ae91 100644 --- a/matchers/compat/parse.go +++ b/matchers/compat/parse.go @@ -16,69 +16,90 @@ package compat import ( "github.com/go-kit/log" "github.com/go-kit/log/level" + "github.com/prometheus/alertmanager/featurecontrol" "github.com/prometheus/alertmanager/matchers/parse" "github.com/prometheus/alertmanager/pkg/labels" ) var ( - // ParseMatcher is the default parser for parsing individual matchers. - ParseMatcher = FallbackMatcherParser(log.NewNopLogger()) - - // ParseMatchers is the default parser for parsing a series of zero or - // more matchers. - ParseMatchers = FallbackMatchersParser(log.NewNopLogger()) + parseMatcher = stableMatcherParser(log.NewNopLogger()) + parseMatchers = stableMatchersParser(log.NewNopLogger()) ) -// MatcherParser is an interface for parsing a single matcher in the input string. -type MatcherParser func(s string) (*labels.Matcher, error) +type matcherParser func(s string) (*labels.Matcher, error) +type matchersParser func(s 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) +} + +// 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) +} -// MatchersParser is an interface for parsing one or more matchers in the input string. -type MatchersParser func(s string) (labels.Matchers, error) +// InitFromFlags initializes the compat package from the flagger. +func InitFromFlags(l log.Logger, f featurecontrol.Flagger) { + if f.StableMatchersParsing() { + parseMatcher = stableMatcherParser(l) + parseMatchers = stableMatchersParser(l) + } else if f.UTF8MatchersParsing() { + parseMatcher = utf8MatcherParser(l) + parseMatchers = utf8MatchersParser(l) + } else { + parseMatcher = fallbackMatcherParser(l) + parseMatchers = fallbackMatchersParser(l) + } +} -// OldMatcherParser uses the old pkg/labels parser to parse the matcher in +// stableMatcherParser uses the old pkg/labels parser to parse the matcher in // the input string. -func OldMatcherParser(l log.Logger) MatcherParser { +func stableMatcherParser(_ log.Logger) matcherParser { + return func(s string) (*labels.Matcher, error) { + return labels.ParseMatcher(s) + } +} + +// stableMatchersParser uses the old pkg/labels parser to parse zero or more +// matchers in the input string. It returns an error if the input is invalid. +func stableMatchersParser(_ log.Logger) matchersParser { + return func(s string) (labels.Matchers, error) { + return labels.ParseMatchers(s) + } +} + +// 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(_ log.Logger) matcherParser { return func(s string) (*labels.Matcher, error) { - level.Debug(l).Log( - "msg", - "Parsing matcher with old regular expressions parser", - "matcher", - s, - ) return labels.ParseMatcher(s) } } -// OldMatchersParser uses the old pkg/labels parser to parse zero or more -// matchers in the string. It returns an error if the input is invalid. -func OldMatchersParser(l log.Logger) MatchersParser { +// 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(_ log.Logger) matchersParser { return func(s string) (labels.Matchers, error) { - level.Debug(l).Log( - "msg", - "Parsing matchers with old regular expressions parser", - "matchers", - s) return labels.ParseMatchers(s) } } -// FallbackMatcherParser uses the new matchers/parse parser to parse +// 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) MatcherParser { +func fallbackMatcherParser(l log.Logger) matcherParser { return func(s string) (*labels.Matcher, error) { var ( m *labels.Matcher err error invalidErr error ) - level.Debug(l).Log( - "msg", - "Parsing matcher with new parser", - "matcher", - s, - ) m, err = parse.Matcher(s) if err != nil { // The input is not valid in the old pkg/labels parser either, @@ -102,22 +123,16 @@ func FallbackMatcherParser(l log.Logger) MatcherParser { } } -// FallbackMatchersParser uses the new matchers/parse parser to parse the +// 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) MatchersParser { +func fallbackMatchersParser(l log.Logger) matchersParser { return func(s string) (labels.Matchers, error) { var ( m []*labels.Matcher err error invalidErr error ) - level.Debug(l).Log( - "msg", - "Parsing matchers with new parser", - "matchers", - s, - ) m, err = parse.Matchers(s) if err != nil { // The input is not valid in the old pkg/labels parser either, diff --git a/matchers/compat/parse_test.go b/matchers/compat/parse_test.go index b92b1d421b..b3d1f15a35 100644 --- a/matchers/compat/parse_test.go +++ b/matchers/compat/parse_test.go @@ -45,7 +45,7 @@ func TestFallbackMatcherParser(t *testing.T) { input: "foo!bar", err: "bad matcher format: foo!bar", }} - f := FallbackMatcherParser(log.NewNopLogger()) + f := fallbackMatcherParser(log.NewNopLogger()) for _, test := range tests { t.Run(test.name, func(t *testing.T) { matcher, err := f(test.input) @@ -91,7 +91,7 @@ func TestFallbackMatchersParser(t *testing.T) { input: "{foo!bar}", err: "bad matcher format: foo!bar", }} - f := FallbackMatchersParser(log.NewNopLogger()) + f := fallbackMatchersParser(log.NewNopLogger()) for _, test := range tests { t.Run(test.name, func(t *testing.T) { matchers, err := f(test.input) From 2278d78626cb78cf354094391df46cc78128da68 Mon Sep 17 00:00:00 2001 From: George Robinson Date: Tue, 12 Sep 2023 11:26:28 +0100 Subject: [PATCH 12/27] Fix lint Signed-off-by: George Robinson --- matchers/compat/parse.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/matchers/compat/parse.go b/matchers/compat/parse.go index 7e6814ae91..9f001423e8 100644 --- a/matchers/compat/parse.go +++ b/matchers/compat/parse.go @@ -16,8 +16,8 @@ package compat import ( "github.com/go-kit/log" "github.com/go-kit/log/level" - "github.com/prometheus/alertmanager/featurecontrol" + "github.com/prometheus/alertmanager/featurecontrol" "github.com/prometheus/alertmanager/matchers/parse" "github.com/prometheus/alertmanager/pkg/labels" ) @@ -28,6 +28,7 @@ var ( ) type matcherParser func(s string) (*labels.Matcher, error) + type matchersParser func(s string) (labels.Matchers, error) // Matcher parses the matcher in the input string. It returns an error From 846a93347fc27e66c0530a35f6d113d1ec1e0981 Mon Sep 17 00:00:00 2001 From: George Robinson Date: Tue, 12 Sep 2023 16:26:29 +0100 Subject: [PATCH 13/27] Update warning log on fallback Signed-off-by: George Robinson --- matchers/compat/parse.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/matchers/compat/parse.go b/matchers/compat/parse.go index 9f001423e8..1d63c31967 100644 --- a/matchers/compat/parse.go +++ b/matchers/compat/parse.go @@ -146,7 +146,7 @@ func fallbackMatchersParser(l log.Logger) matchersParser { // new matchers/parse parser. level.Warn(l).Log( "msg", - "Failed to parse input with matchers/parse, falling back to pkg/labels parser", + "Alertmanager is moving to a new parser for label matchers, and this input is incompatible. Please make sure all regular expressions and values are double-quoted. If you are still seeing this message please open an issue.", "matchers", s, "err", From 6e76633b4974130baf1484fda7db5f980a731acf Mon Sep 17 00:00:00 2001 From: George Robinson Date: Wed, 13 Sep 2023 10:37:39 +0100 Subject: [PATCH 14/27] Use same error when parsing matchers plural Signed-off-by: George Robinson --- matchers/compat/parse.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/matchers/compat/parse.go b/matchers/compat/parse.go index 1d63c31967..de8aaf5f04 100644 --- a/matchers/compat/parse.go +++ b/matchers/compat/parse.go @@ -113,7 +113,7 @@ func fallbackMatcherParser(l log.Logger) matcherParser { // new matchers/parse parser. level.Warn(l).Log( "msg", - "Failed to parse input with matchers/parse, falling back to pkg/labels parser", + "Alertmanager is moving to a new parser for label matchers, and this input is incompatible. Please make sure all regular expressions and values are double-quoted. If you are still seeing this message please open an issue.", "matcher", s, "err", From d2e0b07fe2bfabcf530aaddc967b464d51d87a45 Mon Sep 17 00:00:00 2001 From: George Robinson Date: Wed, 13 Sep 2023 12:26:02 +0100 Subject: [PATCH 15/27] Fix TestQuerySilence test Signed-off-by: George Robinson --- cli/silence_add.go | 3 ++- cli/silence_query.go | 6 +++--- cli/utils.go | 13 +++++++------ 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/cli/silence_add.go b/cli/silence_add.go index 3125c512f8..c50496743f 100644 --- a/cli/silence_add.go +++ b/cli/silence_add.go @@ -18,6 +18,7 @@ import ( "errors" "fmt" "os/user" + "strconv" "time" "github.com/alecthomas/kingpin/v2" @@ -94,7 +95,7 @@ func (c *silenceAddCmd) add(ctx context.Context, _ *kingpin.ParseContext) error // to the front. _, err := parseMatchers([]string{c.matchers[0]}) if err != nil { - c.matchers[0] = fmt.Sprintf("alertname=%s", c.matchers[0]) + c.matchers[0] = fmt.Sprintf("alertname=%s", strconv.Quote(c.matchers[0])) } } diff --git a/cli/silence_query.go b/cli/silence_query.go index 5502fed2c1..85540c82e7 100644 --- a/cli/silence_query.go +++ b/cli/silence_query.go @@ -17,6 +17,7 @@ import ( "context" "errors" "fmt" + "strconv" "time" kingpin "github.com/alecthomas/kingpin/v2" @@ -24,7 +25,6 @@ import ( "github.com/prometheus/alertmanager/api/v2/client/silence" "github.com/prometheus/alertmanager/api/v2/models" "github.com/prometheus/alertmanager/cli/format" - "github.com/prometheus/alertmanager/matchers/compat" ) type silenceQueryCmd struct { @@ -98,9 +98,9 @@ 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 := parseMatchers([]string{c.matchers[0]}) if err != nil { - c.matchers[0] = fmt.Sprintf("alertname=%s", c.matchers[0]) + c.matchers[0] = fmt.Sprintf("alertname=%s", strconv.Quote(c.matchers[0])) } } diff --git a/cli/utils.go b/cli/utils.go index 22e18c3dd8..8466c2ccd6 100644 --- a/cli/utils.go +++ b/cli/utils.go @@ -19,8 +19,9 @@ import ( "fmt" "net/url" "os" + "strings" - kingpin "github.com/alecthomas/kingpin/v2" + "github.com/alecthomas/kingpin/v2" "github.com/prometheus/common/model" "github.com/prometheus/alertmanager/api/v2/client/general" @@ -33,16 +34,16 @@ import ( // parseMatchers parses a list of matchers (cli arguments). func parseMatchers(inputMatchers []string) ([]labels.Matcher, error) { matchers := make([]labels.Matcher, 0, len(inputMatchers)) - - for _, v := range inputMatchers { - matcher, err := compat.Matcher(v) + for _, s := range inputMatchers { + if strings.HasPrefix(s, "{") || strings.HasSuffix(s, "}") { + return []labels.Matcher{}, fmt.Errorf("unexpected open or close brace: %s", s) + } + matcher, err := compat.Matcher(s) if err != nil { return []labels.Matcher{}, err } - matchers = append(matchers, *matcher) } - return matchers, nil } From b1f8e9c410f542d8feee2e5eb8f472962bcff450 Mon Sep 17 00:00:00 2001 From: George Robinson Date: Thu, 14 Sep 2023 14:50:26 +0100 Subject: [PATCH 16/27] Add check for braces to parseLabels Signed-off-by: George Robinson --- cli/utils.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cli/utils.go b/cli/utils.go index 8466c2ccd6..5038c92221 100644 --- a/cli/utils.go +++ b/cli/utils.go @@ -101,6 +101,9 @@ func parseLabels(inputLabels []string) (models.LabelSet, error) { labelSet := make(models.LabelSet, len(inputLabels)) for _, l := range inputLabels { + if strings.HasPrefix(l, "{") || strings.HasSuffix(l, "}") { + return models.LabelSet{}, fmt.Errorf("unexpected open or close brace: %s", l) + } matcher, err := compat.Matcher(l) if err != nil { return models.LabelSet{}, err From 0fa923e17ff1896dd1b75c902d0036155b532f3d Mon Sep 17 00:00:00 2001 From: George Robinson Date: Thu, 14 Sep 2023 14:53:55 +0100 Subject: [PATCH 17/27] Remove extra : in error message Signed-off-by: George Robinson --- cli/root.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/root.go b/cli/root.go index bff9dd747c..389600257e 100644 --- a/cli/root.go +++ b/cli/root.go @@ -146,7 +146,7 @@ func Execute() { logger := log.NewLogfmtLogger(os.Stdout) featureConfig, err := featurecontrol.NewFlags(logger, featureFlags) if err != nil { - kingpin.Fatalf(":error parsing the feature flag list: %v\n", err) + kingpin.Fatalf("error parsing the feature flag list: %v\n", err) } compat.InitFromFlags(logger, featureConfig) From c8691b457c5870395b175a38fb70b2a5b060e9ec Mon Sep 17 00:00:00 2001 From: George Robinson Date: Fri, 15 Sep 2023 12:18:11 +0100 Subject: [PATCH 18/27] Add verbose logging and suggestions to warning logs Signed-off-by: George Robinson --- cli/alert_add.go | 3 +- cli/root.go | 25 +++++++++---- cli/utils.go | 7 ---- matchers/compat/parse.go | 76 +++++++++++++++++++++++++++++++++++----- 4 files changed, 88 insertions(+), 23 deletions(-) diff --git a/cli/alert_add.go b/cli/alert_add.go index e27d1522b3..035d5f58a0 100644 --- a/cli/alert_add.go +++ b/cli/alert_add.go @@ -16,6 +16,7 @@ package cli import ( "context" "fmt" + "strconv" "time" "github.com/alecthomas/kingpin/v2" @@ -74,7 +75,7 @@ func (a *alertAddCmd) addAlert(ctx context.Context, _ *kingpin.ParseContext) err // Allow the alertname label to be defined implicitly as the first argument rather // than explicitly as a key=value pair. if _, err := parseLabels([]string{a.labels[0]}); err != nil { - a.labels[0] = fmt.Sprintf("alertname=%s", a.labels[0]) + a.labels[0] = fmt.Sprintf("alertname=%s", strconv.Quote(a.labels[0])) } } diff --git a/cli/root.go b/cli/root.go index 389600257e..69c1022c6a 100644 --- a/cli/root.go +++ b/cli/root.go @@ -23,6 +23,7 @@ import ( "github.com/alecthomas/kingpin/v2" "github.com/go-kit/log" + "github.com/go-kit/log/level" clientruntime "github.com/go-openapi/runtime/client" "github.com/go-openapi/strfmt" promconfig "github.com/prometheus/common/config" @@ -49,6 +50,21 @@ var ( legacyFlags = map[string]string{"comment_required": "require-comment"} ) +func initMatchersCompat(_ *kingpin.ParseContext) error { + logger := log.NewLogfmtLogger(os.Stdout) + if verbose { + logger = level.NewFilter(logger, level.AllowDebug()) + } else { + logger = level.NewFilter(logger, level.AllowInfo()) + } + featureConfig, err := featurecontrol.NewFlags(logger, featureFlags) + if err != nil { + kingpin.Fatalf("error parsing the feature flag list: %v\n", err) + } + compat.InitFromFlags(logger, featureConfig) + return nil +} + func requireAlertManagerURL(pc *kingpin.ParseContext) error { // Return without error if any help flag is set. for _, elem := range pc.Elements { @@ -143,13 +159,6 @@ func Execute() { app.Flag("version-check", "Check alertmanager version. Use --no-version-check to disable.").Default("true").BoolVar(&versionCheck) app.Flag("enable-feature", fmt.Sprintf("Experimental features to enable. The flag can be repeated to enable multiple features. Valid options: %s", strings.Join(featurecontrol.AllowedFlags, ", "))).Default("").StringVar(&featureFlags) - logger := log.NewLogfmtLogger(os.Stdout) - featureConfig, err := featurecontrol.NewFlags(logger, featureFlags) - if err != nil { - kingpin.Fatalf("error parsing the feature flag list: %v\n", err) - } - compat.InitFromFlags(logger, featureConfig) - app.Version(version.Print("amtool")) app.GetFlag("help").Short('h') app.UsageTemplate(kingpin.CompactUsageTemplate) @@ -166,6 +175,8 @@ func Execute() { configureConfigCmd(app) configureTemplateCmd(app) + app.Action(initMatchersCompat) + err = resolver.Bind(app, os.Args[1:]) if err != nil { kingpin.Fatalf("%v\n", err) diff --git a/cli/utils.go b/cli/utils.go index 5038c92221..3cb178fa6e 100644 --- a/cli/utils.go +++ b/cli/utils.go @@ -19,7 +19,6 @@ import ( "fmt" "net/url" "os" - "strings" "github.com/alecthomas/kingpin/v2" "github.com/prometheus/common/model" @@ -35,9 +34,6 @@ import ( func parseMatchers(inputMatchers []string) ([]labels.Matcher, error) { matchers := make([]labels.Matcher, 0, len(inputMatchers)) for _, s := range inputMatchers { - if strings.HasPrefix(s, "{") || strings.HasSuffix(s, "}") { - return []labels.Matcher{}, fmt.Errorf("unexpected open or close brace: %s", s) - } matcher, err := compat.Matcher(s) if err != nil { return []labels.Matcher{}, err @@ -101,9 +97,6 @@ func parseLabels(inputLabels []string) (models.LabelSet, error) { labelSet := make(models.LabelSet, len(inputLabels)) for _, l := range inputLabels { - if strings.HasPrefix(l, "{") || strings.HasSuffix(l, "}") { - return models.LabelSet{}, fmt.Errorf("unexpected open or close brace: %s", l) - } matcher, err := compat.Matcher(l) if err != nil { return models.LabelSet{}, err diff --git a/matchers/compat/parse.go b/matchers/compat/parse.go index de8aaf5f04..305b6f9b25 100644 --- a/matchers/compat/parse.go +++ b/matchers/compat/parse.go @@ -14,6 +14,9 @@ package compat import ( + "fmt" + "strings" + "github.com/go-kit/log" "github.com/go-kit/log/level" @@ -59,16 +62,28 @@ func InitFromFlags(l log.Logger, f featurecontrol.Flagger) { // stableMatcherParser uses the old pkg/labels parser to parse the matcher in // the input string. -func stableMatcherParser(_ log.Logger) matcherParser { +func stableMatcherParser(l log.Logger) matcherParser { return func(s string) (*labels.Matcher, error) { + level.Debug(l).Log( + "msg", + "Parsing with stable matchers parser", + "input", + s, + ) return labels.ParseMatcher(s) } } // stableMatchersParser uses the old pkg/labels parser to parse zero or more // matchers in the input string. It returns an error if the input is invalid. -func stableMatchersParser(_ log.Logger) matchersParser { +func stableMatchersParser(l log.Logger) matchersParser { return func(s string) (labels.Matchers, error) { + level.Debug(l).Log( + "msg", + "Parsing with stable matchers parser", + "input", + s, + ) return labels.ParseMatchers(s) } } @@ -76,8 +91,14 @@ func stableMatchersParser(_ log.Logger) matchersParser { // 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(_ log.Logger) matcherParser { +func utf8MatcherParser(l log.Logger) matcherParser { return func(s string) (*labels.Matcher, error) { + level.Debug(l).Log( + "msg", + "Parsing with UTF-8 matchers parser", + "input", + s, + ) return labels.ParseMatcher(s) } } @@ -85,8 +106,17 @@ func utf8MatcherParser(_ log.Logger) matcherParser { // 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(_ log.Logger) matchersParser { +func utf8MatchersParser(l log.Logger) matchersParser { return func(s string) (labels.Matchers, 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) + } return labels.ParseMatchers(s) } } @@ -101,6 +131,15 @@ func fallbackMatcherParser(l log.Logger) matcherParser { err error invalidErr error ) + level.Debug(l).Log( + "msg", + "Parsing with UTF-8 matchers parser, with fallback to stable matchers parser", + "input", + s, + ) + if strings.HasPrefix(s, "{") || strings.HasSuffix(s, "}") { + return nil, fmt.Errorf("unexpected open or close brace: %s", s) + } m, err = parse.Matcher(s) if err != nil { // The input is not valid in the old pkg/labels parser either, @@ -111,13 +150,16 @@ func fallbackMatcherParser(l log.Logger) matcherParser { } // 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 label matchers, and this input is incompatible. Please make sure all regular expressions and values are double-quoted. If you are still seeing this message please open an issue.", - "matcher", + "Alertmanager is moving to a new parser for label 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, ) } return m, nil @@ -134,6 +176,12 @@ func fallbackMatchersParser(l log.Logger) matchersParser { err error invalidErr error ) + level.Debug(l).Log( + "msg", + "Parsing with UTF-8 matchers parser, with fallback to stable matchers parser", + "input", + s, + ) m, err = parse.Matchers(s) if err != nil { // The input is not valid in the old pkg/labels parser either, @@ -142,15 +190,27 @@ func fallbackMatchersParser(l log.Logger) matchersParser { if invalidErr != nil { return nil, invalidErr } + var sb strings.Builder + sb.WriteRune('{') + for i, n := range m { + sb.WriteString(n.String()) + if i < len(m)-1 { + sb.WriteRune(',') + } + } + sb.WriteRune('}') + suggestion := sb.String() // The input is valid in the old pkg/labels parser, but not the // new matchers/parse parser. level.Warn(l).Log( "msg", - "Alertmanager is moving to a new parser for label matchers, and this input is incompatible. Please make sure all regular expressions and values are double-quoted. If you are still seeing this message please open an issue.", - "matchers", + "Alertmanager is moving to a new parser for label 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, ) } return m, nil From 794361dda36d057684154dbf5b907d48ff9e8fe8 Mon Sep 17 00:00:00 2001 From: George Robinson Date: Fri, 15 Sep 2023 12:21:31 +0100 Subject: [PATCH 19/27] Fix check for braces in wrong place Signed-off-by: George Robinson --- matchers/compat/parse.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/matchers/compat/parse.go b/matchers/compat/parse.go index 305b6f9b25..2f1ef23a50 100644 --- a/matchers/compat/parse.go +++ b/matchers/compat/parse.go @@ -99,6 +99,9 @@ func utf8MatcherParser(l log.Logger) matcherParser { "input", s, ) + if strings.HasPrefix(s, "{") || strings.HasSuffix(s, "}") { + return nil, fmt.Errorf("unexpected open or close brace: %s", s) + } return labels.ParseMatcher(s) } } @@ -114,9 +117,6 @@ func utf8MatchersParser(l log.Logger) matchersParser { "input", s, ) - if strings.HasPrefix(s, "{") || strings.HasSuffix(s, "}") { - return nil, fmt.Errorf("unexpected open or close brace: %s", s) - } return labels.ParseMatchers(s) } } From 5371e4aea479b96ffecea68322dc4ce83a038592 Mon Sep 17 00:00:00 2001 From: George Robinson Date: Thu, 21 Sep 2023 10:35:14 +0100 Subject: [PATCH 20/27] Fix incorrect parser used Signed-off-by: George Robinson --- matchers/compat/parse.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/matchers/compat/parse.go b/matchers/compat/parse.go index 2f1ef23a50..9a958e8c55 100644 --- a/matchers/compat/parse.go +++ b/matchers/compat/parse.go @@ -102,7 +102,7 @@ func utf8MatcherParser(l log.Logger) matcherParser { if strings.HasPrefix(s, "{") || strings.HasSuffix(s, "}") { return nil, fmt.Errorf("unexpected open or close brace: %s", s) } - return labels.ParseMatcher(s) + return parse.Matcher(s) } } @@ -117,7 +117,7 @@ func utf8MatchersParser(l log.Logger) matchersParser { "input", s, ) - return labels.ParseMatchers(s) + return parse.Matchers(s) } } From 9dbd07df3b833e4729f198a3c7647a640373c067 Mon Sep 17 00:00:00 2001 From: George Robinson Date: Sat, 23 Sep 2023 11:39:28 +0100 Subject: [PATCH 21/27] Put enabled at the end of the sentence Signed-off-by: George Robinson --- featurecontrol/featurecontrol.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/featurecontrol/featurecontrol.go b/featurecontrol/featurecontrol.go index 0b61a77386..9e0a54cdc6 100644 --- a/featurecontrol/featurecontrol.go +++ b/featurecontrol/featurecontrol.go @@ -94,10 +94,10 @@ func NewFlags(logger log.Logger, features string) (Flagger, error) { level.Warn(logger).Log("msg", "Experimental receiver name in metrics enabled") case fcStableMatchersParsing: opts = append(opts, enableStableMatchersParsing()) - level.Warn(logger).Log("msg", "Enabled stable matchers parsing") + level.Warn(logger).Log("msg", "Stable matchers parsing enabled") case fcUTF8MatchersParsing: opts = append(opts, enableUTF8MatchersParsing()) - level.Warn(logger).Log("msg", "Enabled UTF-8 matchers parsing") + level.Warn(logger).Log("msg", "UTF-8 matchers parsing enabled") default: return nil, fmt.Errorf("Unknown option '%s' for --enable-feature", feature) } From 9543033303df3f6c0e78b8468a39ab1f2bfc74bb Mon Sep 17 00:00:00 2001 From: George Robinson Date: Sat, 23 Sep 2023 11:49:56 +0100 Subject: [PATCH 22/27] Use single line when emitting logs Signed-off-by: George Robinson --- matchers/compat/parse.go | 64 +++++----------------------------------- 1 file changed, 8 insertions(+), 56 deletions(-) diff --git a/matchers/compat/parse.go b/matchers/compat/parse.go index 9a958e8c55..11627b278c 100644 --- a/matchers/compat/parse.go +++ b/matchers/compat/parse.go @@ -64,12 +64,7 @@ func InitFromFlags(l log.Logger, f featurecontrol.Flagger) { // the input string. func stableMatcherParser(l log.Logger) matcherParser { return func(s string) (*labels.Matcher, error) { - level.Debug(l).Log( - "msg", - "Parsing with stable matchers parser", - "input", - s, - ) + level.Debug(l).Log("msg", "Parsing with stable matchers parser", "input", s) return labels.ParseMatcher(s) } } @@ -78,12 +73,7 @@ func stableMatcherParser(l log.Logger) matcherParser { // matchers in the input string. It returns an error if the input is invalid. func stableMatchersParser(l log.Logger) matchersParser { return func(s string) (labels.Matchers, error) { - level.Debug(l).Log( - "msg", - "Parsing with stable matchers parser", - "input", - s, - ) + level.Debug(l).Log("msg", "Parsing with stable matchers parser", "input", s) return labels.ParseMatchers(s) } } @@ -93,12 +83,7 @@ func stableMatchersParser(l log.Logger) matchersParser { // to the old pkg/labels parser. func utf8MatcherParser(l log.Logger) matcherParser { return func(s string) (*labels.Matcher, error) { - level.Debug(l).Log( - "msg", - "Parsing with UTF-8 matchers parser", - "input", - s, - ) + 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) } @@ -111,12 +96,7 @@ func utf8MatcherParser(l log.Logger) matcherParser { // does not fallback to the old pkg/labels parser. func utf8MatchersParser(l log.Logger) matchersParser { return func(s string) (labels.Matchers, error) { - level.Debug(l).Log( - "msg", - "Parsing with UTF-8 matchers parser", - "input", - s, - ) + level.Debug(l).Log("msg", "Parsing with UTF-8 matchers parser", "input", s) return parse.Matchers(s) } } @@ -131,12 +111,7 @@ func fallbackMatcherParser(l log.Logger) matcherParser { err error invalidErr error ) - level.Debug(l).Log( - "msg", - "Parsing with UTF-8 matchers parser, with fallback to stable matchers parser", - "input", - s, - ) + level.Debug(l).Log("msg", "Parsing with UTF-8 matchers parser, with fallback to stable matchers parser", "input", s) if strings.HasPrefix(s, "{") || strings.HasSuffix(s, "}") { return nil, fmt.Errorf("unexpected open or close brace: %s", s) } @@ -151,16 +126,7 @@ func fallbackMatcherParser(l log.Logger) matcherParser { // 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 label 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 label 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) } return m, nil } @@ -176,12 +142,7 @@ func fallbackMatchersParser(l log.Logger) matchersParser { err error invalidErr error ) - level.Debug(l).Log( - "msg", - "Parsing with UTF-8 matchers parser, with fallback to stable matchers parser", - "input", - s, - ) + level.Debug(l).Log("msg", "Parsing with UTF-8 matchers parser, with fallback to stable matchers parser", "input", s) m, err = parse.Matchers(s) if err != nil { // The input is not valid in the old pkg/labels parser either, @@ -202,16 +163,7 @@ func fallbackMatchersParser(l log.Logger) matchersParser { suggestion := sb.String() // The input is valid in the old pkg/labels parser, but not the // new matchers/parse parser. - level.Warn(l).Log( - "msg", - "Alertmanager is moving to a new parser for label 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 label 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) } return m, nil } From 40d3e53ce59266cadb50962627d0cb9838eb16c0 Mon Sep 17 00:00:00 2001 From: George Robinson Date: Sat, 23 Sep 2023 11:50:31 +0100 Subject: [PATCH 23/27] Move comment inside if Signed-off-by: George Robinson --- matchers/compat/parse.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/matchers/compat/parse.go b/matchers/compat/parse.go index 11627b278c..d2f1c437c1 100644 --- a/matchers/compat/parse.go +++ b/matchers/compat/parse.go @@ -117,10 +117,10 @@ func fallbackMatcherParser(l log.Logger) matcherParser { } m, err = parse.Matcher(s) if err != nil { - // The input is not valid in the old pkg/labels parser either, - // it cannot be valid input. 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 } // The input is valid in the old pkg/labels parser, but not the @@ -145,10 +145,10 @@ func fallbackMatchersParser(l log.Logger) matchersParser { level.Debug(l).Log("msg", "Parsing with UTF-8 matchers parser, with fallback to stable matchers parser", "input", s) m, err = parse.Matchers(s) if err != nil { - // The input is not valid in the old pkg/labels parser either, - // it cannot be valid input. 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 } var sb strings.Builder From f85a75e7bc0994b0f7960add2f773c65d0972845 Mon Sep 17 00:00:00 2001 From: George Robinson Date: Sat, 23 Sep 2023 11:51:27 +0100 Subject: [PATCH 24/27] Update log line to include both labels and matchers Signed-off-by: George Robinson --- matchers/compat/parse.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/matchers/compat/parse.go b/matchers/compat/parse.go index d2f1c437c1..7ba63ca794 100644 --- a/matchers/compat/parse.go +++ b/matchers/compat/parse.go @@ -126,7 +126,7 @@ func fallbackMatcherParser(l log.Logger) matcherParser { // 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 label 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", s, "err", err, "suggestion", suggestion) } return m, nil } @@ -163,7 +163,7 @@ func fallbackMatchersParser(l log.Logger) matchersParser { suggestion := sb.String() // The input is valid in the old pkg/labels parser, but not the // new matchers/parse parser. - level.Warn(l).Log("msg", "Alertmanager is moving to a new parser for label 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", s, "err", err, "suggestion", suggestion) } return m, nil } From 1efe79b799ff3d380d3557e3013f2e48ea6702e0 Mon Sep 17 00:00:00 2001 From: George Robinson Date: Sat, 23 Sep 2023 11:54:57 +0100 Subject: [PATCH 25/27] Rename stable to classic Signed-off-by: George Robinson --- featurecontrol/featurecontrol.go | 32 ++++++++++++++++---------------- matchers/compat/parse.go | 26 +++++++++++++------------- 2 files changed, 29 insertions(+), 29 deletions(-) diff --git a/featurecontrol/featurecontrol.go b/featurecontrol/featurecontrol.go index 9e0a54cdc6..0108a680e7 100644 --- a/featurecontrol/featurecontrol.go +++ b/featurecontrol/featurecontrol.go @@ -23,36 +23,36 @@ import ( ) const ( - fcReceiverNameInMetrics = "receiver-name-in-metrics" - fcStableMatchersParsing = "stable-matchers-parsing" - fcUTF8MatchersParsing = "utf8-matchers-parsing" + fcReceiverNameInMetrics = "receiver-name-in-metrics" + fcClassicMatchersParsing = "classic-matchers-parsing" + fcUTF8MatchersParsing = "utf8-matchers-parsing" ) var AllowedFlags = []string{ fcReceiverNameInMetrics, - fcStableMatchersParsing, + fcClassicMatchersParsing, fcUTF8MatchersParsing, } type Flagger interface { EnableReceiverNamesInMetrics() bool - StableMatchersParsing() bool + ClassicMatchersParsing() bool UTF8MatchersParsing() bool } type Flags struct { logger log.Logger enableReceiverNamesInMetrics bool + classicMatchersParsing bool utf8MatchersParsing bool - stableMatchersParsing bool } func (f *Flags) EnableReceiverNamesInMetrics() bool { return f.enableReceiverNamesInMetrics } -func (f *Flags) StableMatchersParsing() bool { - return f.stableMatchersParsing +func (f *Flags) ClassicMatchersParsing() bool { + return f.classicMatchersParsing } func (f *Flags) UTF8MatchersParsing() bool { @@ -67,9 +67,9 @@ func enableReceiverNameInMetrics() flagOption { } } -func enableStableMatchersParsing() flagOption { +func enableClassicMatchersParsing() flagOption { return func(configs *Flags) { - configs.stableMatchersParsing = true + configs.classicMatchersParsing = true } } @@ -92,9 +92,9 @@ func NewFlags(logger log.Logger, features string) (Flagger, error) { case fcReceiverNameInMetrics: opts = append(opts, enableReceiverNameInMetrics()) level.Warn(logger).Log("msg", "Experimental receiver name in metrics enabled") - case fcStableMatchersParsing: - opts = append(opts, enableStableMatchersParsing()) - level.Warn(logger).Log("msg", "Stable matchers parsing enabled") + case fcClassicMatchersParsing: + opts = append(opts, enableClassicMatchersParsing()) + level.Warn(logger).Log("msg", "Classic matchers parsing enabled") case fcUTF8MatchersParsing: opts = append(opts, enableUTF8MatchersParsing()) level.Warn(logger).Log("msg", "UTF-8 matchers parsing enabled") @@ -107,8 +107,8 @@ func NewFlags(logger log.Logger, features string) (Flagger, error) { opt(fc) } - if fc.stableMatchersParsing && fc.utf8MatchersParsing { - return nil, errors.New("Both stable and UTF-8 matchers parsing is enabled, please choose one or remove the flag for both") + if fc.classicMatchersParsing && fc.utf8MatchersParsing { + return nil, errors.New("Both classic and UTF-8 matchers parsing is enabled, please choose one or remove the flag for both") } return fc, nil @@ -118,6 +118,6 @@ type NoopFlags struct{} func (n NoopFlags) EnableReceiverNamesInMetrics() bool { return false } -func (n NoopFlags) StableMatchersParsing() bool { return false } +func (n NoopFlags) ClassicMatchersParsing() bool { return false } func (n NoopFlags) UTF8MatchersParsing() bool { return false } diff --git a/matchers/compat/parse.go b/matchers/compat/parse.go index 7ba63ca794..4d2794d36e 100644 --- a/matchers/compat/parse.go +++ b/matchers/compat/parse.go @@ -26,8 +26,8 @@ import ( ) var ( - parseMatcher = stableMatcherParser(log.NewNopLogger()) - parseMatchers = stableMatchersParser(log.NewNopLogger()) + parseMatcher = classicMatcherParser(log.NewNopLogger()) + parseMatchers = classicMatchersParser(log.NewNopLogger()) ) type matcherParser func(s string) (*labels.Matcher, error) @@ -48,9 +48,9 @@ func Matchers(s string) (labels.Matchers, error) { // InitFromFlags initializes the compat package from the flagger. func InitFromFlags(l log.Logger, f featurecontrol.Flagger) { - if f.StableMatchersParsing() { - parseMatcher = stableMatcherParser(l) - parseMatchers = stableMatchersParser(l) + if f.ClassicMatchersParsing() { + parseMatcher = classicMatcherParser(l) + parseMatchers = classicMatchersParser(l) } else if f.UTF8MatchersParsing() { parseMatcher = utf8MatcherParser(l) parseMatchers = utf8MatchersParser(l) @@ -60,20 +60,20 @@ func InitFromFlags(l log.Logger, f featurecontrol.Flagger) { } } -// stableMatcherParser uses the old pkg/labels parser to parse the matcher in +// classicMatcherParser uses the old pkg/labels parser to parse the matcher in // the input string. -func stableMatcherParser(l log.Logger) matcherParser { +func classicMatcherParser(l log.Logger) matcherParser { return func(s string) (*labels.Matcher, error) { - level.Debug(l).Log("msg", "Parsing with stable matchers parser", "input", s) + level.Debug(l).Log("msg", "Parsing with classic matchers parser", "input", s) return labels.ParseMatcher(s) } } -// stableMatchersParser uses the old pkg/labels parser to parse zero or more +// classicMatchersParser uses the old pkg/labels parser to parse zero or more // matchers in the input string. It returns an error if the input is invalid. -func stableMatchersParser(l log.Logger) matchersParser { +func classicMatchersParser(l log.Logger) matchersParser { return func(s string) (labels.Matchers, error) { - level.Debug(l).Log("msg", "Parsing with stable matchers parser", "input", s) + level.Debug(l).Log("msg", "Parsing with classic matchers parser", "input", s) return labels.ParseMatchers(s) } } @@ -111,7 +111,7 @@ func fallbackMatcherParser(l log.Logger) matcherParser { err error invalidErr error ) - level.Debug(l).Log("msg", "Parsing with UTF-8 matchers parser, with fallback to stable matchers parser", "input", s) + 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) } @@ -142,7 +142,7 @@ func fallbackMatchersParser(l log.Logger) matchersParser { err error invalidErr error ) - level.Debug(l).Log("msg", "Parsing with UTF-8 matchers parser, with fallback to stable matchers parser", "input", s) + 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) From 5f4910169e810b62a4a239e0855f99b013b4fc30 Mon Sep 17 00:00:00 2001 From: George Robinson Date: Mon, 25 Sep 2023 12:21:35 +0100 Subject: [PATCH 26/27] Remove functions from utils.go Signed-off-by: George Robinson --- cli/alert_add.go | 37 ++++++++++++++++++++++++++++--------- cli/silence_add.go | 15 ++++++++++----- cli/silence_query.go | 3 ++- cli/test_routing.go | 15 ++++++++++++--- cli/utils.go | 33 --------------------------------- 5 files changed, 52 insertions(+), 51 deletions(-) diff --git a/cli/alert_add.go b/cli/alert_add.go index 035d5f58a0..6018b95654 100644 --- a/cli/alert_add.go +++ b/cli/alert_add.go @@ -15,6 +15,7 @@ package cli import ( "context" + "errors" "fmt" "strconv" "time" @@ -24,6 +25,8 @@ import ( "github.com/prometheus/alertmanager/api/v2/client/alert" "github.com/prometheus/alertmanager/api/v2/models" + "github.com/prometheus/alertmanager/matchers/compat" + "github.com/prometheus/alertmanager/pkg/labels" ) type alertAddCmd struct { @@ -74,29 +77,45 @@ 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 := parseLabels([]string{a.labels[0]}); err != nil { + if _, err := compat.Matcher(a.labels[0]); err != nil { a.labels[0] = fmt.Sprintf("alertname=%s", strconv.Quote(a.labels[0])) } } - labels, err := parseLabels(a.labels) - if err != nil { - return err + ls := make(models.LabelSet, len(a.labels)) + for _, l := range a.labels { + matcher, err := compat.Matcher(l) + if err != nil { + return err + } + if matcher.Type != labels.MatchEqual { + return errors.New("labels must be specified as key=value pairs") + } + ls[matcher.Name] = matcher.Value } - annotations, err := parseLabels(a.annotations) - if err != nil { - return err + annotations := make(models.LabelSet, len(a.annotations)) + for _, a := range a.annotations { + matcher, err := compat.Matcher(a) + if err != nil { + return err + } + if matcher.Type != labels.MatchEqual { + return errors.New("annotations must be specified as key=value pairs") + } + annotations[matcher.Name] = matcher.Value } var startsAt, endsAt time.Time if a.start != "" { + var err error startsAt, err = time.Parse(time.RFC3339, a.start) if err != nil { return err } } if a.end != "" { + var err error endsAt, err = time.Parse(time.RFC3339, a.end) if err != nil { return err @@ -106,7 +125,7 @@ func (a *alertAddCmd) addAlert(ctx context.Context, _ *kingpin.ParseContext) err pa := &models.PostableAlert{ Alert: models.Alert{ GeneratorURL: strfmt.URI(a.generatorURL), - Labels: labels, + Labels: ls, }, Annotations: annotations, StartsAt: strfmt.DateTime(startsAt), @@ -117,6 +136,6 @@ func (a *alertAddCmd) addAlert(ctx context.Context, _ *kingpin.ParseContext) err amclient := NewAlertmanagerClient(alertmanagerURL) - _, err = amclient.Alert.PostAlerts(alertParams) + _, err := amclient.Alert.PostAlerts(alertParams) return err } diff --git a/cli/silence_add.go b/cli/silence_add.go index c50496743f..d30a523431 100644 --- a/cli/silence_add.go +++ b/cli/silence_add.go @@ -27,6 +27,8 @@ import ( "github.com/prometheus/alertmanager/api/v2/client/silence" "github.com/prometheus/alertmanager/api/v2/models" + "github.com/prometheus/alertmanager/matchers/compat" + "github.com/prometheus/alertmanager/pkg/labels" ) func username() string { @@ -93,17 +95,20 @@ 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 := parseMatchers([]string{c.matchers[0]}) + _, err := compat.Matcher(c.matchers[0]) if err != nil { c.matchers[0] = fmt.Sprintf("alertname=%s", strconv.Quote(c.matchers[0])) } } - matchers, err := parseMatchers(c.matchers) - if err != nil { - return err + matchers := make([]labels.Matcher, 0, len(c.matchers)) + for _, s := range c.matchers { + m, err := compat.Matcher(s) + if err != nil { + return err + } + matchers = append(matchers, *m) } - if len(matchers) < 1 { return fmt.Errorf("no matchers specified") } diff --git a/cli/silence_query.go b/cli/silence_query.go index 85540c82e7..ddffca3b3c 100644 --- a/cli/silence_query.go +++ b/cli/silence_query.go @@ -25,6 +25,7 @@ import ( "github.com/prometheus/alertmanager/api/v2/client/silence" "github.com/prometheus/alertmanager/api/v2/models" "github.com/prometheus/alertmanager/cli/format" + "github.com/prometheus/alertmanager/matchers/compat" ) type silenceQueryCmd struct { @@ -98,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 := parseMatchers([]string{c.matchers[0]}) + _, err := compat.Matcher(c.matchers[0]) 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 e93f40b7a1..85c29a7e2f 100644 --- a/cli/test_routing.go +++ b/cli/test_routing.go @@ -24,6 +24,8 @@ import ( "github.com/prometheus/alertmanager/api/v2/models" "github.com/prometheus/alertmanager/dispatch" + "github.com/prometheus/alertmanager/matchers/compat" + "github.com/prometheus/alertmanager/pkg/labels" ) const routingTestHelp = `Test alert routing @@ -80,9 +82,16 @@ func (c *routingShow) routingTestAction(ctx context.Context, _ *kingpin.ParseCon mainRoute := dispatch.NewRoute(cfg.Route, nil) // Parse labels to LabelSet. - ls, err := parseLabels(c.labels) - if err != nil { - kingpin.Fatalf("Failed to parse labels: %v\n", err) + ls := make(models.LabelSet, len(c.labels)) + for _, l := range c.labels { + matcher, err := compat.Matcher(l) + if err != nil { + kingpin.Fatalf("Failed to parse labels: %v\n", err) + } + if matcher.Type != labels.MatchEqual { + kingpin.Fatalf("%s\n", "Labels must be specified as key=value pairs") + } + ls[matcher.Name] = matcher.Value } if c.debugTree { diff --git a/cli/utils.go b/cli/utils.go index 3cb178fa6e..dfe39dd867 100644 --- a/cli/utils.go +++ b/cli/utils.go @@ -26,23 +26,9 @@ import ( "github.com/prometheus/alertmanager/api/v2/client/general" "github.com/prometheus/alertmanager/api/v2/models" "github.com/prometheus/alertmanager/config" - "github.com/prometheus/alertmanager/matchers/compat" "github.com/prometheus/alertmanager/pkg/labels" ) -// parseMatchers parses a list of matchers (cli arguments). -func parseMatchers(inputMatchers []string) ([]labels.Matcher, error) { - matchers := make([]labels.Matcher, 0, len(inputMatchers)) - for _, s := range inputMatchers { - matcher, err := compat.Matcher(s) - if err != nil { - return []labels.Matcher{}, err - } - matchers = append(matchers, *matcher) - } - return matchers, nil -} - // getRemoteAlertmanagerConfigStatus returns status responsecontaining configuration from remote Alertmanager func getRemoteAlertmanagerConfigStatus(ctx context.Context, alertmanagerURL *url.URL) (*models.AlertmanagerStatus, error) { amclient := NewAlertmanagerClient(alertmanagerURL) @@ -92,25 +78,6 @@ func convertClientToCommonLabelSet(cls models.LabelSet) model.LabelSet { return mls } -// parseLabels parses a list of labels (cli arguments). -func parseLabels(inputLabels []string) (models.LabelSet, error) { - labelSet := make(models.LabelSet, len(inputLabels)) - - for _, l := range inputLabels { - matcher, err := compat.Matcher(l) - if err != nil { - return models.LabelSet{}, err - } - if matcher.Type != labels.MatchEqual { - return models.LabelSet{}, errors.New("labels must be specified as key=value pairs") - } - - labelSet[matcher.Name] = matcher.Value - } - - return labelSet, nil -} - // TypeMatchers only valid for when you are going to add a silence func TypeMatchers(matchers []labels.Matcher) models.Matchers { typeMatchers := make(models.Matchers, len(matchers)) From ff1c27928e8641bc8a039853ede08590144db872 Mon Sep 17 00:00:00 2001 From: George Robinson Date: Wed, 11 Oct 2023 14:50:46 +0100 Subject: [PATCH 27/27] Fix tests Signed-off-by: George Robinson --- test/cli/acceptance/cli_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/cli/acceptance/cli_test.go b/test/cli/acceptance/cli_test.go index bc6af58599..93afe7884f 100644 --- a/test/cli/acceptance/cli_test.go +++ b/test/cli/acceptance/cli_test.go @@ -269,5 +269,5 @@ receivers: // Bad labels should return error out, err := am.TestRoute("{foo=bar}") require.EqualError(t, err, "exit status 1") - require.Equal(t, "amtool: error: Failed to parse labels: bad matcher format: {foo=bar}\n\n", string(out)) + require.Equal(t, "amtool: error: Failed to parse labels: unexpected open or close brace: {foo=bar}\n\n", string(out)) }