From e9e613801e8168516d8998098e565aabddca5cf8 Mon Sep 17 00:00:00 2001 From: Ogulcan Aydogan Date: Tue, 12 May 2026 15:41:00 +0100 Subject: [PATCH] model: fix EscapeLabelName to not preserve colon in label names Colons are reserved for metric names only per the Prometheus data model. EscapeMetricFamily and metricNeedsEscaping were using IsValidLegacyMetricName and EscapeName for label name contexts, causing colons to be preserved when they should be escaped. Add isValidLegacyLabelRune, IsValidLegacyLabelName, and EscapeLabelName that exclude colons. Update EscapeMetricFamily and metricNeedsEscaping to use the label-specific functions for label name contexts. Builds on the approach discussed in prometheus/common#889. Fixes prometheus/prometheus#18380 Signed-off-by: Ogulcan Aydogan --- model/metric.go | 81 ++++++++++++++++++++++++++++++-- model/metric_test.go | 109 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 187 insertions(+), 3 deletions(-) diff --git a/model/metric.go b/model/metric.go index 429a0dab..21ce5873 100644 --- a/model/metric.go +++ b/model/metric.go @@ -321,6 +321,75 @@ func IsValidLegacyMetricName(n string) bool { return LegacyValidation.IsValidMetricName(n) } +// IsValidLegacyLabelName reports whether n is a valid label name under the +// legacy Prometheus data model. Unlike metric names, label names must not +// contain ':'. +func IsValidLegacyLabelName(n string) bool { + return LegacyValidation.IsValidLabelName(n) +} + +// EscapeLabelName escapes the incoming label name according to the provided +// escaping scheme. It behaves like EscapeName but applies label-name rules: +// ':' is not a valid character in a label name and is always escaped. +func EscapeLabelName(name string, scheme EscapingScheme) string { + if len(name) == 0 { + return name + } + var escaped strings.Builder + switch scheme { + case NoEscaping: + return name + case UnderscoreEscaping: + if IsValidLegacyLabelName(name) { + return name + } + for i, b := range name { + if isValidLegacyLabelRune(b, i) { + escaped.WriteRune(b) + } else { + escaped.WriteRune('_') + } + } + return escaped.String() + case DotsEscaping: + for i, b := range name { + switch { + case b == '_': + escaped.WriteString("__") + case b == '.': + escaped.WriteString("_dot_") + case isValidLegacyLabelRune(b, i): + escaped.WriteRune(b) + default: + escaped.WriteString("__") + } + } + return escaped.String() + case ValueEncodingEscaping: + if IsValidLegacyLabelName(name) { + return name + } + escaped.WriteString("U__") + for i, b := range name { + switch { + case b == '_': + escaped.WriteString("__") + case isValidLegacyLabelRune(b, i): + escaped.WriteRune(b) + case !utf8.ValidRune(b): + escaped.WriteString("_FFFD_") + default: + escaped.WriteRune('_') + escaped.WriteString(strconv.FormatInt(int64(b), 16)) + escaped.WriteRune('_') + } + } + return escaped.String() + default: + panic(fmt.Sprintf("invalid escaping scheme %d", scheme)) + } +} + // EscapeMetricFamily escapes the given metric names and labels with the given // escaping scheme. Returns a new object that uses the same pointers to fields // when possible and creates new escaped versions so as not to mutate the @@ -373,12 +442,12 @@ func EscapeMetricFamily(v *dto.MetricFamily, scheme EscapingScheme) *dto.MetricF }) continue } - if l.Name == nil || IsValidLegacyMetricName(l.GetName()) { + if l.Name == nil || IsValidLegacyLabelName(l.GetName()) { escaped.Label = append(escaped.Label, l) continue } escaped.Label = append(escaped.Label, &dto.LabelPair{ - Name: proto.String(EscapeName(l.GetName(), scheme)), + Name: proto.String(EscapeLabelName(l.GetName(), scheme)), Value: l.Value, }) } @@ -392,7 +461,7 @@ func metricNeedsEscaping(m *dto.Metric) bool { if l.GetName() == MetricNameLabel && !IsValidLegacyMetricName(l.GetValue()) { return true } - if !IsValidLegacyMetricName(l.GetName()) { + if !IsValidLegacyLabelName(l.GetName()) { return true } } @@ -550,6 +619,12 @@ func isValidLegacyRune(b rune, i int) bool { return (b >= 'a' && b <= 'z') || (b >= 'A' && b <= 'Z') || b == '_' || b == ':' || (b >= '0' && b <= '9' && i > 0) } +// isValidLegacyLabelRune is like isValidLegacyRune but excludes ':'. +// Colons are reserved for metric names only; they have never been valid in label names. +func isValidLegacyLabelRune(b rune, i int) bool { + return (b >= 'a' && b <= 'z') || (b >= 'A' && b <= 'Z') || b == '_' || (b >= '0' && b <= '9' && i > 0) +} + func (e EscapingScheme) String() string { switch e { case NoEscaping: diff --git a/model/metric_test.go b/model/metric_test.go index c997fa62..dd31d0cb 100644 --- a/model/metric_test.go +++ b/model/metric_test.go @@ -571,6 +571,67 @@ func TestEscapeName(t *testing.T) { } } +func TestEscapeLabelName(t *testing.T) { + scenarios := []struct { + name string + input string + expectedUnderscores string + expectedDots string + expectedValue string + }{ + { + name: "empty string", + }, + { + name: "legacy valid label name, no escaping required", + input: "no_escaping_required", + expectedUnderscores: "no_escaping_required", + expectedDots: "no__escaping__required", + expectedValue: "no_escaping_required", + }, + { + name: "colon only valid in metric names, not label names", + input: "app:instance_id", + expectedUnderscores: "app_instance_id", + expectedDots: "app__instance__id", + expectedValue: "U__app_3a_instance__id", + }, + { + name: "colon and hyphen both escaped in label names", + input: "app:instance-id", + expectedUnderscores: "app_instance_id", + expectedDots: "app__instance__id", + expectedValue: "U__app_3a_instance_2d_id", + }, + { + name: "dot and colon both escaped in label names", + input: "http.status:sum", + expectedUnderscores: "http_status_sum", + expectedDots: "http_dot_status__sum", + expectedValue: "U__http_2e_status_3a_sum", + }, + } + + for _, scenario := range scenarios { + t.Run(scenario.name, func(t *testing.T) { + got := EscapeLabelName(scenario.input, UnderscoreEscaping) + if got != scenario.expectedUnderscores { + t.Errorf("UnderscoreEscaping: expected %q but got %q", scenario.expectedUnderscores, got) + } + + got = EscapeLabelName(scenario.input, DotsEscaping) + if got != scenario.expectedDots { + t.Errorf("DotsEscaping: expected %q but got %q", scenario.expectedDots, got) + } + + got = EscapeLabelName(scenario.input, ValueEncodingEscaping) + if got != scenario.expectedValue { + t.Errorf("ValueEncodingEscaping: expected %q but got %q", scenario.expectedValue, got) + } + }) + } +} + func TestValueUnescapeErrors(t *testing.T) { scenarios := []struct { name string @@ -794,6 +855,54 @@ func TestEscapeMetricFamily(t *testing.T) { }, }, }, + { + name: "colon in label name is escaped, colon in metric name is not", + scheme: UnderscoreEscaping, + input: &dto.MetricFamily{ + Name: proto.String("requests:total"), + Help: proto.String("some help text"), + Type: dto.MetricType_COUNTER.Enum(), + Metric: []*dto.Metric{ + { + Counter: &dto.Counter{ + Value: proto.Float64(1), + }, + Label: []*dto.LabelPair{ + { + Name: proto.String("__name__"), + Value: proto.String("requests:total"), + }, + { + Name: proto.String("app:instance_id"), + Value: proto.String("srv1"), + }, + }, + }, + }, + }, + expected: &dto.MetricFamily{ + Name: proto.String("requests:total"), + Help: proto.String("some help text"), + Type: dto.MetricType_COUNTER.Enum(), + Metric: []*dto.Metric{ + { + Counter: &dto.Counter{ + Value: proto.Float64(1), + }, + Label: []*dto.LabelPair{ + { + Name: proto.String("__name__"), + Value: proto.String("requests:total"), + }, + { + Name: proto.String("app_instance_id"), + Value: proto.String("srv1"), + }, + }, + }, + }, + }, + }, { name: "gauge, escaping needed", scheme: DotsEscaping,