Skip to content

Fix unexpected behavior interpreting matchers in amtool#3386

Closed
grobinson-grafana wants to merge 1 commit intoprometheus:mainfrom
grobinson-grafana:grobinson/3385
Closed

Fix unexpected behavior interpreting matchers in amtool#3386
grobinson-grafana wants to merge 1 commit intoprometheus:mainfrom
grobinson-grafana:grobinson/3385

Conversation

@grobinson-grafana
Copy link
Collaborator

@grobinson-grafana grobinson-grafana commented Jun 6, 2023

This commit fixes some of the unexpected behavior when interpreting matchers in amtool.

Before this commit, <matcher-groups> that looked like a valid matcher such as {foo=bar} would have "alertname" prefixed as amtool considered it to be a label value. However, this would create a silence for alertname="{foo=bar}" which is almost never want the user intended.

Instead, this commit updates the heuristic to be more accurate in determining whether the input is a label value without a label name and operator or if it is a complete matcher that should be parsed from start to end.

Fixes #3385


"github.com/alecthomas/kingpin/v2"
"github.com/go-openapi/strfmt"
new_matchers "github.com/grobinson-grafana/matchers"
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've imported this package to demonstrate how this fix will work, however in the finished pull request it will use the code from #3353.

This commit fixes some of the unexpected behavior when interpreting
matchers in amtool.

Before this commit, <matcher-groups> that looked like a valid matcher
such as {foo=bar} would have "alertname" prefixed as amtool considered
it to be a label value. However, this would create a silence for
alertname="{foo=bar}" which is almost never want the user intended.

Instead, this commit updates the heuristic to be more accurate in
determining whether the input is a label value without a label name
and operator or if it is a complete matcher that should be parsed
from start to end.

Signed-off-by: George Robinson <george.robinson@grafana.com>
@grobinson-grafana
Copy link
Collaborator Author

Here is an example of what is fixed:

Example Before In this PR
foo bar bad matcher format: bar bad matcher format: bar 🔁
foo bar=baz alertname="foo", bar="baz" alertname="foo", bar="baz" 🔁
foo= bar=baz foo="", bar="baz" foo="", bar="baz" 🔁
=foo bar=baz alertname="=foo" bar="baz" bad matcher format: =foo
%foo bar=baz alertname="%foo" bar="baz" bad matcher format: %foo
\"=foo\" bar=baz alertname="=foo" bar="baz" alertname="=foo" bar="baz" 🔁
\"%foo\" bar=baz alertname="=foo" bar="baz" alertname="%foo" bar="baz" 🔁
foo=bar,bar=baz foo="bar,bar=baz" foo="bar,bar=baz" 🔁
{foo=bar} alertname="{foo=bar}" error: bad matcher format: {foo=bar}

🔁 means No change

@grobinson-grafana grobinson-grafana deleted the grobinson/3385 branch September 5, 2023 20:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

amtool silence add shows unexpected behaviour interpreting matchers

1 participant