Skip to content

Integration test: distributor/querier/ruler legacy name validation #11855

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

juliusmh
Copy link
Contributor

@juliusmh juliusmh commented Jun 25, 2025

What this PR does

Add integration test(s) that ensure distributor/ruler/querier use legacy name validation.

There are a couple of interesting things to see here:

  • ruler doesn't validate RuleGroup.Labels, only RuleGroup.Rules.Labels
  • ruler doesn't validate Rule.Expr
  • MQE and PromQL engine don't validate label matchers

Checklist

  • Tests updated.
  • Documentation added.
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]. If changelog entry is not needed, please add the changelog-not-needed label to the PR.
  • about-versioning.md updated with experimental features.

@juliusmh juliusmh requested a review from a team as a code owner June 25, 2025 09:57
@juliusmh juliusmh requested a review from aknuds1 June 25, 2025 09:57
@juliusmh juliusmh added the changelog-not-needed PRs that don't need a CHANGELOG.md entry label Jun 25, 2025
Copy link
Contributor

@dimitarvdimitrov dimitarvdimitrov left a comment

Choose a reason for hiding this comment

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

overall these make sense, but I'm wondering if a unit test wouldn't be a better fit for this. It can be run with much more inputs and takes much less time. There's already some unit tests for this kind of assertions in pkg/distributor/validate_test.go (TestDistributor_ValidateSeries), for the ruler in pkg/ruler/api_test.go (TestAPI_CreateRuleGroup) and the query-frontend in pkg/frontend/v2/frontend_test.go (TestFrontendEnqueueFailures)


// TestLegacyNameValidation_DistributorWrite ensures that distributors discard invalid
// metric and label names in the write path.
func TestLegacyNameValidation_DistributorWrite(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

this looks like a copied version of testDistributorCases. Did you try to reuse that?

}
}

func testLegacyNameValidationQuerier(t *testing.T, queryEngine string) {
Copy link
Contributor

Choose a reason for hiding this comment

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

it might be more valuable to test this on the query-frontend. That's the component which will first receive the queries.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog-not-needed PRs that don't need a CHANGELOG.md entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants