Skip to content

Conversation

ywwg
Copy link
Member

@ywwg ywwg commented Dec 3, 2024

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

@ywwg ywwg force-pushed the owilliams/promqltest branch 3 times, most recently from 0bf137e to 4c0486c Compare December 3, 2024 18:52
@ywwg ywwg marked this pull request as ready for review December 3, 2024 18:53
@ywwg ywwg requested a review from roidelapluie as a code owner December 3, 2024 18:53
@ywwg ywwg force-pushed the owilliams/promqltest branch from d0d65ca to 3a217d4 Compare December 3, 2024 20:05
@ywwg ywwg marked this pull request as draft December 3, 2024 20:34
@ywwg ywwg force-pushed the owilliams/promqltest branch 2 times, most recently from 4b7581b to e6317ef Compare December 3, 2024 21:01
@ywwg
Copy link
Member Author

ywwg commented Dec 3, 2024

any ideas why my TestParseSeriesDesc test is failing on CI but not on my machine? I have Go 1.23.3 locally

@ywwg ywwg marked this pull request as ready for review December 3, 2024 21:11
@ywwg
Copy link
Member Author

ywwg commented Dec 3, 2024

ok I can get it to fail with --tags=dedupelabels

@ywwg
Copy link
Member Author

ywwg commented Dec 3, 2024

ok it's probably because label equality is not the right way to test

@ywwg ywwg force-pushed the owilliams/promqltest branch from e6317ef to 2d74843 Compare December 3, 2024 21:25
@ywwg ywwg requested a review from bboreham December 3, 2024 21:32
Copy link
Member

@bboreham bboreham left a 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?

@ywwg ywwg changed the title promql: Fix subtle bug where label lists could be mutated when printed promql: Fix various UTF-8 bugs related to quoting Dec 4, 2024
@ywwg
Copy link
Member Author

ywwg commented Dec 4, 2024

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.

@ywwg ywwg force-pushed the owilliams/promqltest branch 4 times, most recently from c2e47fe to 5ddeefc Compare December 4, 2024 15:56
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>
@ywwg ywwg force-pushed the owilliams/promqltest branch from 5ddeefc to 8d4bcd2 Compare December 4, 2024 19:19
@ywwg ywwg requested a review from bboreham December 5, 2024 14:59
@ywwg ywwg requested a review from beorn7 December 6, 2024 16:24
Copy link
Member

@bboreham bboreham left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants