Skip to content

Make metric/label name validation scheme explicit#16928

Merged
aknuds1 merged 4 commits intoprometheus:mainfrom
aknuds1:arve/parameterized-name-validation
Aug 18, 2025
Merged

Make metric/label name validation scheme explicit#16928
aknuds1 merged 4 commits intoprometheus:mainfrom
aknuds1:arve/parameterized-name-validation

Conversation

@aknuds1
Copy link
Copy Markdown
Contributor

@aknuds1 aknuds1 commented Jul 25, 2025

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.NameValidationScheme global. 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.IsValid
  • model.LabelName.IsValidLegacy
  • model.IsValidMetricName
  • IsValidLegacyMetricName

Deprecating the above forces us to use explicit equivalents instead.

Does this PR introduce a user-facing change?

NONE

@aknuds1 aknuds1 force-pushed the arve/parameterized-name-validation branch 9 times, most recently from acd7906 to fda15c8 Compare July 28, 2025 06:23
@aknuds1 aknuds1 force-pushed the arve/parameterized-name-validation branch 7 times, most recently from 523d2c1 to 8e0fd04 Compare July 28, 2025 08:21
@aknuds1 aknuds1 changed the title Parameterize metric/label name validation scheme Make metric/label name validation scheme explit Jul 28, 2025
@aknuds1 aknuds1 force-pushed the arve/parameterized-name-validation branch from 8e0fd04 to aed07da Compare July 28, 2025 08:49
@aknuds1 aknuds1 requested a review from ywwg July 28, 2025 08:49
@aknuds1 aknuds1 force-pushed the arve/parameterized-name-validation branch from aed07da to ca33c43 Compare July 28, 2025 09:10
@aknuds1 aknuds1 requested a review from Copilot July 28, 2025 15:23
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

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.UTF8Validation calls
  • Add NameValidationScheme fields 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

Comment thread model/relabel/relabel.go
Comment thread storage/remote/codec.go
Comment thread storage/remote/codec.go
Comment thread prompb/io/prometheus/client/decoder.go Outdated
Comment thread model/textparse/protobufparse.go
@aknuds1 aknuds1 force-pushed the arve/parameterized-name-validation branch 2 times, most recently from e4eb7fa to 52d97d7 Compare July 31, 2025 07:35
@aknuds1 aknuds1 changed the title Make metric/label name validation scheme explit Make metric/label name validation scheme explicit Jul 31, 2025
@aknuds1 aknuds1 force-pushed the arve/parameterized-name-validation branch from 52d97d7 to a1177a4 Compare August 1, 2025 07:16
@aknuds1 aknuds1 marked this pull request as ready for review August 1, 2025 07:17
@aknuds1 aknuds1 force-pushed the arve/parameterized-name-validation branch 3 times, most recently from 2d3be80 to b0c72b9 Compare August 6, 2025 12:43
Parameterized metric/label name validation scheme

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
@aknuds1 aknuds1 force-pushed the arve/parameterized-name-validation branch from b0c72b9 to 099fc32 Compare August 11, 2025 13:06
Copy link
Copy Markdown
Member

@ywwg ywwg left a comment

Choose a reason for hiding this comment

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

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.

Comment thread model/relabel/relabel.go
Comment thread model/rulefmt/rulefmt.go Outdated
Comment thread prompb/io/prometheus/client/decoder.go
Comment thread rules/group.go Outdated
Comment thread scrape/scrape.go Outdated
Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
@aknuds1 aknuds1 force-pushed the arve/parameterized-name-validation branch from e7fd34d to 66efe39 Compare August 11, 2025 15:49
Copy link
Copy Markdown
Member

@ywwg ywwg left a comment

Choose a reason for hiding this comment

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

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>
@aknuds1 aknuds1 force-pushed the arve/parameterized-name-validation branch from 85317db to 839aea6 Compare August 18, 2025 07:38
@aknuds1 aknuds1 enabled auto-merge (squash) August 18, 2025 08:00
@aknuds1 aknuds1 merged commit 0a40df3 into prometheus:main Aug 18, 2025
45 of 49 checks passed
perebaj pushed a commit to perebaj/prometheus that referenced this pull request Aug 22, 2025
* 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>
@aknuds1 aknuds1 deleted the arve/parameterized-name-validation branch August 22, 2025 15:55
carles-grafana added a commit to carles-grafana/tempo that referenced this pull request Feb 6, 2026
…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
carles-grafana added a commit to carles-grafana/tempo that referenced this pull request Feb 6, 2026
…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
carles-grafana added a commit to carles-grafana/tempo that referenced this pull request Feb 6, 2026
…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
carles-grafana added a commit to carles-grafana/tempo that referenced this pull request Feb 6, 2026
…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.
carles-grafana added a commit to grafana/tempo that referenced this pull request Feb 6, 2026
…#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.
carles-grafana added a commit to grafana/tempo that referenced this pull request Feb 13, 2026
…#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.
carles-grafana added a commit to grafana/tempo that referenced this pull request Feb 16, 2026
…#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.
zeekay pushed a commit to hanzoai/traces that referenced this pull request Feb 22, 2026
… (#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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants