Repository navigation
Conversation
Signed-off-by: Raphael Skuza <rapsku.dev@gmail.com>
| name_validation_scheme: utf8 | ||
| ``` | ||
|
|
||
| ### Impact on Cortex components |
There was a problem hiding this comment.
Since the Distributor is a sub-item, it would be better to increase to the ##.
| ### Impact on Cortex components | |
| ## Impact on Cortex components |
There was a problem hiding this comment.
Typo, thank you for pointing it out
|
@RaphSku |
| * [FEATURE] Ingester: Add experimental active series queried metric. #7173 | ||
| * [FEATURE] Update prometheus Alertmanager version to v0.31.1 and add new integration to IncidentIO and Mattermost. #7092 #7267 | ||
| * [FEATURE] Tenant Federation: Add experimental support for partial responses using the `-tenant-federation.allow-partial-data` flag. When enabled, failures from individual tenants during a federated query are treated as warnings, allowing results from successful tenants to be returned. #7232 | ||
| * [ENHANCEMENT] Docs: Guide on UTF-8 Support in Cortex #7091 |
There was a problem hiding this comment.
I don't think it's necessary to leave it in the changelog.
Signed-off-by: Raphael Skuza <rapsku.dev@gmail.com>
I've added the mentioned components to the guide, thank you for the review. If there is something in particular I should mention about either component, let me know. |
|
@RaphSku
Adding a few short YAML or PromQL snippets based on our tests would perfectly bridge the gap between the configuration and actual usage. |
…tilize UTF-8 support Signed-off-by: Raphael Skuza <rapsku.dev@gmail.com>
|
@SungJin1212 Thank you again for your comment, totally valid to have such a section and a good idea. I have included it in the latest commit. |
| series.1{"test.utf8.metric"="😄"} | ||
| ``` | ||
|
|
||
| Note, that the metric name `series.1` is not valid under the `legacy` validation scheme. It is accepted only when UTF-8 validation is enabled. This is useful when metric or label names contain characters, such as `.`, that are supported by UTF-8 validation but not by the legacy validation scheme. |
There was a problem hiding this comment.
| Note, that the metric name `series.1` is not valid under the `legacy` validation scheme. It is accepted only when UTF-8 validation is enabled. This is useful when metric or label names contain characters, such as `.`, that are supported by UTF-8 validation but not by the legacy validation scheme. | |
| Note that the metric name `series.1` is not valid under the `legacy` validation scheme. It is accepted only when UTF-8 validation is enabled. This is useful when metric or label names contain characters, such as `.`, that are supported by UTF-8 validation but not by the legacy validation scheme. |
Signed-off-by: Raphael Skuza <rapsku.dev@gmail.com>
|
@friedrichg friendly ping :) |
sandy2008
left a comment
There was a problem hiding this comment.
Thanks, the earlier review points are all in. I checked the examples and the component notes against the code: two of the snippets don't parse, and the Alertmanager and notifier paragraphs describe a different mechanism from what the code does. Details inline.
| PromQL queries support UTF-8 metric names and label names. For example, a UTF-8 metric name can be queried with the following expression: | ||
|
|
||
| ```promql | ||
| series.1{"test.utf8.metric"="😄"} |
There was a problem hiding this comment.
This doesn't parse: the vendored PromQL parser stops at .1 (1:7: parse error: unexpected number ".1"). A metric name that isn't a legacy identifier has to go inside the braces as a quoted string, which is also what integration/utf8_test.go queries:
| series.1{"test.utf8.metric"="😄"} | |
| {"series.1", "test.utf8.metric"="😄"} |
There was a problem hiding this comment.
You are totally right, my mistake.
|
|
||
| ```yaml | ||
| matchers: | ||
| - "🙂.utf8.label" |
There was a problem hiding this comment.
As written this isn't valid YAML (the list item is a bare string followed by value/isRegex keys); each matcher needs a name field, like the silence the integration test creates:
| - "🙂.utf8.label" | |
| - name: "🙂.utf8.label" |
There was a problem hiding this comment.
Yes, for some reason forgot to include name, thank you for pointing out.
|
|
||
| Ruler external labels are also validated with the selected scheme. With `legacy`, external label names must follow the legacy validation rules. With `utf8`, external label names that are valid under the UTF-8 validation scheme can be used. | ||
|
|
||
| The Ruler also passes the configured validation scheme to the notifier that sends alerts to Alertmanager. As a result, alert labels handled by the notification path are validated according to the selected scheme before alerts are sent. |
There was a problem hiding this comment.
I don't think this validates alert labels: the vendored notifier.NewManager only uses the scheme as the default for alert relabel configs that don't set their own, and the Cortex ruler doesn't configure any. I'd drop this paragraph.
There was a problem hiding this comment.
Dropped, now it should be correct since rule definitions are validated in /pkg/ruler/manager.go and external labels in /pkg/util/validation/limits.go. I hope I didn't miss anything else. Thank you for the information regarding notifier.NewManager.
|
|
||
| ### Alertmanager | ||
|
|
||
| The `name_validation_scheme` setting is not applied directly by Cortex [Alertmanager](https://cortexmetrics.io/docs/architecture/#alertmanager) in the same way it is applied by the Distributor or Ruler. Its effect on Alertmanager is indirect. Alerts sent by the Ruler pass through the Ruler notifier, which is created with the configured validation scheme. As a result, Alertmanager may receive alert labels that were validated according to the selected scheme. |
There was a problem hiding this comment.
This one is applied directly: initAlertManager maps name_validation_scheme to Alertmanager's matcher mode (legacy to classic-mode, utf8 to utf8-strict-mode) and calls compat.InitFromFlags, which decides how matchers in the Alertmanager configuration are parsed and how label names such as group_by entries are validated. So the Alertmanager examples above only work with utf8, and switching to utf8 moves existing Alertmanager configs to the UTF-8 matcher parser with no classic fallback, which seems worth a line in the rollout notes.
There was a problem hiding this comment.
You are right, thank you for pointing that out, rewritten that section and improved recommended rollout strategy section.
- Fixing PromQL expression since it doesn't parse. - Matchers missed "name" key and converted to JSON to better represent the API body. - Removed ruler paragraph claiming validating alert labels. - Correcting Alertmanager section since `name_validation_scheme` is directly mapped to Alertmanager's matcher modes. - Improving recommended rollout strategy. Signed-off-by: Raphael Skuza <rapsku.dev@gmail.com>
|
@sandy2008 Thank you for your detailed review, all very good points. I have tried to include them and improve/fix the guide. |
sandy2008
left a comment
There was a problem hiding this comment.
Re-checked at c4791e0 against the code. The PromQL example now parses with the vendored parser (the old series.1{...} form fails with unexpected number ".1"), the silence matcher has name/value/isRegex/isEqual and unmarshals into Alertmanager's PostableSilence, the notifier paragraph is gone, and the Ruler and Alertmanager sections match ValidateRuleGroup, the ruler_external_labels check in Limits.Validate and initAlertManager. I also loaded the examples: the route config loads in utf8-strict-mode and is rejected in classic-mode, a classic-only matcher like foo=bar baz fails in strict mode as the rollout note warns, and the rule group parses with utf8 and is rejected with legacy. Thanks for the rewrite.
| ## Recommended rollout strategy | ||
|
|
||
| 1. Inventory metric producers and consumers, Ruler rules, external labels, and Alertmanager configurations. Check whether any tools assume legacy metric and label names. | ||
| 2. Verify that existing Alertmanager route matchers parse in UTF-8 strict mode. Update ambiguous or incompatible matchers before switching modes. | ||
| 3. Enable `utf8` in a staging environment. Ingest and query representative metrics with non-legacy names, and exercise label name, label value, and series API endpoints. | ||
| 4. Load representative recording and alerting rules, including group and rule labels, and verify evaluation and notifications. | ||
| 5. Test Alertmanager configuration loading, grouping, routing, and silence creation. Roll out consistently across the Cortex components that need the selected mode. |
There was a problem hiding this comment.
One thing that might be worth covering: what happens when switching back from utf8 to legacy.
- Series already ingested with UTF-8 names stay in storage and remain queryable, but new writes with such names are rejected by the Distributor.
- Stored rule groups still load, but rule outputs with UTF-8 names are rejected when written, and rules with UTF-8 record names or rule labels can no longer be uploaded or updated through the Ruler API.
- Alertmanager configurations that rely on UTF-8 matchers or
group_bylabel names fail to load inclassic-mode, and alerts with UTF-8 label names are rejected. ruler_external_labelsandmetric_relabel_configswith UTF-8 label names fail limits validation: in the default limits Cortex refuses to start, and in per-tenant overrides the runtime config file is rejected.
Before switching back from utf8 to legacy, remove or rename UTF-8 metric and label names from Ruler rule groups, ruler_external_labels, metric_relabel_configs, and Alertmanager configurations.
Could you add a short "Rolling back to legacy" section with this after the rollout steps?
There was a problem hiding this comment.
Added to the guide. Thank you for the very good addition.
- Addressing Distributor, Ruler, Alertmanager and Limits validation that are affected by the potential rollback. Signed-off-by: Raphael Skuza <rapsku.dev@gmail.com>
|
@SungJin1212 Could you please rerun the failed workflow run? Seems to be flaky since the change is unrelated. Thank you. |
What this PR does:
Introduces a Guide on how to setup UTF-8 mode in Cortex
Which issue(s) this PR fixes:
Fixes #7091
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]