From 0d13d50bbbc8f884ca7118d1d326c0a0482133c8 Mon Sep 17 00:00:00 2001 From: Travis Patterson Date: Wed, 24 Jan 2024 14:48:08 -0700 Subject: [PATCH 1/3] reset the basemap when the labelsbuilder is reset --- pkg/logql/log/labels.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/logql/log/labels.go b/pkg/logql/log/labels.go index ea4570e652df..61fc4097fb32 100644 --- a/pkg/logql/log/labels.go +++ b/pkg/logql/log/labels.go @@ -135,6 +135,7 @@ type BaseLabelsBuilder struct { errDetails string groups []string + baseMap map[string]string parserKeyHints ParserHint // label key hints for metric queries that allows to limit parser extractions to only this list of labels. without, noLabels bool referencedStructuredMetadata bool @@ -146,7 +147,6 @@ type BaseLabelsBuilder struct { // LabelsBuilder is the same as labels.Builder but tailored for this package. type LabelsBuilder struct { base labels.Labels - baseMap map[string]string buf labels.Labels currentResult LabelsResult groupedResult LabelsResult @@ -211,6 +211,7 @@ func (b *BaseLabelsBuilder) Reset() { } b.err = "" b.errDetails = "" + b.baseMap = nil b.parserKeyHints.Reset() } From e131ea83ba8fbb45ce48fdc24dadf83316e7aa2d Mon Sep 17 00:00:00 2001 From: Travis Patterson Date: Wed, 24 Jan 2024 15:01:55 -0700 Subject: [PATCH 2/3] test --- pkg/logql/log/labels_test.go | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/pkg/logql/log/labels_test.go b/pkg/logql/log/labels_test.go index b4efaaa1b65b..c9bb9d7f604a 100644 --- a/pkg/logql/log/labels_test.go +++ b/pkg/logql/log/labels_test.go @@ -68,6 +68,38 @@ func TestLabelsBuilder_LabelsError(t *testing.T) { require.Equal(t, labels.FromStrings("already", "in"), lbs) } +func TestLabelsBuilder_IntoMap(t *testing.T) { + strs := []string{ + "namespace", "loki", + "job", "us-central1/loki", + "cluster", "us-central1", + "ToReplace", "text", + } + lbs := labels.FromStrings(strs...) + b := NewBaseLabelsBuilder().ForLabels(lbs, lbs.Hash()) + + m := map[string]string{} + b.IntoMap(m) + + require.Equal(t, map[string]string{ + "namespace": "loki", + "job": "us-central1/loki", + "cluster": "us-central1", + "ToReplace": "text", + }, m) + + b.Reset() + + m2 := map[string]string{} + b.IntoMap(m2) + require.Equal(t, map[string]string{ + "namespace": "loki", + "job": "us-central1/loki", + "cluster": "us-central1", + "ToReplace": "text", + }, m2) +} + func TestLabelsBuilder_LabelsResult(t *testing.T) { strs := []string{ "namespace", "loki", From 893e3af1143bca84d7d778c17bc69f6cf1b70a56 Mon Sep 17 00:00:00 2001 From: Travis Patterson Date: Wed, 24 Jan 2024 15:13:17 -0700 Subject: [PATCH 3/3] allow consecutive copies --- pkg/logql/log/labels.go | 6 ++-- pkg/logql/log/labels_test.go | 62 +++++++++++++++++++++++++----------- 2 files changed, 47 insertions(+), 21 deletions(-) diff --git a/pkg/logql/log/labels.go b/pkg/logql/log/labels.go index 61fc4097fb32..10414a8ed3b8 100644 --- a/pkg/logql/log/labels.go +++ b/pkg/logql/log/labels.go @@ -482,9 +482,9 @@ func (b *LabelsBuilder) IntoMap(m map[string]string) { if !b.hasDel() && !b.hasAdd() && !b.HasErr() { if b.baseMap == nil { b.baseMap = b.base.Map() - for k, v := range b.baseMap { - m[k] = v - } + } + for k, v := range b.baseMap { + m[k] = v } return } diff --git a/pkg/logql/log/labels_test.go b/pkg/logql/log/labels_test.go index c9bb9d7f604a..0f859c812542 100644 --- a/pkg/logql/log/labels_test.go +++ b/pkg/logql/log/labels_test.go @@ -76,28 +76,54 @@ func TestLabelsBuilder_IntoMap(t *testing.T) { "ToReplace", "text", } lbs := labels.FromStrings(strs...) - b := NewBaseLabelsBuilder().ForLabels(lbs, lbs.Hash()) - m := map[string]string{} - b.IntoMap(m) + t.Run("it still copies the map after a Reset", func(t *testing.T) { + b := NewBaseLabelsBuilder().ForLabels(lbs, lbs.Hash()) - require.Equal(t, map[string]string{ - "namespace": "loki", - "job": "us-central1/loki", - "cluster": "us-central1", - "ToReplace": "text", - }, m) + m := map[string]string{} + b.IntoMap(m) - b.Reset() + require.Equal(t, map[string]string{ + "namespace": "loki", + "job": "us-central1/loki", + "cluster": "us-central1", + "ToReplace": "text", + }, m) + + b.Reset() + + m2 := map[string]string{} + b.IntoMap(m2) + require.Equal(t, map[string]string{ + "namespace": "loki", + "job": "us-central1/loki", + "cluster": "us-central1", + "ToReplace": "text", + }, m2) + }) + + t.Run("it can copy the map several times", func(t *testing.T) { + b := NewBaseLabelsBuilder().ForLabels(lbs, lbs.Hash()) + + m := map[string]string{} + b.IntoMap(m) + + require.Equal(t, map[string]string{ + "namespace": "loki", + "job": "us-central1/loki", + "cluster": "us-central1", + "ToReplace": "text", + }, m) - m2 := map[string]string{} - b.IntoMap(m2) - require.Equal(t, map[string]string{ - "namespace": "loki", - "job": "us-central1/loki", - "cluster": "us-central1", - "ToReplace": "text", - }, m2) + m2 := map[string]string{} + b.IntoMap(m2) + require.Equal(t, map[string]string{ + "namespace": "loki", + "job": "us-central1/loki", + "cluster": "us-central1", + "ToReplace": "text", + }, m2) + }) } func TestLabelsBuilder_LabelsResult(t *testing.T) {