Skip to content

Document difference between tab and newline in tests#3383

Merged
gotjosh merged 1 commit intoprometheus:mainfrom
grobinson-grafana:grobinson/more-tests-parse-matchers
Jun 7, 2023
Merged

Document difference between tab and newline in tests#3383
gotjosh merged 1 commit intoprometheus:mainfrom
grobinson-grafana:grobinson/more-tests-parse-matchers

Conversation

@grobinson-grafana
Copy link
Collaborator

I found a possible bug (unexpected behaviour?) in the current parser for label matchers where \n is interpolated and \t is not. I've chosen to document this with a test while I figure out if this is intentional or not.

Related: #3353

@grobinson-grafana
Copy link
Collaborator Author

grobinson-grafana commented Jun 5, 2023

At least, from the PromQL documentation, I would expect \t and \n to behave the same:

"this is a string"
'these are unescaped: \n \\ \t'
`these are not unescaped: \n ' " \t`

Signed-off-by: George Robinson <george.robinson@grafana.com>
@grobinson-grafana grobinson-grafana force-pushed the grobinson/more-tests-parse-matchers branch from dd51bef to f8de618 Compare June 5, 2023 10:25
@gotjosh
Copy link
Member

gotjosh commented Jun 7, 2023

Tagging @beorn7 and @simonpasquier to see if they have any reckons. In the meantime, I'll merge this as tests that document existing behaviour are always a great thing to have.

@gotjosh gotjosh merged commit 9f683fc into prometheus:main Jun 7, 2023
@beorn7
Copy link
Member

beorn7 commented Jun 8, 2023

This is probably inspired by the behavior in the exposition format, in particular: “… the backslash (), double-quote ("), and line feed (\n) characters have to be escaped as \, ", and \n, respectively” (i.e. this is not using the usual Go string escaping, but a very specific limited one).

This is certainly a mixup as the matchers are supposed to be inspired by PromQL, not by the exposition format. So your expectation is spot-on, the matchers should follow escaping rules of PromQL wherever reasonably possible.

gotjosh pushed a commit that referenced this pull request Aug 7, 2023
Signed-off-by: George Robinson <george.robinson@grafana.com>
radek-ryckowski pushed a commit to goldmansachs/alertmanager that referenced this pull request Nov 6, 2023
Signed-off-by: George Robinson <george.robinson@grafana.com>
qinxx108 pushed a commit to amazon-contributing/alertmanager that referenced this pull request Mar 28, 2024
Signed-off-by: George Robinson <george.robinson@grafana.com>
@grobinson-grafana grobinson-grafana deleted the grobinson/more-tests-parse-matchers branch April 16, 2024 14:44
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.

3 participants