Skip to content

Conversation

SungJin1212
Copy link
Member

@SungJin1212 SungJin1212 commented Oct 15, 2025

Add a change log for #7040 and a test case for UTF8 querying

Which issue(s) this PR fixes:
Fixes #

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

Signed-off-by: SungJin1212 <tjdwls1201@gmail.com>
@SungJin1212 SungJin1212 force-pushed the Add-changelog-for-utf8-support branch from c9503b1 to 7339ddb Compare October 16, 2025 02:33
Signed-off-by: SungJin1212 <tjdwls1201@gmail.com>
@SungJin1212 SungJin1212 force-pushed the Add-changelog-for-utf8-support branch from 7339ddb to 055e6ee Compare October 16, 2025 02:42
require.Equal(t, 200, res.StatusCode)
require.Equal(t, model.ValVector, queryResult.Type())
vec := queryResult.(model.Vector)
require.Equal(t, 1, len(vec))
Copy link
Contributor

Choose a reason for hiding this comment

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

We should try to cover label names and label values API as well. But we can do it in next PR

Copy link
Member Author

@SungJin1212 SungJin1212 Oct 16, 2025

Choose a reason for hiding this comment

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

Should we remove https://github.com/cortexproject/cortex/blob/master/pkg/cortex/configinit/init.go as we start accepting UTF-8?

I've noticed the labelValues api with UTF-8 label name fails since we currently set the legacy scheme as default.

Copy link
Contributor

Choose a reason for hiding this comment

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

We should. I think we might be missing some test cases so we didn't find this in previous PR. Any components that do label/metric validation needs to accept the validation scheme otherwise you modify the global validation scheme variable

Copy link
Member Author

Choose a reason for hiding this comment

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

I removed it and added label names/values e2e test case.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we are missing more test cases like alertmanager and even ruler configurations with UTF 8 labels.

Signed-off-by: SungJin1212 <tjdwls1201@gmail.com>
@pull-request-size pull-request-size bot added size/M and removed size/S labels Oct 17, 2025
Signed-off-by: SungJin1212 <tjdwls1201@gmail.com>
@pull-request-size pull-request-size bot added size/L and removed size/M labels Oct 17, 2025
Signed-off-by: SungJin1212 <tjdwls1201@gmail.com>
@SungJin1212 SungJin1212 force-pushed the Add-changelog-for-utf8-support branch from 21d3334 to 3a1c61e Compare October 17, 2025 10:59
Signed-off-by: SungJin1212 <tjdwls1201@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants