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

Bugfix: quote label name in matchers when needed #14068

Merged
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
27 changes: 25 additions & 2 deletions model/labels/matcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@
package labels

import (
"fmt"
"bytes"
"strconv"
)

// MatchType is an enum for label matching types.
Expand Down Expand Up @@ -78,7 +79,29 @@ func MustNewMatcher(mt MatchType, name, val string) *Matcher {
}

func (m *Matcher) String() string {
return fmt.Sprintf("%s%s%q", m.Name, m.Type, m.Value)
// Start a buffer with a pre-allocated size on stack to cover most needs.
var bytea [1024]byte
b := bytes.NewBuffer(bytea[:0])

if m.shouldQuoteName() {
b.Write(strconv.AppendQuote(b.AvailableBuffer(), m.Name))
} else {
b.WriteString(m.Name)
}
b.WriteString(m.Type.String())
b.Write(strconv.AppendQuote(b.AvailableBuffer(), m.Value))

return b.String()
}

func (m *Matcher) shouldQuoteName() bool {
for i, c := range m.Name {
if c == '_' || (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (i > 0 && c >= '0' && c <= '9') {
continue
}
return true
}
return false
}

// Matches returns whether the matcher matches the given string value.
Expand Down
126 changes: 126 additions & 0 deletions model/labels/matcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ package labels

import (
"fmt"
"math/rand"
"testing"

"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -225,3 +226,128 @@ func BenchmarkNewMatcher(b *testing.B) {
}
})
}

