Support UTF-8 label matchers: Log an error if the parsers produce different parsings

[grobinson-grafana]

This commit updates the fallback parser to compare the parsings of both the UTF-8 matchers parser and the classic parser, and log an error if the parsers produce different parsings.

This can happen at present if escape sequences such as \n and \\ are used outside of double quotes, as these are not un-escaped in the UTF-8 matchers parser, leaving them as literal \n and \\.

The risk here is that without this change, a small number of Alertmanager user's routes might break on upgrade and without warning.

[grobinson-grafana]

@gotjosh undecided about what to do here. Do we support escape sequences outside of double quotes (a further relaxation of the grammar?) or do we want to warn users to add double quotes around their matchers that have escape sequences?

[gotjosh]

I assume that this means that there is a bug with the current parser as one of the design goals of compat was to make sure we don't introduce any sort of breaking change. Or does the fallback mechanism we have in place takes care of that?

If it is a breaking change, at what point do we update https://github.com/prometheus/alertmanager/blob/main/matchers/compat/parse_test.go with the scenario we're trying to cover?

[grobinson-grafana]

I assume that this means that there is a bug with the current parser

It's not that there is a bug as such, I had omitted openmetrics escape sequences outside double quotes on purpose. The issue is that when we relaxed the grammar to allow unquoted characters other than [a-zA-Z0-9]+, we also permitted escape sequences such as \n, \\, and \".

In the UTF-8 matchers parser, these are not un-escaped and kept as-is:

foo=bar\n

But in the classic parser, these are unescaped as it supports openmetrics:

foo=bar

That said, I'm not convinced we should support openmetrics escape sequences outside double quotes. One option is we make \ a reserved character which would mean foo=bar\n is invalid in the UTF-8 parser (see #3571).

as one of the design goals of compat was to make sure we don't introduce any sort of breaking change. Or does the fallback mechanism we have in place takes care of that?

The fallback mechanism we have ensures that old matchers that do not conform to the grammar still parse. It doesn't ensure there are no breaking changes if the input is valid in both parsers, but produces different parsings. That's what this change does though.

I think we can choose between adding openmetrics support and making \ a reserved character. I would like to keep this change though to detect possible other unforeseen edge cases.

[grobinson-grafana]

I'm going to close this as I'm not 100% sure it's needed.