chore: support dots in label names (#3335)

Author: kolesnikovae

pkg/validation/validate.go

@@ -123,21 +123,57 @@ func ValidateLabels(limits LabelValidationLimits, tenantID string, ls []*typesv1
 	for _, l := range ls {
 		if len(l.Name) > limits.MaxLabelNameLength(tenantID) {
 			return NewErrorf(LabelNameTooLong, LabelNameTooLongErrorMsg, phlaremodel.LabelPairsString(ls), l.Name)
-		} else if len(l.Value) > limits.MaxLabelValueLength(tenantID) {
+		}
+		if len(l.Value) > limits.MaxLabelValueLength(tenantID) {
 			return NewErrorf(LabelValueTooLong, LabelValueTooLongErrorMsg, phlaremodel.LabelPairsString(ls), l.Value)
-		} else if !model.LabelName(l.Name).IsValid() {
-			return NewErrorf(InvalidLabels, InvalidLabelsErrorMsg, phlaremodel.LabelPairsString(ls), "invalid label name '"+l.Name+"'")
-		} else if !model.LabelValue(l.Value).IsValid() {
+		}
+		var origName string
+		var ok bool
+		if origName, l.Name, ok = SanitizeLabelName(l.Name); !ok {
+			return NewErrorf(InvalidLabels, InvalidLabelsErrorMsg, phlaremodel.LabelPairsString(ls), "invalid label name '"+origName+"'")
+		}
+		if !model.LabelValue(l.Value).IsValid() {
 			return NewErrorf(InvalidLabels, InvalidLabelsErrorMsg, phlaremodel.LabelPairsString(ls), "invalid label value '"+l.Value+"'")
-		} else if cmp := strings.Compare(lastLabelName, l.Name); cmp == 0 {
-			return NewErrorf(DuplicateLabelNames, DuplicateLabelNamesErrorMsg, phlaremodel.LabelPairsString(ls), l.Name)
+		}
+		if cmp := strings.Compare(lastLabelName, l.Name); cmp == 0 {
+			return NewErrorf(DuplicateLabelNames, DuplicateLabelNamesErrorMsg, phlaremodel.LabelPairsString(ls), origName)
 		}
 		lastLabelName = l.Name
 	}
 
 	return nil
 }
 
+// SanitizeLabelName reports whether the label name is valid,
+// and returns the sanitized value.
+//
+// The only change the function makes is replacing dots with underscores.
+func SanitizeLabelName(ln string) (old, sanitized string, ok bool) {
+	if len(ln) == 0 {
+		return ln, ln, false
+	}
+	hasDots := false
+	for i, b := range ln {
+		if !((b >= 'a' && b <= 'z') || (b >= 'A' && b <= 'Z') || b == '_' || (b >= '0' && b <= '9' && i > 0)) {
+			if b == '.' {
+				hasDots = true
+			} else {
+				return ln, ln, false
+			}
+		}
+	}
+	if !hasDots {
+		return ln, ln, true
+	}
+	r := []rune(ln)
+	for i, b := range r {
+		if b == '.' {
+			r[i] = '_'
+		}
+	}
+	return ln, string(r), true
+}
+
 type ProfileValidationLimits interface {
 	MaxProfileSizeBytes(tenantID string) int
 	MaxProfileStacktraceSamples(tenantID string) int

pkg/validation/validate_test.go

@@ -5,6 +5,7 @@ import (
 	"time"
 
 	"github.com/prometheus/common/model"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	googlev1 "github.com/grafana/pyroscope/api/gen/proto/go/google/v1"
@@ -50,7 +51,7 @@ func TestValidateLabels(t *testing.T) {
 				{Name: "foo3", Value: "bar"},
 				{Name: "foo4", Value: "bar"},
 			},
-			expectedErr:    `profile series '{foo1="bar", foo2="bar", foo3="bar", foo4="bar", service_name="svc"}' has 5 label names; limit 3`,
+			expectedErr:    `profile series '{foo1="bar", foo2="bar", foo3="bar", foo4="bar", service_name="svc"}' has 5 label names; limit 4`,
 			expectedReason: MaxLabelNamesPerSeries,
 		},
 		{
@@ -112,10 +113,22 @@ func TestValidateLabels(t *testing.T) {
 			expectedReason: DuplicateLabelNames,
 			expectedErr:    "profile with labels '{__name__=\"qux\", service_name=\"svc\", service_name=\"svc\"}' has duplicate label name: 'service_name'",
 		},
+
+		{
+			name: "dupe sanitized",
+			lbs: []*typesv1.LabelPair{
+				{Name: model.MetricNameLabel, Value: "qux"},
+				{Name: "label.name", Value: "foo"},
+				{Name: "label.name", Value: "bar"},
+				{Name: phlaremodel.LabelNameServiceName, Value: "svc"},
+			},
+			expectedReason: DuplicateLabelNames,
+			expectedErr:    "profile with labels '{__name__=\"qux\", label_name=\"foo\", label_name=\"bar\", service_name=\"svc\"}' has duplicate label name: 'label.name'",
+		},
 	} {
 		t.Run(tt.name, func(t *testing.T) {
 			err := ValidateLabels(MockLimits{
-				MaxLabelNamesPerSeriesValue: 3,
+				MaxLabelNamesPerSeriesValue: 4,
 				MaxLabelNameLengthValue:     12,
 				MaxLabelValueLengthValue:    10,
 			}, "foo", tt.lbs)
@@ -400,3 +413,41 @@ func TestValidateFlamegraphMaxNodes(t *testing.T) {
 		})
 	}
 }
+
+func Test_SanitizeLabelName(t *testing.T) {
+	for _, tc := range []struct {
+		input    string
+		expected string
+		valid    bool
+	}{
+		{"", "", false},
+		{".", "_", true},
+		{".a", "_a", true},
+		{"a.", "a_", true},
+		{"..", "__", true},
+		{"..a", "__a", true},
+		{"a..", "a__", true},
+		{"a.a", "a_a", true},
+		{".a.", "_a_", true},
+		{"..a..", "__a__", true},
+		{"世界", "世界", false},
+		{"界世_a", "界世_a", false},
+		{"界世__a", "界世__a", false},
+		{"a_世界", "a_世界", false},
+		{"0.a", "0.a", false},
+		{"0a", "0a", false},
+		{"a.0", "a_0", true},
+		{"a0", "a0", true},
+		{"_", "_", true},
+		{"__a", "__a", true},
+		{"__a__", "__a__", true},
+	} {
+		tc := tc
+		t.Run("", func(t *testing.T) {
+			origName, actual, valid := SanitizeLabelName(tc.input)
+			assert.Equal(t, tc.input, origName)
+			assert.Equal(t, tc.expected, actual)
+			assert.Equal(t, tc.valid, valid)
+		})
+	}
+}