-
Notifications
You must be signed in to change notification settings - Fork 9.8k
promql: Fix various UTF-8 bugs related to quoting #15531
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
0bf137e
to
4c0486c
Compare
d0d65ca
to
3a217d4
Compare
4b7581b
to
e6317ef
Compare
any ideas why my TestParseSeriesDesc test is failing on CI but not on my machine? I have Go 1.23.3 locally |
ok I can get it to fail with --tags=dedupelabels |
ok it's probably because label equality is not the right way to test |
e6317ef
to
2d74843
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some suggestions below.
Title needs expanding to cover all changes. (Or split into multiple PRs)
Why does the parser change?
the parser change is the second item -- quoted metric names were not supported in the "series description" path, which is separate from the vector selector path that had been updated. |
c2e47fe
to
5ddeefc
Compare
Fixes UTF-8 aggregator label list items getting mutated with quote marks when String-ified. Fixes quoted metric names not supported in metric declarations. Fixes UTF-8 label names not being quoted when String-ified. Fixes #15470 Fixes #15528 Signed-off-by: Owen Williams <owen.williams@grafana.com> Co-authored-by: Bryan Boreham <bjboreham@gmail.com>
5ddeefc
to
8d4bcd2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, LGTM
Fixes UTF-8 aggregator label list items getting mutated with quote marks when String-ified.
Fixes quoted metric names not supported in metric declarations.
Fixes UTF-8 label names not being quoted when String-ified.
Fixes #15470
Fixes #15528