Make metric/label name validation scheme explicit#16928
Make metric/label name validation scheme explicit#16928aknuds1 merged 4 commits intoprometheus:mainfrom
Conversation
acd7906 to
fda15c8
Compare
523d2c1 to
8e0fd04
Compare
8e0fd04 to
aed07da
Compare
aed07da to
ca33c43
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR makes metric and label name validation schemes explicit throughout the codebase instead of relying on the deprecated global github.com/prometheus/common/model.NameValidationScheme. The change replaces deprecated functions with explicit validation scheme parameters and updates the common library dependency.
Key changes:
- Replace deprecated validation functions with explicit
model.UTF8Validationcalls - Add
NameValidationSchemefields to configuration structs where needed - Update function signatures to accept explicit validation schemes as parameters
Reviewed Changes
Copilot reviewed 38 out of 39 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| go.mod | Updates prometheus/common dependency version |
| Multiple config files | Add NameValidationScheme: model.UTF8Validation to relabel configurations in tests |
| Multiple source files | Replace deprecated validation calls with explicit scheme-based validation |
| rulefmt package | Update Parse/ParseFile functions to accept validation scheme parameter |
| relabel package | Add validation scheme field and update validation logic |
| main.go files | Pass validation schemes to managers and parsers |
e4eb7fa to
52d97d7
Compare
52d97d7 to
a1177a4
Compare
2d3be80 to
b0c72b9
Compare
Parameterized metric/label name validation scheme Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
b0c72b9 to
099fc32
Compare
ywwg
left a comment
There was a problem hiding this comment.
This is good, and pretty clean! What worries me is the proliferation of validation and default-setting behavior that seems to be spread far and wide. I am wondering if there is a way to constrain that to specific places so that other people will not have to remember to do the right thing.
Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
e7fd34d to
66efe39
Compare
ywwg
left a comment
There was a problem hiding this comment.
I think the new util is helpful, thanks! For the yolostring discussion, resolve it however you like.
…d-name-validation Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
85317db to
839aea6
Compare
* Parameterize metric/label name validation scheme Parameterized metric/label name validation scheme --------- Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com> Co-authored-by: Julius Hinze <julius.hinze@grafana.com> Signed-off-by: perebaj <perebaj@gmail.com>
…a#6396) The Prometheus dependency upgrade from v0.304.2 to v0.307.3 (PR prometheus/prometheus#16928) moved label name validation from a global to a per-config NameValidationScheme field on relabel.Config. This field must be initialized by calling Validate() before use, otherwise relabeling panics with "Invalid name validation scheme requested: unset". Tempo constructs RemoteWriteConfig programmatically rather than going through Prometheus's config.Load() path, so Validate() was never called. Changes: - Call RemoteWriteConfig.Validate(model.UTF8Validation) in generateTenantRemoteWriteConfigs to initialize NameValidationScheme - Deep copy WriteRelabelConfigs before validation to avoid mutating shared input configs across concurrent watchOverrides goroutines - Move config generation before remote.NewStorage allocation in New() to avoid leaking resources on validation failure - Fix pre-existing bug in watchOverrides where cached state was updated before config was successfully applied, preventing retries on failure
…a#6396) The Prometheus dependency upgrade from v0.304.2 to v0.307.3 (PR prometheus/prometheus#16928) moved label name validation from a global to a per-config NameValidationScheme field on relabel.Config. This field must be initialized by calling Validate() before use, otherwise relabeling panics with "Invalid name validation scheme requested: unset". Tempo constructs RemoteWriteConfig programmatically rather than going through Prometheus's config.Load() path, so Validate() was never called. Changes: - Call RemoteWriteConfig.Validate(model.UTF8Validation) in generateTenantRemoteWriteConfigs to initialize NameValidationScheme - Deep copy WriteRelabelConfigs before validation to avoid mutating shared input configs across concurrent watchOverrides goroutines - Move config generation before remote.NewStorage allocation in New() to avoid leaking resources on validation failure - Fix pre-existing bug in watchOverrides where cached state was updated before config was successfully applied, preventing retries on failure
…mote write endpoints The Prometheus dependency upgrade from v0.304.2 to v0.307.3 (PR prometheus/prometheus#16928) moved label name validation from a global to a per-config NameValidationScheme field on relabel.Config. This field must be initialized by calling Validate() before use, otherwise relabeling panics with "Invalid name validation scheme requested: unset". Tempo constructs RemoteWriteConfig programmatically rather than going through Prometheus's config.Load() path, so Validate() was never called. Changes: - Call RemoteWriteConfig.Validate(model.UTF8Validation) in generateTenantRemoteWriteConfigs to initialize NameValidationScheme - Deep copy WriteRelabelConfigs before validation to avoid mutating shared input configs across concurrent watchOverrides goroutines - Move config generation before remote.NewStorage allocation in New() to avoid leaking resources on validation failure - Fix pre-existing bug in watchOverrides where cached state was updated before config was successfully applied, preventing retries on failure
…a#6396) The Prometheus dependency upgrade from v0.304.2 to v0.307.3 (PR prometheus/prometheus#16928) moved label name validation from a global to a per-config NameValidationScheme field on relabel.Config. This field must be initialized by calling Validate() before use, otherwise relabeling panics with "Invalid name validation scheme requested: unset". Tempo constructs RemoteWriteConfig programmatically rather than going through Prometheus's config.Load() path, so Validate() was never called. Call RemoteWriteConfig.Validate(model.UTF8Validation) once at startup in storage.Config.Validate(), which initializes NameValidationScheme on all write relabel configs before any tenant storage instances are created. Also fix a pre-existing bug in watchOverrides where cached state was updated before ApplyConfig succeeded, preventing retries on failure.
…#6399) The Prometheus dependency upgrade from v0.304.2 to v0.307.3 (PR prometheus/prometheus#16928) moved label name validation from a global to a per-config NameValidationScheme field on relabel.Config. This field must be initialized by calling Validate() before use, otherwise relabeling panics with "Invalid name validation scheme requested: unset". Tempo constructs RemoteWriteConfig programmatically rather than going through Prometheus's config.Load() path, so Validate() was never called. Call RemoteWriteConfig.Validate(model.UTF8Validation) once at startup in storage.Config.Validate(), which initializes NameValidationScheme on all write relabel configs before any tenant storage instances are created. Also fix a pre-existing bug in watchOverrides where cached state was updated before ApplyConfig succeeded, preventing retries on failure.
…#6399) The Prometheus dependency upgrade from v0.304.2 to v0.307.3 (PR prometheus/prometheus#16928) moved label name validation from a global to a per-config NameValidationScheme field on relabel.Config. This field must be initialized by calling Validate() before use, otherwise relabeling panics with "Invalid name validation scheme requested: unset". Tempo constructs RemoteWriteConfig programmatically rather than going through Prometheus's config.Load() path, so Validate() was never called. Call RemoteWriteConfig.Validate(model.UTF8Validation) once at startup in storage.Config.Validate(), which initializes NameValidationScheme on all write relabel configs before any tenant storage instances are created. Also fix a pre-existing bug in watchOverrides where cached state was updated before ApplyConfig succeeded, preventing retries on failure.
…#6399) (#6455) The Prometheus dependency upgrade from v0.304.2 to v0.307.3 (PR prometheus/prometheus#16928) moved label name validation from a global to a per-config NameValidationScheme field on relabel.Config. This field must be initialized by calling Validate() before use, otherwise relabeling panics with "Invalid name validation scheme requested: unset". Tempo constructs RemoteWriteConfig programmatically rather than going through Prometheus's config.Load() path, so Validate() was never called. Call RemoteWriteConfig.Validate(model.UTF8Validation) once at startup in storage.Config.Validate(), which initializes NameValidationScheme on all write relabel configs before any tenant storage instances are created. Also fix a pre-existing bug in watchOverrides where cached state was updated before ApplyConfig succeeded, preventing retries on failure.
… (#6399) The Prometheus dependency upgrade from v0.304.2 to v0.307.3 (PR prometheus/prometheus#16928) moved label name validation from a global to a per-config NameValidationScheme field on relabel.Config. This field must be initialized by calling Validate() before use, otherwise relabeling panics with "Invalid name validation scheme requested: unset". Tempo constructs RemoteWriteConfig programmatically rather than going through Prometheus's config.Load() path, so Validate() was never called. Call RemoteWriteConfig.Validate(model.UTF8Validation) once at startup in storage.Config.Validate(), which initializes NameValidationScheme on all write relabel configs before any tenant storage instances are created. Also fix a pre-existing bug in watchOverrides where cached state was updated before ApplyConfig succeeded, preventing retries on failure.
Make the metric/label name validation scheme explicit throughout the code base, instead of directly or indirectly using the deprecated
github.com/prometheus/common/model.NameValidationSchemeglobal. I'm updating to a version of github.com/prometheus/common that deprecates functions using the aforementioned global as well as legacy equivalents:model.LabelName.IsValidmodel.LabelName.IsValidLegacymodel.IsValidMetricNameIsValidLegacyMetricNameDeprecating the above forces us to use explicit equivalents instead.
Does this PR introduce a user-facing change?