From 047585360b6fe948cbe230899ebaf285b286b3ae Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 9 Mar 2022 22:35:14 +0000 Subject: [PATCH] Update package storage/remote tests for new labels.Labels type Use ScratchBuilder to create labels. Signed-off-by: Bryan Boreham --- storage/remote/codec_test.go | 83 ++++++++++++++-------------- storage/remote/queue_manager_test.go | 67 +++++++++++++--------- storage/remote/read_test.go | 12 ++-- storage/remote/storage_test.go | 4 +- storage/remote/write_test.go | 4 +- 5 files changed, 92 insertions(+), 78 deletions(-) diff --git a/storage/remote/codec_test.go b/storage/remote/codec_test.go index 596eb0861..74328e1c7 100644 --- a/storage/remote/codec_test.go +++ b/storage/remote/codec_test.go @@ -71,86 +71,86 @@ var writeRequestFixture = &prompb.WriteRequest{ func TestValidateLabelsAndMetricName(t *testing.T) { tests := []struct { - input labels.Labels + input []prompb.Label expectedErr string description string }{ { - input: labels.FromStrings( - "__name__", "name", - "labelName", "labelValue", - ), + input: []prompb.Label{ + {Name: "__name__", Value: "name"}, + {Name: "labelName", Value: "labelValue"}, + }, expectedErr: "", description: "regular labels", }, { - input: labels.FromStrings( - "__name__", "name", - "_labelName", "labelValue", - ), + input: []prompb.Label{ + {Name: "__name__", Value: "name"}, + {Name: "_labelName", Value: "labelValue"}, + }, expectedErr: "", description: "label name with _", }, { - input: labels.FromStrings( - "__name__", "name", - "@labelName", "labelValue", - ), + input: []prompb.Label{ + {Name: "__name__", Value: "name"}, + {Name: "@labelName", Value: "labelValue"}, + }, expectedErr: "invalid label name: @labelName", description: "label name with @", }, { - input: labels.FromStrings( - "__name__", "name", - "123labelName", "labelValue", - ), + input: []prompb.Label{ + {Name: "__name__", Value: "name"}, + {Name: "123labelName", Value: "labelValue"}, + }, expectedErr: "invalid label name: 123labelName", description: "label name starts with numbers", }, { - input: labels.FromStrings( - "__name__", "name", - "", "labelValue", - ), + input: []prompb.Label{ + {Name: "__name__", Value: "name"}, + {Name: "", Value: "labelValue"}, + }, expectedErr: "invalid label name: ", description: "label name is empty string", }, { - input: labels.FromStrings( - "__name__", "name", - "labelName", string([]byte{0xff}), - ), + input: []prompb.Label{ + {Name: "__name__", Value: "name"}, + {Name: "labelName", Value: string([]byte{0xff})}, + }, expectedErr: "invalid label value: " + string([]byte{0xff}), description: "label value is an invalid UTF-8 value", }, { - input: labels.FromStrings( - "__name__", "@invalid_name", - ), + input: []prompb.Label{ + {Name: "__name__", Value: "@invalid_name"}, + }, expectedErr: "invalid metric name: @invalid_name", description: "metric name starts with @", }, { - input: labels.FromStrings( - "__name__", "name1", - "__name__", "name2", - ), + input: []prompb.Label{ + {Name: "__name__", Value: "name1"}, + {Name: "__name__", Value: "name2"}, + }, expectedErr: "duplicate label with name: __name__", description: "duplicate label names", }, { - input: labels.FromStrings( - "label1", "name", - "label2", "name", - ), + input: []prompb.Label{ + {Name: "label1", Value: "name"}, + {Name: "label2", Value: "name"}, + }, expectedErr: "", description: "duplicate label values", }, { - input: labels.FromStrings( - "", "name", - "label2", "name", - ), + input: []prompb.Label{ + {Name: "", Value: "name"}, + {Name: "label2", Value: "name"}, + }, expectedErr: "invalid label name: ", description: "don't report as duplicate label name", }, @@ -197,8 +197,7 @@ func TestConcreteSeriesClonesLabels(t *testing.T) { gotLabels := cs.Labels() require.Equal(t, lbls, gotLabels) - gotLabels[0].Value = "foo" - gotLabels[1].Value = "bar" + gotLabels.CopyFrom(labels.FromStrings("a", "foo", "c", "foo")) gotLabels = cs.Labels() require.Equal(t, lbls, gotLabels) diff --git a/storage/remote/queue_manager_test.go b/storage/remote/queue_manager_test.go index 86b4e4586..686c7c37b 100644 --- a/storage/remote/queue_manager_test.go +++ b/storage/remote/queue_manager_test.go @@ -161,7 +161,7 @@ func TestMetadataDelivery(t *testing.T) { mcfg := config.DefaultMetadataConfig metrics := newQueueManagerMetrics(nil, "", "") - m := NewQueueManager(metrics, nil, nil, nil, dir, newEWMARate(ewmaWeight, shardUpdateDuration), cfg, mcfg, nil, nil, c, defaultFlushDeadline, newPool(), newHighestTimestampMetric(), nil, false, false) + m := NewQueueManager(metrics, nil, nil, nil, dir, newEWMARate(ewmaWeight, shardUpdateDuration), cfg, mcfg, labels.EmptyLabels(), nil, c, defaultFlushDeadline, newPool(), newHighestTimestampMetric(), nil, false, false) m.Start() defer m.Stop() @@ -539,6 +539,7 @@ func TestShouldReshard(t *testing.T) { func createTimeseries(numSamples, numSeries int, extraLabels ...labels.Label) ([]record.RefSample, []record.RefSeries) { samples := make([]record.RefSample, 0, numSamples) series := make([]record.RefSeries, 0, numSeries) + b := labels.ScratchBuilder{} for i := 0; i < numSeries; i++ { name := fmt.Sprintf("test_metric_%d", i) for j := 0; j < numSamples; j++ { @@ -548,9 +549,16 @@ func createTimeseries(numSamples, numSeries int, extraLabels ...labels.Label) ([ V: float64(i), }) } + // Create Labels that is name of series plus any extra labels supplied. + b.Reset() + b.Add(labels.MetricName, name) + for _, l := range extraLabels { + b.Add(l.Name, l.Value) + } + b.Sort() series = append(series, record.RefSeries{ Ref: chunks.HeadSeriesRef(i), - Labels: append(labels.Labels{{Name: "__name__", Value: name}}, extraLabels...), + Labels: b.Labels(), }) } return samples, series @@ -603,7 +611,7 @@ func createHistograms(numSamples, numSeries int) ([]record.RefHistogramSample, [ } series = append(series, record.RefSeries{ Ref: chunks.HeadSeriesRef(i), - Labels: labels.Labels{{Name: "__name__", Value: name}}, + Labels: labels.FromStrings("__name__", name), }) } return histograms, series @@ -815,7 +823,7 @@ func BenchmarkSampleSend(b *testing.B) { const numSeries = 10000 // Extra labels to make a more realistic workload - taken from Kubernetes' embedded cAdvisor metrics. - extraLabels := labels.Labels{ + extraLabels := []labels.Label{ {Name: "kubernetes_io_arch", Value: "amd64"}, {Name: "kubernetes_io_instance_type", Value: "c3.somesize"}, {Name: "kubernetes_io_os", Value: "linux"}, @@ -902,56 +910,63 @@ func BenchmarkStartup(b *testing.B) { func TestProcessExternalLabels(t *testing.T) { for _, tc := range []struct { labels labels.Labels - externalLabels labels.Labels + externalLabels []labels.Label expected labels.Labels }{ // Test adding labels at the end. { - labels: labels.Labels{{Name: "a", Value: "b"}}, - externalLabels: labels.Labels{{Name: "c", Value: "d"}}, - expected: labels.Labels{{Name: "a", Value: "b"}, {Name: "c", Value: "d"}}, + labels: labels.FromStrings("a", "b"), + externalLabels: []labels.Label{{Name: "c", Value: "d"}}, + expected: labels.FromStrings("a", "b", "c", "d"), }, // Test adding labels at the beginning. { - labels: labels.Labels{{Name: "c", Value: "d"}}, - externalLabels: labels.Labels{{Name: "a", Value: "b"}}, - expected: labels.Labels{{Name: "a", Value: "b"}, {Name: "c", Value: "d"}}, + labels: labels.FromStrings("c", "d"), + externalLabels: []labels.Label{{Name: "a", Value: "b"}}, + expected: labels.FromStrings("a", "b", "c", "d"), }, // Test we don't override existing labels. { - labels: labels.Labels{{Name: "a", Value: "b"}}, - externalLabels: labels.Labels{{Name: "a", Value: "c"}}, - expected: labels.Labels{{Name: "a", Value: "b"}}, + labels: labels.FromStrings("a", "b"), + externalLabels: []labels.Label{{Name: "a", Value: "c"}}, + expected: labels.FromStrings("a", "b"), }, // Test empty externalLabels. { - labels: labels.Labels{{Name: "a", Value: "b"}}, - externalLabels: labels.Labels{}, - expected: labels.Labels{{Name: "a", Value: "b"}}, + labels: labels.FromStrings("a", "b"), + externalLabels: []labels.Label{}, + expected: labels.FromStrings("a", "b"), }, // Test empty labels. { - labels: labels.Labels{}, - externalLabels: labels.Labels{{Name: "a", Value: "b"}}, - expected: labels.Labels{{Name: "a", Value: "b"}}, + labels: labels.EmptyLabels(), + externalLabels: []labels.Label{{Name: "a", Value: "b"}}, + expected: labels.FromStrings("a", "b"), }, // Test labels is longer than externalLabels. { - labels: labels.Labels{{Name: "a", Value: "b"}, {Name: "c", Value: "d"}}, - externalLabels: labels.Labels{{Name: "e", Value: "f"}}, - expected: labels.Labels{{Name: "a", Value: "b"}, {Name: "c", Value: "d"}, {Name: "e", Value: "f"}}, + labels: labels.FromStrings("a", "b", "c", "d"), + externalLabels: []labels.Label{{Name: "e", Value: "f"}}, + expected: labels.FromStrings("a", "b", "c", "d", "e", "f"), }, // Test externalLabels is longer than labels. { - labels: labels.Labels{{Name: "c", Value: "d"}}, - externalLabels: labels.Labels{{Name: "a", Value: "b"}, {Name: "e", Value: "f"}}, - expected: labels.Labels{{Name: "a", Value: "b"}, {Name: "c", Value: "d"}, {Name: "e", Value: "f"}}, + labels: labels.FromStrings("c", "d"), + externalLabels: []labels.Label{{Name: "a", Value: "b"}, {Name: "e", Value: "f"}}, + expected: labels.FromStrings("a", "b", "c", "d", "e", "f"), + }, + + // Adding with and without clashing labels. + { + labels: labels.FromStrings("a", "b", "c", "d"), + externalLabels: []labels.Label{{Name: "a", Value: "xxx"}, {Name: "c", Value: "yyy"}, {Name: "e", Value: "f"}}, + expected: labels.FromStrings("a", "b", "c", "d", "e", "f"), }, } { require.Equal(t, tc.expected, processExternalLabels(tc.labels, tc.externalLabels)) diff --git a/storage/remote/read_test.go b/storage/remote/read_test.go index e3f7f4dee..6cf7d8685 100644 --- a/storage/remote/read_test.go +++ b/storage/remote/read_test.go @@ -110,7 +110,7 @@ func TestExternalLabelsQuerierAddExternalLabels(t *testing.T) { el labels.Labels inMatchers []*labels.Matcher outMatchers []*labels.Matcher - added labels.Labels + added []string }{ { inMatchers: []*labels.Matcher{ @@ -119,7 +119,7 @@ func TestExternalLabelsQuerierAddExternalLabels(t *testing.T) { outMatchers: []*labels.Matcher{ labels.MustNewMatcher(labels.MatchEqual, "job", "api-server"), }, - added: labels.Labels{}, + added: []string{}, }, { el: labels.FromStrings("dc", "berlin-01", "region", "europe"), @@ -131,7 +131,7 @@ func TestExternalLabelsQuerierAddExternalLabels(t *testing.T) { labels.MustNewMatcher(labels.MatchEqual, "region", "europe"), labels.MustNewMatcher(labels.MatchEqual, "dc", "berlin-01"), }, - added: labels.FromStrings("dc", "berlin-01", "region", "europe"), + added: []string{"dc", "region"}, }, { el: labels.FromStrings("dc", "berlin-01", "region", "europe"), @@ -144,7 +144,7 @@ func TestExternalLabelsQuerierAddExternalLabels(t *testing.T) { labels.MustNewMatcher(labels.MatchEqual, "region", "europe"), labels.MustNewMatcher(labels.MatchEqual, "dc", "munich-02"), }, - added: labels.FromStrings("region", "europe"), + added: []string{"region"}, }, } @@ -163,12 +163,12 @@ func TestExternalLabelsQuerierAddExternalLabels(t *testing.T) { func TestSeriesSetFilter(t *testing.T) { tests := []struct { in *prompb.QueryResult - toRemove labels.Labels + toRemove []string expected *prompb.QueryResult }{ { - toRemove: labels.Labels{{Name: "foo", Value: "bar"}}, + toRemove: []string{"foo"}, in: &prompb.QueryResult{ Timeseries: []*prompb.TimeSeries{ {Labels: labelsToLabelsProto(labels.FromStrings("foo", "bar", "a", "b"), nil), Samples: []prompb.Sample{}}, diff --git a/storage/remote/storage_test.go b/storage/remote/storage_test.go index 8a741f7f5..1bca61fdd 100644 --- a/storage/remote/storage_test.go +++ b/storage/remote/storage_test.go @@ -91,7 +91,7 @@ func TestFilterExternalLabels(t *testing.T) { require.NoError(t, s.ApplyConfig(conf)) require.Equal(t, 1, len(s.queryables)) - require.Equal(t, 1, len(s.queryables[0].(*sampleAndChunkQueryableClient).externalLabels)) + require.Equal(t, 1, s.queryables[0].(*sampleAndChunkQueryableClient).externalLabels.Len()) err := s.Close() require.NoError(t, err) @@ -118,7 +118,7 @@ func TestIgnoreExternalLabels(t *testing.T) { require.NoError(t, s.ApplyConfig(conf)) require.Equal(t, 1, len(s.queryables)) - require.Equal(t, 0, len(s.queryables[0].(*sampleAndChunkQueryableClient).externalLabels)) + require.Equal(t, 0, s.queryables[0].(*sampleAndChunkQueryableClient).externalLabels.Len()) err := s.Close() require.NoError(t, err) diff --git a/storage/remote/write_test.go b/storage/remote/write_test.go index 3210f9018..b1f1acbc7 100644 --- a/storage/remote/write_test.go +++ b/storage/remote/write_test.go @@ -228,14 +228,14 @@ func TestUpdateExternalLabels(t *testing.T) { require.NoError(t, err) require.NoError(t, s.ApplyConfig(conf)) require.Equal(t, 1, len(s.queues)) - require.Equal(t, labels.Labels(nil), s.queues[hash].externalLabels) + require.Equal(t, 0, len(s.queues[hash].externalLabels)) conf.GlobalConfig.ExternalLabels = externalLabels hash, err = toHash(conf.RemoteWriteConfigs[0]) require.NoError(t, err) require.NoError(t, s.ApplyConfig(conf)) require.Equal(t, 1, len(s.queues)) - require.Equal(t, externalLabels, s.queues[hash].externalLabels) + require.Equal(t, []labels.Label{{Name: "external", Value: "true"}}, s.queues[hash].externalLabels) err = s.Close() require.NoError(t, err)