From 6caa6ccca8f6aa689976669448e7947e02b982fb Mon Sep 17 00:00:00 2001 From: Arve Knudsen Date: Fri, 25 Jul 2025 17:37:19 +0200 Subject: [PATCH 1/5] Add ValidationScheme methods IsValidMetricName and IsValidLabelName Signed-off-by: Arve Knudsen --- expfmt/decode.go | 2 + expfmt/openmetrics_create.go | 2 +- expfmt/text_create.go | 4 +- model/labels.go | 29 ++++---------- model/labels_test.go | 43 ++++++++++++++------- model/metric.go | 75 ++++++++++++++++++++++++++---------- model/metric_test.go | 45 +++++++++++++++------- 7 files changed, 128 insertions(+), 72 deletions(-) diff --git a/expfmt/decode.go b/expfmt/decode.go index 1448439b7..24a24b53a 100644 --- a/expfmt/decode.go +++ b/expfmt/decode.go @@ -93,6 +93,7 @@ func (d *protoDecoder) Decode(v *dto.MetricFamily) error { if err := opts.UnmarshalFrom(d.r, v); err != nil { return err } + //nolint:staticcheck // model.IsValidMetricName is deprecated. if !model.IsValidMetricName(model.LabelValue(v.GetName())) { return fmt.Errorf("invalid metric name %q", v.GetName()) } @@ -107,6 +108,7 @@ func (d *protoDecoder) Decode(v *dto.MetricFamily) error { if !model.LabelValue(l.GetValue()).IsValid() { return fmt.Errorf("invalid label value %q", l.GetValue()) } + //nolint:staticcheck // model.LabelName.IsValid is deprecated. if !model.LabelName(l.GetName()).IsValid() { return fmt.Errorf("invalid label name %q", l.GetName()) } diff --git a/expfmt/openmetrics_create.go b/expfmt/openmetrics_create.go index a21ed4ec1..fed8253ca 100644 --- a/expfmt/openmetrics_create.go +++ b/expfmt/openmetrics_create.go @@ -477,7 +477,7 @@ func writeOpenMetricsNameAndLabelPairs( if name != "" { // If the name does not pass the legacy validity check, we must put the // metric name inside the braces, quoted. - if !model.IsValidLegacyMetricName(name) { + if !model.LegacyValidation.IsValidMetricName(name) { metricInsideBraces = true err := w.WriteByte(separator) written++ diff --git a/expfmt/text_create.go b/expfmt/text_create.go index 4b86434b3..e242b3720 100644 --- a/expfmt/text_create.go +++ b/expfmt/text_create.go @@ -354,7 +354,7 @@ func writeNameAndLabelPairs( if name != "" { // If the name does not pass the legacy validity check, we must put the // metric name inside the braces. - if !model.IsValidLegacyMetricName(name) { + if !model.LegacyValidation.IsValidMetricName(name) { metricInsideBraces = true err := w.WriteByte(separator) written++ @@ -498,7 +498,7 @@ func writeInt(w enhancedWriter, i int64) (int, error) { // writeName writes a string as-is if it complies with the legacy naming // scheme, or escapes it in double quotes if not. func writeName(w enhancedWriter, name string) (int, error) { - if model.IsValidLegacyMetricName(name) { + if model.LegacyValidation.IsValidMetricName(name) { return w.WriteString(name) } var written int diff --git a/model/labels.go b/model/labels.go index e2ff83595..dfeb34be5 100644 --- a/model/labels.go +++ b/model/labels.go @@ -106,34 +106,21 @@ type LabelName string // IsValid returns true iff the name matches the pattern of LabelNameRE when // NameValidationScheme is set to LegacyValidation, or valid UTF-8 if // NameValidationScheme is set to UTF8Validation. +// +// Deprecated: This method should not be used and may be removed in the future. +// Use [ValidationScheme.IsValidLabelName] instead. func (ln LabelName) IsValid() bool { - if len(ln) == 0 { - return false - } - switch NameValidationScheme { - case LegacyValidation: - return ln.IsValidLegacy() - case UTF8Validation: - return utf8.ValidString(string(ln)) - default: - panic(fmt.Sprintf("Invalid name validation scheme requested: %d", NameValidationScheme)) - } + return NameValidationScheme.IsValidLabelName(string(ln)) } // IsValidLegacy returns true iff name matches the pattern of LabelNameRE for // legacy names. It does not use LabelNameRE for the check but a much faster // hardcoded implementation. +// +// Deprecated: This method should not be used and may be removed in the future. +// Use [LegacyValidation.IsValidLabelName] instead. func (ln LabelName) IsValidLegacy() bool { - if len(ln) == 0 { - return false - } - for i, b := range ln { - // TODO: Apply De Morgan's law. Make sure there are tests for this. - if !((b >= 'a' && b <= 'z') || (b >= 'A' && b <= 'Z') || b == '_' || (b >= '0' && b <= '9' && i > 0)) { //nolint:staticcheck - return false - } - } - return true + return LegacyValidation.IsValidLabelName(string(ln)) } // UnmarshalYAML implements the yaml.Unmarshaler interface. diff --git a/model/labels_test.go b/model/labels_test.go index 233954326..441c138a3 100644 --- a/model/labels_test.go +++ b/model/labels_test.go @@ -14,6 +14,7 @@ package model import ( + "fmt" "sort" "testing" ) @@ -90,9 +91,9 @@ func BenchmarkLabelValues(b *testing.B) { } } -func TestLabelNameIsValid(t *testing.T) { +func TestValidationScheme_IsLabelNameValid(t *testing.T) { scenarios := []struct { - ln LabelName + ln string legacyValid bool utf8Valid bool }{ @@ -142,19 +143,33 @@ func TestLabelNameIsValid(t *testing.T) { utf8Valid: false, }, } - for _, s := range scenarios { - NameValidationScheme = LegacyValidation - if s.ln.IsValid() != s.legacyValid { - t.Errorf("Expected %v for %q using legacy IsValid method", s.legacyValid, s.ln) - } - if LabelNameRE.MatchString(string(s.ln)) != s.legacyValid { - t.Errorf("Expected %v for %q using legacy regexp match", s.legacyValid, s.ln) - } - NameValidationScheme = UTF8Validation - if s.ln.IsValid() != s.utf8Valid { - t.Errorf("Expected %v for %q using UTF-8 IsValid method", s.legacyValid, s.ln) - } + t.Run(fmt.Sprintf("%s,%t,%t", s.ln, s.legacyValid, s.utf8Valid), func(t *testing.T) { + if LegacyValidation.IsValidLabelName(s.ln) != s.legacyValid { + t.Errorf("Expected %v for %q using LegacyValidation.IsValidLabelName", s.legacyValid, s.ln) + } + if LabelNameRE.MatchString(s.ln) != s.legacyValid { + t.Errorf("Expected %v for %q using legacy regexp match", s.legacyValid, s.ln) + } + if UTF8Validation.IsValidLabelName(s.ln) != s.utf8Valid { + t.Errorf("Expected %v for %q using UTF8Validation.IsValidLabelName", s.utf8Valid, s.ln) + } + + // Test deprecated functions. + origScheme := NameValidationScheme + t.Cleanup(func() { + NameValidationScheme = origScheme + }) + NameValidationScheme = LegacyValidation + labelName := LabelName(s.ln) + if labelName.IsValid() != s.legacyValid { + t.Errorf("Expected %v for %q using legacy IsValid method", s.legacyValid, s.ln) + } + NameValidationScheme = UTF8Validation + if labelName.IsValid() != s.utf8Valid { + t.Errorf("Expected %v for %q using UTF-8 IsValid method", s.legacyValid, s.ln) + } + }) } } diff --git a/model/metric.go b/model/metric.go index 2bd913fff..55d6a5847 100644 --- a/model/metric.go +++ b/model/metric.go @@ -127,6 +127,53 @@ func (s *ValidationScheme) UnmarshalYAML(unmarshal func(any) error) error { return nil } +// IsValidMetricName returns whether metricName is valid according to s. +func (s ValidationScheme) IsValidMetricName(metricName string) bool { + switch s { + case LegacyValidation: + if len(metricName) == 0 { + return false + } + for i, b := range metricName { + if !isValidLegacyRune(b, i) { + return false + } + } + return true + case UTF8Validation: + if len(metricName) == 0 { + return false + } + return utf8.ValidString(string(metricName)) + default: + panic(fmt.Sprintf("Invalid metricName validation scheme requested: %s", s.String())) + } +} + +// IsValidLabelName returns whether labelName is valid according to s. +func (s ValidationScheme) IsValidLabelName(labelName string) bool { + if len(labelName) == 0 { + return false + } + switch s { + case LegacyValidation: + if len(labelName) == 0 { + return false + } + for i, b := range labelName { + // TODO: Apply De Morgan's law. Make sure there are tests for this. + if !((b >= 'a' && b <= 'z') || (b >= 'A' && b <= 'Z') || b == '_' || (b >= '0' && b <= '9' && i > 0)) { //nolint:staticcheck + return false + } + } + return true + case UTF8Validation: + return utf8.ValidString(string(labelName)) + default: + panic(fmt.Sprintf("Invalid name validation scheme requested: %s", s)) + } +} + type EscapingScheme int const ( @@ -230,34 +277,22 @@ func (m Metric) FastFingerprint() Fingerprint { // IsValidMetricName returns true iff name matches the pattern of MetricNameRE // for legacy names, and iff it's valid UTF-8 if the UTF8Validation scheme is // selected. +// +// Deprecated: This function should not be used and might be removed in the future. +// Use [ValidationScheme.IsValidMetricName] instead. func IsValidMetricName(n LabelValue) bool { - switch NameValidationScheme { - case LegacyValidation: - return IsValidLegacyMetricName(string(n)) - case UTF8Validation: - if len(n) == 0 { - return false - } - return utf8.ValidString(string(n)) - default: - panic(fmt.Sprintf("Invalid name validation scheme requested: %s", NameValidationScheme.String())) - } + return NameValidationScheme.IsValidMetricName(string(n)) } // IsValidLegacyMetricName is similar to IsValidMetricName but always uses the // legacy validation scheme regardless of the value of NameValidationScheme. // This function, however, does not use MetricNameRE for the check but a much // faster hardcoded implementation. +// +// Deprecated: This function should not be used and might be removed in the future. +// Use [LegacyValidation.IsValidMetricName] instead. func IsValidLegacyMetricName(n string) bool { - if len(n) == 0 { - return false - } - for i, b := range n { - if !isValidLegacyRune(b, i) { - return false - } - } - return true + return LegacyValidation.IsValidMetricName(n) } // EscapeMetricFamily escapes the given metric names and labels with the given diff --git a/model/metric_test.go b/model/metric_test.go index 662a53d56..358281b0e 100644 --- a/model/metric_test.go +++ b/model/metric_test.go @@ -15,6 +15,7 @@ package model import ( "errors" + "fmt" "strings" "testing" @@ -202,9 +203,9 @@ func TestValidationScheme_UnmarshalYAML(t *testing.T) { } } -func TestMetricNameIsLegacyValid(t *testing.T) { +func TestValidationScheme_IsMetricNameValid(t *testing.T) { scenarios := []struct { - mn LabelValue + mn string legacyValid bool utf8Valid bool }{ @@ -259,19 +260,35 @@ func TestMetricNameIsLegacyValid(t *testing.T) { utf8Valid: false, }, } - for _, s := range scenarios { - NameValidationScheme = LegacyValidation - if IsValidMetricName(s.mn) != s.legacyValid { - t.Errorf("Expected %v for %q using legacy IsValidMetricName method", s.legacyValid, s.mn) - } - if MetricNameRE.MatchString(string(s.mn)) != s.legacyValid { - t.Errorf("Expected %v for %q using regexp matching", s.legacyValid, s.mn) - } - NameValidationScheme = UTF8Validation - if IsValidMetricName(s.mn) != s.utf8Valid { - t.Errorf("Expected %v for %q using utf-8 IsValidMetricName method", s.legacyValid, s.mn) - } + t.Run(fmt.Sprintf("%s,%t,%t", s.mn, s.legacyValid, s.utf8Valid), func(t *testing.T) { + if LegacyValidation.IsValidMetricName(s.mn) != s.legacyValid { + t.Errorf("Expected %v for %q using LegacyValidation.IsValidMetricName", s.legacyValid, s.mn) + } + if MetricNameRE.MatchString(string(s.mn)) != s.legacyValid { + t.Errorf("Expected %v for %q using regexp matching", s.legacyValid, s.mn) + } + if UTF8Validation.IsValidMetricName(s.mn) != s.utf8Valid { + t.Errorf("Expected %v for %q using UTF8Validation.IsValidMetricName", s.utf8Valid, s.mn) + } + + // Test deprecated functions. + if IsValidLegacyMetricName(s.mn) != s.legacyValid { + t.Errorf("Expected %v for %q using IsValidLegacyMetricNames", s.legacyValid, s.mn) + } + origScheme := NameValidationScheme + t.Cleanup(func() { + NameValidationScheme = origScheme + }) + NameValidationScheme = LegacyValidation + if IsValidMetricName(LabelValue(s.mn)) != s.legacyValid { + t.Errorf("Expected %v for %q using legacy IsValidMetricName", s.legacyValid, s.mn) + } + NameValidationScheme = UTF8Validation + if IsValidMetricName(LabelValue(s.mn)) != s.utf8Valid { + t.Errorf("Expected %v for %q using utf-8 IsValidMetricName method", s.legacyValid, s.mn) + } + }) } } From 4f3db05ca06f44021cccdb1bf45d1aa2188822af Mon Sep 17 00:00:00 2001 From: Arve Knudsen Date: Thu, 31 Jul 2025 16:50:22 +0200 Subject: [PATCH 2/5] ValidationScheme.IsValidLabelName: Fix checking of empty string Signed-off-by: Arve Knudsen --- model/metric.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/model/metric.go b/model/metric.go index 55d6a5847..4dd36cc1d 100644 --- a/model/metric.go +++ b/model/metric.go @@ -152,9 +152,6 @@ func (s ValidationScheme) IsValidMetricName(metricName string) bool { // IsValidLabelName returns whether labelName is valid according to s. func (s ValidationScheme) IsValidLabelName(labelName string) bool { - if len(labelName) == 0 { - return false - } switch s { case LegacyValidation: if len(labelName) == 0 { @@ -168,6 +165,9 @@ func (s ValidationScheme) IsValidLabelName(labelName string) bool { } return true case UTF8Validation: + if len(labelName) == 0 { + return false + } return utf8.ValidString(string(labelName)) default: panic(fmt.Sprintf("Invalid name validation scheme requested: %s", s)) From 2fae0604fbe53b5fad398624d613b82705d45d0e Mon Sep 17 00:00:00 2001 From: Arve Knudsen Date: Thu, 31 Jul 2025 16:56:12 +0200 Subject: [PATCH 3/5] Add ValidationScheme.IsLabelNameValid test case for empty input Signed-off-by: Arve Knudsen --- model/labels_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/model/labels_test.go b/model/labels_test.go index 441c138a3..53ca1e258 100644 --- a/model/labels_test.go +++ b/model/labels_test.go @@ -142,6 +142,11 @@ func TestValidationScheme_IsLabelNameValid(t *testing.T) { legacyValid: false, utf8Valid: false, }, + { + ln: "", + legacyValid: false, + utf8Valid: false, + }, } for _, s := range scenarios { t.Run(fmt.Sprintf("%s,%t,%t", s.ln, s.legacyValid, s.utf8Valid), func(t *testing.T) { From 93ab601e3aff97a572390ab5d7837496cfe68922 Mon Sep 17 00:00:00 2001 From: Arve Knudsen Date: Fri, 1 Aug 2025 09:06:54 +0200 Subject: [PATCH 4/5] Drop unnecessary type conversions Signed-off-by: Arve Knudsen --- model/metric.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/model/metric.go b/model/metric.go index 4dd36cc1d..31981bb4e 100644 --- a/model/metric.go +++ b/model/metric.go @@ -144,9 +144,9 @@ func (s ValidationScheme) IsValidMetricName(metricName string) bool { if len(metricName) == 0 { return false } - return utf8.ValidString(string(metricName)) + return utf8.ValidString(metricName) default: - panic(fmt.Sprintf("Invalid metricName validation scheme requested: %s", s.String())) + panic(fmt.Sprintf("Invalid name validation scheme requested: %s", s.String())) } } @@ -168,7 +168,7 @@ func (s ValidationScheme) IsValidLabelName(labelName string) bool { if len(labelName) == 0 { return false } - return utf8.ValidString(string(labelName)) + return utf8.ValidString(labelName) default: panic(fmt.Sprintf("Invalid name validation scheme requested: %s", s)) } From 59b710829f08b01819f05a6192bde92814ac71c6 Mon Sep 17 00:00:00 2001 From: Arve Knudsen Date: Fri, 1 Aug 2025 09:11:10 +0200 Subject: [PATCH 5/5] Fix test error messages Signed-off-by: Arve Knudsen --- model/labels_test.go | 2 +- model/metric_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/model/labels_test.go b/model/labels_test.go index 53ca1e258..e66e3316a 100644 --- a/model/labels_test.go +++ b/model/labels_test.go @@ -172,7 +172,7 @@ func TestValidationScheme_IsLabelNameValid(t *testing.T) { } NameValidationScheme = UTF8Validation if labelName.IsValid() != s.utf8Valid { - t.Errorf("Expected %v for %q using UTF-8 IsValid method", s.legacyValid, s.ln) + t.Errorf("Expected %v for %q using UTF-8 IsValid method", s.utf8Valid, s.ln) } }) } diff --git a/model/metric_test.go b/model/metric_test.go index 358281b0e..b64c8a7e5 100644 --- a/model/metric_test.go +++ b/model/metric_test.go @@ -286,7 +286,7 @@ func TestValidationScheme_IsMetricNameValid(t *testing.T) { } NameValidationScheme = UTF8Validation if IsValidMetricName(LabelValue(s.mn)) != s.utf8Valid { - t.Errorf("Expected %v for %q using utf-8 IsValidMetricName method", s.legacyValid, s.mn) + t.Errorf("Expected %v for %q using utf-8 IsValidMetricName method", s.utf8Valid, s.mn) } }) }