func BenchmarkMatcher_String(b *testing.B) {
type benchCase struct {
name string
matchers []*Matcher
}
cases := []benchCase{
{
name: "short name equal",
matchers: []*Matcher{
MustNewMatcher(MatchEqual, "foo", "bar"),
MustNewMatcher(MatchEqual, "bar", "baz"),
MustNewMatcher(MatchEqual, "abc", "def"),
MustNewMatcher(MatchEqual, "ghi", "klm"),
MustNewMatcher(MatchEqual, "nop", "qrs"),
},
},
{
name: "short quoted name not equal",
matchers: []*Matcher{
MustNewMatcher(MatchEqual, "f.o", "bar"),
MustNewMatcher(MatchEqual, "b.r", "baz"),
MustNewMatcher(MatchEqual, "a.c", "def"),
MustNewMatcher(MatchEqual, "g.i", "klm"),
MustNewMatcher(MatchEqual, "n.p", "qrs"),
},
},
{
name: "short quoted name with quotes not equal",
matchers: []*Matcher{
MustNewMatcher(MatchEqual, `"foo"`, "bar"),
MustNewMatcher(MatchEqual, `"foo"`, "baz"),
MustNewMatcher(MatchEqual, `"foo"`, "def"),
MustNewMatcher(MatchEqual, `"foo"`, "klm"),
MustNewMatcher(MatchEqual, `"foo"`, "qrs"),
},
},
{
name: "short name value with quotes equal",
matchers: []*Matcher{
MustNewMatcher(MatchEqual, "foo", `"bar"`),
MustNewMatcher(MatchEqual, "bar", `"baz"`),
MustNewMatcher(MatchEqual, "abc", `"def"`),
MustNewMatcher(MatchEqual, "ghi", `"klm"`),
MustNewMatcher(MatchEqual, "nop", `"qrs"`),
},
},
{
name: "short name and long value regexp",
matchers: []*Matcher{
MustNewMatcher(MatchRegexp, "foo", "five_six_seven_eight_nine_ten_one_two_three_four"),
MustNewMatcher(MatchRegexp, "bar", "one_two_three_four_five_six_seven_eight_nine_ten"),
MustNewMatcher(MatchRegexp, "abc", "two_three_four_five_six_seven_eight_nine_ten_one"),
MustNewMatcher(MatchRegexp, "ghi", "three_four_five_six_seven_eight_nine_ten_one_two"),
MustNewMatcher(MatchRegexp, "nop", "four_five_six_seven_eight_nine_ten_one_two_three"),
},
},
{
name: "short name and long value with quotes equal",
matchers: []*Matcher{
MustNewMatcher(MatchEqual, "foo", `five_six_seven_eight_nine_ten_"one"_two_three_four`),
MustNewMatcher(MatchEqual, "bar", `one_two_three_four_five_six_"seven"_eight_nine_ten`),
MustNewMatcher(MatchEqual, "abc", `two_three_four_five_six_seven_"eight"_nine_ten_one`),
MustNewMatcher(MatchEqual, "ghi", `three_four_five_six_seven_eight_"nine"_ten_one_two`),
MustNewMatcher(MatchEqual, "nop", `four_five_six_seven_eight_nine_"ten"_one_two_three`),
},
},
{
name: "long name regexp",
matchers: []*Matcher{
MustNewMatcher(MatchRegexp, "one_two_three_four_five_six_seven_eight_nine_ten", "val"),
MustNewMatcher(MatchRegexp, "two_three_four_five_six_seven_eight_nine_ten_one", "val"),
MustNewMatcher(MatchRegexp, "three_four_five_six_seven_eight_nine_ten_one_two", "val"),
MustNewMatcher(MatchRegexp, "four_five_six_seven_eight_nine_ten_one_two_three", "val"),
MustNewMatcher(MatchRegexp, "five_six_seven_eight_nine_ten_one_two_three_four", "val"),
},
},
{
name: "long quoted name regexp",
matchers: []*Matcher{
MustNewMatcher(MatchRegexp, "one.two.three.four.five.six.seven.eight.nine.ten", "val"),
MustNewMatcher(MatchRegexp, "two.three.four.five.six.seven.eight.nine.ten.one", "val"),
MustNewMatcher(MatchRegexp, "three.four.five.six.seven.eight.nine.ten.one.two", "val"),
MustNewMatcher(MatchRegexp, "four.five.six.seven.eight.nine.ten.one.two.three", "val"),
MustNewMatcher(MatchRegexp, "five.six.seven.eight.nine.ten.one.two.three.four", "val"),
},
},
{
name: "long name and long value regexp",
matchers: []*Matcher{
MustNewMatcher(MatchRegexp, "one_two_three_four_five_six_seven_eight_nine_ten", "five_six_seven_eight_nine_ten_one_two_three_four"),
MustNewMatcher(MatchRegexp, "two_three_four_five_six_seven_eight_nine_ten_one", "one_two_three_four_five_six_seven_eight_nine_ten"),
MustNewMatcher(MatchRegexp, "three_four_five_six_seven_eight_nine_ten_one_two", "two_three_four_five_six_seven_eight_nine_ten_one"),
MustNewMatcher(MatchRegexp, "four_five_six_seven_eight_nine_ten_one_two_three", "three_four_five_six_seven_eight_nine_ten_one_two"),
MustNewMatcher(MatchRegexp, "five_six_seven_eight_nine_ten_one_two_three_four", "four_five_six_seven_eight_nine_ten_one_two_three"),
},
},
{
name: "long quoted name and long value regexp",
matchers: []*Matcher{
MustNewMatcher(MatchRegexp, "one.two.three.four.five.six.seven.eight.nine.ten", "five.six.seven.eight.nine.ten.one.two.three.four"),
MustNewMatcher(MatchRegexp, "two.three.four.five.six.seven.eight.nine.ten.one", "one.two.three.four.five.six.seven.eight.nine.ten"),
MustNewMatcher(MatchRegexp, "three.four.five.six.seven.eight.nine.ten.one.two", "two.three.four.five.six.seven.eight.nine.ten.one"),
MustNewMatcher(MatchRegexp, "four.five.six.seven.eight.nine.ten.one.two.three", "three.four.five.six.seven.eight.nine.ten.one.two"),
MustNewMatcher(MatchRegexp, "five.six.seven.eight.nine.ten.one.two.three.four", "four.five.six.seven.eight.nine.ten.one.two.three"),
},
},
}

var mixed []*Matcher
for _, bc := range cases {
mixed = append(mixed, bc.matchers...)
}
rand.Shuffle(len(mixed), func(i, j int) { mixed[i], mixed[j] = mixed[j], mixed[i] })
cases = append(cases, benchCase{name: "mixed", matchers: mixed})

for _, bc := range cases {
b.Run(bc.name, func(b *testing.B) {
for i := 0; i <= b.N; i++ {
m := bc.matchers[i%len(bc.matchers)]
_ = m.String()
}
})
}
}
10 changes: 10 additions & 0 deletions promql/parser/printer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,16 @@ func TestExprString(t *testing.T) {
{
in: `{__name__="",a="x"}`,
},
{
in: `{"a.b"="c"}`,
},
{
in: `{"0"="1"}`,
},
{
in: `{"_0"="1"}`,
out: `{_0="1"}`,
},
}

for _, test := range inputs {
Expand Down
Loading