From bb769669923a830ae11f284905979017c2ac7a12 Mon Sep 17 00:00:00 2001 From: Lukasz Mierzwa Date: Tue, 15 Apr 2025 17:52:24 +0100 Subject: [PATCH 1/3] Use stringlabels by default This removes the stringlabels build tag, makes that implementation the default one, and moves the old labels implementation under the slicelabels build tag. Fixes #16064. Signed-off-by: Lukasz Mierzwa --- .github/workflows/ci.yml | 4 ++-- model/labels/labels.go | 4 ++-- model/labels/labels_stringlabels.go | 2 +- model/labels/sharding.go | 2 +- model/labels/sharding_stringlabels.go | 2 +- tsdb/agent/series_test.go | 10 +++++----- tsdb/head_test.go | 10 +++++----- 7 files changed, 17 insertions(+), 17 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f2263689a3..f280e9ad78 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -18,8 +18,8 @@ jobs: - uses: ./.github/promci/actions/setup_environment with: enable_npm: true - - run: make GOOPTS=--tags=stringlabels GO_ONLY=1 SKIP_GOLANGCI_LINT=1 - - run: go test --tags=stringlabels ./tsdb/ -test.tsdb-isolation=false + - run: make GO_ONLY=1 SKIP_GOLANGCI_LINT=1 + - run: go test ./tsdb/ -test.tsdb-isolation=false - run: make -C documentation/examples/remote_storage - run: make -C documentation/examples diff --git a/model/labels/labels.go b/model/labels/labels.go index ed66d73cbf..5ebdf6a3fe 100644 --- a/model/labels/labels.go +++ b/model/labels/labels.go @@ -11,7 +11,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -//go:build !stringlabels && !dedupelabels +//go:build slicelabels package labels @@ -453,7 +453,7 @@ func (b *ScratchBuilder) Add(name, value string) { } // UnsafeAddBytes adds a name/value pair, using []byte instead of string. -// The '-tags stringlabels' version of this function is unsafe, hence the name. +// The default version of this function is unsafe, hence the name. // This version is safe - it copies the strings immediately - but we keep the same name so everything compiles. func (b *ScratchBuilder) UnsafeAddBytes(name, value []byte) { b.add = append(b.add, Label{Name: string(name), Value: string(value)}) diff --git a/model/labels/labels_stringlabels.go b/model/labels/labels_stringlabels.go index f49ed96f65..8d611492b5 100644 --- a/model/labels/labels_stringlabels.go +++ b/model/labels/labels_stringlabels.go @@ -11,7 +11,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -//go:build stringlabels +//go:build !slicelabels && !dedupelabels package labels diff --git a/model/labels/sharding.go b/model/labels/sharding.go index 8b3a369397..ed05da675f 100644 --- a/model/labels/sharding.go +++ b/model/labels/sharding.go @@ -11,7 +11,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -//go:build !stringlabels && !dedupelabels +//go:build slicelabels package labels diff --git a/model/labels/sharding_stringlabels.go b/model/labels/sharding_stringlabels.go index 798f268eb9..4dcbaa21d1 100644 --- a/model/labels/sharding_stringlabels.go +++ b/model/labels/sharding_stringlabels.go @@ -11,7 +11,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -//go:build stringlabels +//go:build !slicelabels && !dedupelabels package labels diff --git a/tsdb/agent/series_test.go b/tsdb/agent/series_test.go index bc5a4af5d3..257f5815f9 100644 --- a/tsdb/agent/series_test.go +++ b/tsdb/agent/series_test.go @@ -77,13 +77,13 @@ func TestNoDeadlock(t *testing.T) { func labelsWithHashCollision() (labels.Labels, labels.Labels) { // These two series have the same XXHash; thanks to https://github.com/pstibrany/labels_hash_collisions - ls1 := labels.FromStrings("__name__", "metric", "lbl1", "value", "lbl2", "l6CQ5y") - ls2 := labels.FromStrings("__name__", "metric", "lbl1", "value", "lbl2", "v7uDlF") + ls1 := labels.FromStrings("__name__", "metric", "lbl", "HFnEaGl") + ls2 := labels.FromStrings("__name__", "metric", "lbl", "RqcXatm") if ls1.Hash() != ls2.Hash() { - // These ones are the same when using -tags stringlabels - ls1 = labels.FromStrings("__name__", "metric", "lbl", "HFnEaGl") - ls2 = labels.FromStrings("__name__", "metric", "lbl", "RqcXatm") + // These ones are the same when using -tags slicelabels + ls1 = labels.FromStrings("__name__", "metric", "lbl1", "value", "lbl2", "l6CQ5y") + ls2 = labels.FromStrings("__name__", "metric", "lbl1", "value", "lbl2", "v7uDlF") } if ls1.Hash() != ls2.Hash() { diff --git a/tsdb/head_test.go b/tsdb/head_test.go index 8cd44db841..100d5b1265 100644 --- a/tsdb/head_test.go +++ b/tsdb/head_test.go @@ -6286,13 +6286,13 @@ func TestHeadCompactionWhileAppendAndCommitExemplar(t *testing.T) { func labelsWithHashCollision() (labels.Labels, labels.Labels) { // These two series have the same XXHash; thanks to https://github.com/pstibrany/labels_hash_collisions - ls1 := labels.FromStrings("__name__", "metric", "lbl1", "value", "lbl2", "l6CQ5y") - ls2 := labels.FromStrings("__name__", "metric", "lbl1", "value", "lbl2", "v7uDlF") + ls1 := labels.FromStrings("__name__", "metric", "lbl", "HFnEaGl") + ls2 := labels.FromStrings("__name__", "metric", "lbl", "RqcXatm") if ls1.Hash() != ls2.Hash() { - // These ones are the same when using -tags stringlabels - ls1 = labels.FromStrings("__name__", "metric", "lbl", "HFnEaGl") - ls2 = labels.FromStrings("__name__", "metric", "lbl", "RqcXatm") + // These ones are the same when using -tags slicelabels + ls1 = labels.FromStrings("__name__", "metric", "lbl1", "value", "lbl2", "l6CQ5y") + ls2 = labels.FromStrings("__name__", "metric", "lbl1", "value", "lbl2", "v7uDlF") } if ls1.Hash() != ls2.Hash() { From 05088aaa1285be8c21ef652d704f431d9eae1734 Mon Sep 17 00:00:00 2001 From: Lukasz Mierzwa Date: Tue, 15 Apr 2025 18:02:34 +0100 Subject: [PATCH 2/3] Fix linter errors Mostly comment issues and unused variables. Signed-off-by: Lukasz Mierzwa --- model/labels/labels_stringlabels.go | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/model/labels/labels_stringlabels.go b/model/labels/labels_stringlabels.go index 8d611492b5..fa0bd7bc27 100644 --- a/model/labels/labels_stringlabels.go +++ b/model/labels/labels_stringlabels.go @@ -76,7 +76,7 @@ func (ls Labels) IsZero() bool { // MatchLabels returns a subset of Labels that matches/does not match with the provided label names based on the 'on' boolean. // If on is set to true, it returns the subset of labels that match with the provided label names and its inverse when 'on' is set to false. -// TODO: This is only used in printing an error message +// TODO: This is only used in printing an error message. func (ls Labels) MatchLabels(on bool, names ...string) Labels { b := NewBuilder(ls) if on { @@ -298,6 +298,7 @@ func Equal(ls, o Labels) bool { func EmptyLabels() Labels { return Labels{} } + func yoloBytes(s string) []byte { return unsafe.Slice(unsafe.StringData(s), len(s)) } @@ -370,7 +371,7 @@ func Compare(a, b Labels) int { return +1 } -// Copy labels from b on top of whatever was in ls previously, reusing memory or expanding if needed. +// CopyFrom will copy labels from b on top of whatever was in ls previously, reusing memory or expanding if needed. func (ls *Labels) CopyFrom(b Labels) { ls.data = b.data // strings are immutable } @@ -440,11 +441,11 @@ func (ls Labels) DropMetricName() Labels { } // InternStrings is a no-op because it would only save when the whole set of labels is identical. -func (ls *Labels) InternStrings(intern func(string) string) { +func (ls *Labels) InternStrings(_ func(string) string) { } // ReleaseStrings is a no-op for the same reason as InternStrings. -func (ls Labels) ReleaseStrings(release func(string)) { +func (ls Labels) ReleaseStrings(_ func(string)) { } // Builder allows modifying Labels. @@ -561,7 +562,7 @@ func encodeVarint(data []byte, offset int, v uint64) int { return base } -// Special code for the common case that a size is less than 128 +// Special code for the common case that a size is less than 128. func encodeSize(data []byte, offset, v int) int { if v < 1<<7 { offset-- @@ -630,7 +631,7 @@ func (b *ScratchBuilder) Add(name, value string) { b.add = append(b.add, Label{Name: name, Value: value}) } -// Add a name/value pair, using []byte instead of string to reduce memory allocations. +// UnsafeAddBytes adds a name/value pair using []byte instead of string to reduce memory allocations. // The values must remain live until Labels() is called. func (b *ScratchBuilder) UnsafeAddBytes(name, value []byte) { b.add = append(b.add, Label{Name: yoloString(name), Value: yoloString(value)}) @@ -658,7 +659,7 @@ func (b *ScratchBuilder) Labels() Labels { return b.output } -// Write the newly-built Labels out to ls, reusing an internal buffer. +// Overwrite will write the newly-built Labels out to ls, reusing an internal buffer. // Callers must ensure that there are no other references to ls, or any strings fetched from it. func (b *ScratchBuilder) Overwrite(ls *Labels) { size := labelsSize(b.add) @@ -671,7 +672,7 @@ func (b *ScratchBuilder) Overwrite(ls *Labels) { ls.data = yoloString(b.overwriteBuffer) } -// Symbol-table is no-op, just for api parity with dedupelabels. +// SymbolTable is no-op, just for api parity with dedupelabels. type SymbolTable struct{} func NewSymbolTable() *SymbolTable { return nil } From bec3a125a07010087aed1ecdbdca65bae89848d4 Mon Sep 17 00:00:00 2001 From: Lukasz Mierzwa Date: Tue, 15 Apr 2025 18:21:57 +0100 Subject: [PATCH 3/3] Remove stringlabels from promu build tags Signed-off-by: Lukasz Mierzwa --- .promu.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.promu.yml b/.promu.yml index 23379cc1ef..d16bceeed9 100644 --- a/.promu.yml +++ b/.promu.yml @@ -14,10 +14,8 @@ build: all: - netgo - builtinassets - - stringlabels windows: - builtinassets - - stringlabels ldflags: | -X github.com/prometheus/common/version.Version={{.Version}} -X github.com/prometheus/common/version.Revision={{.Revision}}