-
Notifications
You must be signed in to change notification settings - Fork 11
remove global name validation #892
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
f9c5913 to
e0e77cf
Compare
e0e77cf to
8092852
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.
Instead of pkg/util/validation.IsValidLabelName and pkg/util/validation.IsValidMetricName, I think a more forward looking solution would be to implement a method pkg/util/validation.Overrides.NameValidationScheme(tenantID string) validation.NamingScheme, and use that wherever you need to validate metric or label names. It can hard-code the legacy scheme, so we don't make it configurable at first. If we have such a solution, we can easily implement the next step, which is per-tenant configurability of metric/label name validation scheme.
31aff3b to
2ceae43
Compare
af1fe5f to
0f47658
Compare
aknuds1
left a comment
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.
It doesn't seem right to me to essentially duplicate prometheus/common/model.ValidationScheme via model/validation.NamingScheme. It's going to be bad to maintain, and a potential footgun. Please see my alternative suggestion :)
0f47658 to
ef1d8b1
Compare
…ng and query evaluation
…t, textparse, labels.
ef1d8b1 to
c0349fd
Compare
|
Superseded by #946 |
This PR introduces explicit APIs to set validation scheme for various components of prometheus.
Uses / Depends on: