-
Notifications
You must be signed in to change notification settings - Fork 343
model.ValidationScheme: Support encoding as YAML #799
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -24,6 +24,7 @@ import ( | |||||
|
|
||||||
| dto "github.com/prometheus/client_model/go" | ||||||
| "google.golang.org/protobuf/proto" | ||||||
| "gopkg.in/yaml.v2" | ||||||
| ) | ||||||
|
|
||||||
| var ( | ||||||
|
|
@@ -62,16 +63,70 @@ var ( | |||||
| type ValidationScheme int | ||||||
|
|
||||||
| const ( | ||||||
| // UnsetValidation represents an undefined ValidationScheme. | ||||||
| // Should not be used in practice. | ||||||
| UnsetValidation ValidationScheme = iota | ||||||
|
|
||||||
| // LegacyValidation is a setting that requires that all metric and label names | ||||||
| // conform to the original Prometheus character requirements described by | ||||||
| // MetricNameRE and LabelNameRE. | ||||||
| LegacyValidation ValidationScheme = iota | ||||||
| LegacyValidation | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did you check if the change of ordering here is going to cause any issues?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's not really a change of ordering is it, but a change of default, right? It's a breaking change if anyone depends on
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I see that now,. Well if you cant find a dependency in mimir or prom to the current default then I'm happy with this change.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks @jesusvazquez - I will try to look a bit.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see anything breaking in Prometheus/Mimir from this change at least. I also did some more testing, and found that YAML decoding depends on |
||||||
|
|
||||||
| // UTF8Validation only requires that metric and label names be valid UTF-8 | ||||||
| // strings. | ||||||
| UTF8Validation | ||||||
| ) | ||||||
|
|
||||||
| var ( | ||||||
| _ yaml.Marshaler = UnsetValidation | ||||||
| _ fmt.Stringer = UnsetValidation | ||||||
| ) | ||||||
|
|
||||||
| // String returns the string representation of s. | ||||||
| func (s ValidationScheme) String() string { | ||||||
| switch s { | ||||||
| case UnsetValidation: | ||||||
| return "unset" | ||||||
| case LegacyValidation: | ||||||
| return "legacy" | ||||||
| case UTF8Validation: | ||||||
| return "utf8" | ||||||
| default: | ||||||
| panic(fmt.Errorf("unhandled ValidationScheme: %d", s)) | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| // MarshalYAML implements the yaml.Marshaler interface. | ||||||
| func (s ValidationScheme) MarshalYAML() (any, error) { | ||||||
| switch s { | ||||||
|
aknuds1 marked this conversation as resolved.
|
||||||
| case UnsetValidation: | ||||||
| return "", nil | ||||||
| case LegacyValidation, UTF8Validation: | ||||||
| return s.String(), nil | ||||||
| default: | ||||||
| panic(fmt.Errorf("unhandled ValidationScheme: %d", s)) | ||||||
|
aknuds1 marked this conversation as resolved.
|
||||||
| } | ||||||
| } | ||||||
|
|
||||||
| // UnmarshalYAML implements the yaml.Unmarshaler interface. | ||||||
| func (s *ValidationScheme) UnmarshalYAML(unmarshal func(any) error) error { | ||||||
| var scheme string | ||||||
| if err := unmarshal(&scheme); err != nil { | ||||||
| return err | ||||||
| } | ||||||
| switch scheme { | ||||||
| case "": | ||||||
| // Don't change the value. | ||||||
| case "legacy": | ||||||
| *s = LegacyValidation | ||||||
| case "utf8": | ||||||
| *s = UTF8Validation | ||||||
| default: | ||||||
| return fmt.Errorf("unrecognized ValidationScheme: %q", scheme) | ||||||
| } | ||||||
| return nil | ||||||
| } | ||||||
|
|
||||||
| type EscapingScheme int | ||||||
|
|
||||||
| const ( | ||||||
|
|
@@ -185,7 +240,7 @@ func IsValidMetricName(n LabelValue) bool { | |||||
| } | ||||||
| return utf8.ValidString(string(n)) | ||||||
| default: | ||||||
| panic(fmt.Sprintf("Invalid name validation scheme requested: %d", NameValidationScheme)) | ||||||
| panic(fmt.Sprintf("Invalid name validation scheme requested: %s", NameValidationScheme.String())) | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: since we implement
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't understand the change to |
||||||
| } | ||||||
| } | ||||||
|
|
||||||
|
|
||||||
Uh oh!
There was an error while loading. Please reload this page.