Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: support dots in label names #3335

Merged
merged 2 commits into from
Jun 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 42 additions & 6 deletions pkg/validation/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
55 changes: 53 additions & 2 deletions pkg/validation/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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,
},
{
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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},
kolesnikovae marked this conversation as resolved.
Show resolved Hide resolved
{"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)
})
}
}
Loading