From f1d1845a2990226f2cf031993b478022af3198e2 Mon Sep 17 00:00:00 2001 From: Syed Nihal Date: Fri, 9 Feb 2024 16:42:22 +0530 Subject: [PATCH] fix sorting issue of prometheus labelset. see: https://github.com/prometheus/common/issues/543 Signed-off-by: Syed Nihal --- model/labelset.go | 108 +++-------------------------------------- model/labelset_test.go | 20 ++------ 2 files changed, 11 insertions(+), 117 deletions(-) diff --git a/model/labelset.go b/model/labelset.go index c685e4d4..ca855a0c 100644 --- a/model/labelset.go +++ b/model/labelset.go @@ -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, ", ")) } @@ -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:]) -} diff --git a/model/labelset_test.go b/model/labelset_test.go index dbe0d2e0..3f8824d6 100644 --- a/model/labelset_test.go +++ b/model/labelset_test.go @@ -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 @@ -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) @@ -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() }