From ac1267c909d80dc4e67056ba5c0fd834aaec5ca9 Mon Sep 17 00:00:00 2001 From: Julius Hinze Date: Tue, 26 Aug 2025 12:47:19 +0200 Subject: [PATCH] validation/limits: remove name ValidationSchemeValue in favor of prometheus/common/model.ValidationScheme --- pkg/distributor/otel_test.go | 4 +- pkg/distributor/validate_test.go | 4 +- pkg/util/validation/limits.go | 10 +- pkg/util/validation/limits_test.go | 54 +++---- .../validation/validation_scheme_value.go | 79 ---------- .../validation_scheme_value_test.go | 147 ------------------ 6 files changed, 36 insertions(+), 262 deletions(-) delete mode 100644 pkg/util/validation/validation_scheme_value.go delete mode 100644 pkg/util/validation/validation_scheme_value_test.go diff --git a/pkg/distributor/otel_test.go b/pkg/distributor/otel_test.go index 8056f483742..3cfbdc2938e 100644 --- a/pkg/distributor/otel_test.go +++ b/pkg/distributor/otel_test.go @@ -1290,7 +1290,7 @@ func TestHandlerOTLPPush(t *testing.T) { testLimits := &validation.Limits{ PromoteOTelResourceAttributes: tt.promoteResourceAttributes, - NameValidationScheme: validation.ValidationSchemeValue(model.LegacyValidation), + NameValidationScheme: model.LegacyValidation, OTelMetricSuffixesEnabled: false, } limits := validation.NewOverrides( @@ -1381,7 +1381,7 @@ func TestHandler_otlpDroppedMetricsPanic(t *testing.T) { limits := validation.NewOverrides( validation.Limits{ - NameValidationScheme: validation.ValidationSchemeValue(model.LegacyValidation), + NameValidationScheme: model.LegacyValidation, OTelTranslationStrategy: validation.OTelTranslationStrategyValue(otlptranslator.UnderscoreEscapingWithoutSuffixes), }, validation.NewMockTenantLimits(map[string]*validation.Limits{}), diff --git a/pkg/distributor/validate_test.go b/pkg/distributor/validate_test.go index 8a7d39d4a50..d5c8e81932a 100644 --- a/pkg/distributor/validate_test.go +++ b/pkg/distributor/validate_test.go @@ -100,8 +100,8 @@ func TestValidateLabels(t *testing.T) { limits := *limits perTenant[userID] = &limits } - perTenant[defaultUserID].NameValidationScheme = validation.ValidationSchemeValue(model.LegacyValidation) - perTenant[utf8UserID].NameValidationScheme = validation.ValidationSchemeValue(model.UTF8Validation) + perTenant[defaultUserID].NameValidationScheme = model.LegacyValidation + perTenant[utf8UserID].NameValidationScheme = model.UTF8Validation overrides := func(limits *validation.Limits) *validation.Overrides { return testutils.NewMockCostAttributionOverrides(*limits, perTenant, 0, diff --git a/pkg/util/validation/limits.go b/pkg/util/validation/limits.go index 63138315f3d..c41c964cf30 100644 --- a/pkg/util/validation/limits.go +++ b/pkg/util/validation/limits.go @@ -303,7 +303,7 @@ type Limits struct { IngestionPartitionsTenantShardSize int `yaml:"ingestion_partitions_tenant_shard_size" json:"ingestion_partitions_tenant_shard_size" category:"experimental"` // NameValidationScheme is the validation scheme for metric and label names. - NameValidationScheme ValidationSchemeValue `yaml:"name_validation_scheme" json:"name_validation_scheme" category:"experimental"` + NameValidationScheme model.ValidationScheme `yaml:"name_validation_scheme" json:"name_validation_scheme" category:"experimental"` extensions map[string]interface{} } @@ -568,15 +568,15 @@ func (l *Limits) MarshalYAML() (interface{}, error) { // Validate the Limits. func (l *Limits) Validate() error { - switch model.ValidationScheme(l.NameValidationScheme) { + switch l.NameValidationScheme { case model.UTF8Validation, model.LegacyValidation: case model.UnsetValidation: - l.NameValidationScheme = ValidationSchemeValue(model.LegacyValidation) + l.NameValidationScheme = model.LegacyValidation default: return fmt.Errorf("unrecognized name validation scheme: %s", l.NameValidationScheme) } - validationScheme := model.ValidationScheme(l.NameValidationScheme) + validationScheme := l.NameValidationScheme switch otlptranslator.TranslationStrategyOption(l.OTelTranslationStrategy) { case otlptranslator.UnderscoreEscapingWithoutSuffixes: if validationScheme != model.LegacyValidation { @@ -1518,7 +1518,7 @@ func (o *Overrides) LabelsQueryOptimizerEnabled(userID string) bool { // NameValidationScheme returns the name validation scheme to use for a particular tenant. func (o *Overrides) NameValidationScheme(userID string) model.ValidationScheme { - return model.ValidationScheme(o.getOverridesForUser(userID).NameValidationScheme) + return o.getOverridesForUser(userID).NameValidationScheme } // CardinalityAnalysisMaxResults returns the maximum number of results that diff --git a/pkg/util/validation/limits_test.go b/pkg/util/validation/limits_test.go index 532c8690e33..7b8f369ba21 100644 --- a/pkg/util/validation/limits_test.go +++ b/pkg/util/validation/limits_test.go @@ -73,7 +73,7 @@ func TestLimitsLoadingFromYaml(t *testing.T) { input: `{}`, testFunc: func(t *testing.T, l Limits) { assert.Equal(t, 1024, l.MaxLabelNameLength) - assert.Equal(t, ValidationSchemeValue(model.LegacyValidation), l.NameValidationScheme) + assert.Equal(t, model.LegacyValidation, l.NameValidationScheme) }, }, { @@ -87,14 +87,14 @@ func TestLimitsLoadingFromYaml(t *testing.T) { name: "name_validation_scheme: legacy", input: `name_validation_scheme: "legacy"`, testFunc: func(t *testing.T, l Limits) { - assert.Equal(t, ValidationSchemeValue(model.LegacyValidation), l.NameValidationScheme) + assert.Equal(t, model.LegacyValidation, l.NameValidationScheme) }, }, { name: "name_validation_scheme: utf8", input: `name_validation_scheme: "utf8"`, testFunc: func(t *testing.T, l Limits) { - assert.Equal(t, ValidationSchemeValue(model.UTF8Validation), l.NameValidationScheme) + assert.Equal(t, model.UTF8Validation, l.NameValidationScheme) }, }, } @@ -120,7 +120,7 @@ func TestLimitsLoadingFromJson(t *testing.T) { input: `{}`, testFunc: func(t *testing.T, l Limits) { assert.Equal(t, 1024, l.MaxLabelNameLength) - assert.Equal(t, ValidationSchemeValue(model.LegacyValidation), l.NameValidationScheme) + assert.Equal(t, model.LegacyValidation, l.NameValidationScheme) }, }, { @@ -134,7 +134,7 @@ func TestLimitsLoadingFromJson(t *testing.T) { name: "name_validation_scheme: utf8", input: `{"name_validation_scheme": "utf8"}`, testFunc: func(t *testing.T, l Limits) { - assert.Equal(t, ValidationSchemeValue(model.UTF8Validation), l.NameValidationScheme) + assert.Equal(t, model.UTF8Validation, l.NameValidationScheme) }, }, } @@ -1589,7 +1589,7 @@ func TestLimits_Validate(t *testing.T) { cfg: func() Limits { cfg := Limits{} flagext.DefaultValues(&cfg) - cfg.NameValidationScheme = ValidationSchemeValue(model.LegacyValidation) + cfg.NameValidationScheme = model.LegacyValidation cfg.OTelMetricSuffixesEnabled = false cfg.OTelTranslationStrategy = OTelTranslationStrategyValue(otlptranslator.UnderscoreEscapingWithoutSuffixes) return cfg @@ -1600,7 +1600,7 @@ func TestLimits_Validate(t *testing.T) { cfg: func() Limits { cfg := Limits{} flagext.DefaultValues(&cfg) - cfg.NameValidationScheme = ValidationSchemeValue(model.LegacyValidation) + cfg.NameValidationScheme = model.LegacyValidation cfg.OTelMetricSuffixesEnabled = true cfg.OTelTranslationStrategy = OTelTranslationStrategyValue(otlptranslator.UnderscoreEscapingWithSuffixes) return cfg @@ -1611,7 +1611,7 @@ func TestLimits_Validate(t *testing.T) { cfg: func() Limits { cfg := Limits{} flagext.DefaultValues(&cfg) - cfg.NameValidationScheme = ValidationSchemeValue(model.UTF8Validation) + cfg.NameValidationScheme = model.UTF8Validation cfg.OTelMetricSuffixesEnabled = true cfg.OTelTranslationStrategy = OTelTranslationStrategyValue(otlptranslator.NoUTF8EscapingWithSuffixes) return cfg @@ -1622,7 +1622,7 @@ func TestLimits_Validate(t *testing.T) { cfg: func() Limits { cfg := Limits{} flagext.DefaultValues(&cfg) - cfg.NameValidationScheme = ValidationSchemeValue(model.UTF8Validation) + cfg.NameValidationScheme = model.UTF8Validation cfg.OTelMetricSuffixesEnabled = false cfg.OTelTranslationStrategy = OTelTranslationStrategyValue(otlptranslator.NoTranslation) return cfg @@ -1633,7 +1633,7 @@ func TestLimits_Validate(t *testing.T) { cfg: func() Limits { cfg := Limits{} flagext.DefaultValues(&cfg) - cfg.NameValidationScheme = ValidationSchemeValue(model.LegacyValidation) + cfg.NameValidationScheme = model.LegacyValidation cfg.OTelMetricSuffixesEnabled = false cfg.OTelTranslationStrategy = OTelTranslationStrategyValue("") return cfg @@ -1648,7 +1648,7 @@ func TestLimits_Validate(t *testing.T) { cfg: func() Limits { cfg := Limits{} flagext.DefaultValues(&cfg) - cfg.NameValidationScheme = ValidationSchemeValue(model.LegacyValidation) + cfg.NameValidationScheme = model.LegacyValidation cfg.OTelMetricSuffixesEnabled = true cfg.OTelTranslationStrategy = OTelTranslationStrategyValue("") return cfg @@ -1663,7 +1663,7 @@ func TestLimits_Validate(t *testing.T) { cfg: func() Limits { cfg := Limits{} flagext.DefaultValues(&cfg) - cfg.NameValidationScheme = ValidationSchemeValue(model.UTF8Validation) + cfg.NameValidationScheme = model.UTF8Validation cfg.OTelMetricSuffixesEnabled = true cfg.OTelTranslationStrategy = OTelTranslationStrategyValue("") return cfg @@ -1678,7 +1678,7 @@ func TestLimits_Validate(t *testing.T) { cfg: func() Limits { cfg := Limits{} flagext.DefaultValues(&cfg) - cfg.NameValidationScheme = ValidationSchemeValue(model.UTF8Validation) + cfg.NameValidationScheme = model.UTF8Validation cfg.OTelMetricSuffixesEnabled = false cfg.OTelTranslationStrategy = OTelTranslationStrategyValue("") return cfg @@ -1693,7 +1693,7 @@ func TestLimits_Validate(t *testing.T) { cfg: func() Limits { cfg := Limits{} flagext.DefaultValues(&cfg) - cfg.NameValidationScheme = ValidationSchemeValue(model.UTF8Validation) + cfg.NameValidationScheme = model.UTF8Validation cfg.OTelMetricSuffixesEnabled = false cfg.OTelTranslationStrategy = OTelTranslationStrategyValue(otlptranslator.UnderscoreEscapingWithoutSuffixes) return cfg @@ -1704,7 +1704,7 @@ func TestLimits_Validate(t *testing.T) { cfg: func() Limits { cfg := Limits{} flagext.DefaultValues(&cfg) - cfg.NameValidationScheme = ValidationSchemeValue(model.LegacyValidation) + cfg.NameValidationScheme = model.LegacyValidation cfg.OTelMetricSuffixesEnabled = true cfg.OTelTranslationStrategy = OTelTranslationStrategyValue(otlptranslator.UnderscoreEscapingWithoutSuffixes) return cfg @@ -1715,7 +1715,7 @@ func TestLimits_Validate(t *testing.T) { cfg: func() Limits { cfg := Limits{} flagext.DefaultValues(&cfg) - cfg.NameValidationScheme = ValidationSchemeValue(model.UTF8Validation) + cfg.NameValidationScheme = model.UTF8Validation cfg.OTelMetricSuffixesEnabled = true cfg.OTelTranslationStrategy = OTelTranslationStrategyValue(otlptranslator.UnderscoreEscapingWithSuffixes) return cfg @@ -1726,7 +1726,7 @@ func TestLimits_Validate(t *testing.T) { cfg: func() Limits { cfg := Limits{} flagext.DefaultValues(&cfg) - cfg.NameValidationScheme = ValidationSchemeValue(model.LegacyValidation) + cfg.NameValidationScheme = model.LegacyValidation cfg.OTelMetricSuffixesEnabled = false cfg.OTelTranslationStrategy = OTelTranslationStrategyValue(otlptranslator.UnderscoreEscapingWithSuffixes) return cfg @@ -1737,7 +1737,7 @@ func TestLimits_Validate(t *testing.T) { cfg: func() Limits { cfg := Limits{} flagext.DefaultValues(&cfg) - cfg.NameValidationScheme = ValidationSchemeValue(model.LegacyValidation) + cfg.NameValidationScheme = model.LegacyValidation cfg.OTelMetricSuffixesEnabled = true cfg.OTelTranslationStrategy = OTelTranslationStrategyValue(otlptranslator.NoUTF8EscapingWithSuffixes) return cfg @@ -1748,7 +1748,7 @@ func TestLimits_Validate(t *testing.T) { cfg: func() Limits { cfg := Limits{} flagext.DefaultValues(&cfg) - cfg.NameValidationScheme = ValidationSchemeValue(model.UTF8Validation) + cfg.NameValidationScheme = model.UTF8Validation cfg.OTelMetricSuffixesEnabled = false cfg.OTelTranslationStrategy = OTelTranslationStrategyValue(otlptranslator.NoUTF8EscapingWithSuffixes) return cfg @@ -1759,7 +1759,7 @@ func TestLimits_Validate(t *testing.T) { cfg: func() Limits { cfg := Limits{} flagext.DefaultValues(&cfg) - cfg.NameValidationScheme = ValidationSchemeValue(model.LegacyValidation) + cfg.NameValidationScheme = model.LegacyValidation cfg.OTelMetricSuffixesEnabled = false cfg.OTelTranslationStrategy = OTelTranslationStrategyValue(otlptranslator.NoTranslation) return cfg @@ -1770,7 +1770,7 @@ func TestLimits_Validate(t *testing.T) { cfg: func() Limits { cfg := Limits{} flagext.DefaultValues(&cfg) - cfg.NameValidationScheme = ValidationSchemeValue(model.UTF8Validation) + cfg.NameValidationScheme = model.UTF8Validation cfg.OTelMetricSuffixesEnabled = true cfg.OTelTranslationStrategy = OTelTranslationStrategyValue(otlptranslator.NoTranslation) return cfg @@ -2180,7 +2180,7 @@ func TestOverrides_OTelTranslationStrategy(t *testing.T) { limits: map[string]*Limits{ "tenant1": { OTelTranslationStrategy: OTelTranslationStrategyValue(otlptranslator.UnderscoreEscapingWithSuffixes), - NameValidationScheme: ValidationSchemeValue(model.UTF8Validation), + NameValidationScheme: model.UTF8Validation, OTelMetricSuffixesEnabled: false, }, }, @@ -2192,7 +2192,7 @@ func TestOverrides_OTelTranslationStrategy(t *testing.T) { limits: map[string]*Limits{ "tenant1": { OTelTranslationStrategy: OTelTranslationStrategyValue(""), - NameValidationScheme: ValidationSchemeValue(model.LegacyValidation), + NameValidationScheme: model.LegacyValidation, OTelMetricSuffixesEnabled: true, }, }, @@ -2204,7 +2204,7 @@ func TestOverrides_OTelTranslationStrategy(t *testing.T) { limits: map[string]*Limits{ "tenant1": { OTelTranslationStrategy: OTelTranslationStrategyValue(""), - NameValidationScheme: ValidationSchemeValue(model.LegacyValidation), + NameValidationScheme: model.LegacyValidation, OTelMetricSuffixesEnabled: false, }, }, @@ -2216,7 +2216,7 @@ func TestOverrides_OTelTranslationStrategy(t *testing.T) { limits: map[string]*Limits{ "tenant1": { OTelTranslationStrategy: OTelTranslationStrategyValue(""), - NameValidationScheme: ValidationSchemeValue(model.UTF8Validation), + NameValidationScheme: model.UTF8Validation, OTelMetricSuffixesEnabled: true, }, }, @@ -2228,7 +2228,7 @@ func TestOverrides_OTelTranslationStrategy(t *testing.T) { limits: map[string]*Limits{ "tenant1": { OTelTranslationStrategy: OTelTranslationStrategyValue(""), - NameValidationScheme: ValidationSchemeValue(model.UTF8Validation), + NameValidationScheme: model.UTF8Validation, OTelMetricSuffixesEnabled: false, }, }, @@ -2257,7 +2257,7 @@ func TestOverrides_OTelTranslationStrategy(t *testing.T) { limits := map[string]*Limits{ "tenant1": { OTelTranslationStrategy: OTelTranslationStrategyValue(""), - NameValidationScheme: ValidationSchemeValue(999), // Invalid scheme + NameValidationScheme: model.ValidationScheme(999), // Invalid scheme OTelMetricSuffixesEnabled: true, }, } diff --git a/pkg/util/validation/validation_scheme_value.go b/pkg/util/validation/validation_scheme_value.go deleted file mode 100644 index 1df9fab9db5..00000000000 --- a/pkg/util/validation/validation_scheme_value.go +++ /dev/null @@ -1,79 +0,0 @@ -// SPDX-License-Identifier: AGPL-3.0-only - -package validation - -import ( - "encoding/json" - "fmt" - - "github.com/prometheus/common/model" - "github.com/spf13/pflag" - "gopkg.in/yaml.v3" -) - -var ( - _ interface { - yaml.Marshaler - yaml.Unmarshaler - json.Marshaler - json.Unmarshaler - pflag.Value - } = new(ValidationSchemeValue) -) - -// ValidationSchemeValue wraps model.ValidationScheme for use in limits. -type ValidationSchemeValue model.ValidationScheme - -func (s ValidationSchemeValue) MarshalYAML() (any, error) { - return model.ValidationScheme(s).MarshalYAML() -} - -func (s *ValidationSchemeValue) UnmarshalYAML(value *yaml.Node) error { - var repr model.ValidationScheme - if err := value.Decode(&repr); err != nil { - return err - } - *s = ValidationSchemeValue(repr) - return nil -} - -func (s ValidationSchemeValue) MarshalJSON() ([]byte, error) { - switch model.ValidationScheme(s) { - case model.UTF8Validation, model.LegacyValidation: - return json.Marshal(s.String()) - case model.UnsetValidation: - return json.Marshal("") - default: - return nil, fmt.Errorf("unrecognized name validation scheme: %s", s) - } -} - -func (s *ValidationSchemeValue) UnmarshalJSON(bytes []byte) error { - var repr string - if err := json.Unmarshal(bytes, &repr); err != nil { - return err - } - return s.Set(repr) -} - -func (s ValidationSchemeValue) String() string { - return model.ValidationScheme(s).String() -} - -func (s *ValidationSchemeValue) Set(text string) error { - switch text { - case "": - // Don't change value. - case model.LegacyValidation.String(): - *s = ValidationSchemeValue(model.LegacyValidation) - case model.UTF8Validation.String(): - *s = ValidationSchemeValue(model.UTF8Validation) - default: - return fmt.Errorf("unrecognized validation scheme %s", text) - } - return nil -} - -func (s ValidationSchemeValue) Type() string { - return "validationScheme" -} diff --git a/pkg/util/validation/validation_scheme_value_test.go b/pkg/util/validation/validation_scheme_value_test.go deleted file mode 100644 index bcb97ff3101..00000000000 --- a/pkg/util/validation/validation_scheme_value_test.go +++ /dev/null @@ -1,147 +0,0 @@ -// SPDX-License-Identifier: AGPL-3.0-only - -package validation - -import ( - "encoding/json" - "strings" - "testing" - - "github.com/prometheus/common/model" - "github.com/stretchr/testify/require" - "gopkg.in/yaml.v3" -) - -func TestValidationSchemeValue_UnmarshalYAML(t *testing.T) { - testCases := []struct { - name string - input string - want model.ValidationScheme - wantErr bool - }{ - { - name: "invalid", - input: `invalid`, - wantErr: true, - }, - { - name: "empty", - input: `""`, - want: model.UnsetValidation, - }, - { - name: "legacy validation", - input: `legacy`, - want: model.LegacyValidation, - }, - { - name: "utf8 validation", - input: `utf8`, - want: model.UTF8Validation, - }, - } - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - var got ValidationSchemeValue - err := yaml.Unmarshal([]byte(tc.input), &got) - if tc.wantErr { - require.Error(t, err) - return - } - require.NoError(t, err) - require.Equal(t, tc.want, model.ValidationScheme(got)) - - output, err := yaml.Marshal(got) - require.NoError(t, err) - require.Equal(t, tc.input, strings.TrimSpace(string(output))) - }) - } -} - -func TestValidationSchemeValue_UnmarshalJSON(t *testing.T) { - testCases := []struct { - name string - input string - want model.ValidationScheme - wantErr bool - }{ - { - name: "invalid", - input: `invalid`, - wantErr: true, - }, - { - name: "empty", - input: `""`, - want: model.UnsetValidation, - }, - { - name: "legacy validation", - input: `"legacy"`, - want: model.LegacyValidation, - }, - { - name: "utf8 validation", - input: `"utf8"`, - want: model.UTF8Validation, - }, - } - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - var got ValidationSchemeValue - err := json.Unmarshal([]byte(tc.input), &got) - if tc.wantErr { - require.Error(t, err) - return - } - require.NoError(t, err) - require.Equal(t, tc.want, model.ValidationScheme(got)) - - output, err := json.Marshal(got) - require.NoError(t, err) - require.Equal(t, tc.input, string(output)) - }) - } -} - -func TestValidationSchemeValue_Set(t *testing.T) { - testCases := []struct { - name string - input string - want model.ValidationScheme - wantErr bool - }{ - { - name: "invalid", - input: `invalid`, - wantErr: true, - }, - { - name: "empty", - input: ``, - want: model.UnsetValidation, - }, - { - name: "legacy validation", - input: `legacy`, - want: model.LegacyValidation, - }, - { - name: "utf8 validation", - input: `utf8`, - want: model.UTF8Validation, - }, - } - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - var got ValidationSchemeValue - err := got.Set(tc.input) - if tc.wantErr { - require.Error(t, err) - return - } - require.NoError(t, err) - require.Equal(t, tc.want, model.ValidationScheme(got)) - }) - } -}