Skip to content

Commit

Permalink
fix sorting issue of prometheus labelset. see: prometheus#543
Browse files Browse the repository at this point in the history
Signed-off-by: Syed Nihal <syed.nihal@nokia.com>
  • Loading branch information
wasim-nihal committed Feb 9, 2024
1 parent dd21376 commit f1d1845
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 117 deletions.
108 changes: 7 additions & 101 deletions model/labelset.go
Expand Up @@ -130,11 +130,15 @@ func (l LabelSet) Merge(other LabelSet) LabelSet {
}

func (l LabelSet) String() string {
labelNames := make([]string, 0, len(l))
for name := range l {
labelNames = append(labelNames, string(name))
}
sort.Strings(labelNames)
lstrs := make([]string, 0, len(l))
for l, v := range l {
lstrs = append(lstrs, fmt.Sprintf("%s=%q", l, v))
for _, name := range labelNames {
lstrs = append(lstrs, fmt.Sprintf("%s=%q", name, l[LabelName(name)]))
}
sort.Stable(LabelSorter(lstrs))
return fmt.Sprintf("{%s}", strings.Join(lstrs, ", "))
}

Expand Down Expand Up @@ -166,101 +170,3 @@ func (l *LabelSet) UnmarshalJSON(b []byte) error {
*l = LabelSet(m)
return nil
}

// LabelSorter implements custom sorting functionality for prometheus LabelSets.
// See: https://github.com/prometheus/common/issues/543
type LabelSorter []string

func (p LabelSorter) Len() int { return len(p) }
func (p LabelSorter) Less(i, j int) bool { return Less(p[i], p[j]) }
func (p LabelSorter) Swap(i, j int) { p[i], p[j] = p[j], p[i] }

func Less(a, b string) bool {
for len(a) > 0 && len(b) > 0 {
// get the length of common prefix
p := lengthOfCommonPrefix(a, b)
// If there is a common prefix, remove the prefix from both the strings
if p > 0 {
a = a[p:]
b = b[p:]
}
if len(a) == 0 {
return len(b) != 0
}
ia := firstNonNumericCharacterIndex(a)
ib := firstNonNumericCharacterIndex(b)
switch {
// If both strings a and b, begin with a numeric character
case ia > 0 && ib > 0:
// remove leading zeros if any
trimmedA, lenTrimmedA := removeLeadingZeros(a[:ia])
trimmedB, lenTrimmedB := removeLeadingZeros(b[:ib])
// check the the numeric weightage based on number of digits in the numeric substring
if lenTrimmedA > lenTrimmedB {
return false
} else if lenTrimmedA < lenTrimmedB {
return true
}
// if the length is same, compare them lexicographically
if trimmedA != trimmedB {
return trimmedA < trimmedB
}
// if the length is same and the value is also same, remove the substring from both the strings
// and continue with next set of characters
if ia != len(a) && ib != len(b) {
a = a[ia:]
b = b[ib:]
continue
}
// If string a begins with a numeric character and b begins with '=', do not swap
case ia > 0 && b[0] == '=':
return false
// If string b begins with a numeric character and a begins with '=', swap
case ib > 0 && a[0] == '=':
return true
// all the other cases, compare them lexicographically
default:
return a < b
}
}
return a < b
}

// lengthOfCommonPrefix, returns a length of common prefix between the given two strings a and b.
// Returns 0 if there is no common prefix
func lengthOfCommonPrefix(a, b string) int {
i, j := 0, 0
lenA, lenB := len(a), len(b)
for i < lenA && j < lenB {
if a[i] == b[j] {
i++
j++
} else {
break
}
}
return i
}

// firstNonNumericCharacterIndex returns the index of first non-numeric character
func firstNonNumericCharacterIndex(s string) int {
for i, c := range s {
if c < '0' || c > '9' {
return i
}
}
return len(s)
}

// removeLeadingZeros removes all the leading zeros from the numeric string, and returns it with the new length
func removeLeadingZeros(s string) (string, int) {
// if the numeric string does not begin with 0, return the same string with its length
if s[0] != '0' {
return s, len(s)
}
index := 0
for index < len(s) && s[index] == '0' {
index++
}
return s[index:], len(s[index:])
}
20 changes: 4 additions & 16 deletions model/labelset_test.go
Expand Up @@ -30,9 +30,7 @@ func TestUnmarshalJSONLabelSet(t *testing.T) {
"foo": "bar",
"foo2": "bar",
"abc": "prometheus",
"foo11": "bar11",
"foo20abc30": "bar",
"foo20abc9": "bar"
"foo11": "bar11"
}
}`
var c testConfig
Expand All @@ -43,7 +41,7 @@ func TestUnmarshalJSONLabelSet(t *testing.T) {

labelSetString := c.LabelSet.String()

expected := `{abc="prometheus", foo="bar", foo2="bar", foo11="bar11", foo20abc9="bar", foo20abc30="bar", monitor="codelab"}`
expected := `{abc="prometheus", foo="bar", foo11="bar11", foo2="bar", monitor="codelab"}`

if expected != labelSetString {
t.Errorf("expected %s but got %s", expected, labelSetString)
Expand Down Expand Up @@ -125,29 +123,19 @@ func TestLabelSetMerge(t *testing.T) {

// Benchmark Results for LabelSet's String() method
// ---------------------------------------------------------------------------------------------------------
// Standard Sort(current sorting via LabelSet.String())
// goos: linux
// goarch: amd64
// pkg: github.com/prometheus/common/model
// cpu: 11th Gen Intel(R) Core(TM) i5-1145G7 @ 2.60GHz
// BenchmarkStandardSortString-8 487034 2128 ns/op
// ---------------------------------------------------------------------------------------------------------
// Label Sort (proposed solution)
// goos: linux
// goarch: amd64
// pkg: github.com/prometheus/common/model
// cpu: 11th Gen Intel(R) Core(TM) i5-1145G7 @ 2.60GHz
// BenchmarkLabelSortString-8 451951 2257 ns/op
// BenchmarkLabelSetStringMethod-8 732376 1532 ns/op

func BenchmarkLabelSortString(b *testing.B) {
func BenchmarkLabelSetStringMethod(b *testing.B) {
ls := make(LabelSet)
ls["monitor"] = "codelab"
ls["foo2"] = "bar"
ls["foo"] = "bar"
ls["abc"] = "prometheus"
ls["foo11"] = "bar11"
ls["foo20abc30"] = "bar"
ls["foo20abc9"] = "bar"
for i := 0; i < b.N; i++ {
_ = ls.String()
}
Expand Down

0 comments on commit f1d1845

Please sign in to comment.