From 177b4f2383fa8a910b25bd3b02a6eaab677f9b4f Mon Sep 17 00:00:00 2001 From: Bartlomiej Plotka Date: Thu, 23 Sep 2021 12:01:53 +0200 Subject: [PATCH] labels: Fixed label matching using matchers + external labels; unify tests and behaviour. (#4690) Signed-off-by: Bartlomiej Plotka --- pkg/promclient/promclient.go | 10 +- pkg/store/acceptance_test.go | 239 ++++++++++++++++++++++++++++++++++ pkg/store/prometheus.go | 67 +++++----- pkg/store/prometheus_test.go | 226 ++++---------------------------- pkg/store/tsdb.go | 36 +++-- pkg/store/tsdb_test.go | 245 ++--------------------------------- test/e2e/query_test.go | 13 +- 7 files changed, 348 insertions(+), 488 deletions(-) create mode 100644 pkg/store/acceptance_test.go diff --git a/pkg/promclient/promclient.go b/pkg/promclient/promclient.go index 45a3c49638..73043283b7 100644 --- a/pkg/promclient/promclient.go +++ b/pkg/promclient/promclient.go @@ -704,15 +704,15 @@ func (c *Client) SeriesInGRPC(ctx context.Context, base *url.URL, matchers []*la return m.Data, c.get2xxResultWithGRPCErrors(ctx, "/prom_series HTTP[client]", &u, &m) } -// LabelNames returns all known label names constrained by the given matchers. It uses gRPC errors. +// LabelNamesInGRPC returns all known label names constrained by the given matchers. It uses gRPC errors. // NOTE: This method is tested in pkg/store/prometheus_test.go against Prometheus. -func (c *Client) LabelNamesInGRPC(ctx context.Context, base *url.URL, matchers []storepb.LabelMatcher, startTime, endTime int64) ([]string, error) { +func (c *Client) LabelNamesInGRPC(ctx context.Context, base *url.URL, matchers []*labels.Matcher, startTime, endTime int64) ([]string, error) { u := *base u.Path = path.Join(u.Path, "/api/v1/labels") q := u.Query() if len(matchers) > 0 { - q.Add("match[]", storepb.MatchersToString(matchers...)) + q.Add("match[]", storepb.PromMatchersToString(matchers...)) } q.Add("start", formatTime(timestamp.Time(startTime))) q.Add("end", formatTime(timestamp.Time(endTime))) @@ -726,13 +726,13 @@ func (c *Client) LabelNamesInGRPC(ctx context.Context, base *url.URL, matchers [ // LabelValuesInGRPC returns all known label values for a given label name. It uses gRPC errors. // NOTE: This method is tested in pkg/store/prometheus_test.go against Prometheus. -func (c *Client) LabelValuesInGRPC(ctx context.Context, base *url.URL, label string, matchers []storepb.LabelMatcher, startTime, endTime int64) ([]string, error) { +func (c *Client) LabelValuesInGRPC(ctx context.Context, base *url.URL, label string, matchers []*labels.Matcher, startTime, endTime int64) ([]string, error) { u := *base u.Path = path.Join(u.Path, "/api/v1/label/", label, "/values") q := u.Query() if len(matchers) > 0 { - q.Add("match[]", storepb.MatchersToString(matchers...)) + q.Add("match[]", storepb.PromMatchersToString(matchers...)) } q.Add("start", formatTime(timestamp.Time(startTime))) q.Add("end", formatTime(timestamp.Time(endTime))) diff --git a/pkg/store/acceptance_test.go b/pkg/store/acceptance_test.go new file mode 100644 index 0000000000..56d30565b5 --- /dev/null +++ b/pkg/store/acceptance_test.go @@ -0,0 +1,239 @@ +// Copyright (c) The Thanos Authors. +// Licensed under the Apache License 2.0. + +package store + +import ( + "context" + "testing" + "time" + + "github.com/pkg/errors" + "github.com/prometheus/prometheus/pkg/labels" + "github.com/prometheus/prometheus/pkg/timestamp" + "github.com/prometheus/prometheus/storage" + "github.com/thanos-io/thanos/pkg/store/storepb" + "github.com/thanos-io/thanos/pkg/testutil" +) + +type labelNameCallCase struct { + matchers []storepb.LabelMatcher + start int64 + end int64 + + expectedNames []string + expectErr error +} + +type labelValuesCallCase struct { + label string + + matchers []storepb.LabelMatcher + start int64 + end int64 + + expectedValues []string + expectErr error +} + +// testLabelAPIs tests labels methods from StoreAPI from closed box perspective. +func testLabelAPIs(t *testing.T, startStore func(extLset labels.Labels, append func(app storage.Appender)) storepb.StoreServer) { + t.Helper() + + now := time.Now() + extLset := labels.FromStrings("region", "eu-west") + for _, tc := range []struct { + desc string + appendFn func(app storage.Appender) + labelNameCalls []labelNameCallCase + labelValuesCalls []labelValuesCallCase + }{ + { + desc: "no label in tsdb, empty results", + labelNameCalls: []labelNameCallCase{ + {start: timestamp.FromTime(minTime), end: timestamp.FromTime(maxTime)}, + }, + labelValuesCalls: []labelValuesCallCase{ + {start: timestamp.FromTime(minTime), end: timestamp.FromTime(maxTime), expectErr: errors.New("rpc error: code = InvalidArgument desc = label name parameter cannot be empty")}, + {start: timestamp.FromTime(minTime), end: timestamp.FromTime(maxTime), label: "foo"}, + {start: timestamp.FromTime(minTime), end: timestamp.FromTime(maxTime), label: "region", expectedValues: []string{"eu-west"}}, // External labels should be visible. + }, + }, + { + desc: "{foo=foovalue1} 1", + appendFn: func(app storage.Appender) { + _, err := app.Append(0, labels.FromStrings("foo", "foovalue1"), timestamp.FromTime(now), 1) + testutil.Ok(t, err) + testutil.Ok(t, app.Commit()) + }, + labelNameCalls: []labelNameCallCase{ + {start: timestamp.FromTime(minTime), end: timestamp.FromTime(maxTime), expectedNames: []string{"foo", "region"}}, + }, + labelValuesCalls: []labelValuesCallCase{ + {start: timestamp.FromTime(minTime), end: timestamp.FromTime(maxTime), label: "foo", expectedValues: []string{"foovalue1"}}, + }, + }, + { + desc: "{foo=foovalue2} 1 and {foo=foovalue2} 1", + appendFn: func(app storage.Appender) { + _, err := app.Append(0, labels.FromStrings("foo", "foovalue1"), timestamp.FromTime(now), 1) + testutil.Ok(t, err) + _, err = app.Append(0, labels.FromStrings("foo", "foovalue2"), timestamp.FromTime(now), 1) + testutil.Ok(t, err) + testutil.Ok(t, app.Commit()) + }, + labelNameCalls: []labelNameCallCase{ + {start: timestamp.FromTime(minTime), end: timestamp.FromTime(maxTime), expectedNames: []string{"foo", "region"}}, + }, + labelValuesCalls: []labelValuesCallCase{ + {start: timestamp.FromTime(minTime), end: timestamp.FromTime(maxTime), label: "foo", expectedValues: []string{"foovalue1", "foovalue2"}}, + }, + }, + { + desc: "{foo=foovalue1, bar=barvalue1} 1 and {foo=foovalue2} 1 and {foo=foovalue2} 1", + appendFn: func(app storage.Appender) { + _, err := app.Append(0, labels.FromStrings("foo", "foovalue1"), timestamp.FromTime(now), 1) + testutil.Ok(t, err) + _, err = app.Append(0, labels.FromStrings("foo", "foovalue2"), timestamp.FromTime(now), 1) + testutil.Ok(t, err) + _, err = app.Append(0, labels.FromStrings("foo", "foovalue1", "bar", "barvalue1"), timestamp.FromTime(now), 1) + testutil.Ok(t, err) + testutil.Ok(t, app.Commit()) + }, + labelNameCalls: []labelNameCallCase{ + {start: timestamp.FromTime(minTime), end: timestamp.FromTime(maxTime), expectedNames: []string{"bar", "foo", "region"}}, + // Query range outside added samples timestamp. + // NOTE: Ideally we could do 'end: timestamp.FromTime(now.Add(-1 * time.Second))'. In practice however we index labels within block range, so we approximate label and label values to chunk of block time. + {start: timestamp.FromTime(minTime), end: timestamp.FromTime(now.Add(-4 * time.Hour))}, + // Matchers on normal series. + { + start: timestamp.FromTime(minTime), + end: timestamp.FromTime(maxTime), + expectedNames: []string{"bar", "foo", "region"}, + matchers: []storepb.LabelMatcher{{Type: storepb.LabelMatcher_EQ, Name: "bar", Value: "barvalue1"}}, + }, + { + start: timestamp.FromTime(minTime), + end: timestamp.FromTime(maxTime), + expectedNames: []string{"foo", "region"}, + matchers: []storepb.LabelMatcher{{Type: storepb.LabelMatcher_EQ, Name: "foo", Value: "foovalue2"}}, + }, + { + start: timestamp.FromTime(minTime), + end: timestamp.FromTime(maxTime), + matchers: []storepb.LabelMatcher{{Type: storepb.LabelMatcher_EQ, Name: "bar", Value: "different"}}, + }, + // Matchers on external labels. + { + start: timestamp.FromTime(minTime), + end: timestamp.FromTime(maxTime), + expectedNames: []string{"bar", "foo", "region"}, + matchers: []storepb.LabelMatcher{{Type: storepb.LabelMatcher_EQ, Name: "region", Value: "eu-west"}}, + }, + { + start: timestamp.FromTime(minTime), + end: timestamp.FromTime(maxTime), + matchers: []storepb.LabelMatcher{{Type: storepb.LabelMatcher_EQ, Name: "region", Value: "different"}}, + }, + }, + labelValuesCalls: []labelValuesCallCase{ + {start: timestamp.FromTime(minTime), end: timestamp.FromTime(maxTime), label: "foo", expectedValues: []string{"foovalue1", "foovalue2"}}, + {start: timestamp.FromTime(minTime), end: timestamp.FromTime(maxTime), label: "bar", expectedValues: []string{"barvalue1"}}, + // Query range outside added samples timestamp. + // NOTE: Ideally we could do 'end: timestamp.FromTime(now.Add(-1 * time.Second))'. In practice however we index labels within block range, so we approximate label and label values to chunk of block time. + {start: timestamp.FromTime(minTime), end: timestamp.FromTime(now.Add(-4 * time.Hour)), label: "foo"}, + {start: timestamp.FromTime(minTime), end: timestamp.FromTime(now.Add(-4 * time.Hour)), label: "bar"}, + // Matchers on normal series. + { + start: timestamp.FromTime(minTime), + end: timestamp.FromTime(maxTime), + label: "foo", + expectedValues: []string{"foovalue1"}, + matchers: []storepb.LabelMatcher{{Type: storepb.LabelMatcher_EQ, Name: "bar", Value: "barvalue1"}}, + }, + { + start: timestamp.FromTime(minTime), + end: timestamp.FromTime(maxTime), + label: "foo", + matchers: []storepb.LabelMatcher{{Type: storepb.LabelMatcher_EQ, Name: "bar", Value: "different"}}, + }, + // Matchers on external labels. + { + start: timestamp.FromTime(minTime), + end: timestamp.FromTime(maxTime), + label: "foo", + expectedValues: []string{"foovalue1", "foovalue2"}, + matchers: []storepb.LabelMatcher{{Type: storepb.LabelMatcher_EQ, Name: "region", Value: "eu-west"}}, + }, + { + start: timestamp.FromTime(minTime), + end: timestamp.FromTime(maxTime), + label: "bar", + expectedValues: []string{"barvalue1"}, + matchers: []storepb.LabelMatcher{{Type: storepb.LabelMatcher_EQ, Name: "region", Value: "eu-west"}}, + }, + { + start: timestamp.FromTime(minTime), + end: timestamp.FromTime(maxTime), + label: "foo", + matchers: []storepb.LabelMatcher{{Type: storepb.LabelMatcher_EQ, Name: "region", Value: "different"}}, + }, + { + start: timestamp.FromTime(minTime), + end: timestamp.FromTime(maxTime), + label: "bar", + matchers: []storepb.LabelMatcher{{Type: storepb.LabelMatcher_EQ, Name: "region", Value: "different"}}, + }, + }, + }, + } { + t.Run(tc.desc, func(t *testing.T) { + appendFn := tc.appendFn + if appendFn == nil { + appendFn = func(storage.Appender) {} + } + store := startStore(extLset, appendFn) + for _, c := range tc.labelNameCalls { + t.Run("label_names", func(t *testing.T) { + resp, err := store.LabelNames(context.Background(), &storepb.LabelNamesRequest{ + Start: c.start, + End: c.end, + Matchers: c.matchers, + }) + if c.expectErr != nil { + testutil.NotOk(t, err) + testutil.Equals(t, c.expectErr.Error(), err.Error()) + return + } + testutil.Ok(t, err) + testutil.Equals(t, 0, len(resp.Warnings)) + if len(resp.Names) == 0 { + resp.Names = nil + } + testutil.Equals(t, c.expectedNames, resp.Names) + }) + } + for _, c := range tc.labelValuesCalls { + t.Run("label_name_values", func(t *testing.T) { + resp, err := store.LabelValues(context.Background(), &storepb.LabelValuesRequest{ + Start: c.start, + End: c.end, + Label: c.label, + Matchers: c.matchers, + }) + if c.expectErr != nil { + testutil.NotOk(t, err) + testutil.Equals(t, c.expectErr.Error(), err.Error()) + return + } + testutil.Ok(t, err) + testutil.Equals(t, 0, len(resp.Warnings)) + if len(resp.Values) == 0 { + resp.Values = nil + } + testutil.Equals(t, c.expectedValues, resp.Values) + }) + } + }) + } +} diff --git a/pkg/store/prometheus.go b/pkg/store/prometheus.go index 3b024cc273..2952e4467b 100644 --- a/pkg/store/prometheus.go +++ b/pkg/store/prometheus.go @@ -508,25 +508,24 @@ func (p *PrometheusStore) encodeChunk(ss []prompb.Sample) (storepb.Chunk_Encodin // LabelNames returns all known label names of series that match the given matchers. func (p *PrometheusStore) LabelNames(ctx context.Context, r *storepb.LabelNamesRequest) (*storepb.LabelNamesResponse, error) { - lnc := false - v := p.promVersion() - lbls := []string{} + extLset := p.externalLabelsFn() - version, err := semver.Parse(v) - if err == nil && version.GTE(baseVer) { - lnc = true + match, matchers, err := matchesExternalLabels(r.Matchers, extLset) + if err != nil { + return nil, status.Error(codes.InvalidArgument, err.Error()) + } + if !match { + return &storepb.LabelNamesResponse{Names: nil}, nil } - if lnc || len(r.Matchers) == 0 { - lbls, err = p.client.LabelNamesInGRPC(ctx, p.base, r.Matchers, r.Start, r.End) + var lbls []string + version, parseErr := semver.Parse(p.promVersion()) + if len(matchers) == 0 || (parseErr == nil && version.GTE(baseVer)) { + lbls, err = p.client.LabelNamesInGRPC(ctx, p.base, matchers, r.Start, r.End) if err != nil { return nil, err } } else { - matchers, err := storepb.MatchersToPromMatchers(r.Matchers...) - if err != nil { - return nil, status.Error(codes.Internal, err.Error()) - } sers, err := p.client.SeriesInGRPC(ctx, p.base, matchers, r.Start, r.End) if err != nil { return nil, err @@ -545,42 +544,49 @@ func (p *PrometheusStore) LabelNames(ctx context.Context, r *storepb.LabelNamesR } } + if len(lbls) > 0 { + for _, extLbl := range extLset { + lbls = append(lbls, extLbl.Name) + } + sort.Strings(lbls) + } + return &storepb.LabelNamesResponse{Names: lbls}, nil } // LabelValues returns all known label values for a given label name. func (p *PrometheusStore) LabelValues(ctx context.Context, r *storepb.LabelValuesRequest) (*storepb.LabelValuesResponse, error) { - externalLset := p.externalLabelsFn() + if r.Label == "" { + return nil, status.Error(codes.InvalidArgument, "label name parameter cannot be empty") + } + + extLset := p.externalLabelsFn() // First check for matching external label which has priority. - if l := externalLset.Get(r.Label); l != "" { + if l := extLset.Get(r.Label); l != "" { return &storepb.LabelValuesResponse{Values: []string{l}}, nil } + match, matchers, err := matchesExternalLabels(r.Matchers, extLset) + if err != nil { + return nil, status.Error(codes.InvalidArgument, err.Error()) + } + if !match { + return &storepb.LabelValuesResponse{Values: nil}, nil + } + var ( sers []map[string]string - err error + vals []string ) - lvc := false // LabelValuesCall - vals := []string{} - v := p.promVersion() - - version, err := semver.Parse(v) - if err == nil && version.GTE(baseVer) { - lvc = true - } - - if len(r.Matchers) == 0 || lvc { - vals, err = p.client.LabelValuesInGRPC(ctx, p.base, r.Label, r.Matchers, r.Start, r.End) + version, parseErr := semver.Parse(p.promVersion()) + if len(matchers) == 0 || (parseErr == nil && version.GTE(baseVer)) { + vals, err = p.client.LabelValuesInGRPC(ctx, p.base, r.Label, matchers, r.Start, r.End) if err != nil { return nil, err } } else { - matchers, err := storepb.MatchersToPromMatchers(r.Matchers...) - if err != nil { - return nil, status.Error(codes.Internal, err.Error()) - } sers, err = p.client.SeriesInGRPC(ctx, p.base, matchers, r.Start, r.End) if err != nil { return nil, err @@ -597,6 +603,7 @@ func (p *PrometheusStore) LabelValues(ctx context.Context, r *storepb.LabelValue vals = append(vals, key) } } + sort.Strings(vals) return &storepb.LabelValuesResponse{Values: vals}, nil } diff --git a/pkg/store/prometheus_test.go b/pkg/store/prometheus_test.go index cf32127e92..a532a70db6 100644 --- a/pkg/store/prometheus_test.go +++ b/pkg/store/prometheus_test.go @@ -161,15 +161,7 @@ func expandChunk(cit chunkenc.Iterator) (res []sample) { return res } -func getExternalLabels() labels.Labels { - return labels.Labels{ - {Name: "ext_a", Value: "a"}, - {Name: "ext_b", Value: "a"}} -} - func TestPrometheusStore_SeriesLabels_e2e(t *testing.T) { - t.Helper() - defer testutil.TolerantVerifyLeak(t) p, err := e2eutil.NewPrometheus() @@ -351,201 +343,32 @@ func TestPrometheusStore_SeriesLabels_e2e(t *testing.T) { } } -// Tests retrieving label names and their values via the gRPC API. -func TestPrometheusStore_LabelAPIs_e2e(t *testing.T) { - defer testutil.TolerantVerifyLeak(t) - - p, err := e2eutil.NewPrometheus() - testutil.Ok(t, err) - defer func() { testutil.Ok(t, p.Stop()) }() - - a := p.Appender() - _, err = a.Append(0, labels.FromStrings("a", "b"), 0, 1) - testutil.Ok(t, err) - _, err = a.Append(0, labels.FromStrings("bb", "c"), 0, 1) - testutil.Ok(t, err) - _, err = a.Append(0, labels.FromStrings("a", "a"), 0, 1) - testutil.Ok(t, err) - testutil.Ok(t, a.Commit()) - - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - - testutil.Ok(t, p.Start()) - - u, err := url.Parse(fmt.Sprintf("http://%s", p.Addr())) - testutil.Ok(t, err) - - version, err := promclient.NewDefaultClient().BuildVersion(ctx, u) - testutil.Ok(t, err) - - proxy, err := NewPrometheusStore(nil, nil, promclient.NewDefaultClient(), u, component.Sidecar, getExternalLabels, nil, - func() string { return version }) - testutil.Ok(t, err) - - // All values/names. - respValues, err := proxy.LabelValues(ctx, &storepb.LabelValuesRequest{ - Label: "a", - Start: timestamp.FromTime(minTime), - End: timestamp.FromTime(maxTime), - }) - testutil.Ok(t, err) - testutil.Equals(t, []string(nil), respValues.Warnings) - testutil.Equals(t, []string{"a", "b"}, respValues.Values) - - respValues, err = proxy.LabelValues(ctx, &storepb.LabelValuesRequest{ - Label: "bb", - Start: timestamp.FromTime(minTime), - End: timestamp.FromTime(maxTime), - }) - testutil.Ok(t, err) - testutil.Equals(t, []string(nil), respValues.Warnings) - testutil.Equals(t, []string{"c"}, respValues.Values) - - respNames, err := proxy.LabelNames(ctx, &storepb.LabelNamesRequest{ - Start: timestamp.FromTime(minTime), - End: timestamp.FromTime(maxTime), - }) - testutil.Ok(t, err) - testutil.Equals(t, []string(nil), respNames.Warnings) - testutil.Equals(t, []string{"a", "bb"}, respNames.Names) - - // Outside time range. - respValues, err = proxy.LabelValues(ctx, &storepb.LabelValuesRequest{ - Label: "a", - Start: timestamp.FromTime(maxTime.Add(-time.Second)), - End: timestamp.FromTime(maxTime), - }) - testutil.Ok(t, err) - testutil.Equals(t, []string(nil), respValues.Warnings) - testutil.Equals(t, []string{}, respValues.Values) - - respNames, err = proxy.LabelNames(ctx, &storepb.LabelNamesRequest{ - Start: timestamp.FromTime(maxTime.Add(-time.Second)), - End: timestamp.FromTime(maxTime), - }) - testutil.Ok(t, err) - testutil.Equals(t, []string(nil), respNames.Warnings) - testutil.Equals(t, []string{}, respNames.Names) - - // With a matching matcher. - respValues, err = proxy.LabelValues(ctx, &storepb.LabelValuesRequest{ - Label: "a", - Start: timestamp.FromTime(minTime), - End: timestamp.FromTime(maxTime), - Matchers: []storepb.LabelMatcher{ - {Type: storepb.LabelMatcher_EQ, Name: "a", Value: "b"}, - }, - }) - testutil.Ok(t, err) - testutil.Equals(t, []string(nil), respValues.Warnings) - testutil.Equals(t, []string{"b"}, respValues.Values) - - respValues, err = proxy.LabelValues(ctx, &storepb.LabelValuesRequest{ - Label: "bb", - Start: timestamp.FromTime(minTime), - End: timestamp.FromTime(maxTime), - Matchers: []storepb.LabelMatcher{ - {Type: storepb.LabelMatcher_EQ, Name: "bb", Value: "c"}, - }, - }) - testutil.Ok(t, err) - testutil.Equals(t, []string(nil), respValues.Warnings) - testutil.Equals(t, []string{"c"}, respValues.Values) - - respValues, err = proxy.LabelValues(ctx, &storepb.LabelValuesRequest{ - Label: "a", - Start: timestamp.FromTime(minTime), - End: timestamp.FromTime(maxTime), - Matchers: []storepb.LabelMatcher{ - {Type: storepb.LabelMatcher_EQ, Name: "a", Value: "a"}, - }, - }) - testutil.Ok(t, err) - testutil.Equals(t, []string(nil), respValues.Warnings) - testutil.Equals(t, []string{"a"}, respValues.Values) - - respNames, err = proxy.LabelNames(ctx, &storepb.LabelNamesRequest{ - Start: timestamp.FromTime(minTime), - End: timestamp.FromTime(maxTime), - Matchers: []storepb.LabelMatcher{ - {Type: storepb.LabelMatcher_EQ, Name: "a", Value: "b"}, - }, - }) - testutil.Ok(t, err) - testutil.Equals(t, []string(nil), respNames.Warnings) - testutil.Equals(t, []string{"a"}, respNames.Names) - - // A matcher that does not match anything. - respValues, err = proxy.LabelValues(ctx, &storepb.LabelValuesRequest{ - Label: "a", - Start: timestamp.FromTime(minTime), - End: timestamp.FromTime(maxTime), - Matchers: []storepb.LabelMatcher{ - {Type: storepb.LabelMatcher_EQ, Name: "a", Value: "d"}, - }, - }) - testutil.Ok(t, err) - testutil.Equals(t, []string(nil), respValues.Warnings) - testutil.Equals(t, []string{}, respValues.Values) - - respNames, err = proxy.LabelNames(ctx, &storepb.LabelNamesRequest{ - Start: timestamp.FromTime(minTime), - End: timestamp.FromTime(maxTime), - Matchers: []storepb.LabelMatcher{ - {Type: storepb.LabelMatcher_EQ, Name: "a", Value: "d"}, - }, - }) - testutil.Ok(t, err) - testutil.Equals(t, []string(nil), respNames.Warnings) - testutil.Equals(t, []string{}, respNames.Names) -} - -// Test to check external label values retrieve. -func TestPrometheusStore_ExternalLabelValues_e2e(t *testing.T) { - defer testutil.TolerantVerifyLeak(t) - - p, err := e2eutil.NewPrometheus() - testutil.Ok(t, err) - defer func() { testutil.Ok(t, p.Stop()) }() - - a := p.Appender() - _, err = a.Append(0, labels.FromStrings("ext_a", "b"), 0, 1) - testutil.Ok(t, err) - _, err = a.Append(0, labels.FromStrings("a", "b"), 0, 1) - testutil.Ok(t, err) - testutil.Ok(t, a.Commit()) - - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - - testutil.Ok(t, p.Start()) - - u, err := url.Parse(fmt.Sprintf("http://%s", p.Addr())) - testutil.Ok(t, err) +func TestPrometheusStore_LabelAPIs(t *testing.T) { + t.Cleanup(func() { testutil.TolerantVerifyLeak(t) }) + testLabelAPIs(t, func(extLset labels.Labels, appendFn func(app storage.Appender)) storepb.StoreServer { + p, err := e2eutil.NewPrometheus() + testutil.Ok(t, err) + t.Cleanup(func() { testutil.Ok(t, p.Stop()) }) - version, err := promclient.NewDefaultClient().BuildVersion(ctx, u) - testutil.Ok(t, err) + appendFn(p.Appender()) - proxy, err := NewPrometheusStore(nil, nil, promclient.NewDefaultClient(), u, component.Sidecar, getExternalLabels, nil, func() string { return version }) - testutil.Ok(t, err) + testutil.Ok(t, p.Start()) + u, err := url.Parse(fmt.Sprintf("http://%s", p.Addr())) + testutil.Ok(t, err) - resp, err := proxy.LabelValues(ctx, &storepb.LabelValuesRequest{ - Label: "ext_a", - }) - testutil.Ok(t, err) + version, err := promclient.NewDefaultClient().BuildVersion(context.Background(), u) + testutil.Ok(t, err) - testutil.Equals(t, []string{"a"}, resp.Values) + promStore, err := NewPrometheusStore(nil, nil, promclient.NewDefaultClient(), u, component.Sidecar, func() labels.Labels { + return extLset + }, nil, func() string { return version }) + testutil.Ok(t, err) - resp, err = proxy.LabelValues(ctx, &storepb.LabelValuesRequest{ - Label: "a", + return promStore }) - testutil.Ok(t, err) - - testutil.Equals(t, []string{"b"}, resp.Values) } -func TestPrometheusStore_Series_MatchExternalLabel_e2e(t *testing.T) { +func TestPrometheusStore_Series_MatchExternalLabel(t *testing.T) { defer testutil.TolerantVerifyLeak(t) p, err := e2eutil.NewPrometheus() @@ -577,16 +400,14 @@ func TestPrometheusStore_Series_MatchExternalLabel_e2e(t *testing.T) { testutil.Ok(t, err) srv := newStoreSeriesServer(ctx) - err = proxy.Series(&storepb.SeriesRequest{ + testutil.Ok(t, proxy.Series(&storepb.SeriesRequest{ MinTime: baseT + 101, MaxTime: baseT + 300, Matchers: []storepb.LabelMatcher{ {Type: storepb.LabelMatcher_EQ, Name: "a", Value: "b"}, {Type: storepb.LabelMatcher_EQ, Name: "region", Value: "eu-west"}, }, - }, srv) - testutil.Ok(t, err) - + }, srv)) testutil.Equals(t, 1, len(srv.SeriesSet)) testutil.Equals(t, []labelpb.ZLabel{ @@ -595,16 +416,15 @@ func TestPrometheusStore_Series_MatchExternalLabel_e2e(t *testing.T) { }, srv.SeriesSet[0].Labels) srv = newStoreSeriesServer(ctx) - // However it should not match wrong external label. - err = proxy.Series(&storepb.SeriesRequest{ + // However, it should not match wrong external label. + testutil.Ok(t, proxy.Series(&storepb.SeriesRequest{ MinTime: baseT + 101, MaxTime: baseT + 300, Matchers: []storepb.LabelMatcher{ {Type: storepb.LabelMatcher_EQ, Name: "a", Value: "b"}, {Type: storepb.LabelMatcher_EQ, Name: "region", Value: "eu-west2"}, // Non existing label value. }, - }, srv) - testutil.Ok(t, err) + }, srv)) // No series. testutil.Equals(t, 0, len(srv.SeriesSet)) diff --git a/pkg/store/tsdb.go b/pkg/store/tsdb.go index 09de901be3..a58c86bc62 100644 --- a/pkg/store/tsdb.go +++ b/pkg/store/tsdb.go @@ -199,18 +199,22 @@ func (s *TSDBStore) Series(r *storepb.SeriesRequest, srv storepb.Store_SeriesSer func (s *TSDBStore) LabelNames(ctx context.Context, r *storepb.LabelNamesRequest) ( *storepb.LabelNamesResponse, error, ) { - q, err := s.db.ChunkQuerier(ctx, r.Start, r.End) + match, matchers, err := matchesExternalLabels(r.Matchers, s.extLset) if err != nil { - return nil, status.Error(codes.Internal, err.Error()) + return nil, status.Error(codes.InvalidArgument, err.Error()) + } + + if !match { + return &storepb.LabelNamesResponse{Names: nil}, nil } - defer runutil.CloseWithLogOnErr(s.logger, q, "close tsdb querier label names") - promMatchers, err := storepb.MatchersToPromMatchers(r.Matchers...) + q, err := s.db.ChunkQuerier(ctx, r.Start, r.End) if err != nil { return nil, status.Error(codes.Internal, err.Error()) } + defer runutil.CloseWithLogOnErr(s.logger, q, "close tsdb querier label names") - res, _, err := q.LabelNames(promMatchers...) + res, _, err := q.LabelNames(matchers...) if err != nil { return nil, status.Error(codes.Internal, err.Error()) } @@ -229,19 +233,33 @@ func (s *TSDBStore) LabelNames(ctx context.Context, r *storepb.LabelNamesRequest func (s *TSDBStore) LabelValues(ctx context.Context, r *storepb.LabelValuesRequest) ( *storepb.LabelValuesResponse, error, ) { - q, err := s.db.ChunkQuerier(ctx, r.Start, r.End) + if r.Label == "" { + return nil, status.Error(codes.InvalidArgument, "label name parameter cannot be empty") + } + + match, matchers, err := matchesExternalLabels(r.Matchers, s.extLset) if err != nil { - return nil, status.Error(codes.Internal, err.Error()) + return nil, status.Error(codes.InvalidArgument, err.Error()) + } + + if !match { + return &storepb.LabelValuesResponse{Values: nil}, nil } - defer runutil.CloseWithLogOnErr(s.logger, q, "close tsdb querier label values") - matchers, err := storepb.MatchersToPromMatchers(r.Matchers...) + if v := s.extLset.Get(r.Label); v != "" { + return &storepb.LabelValuesResponse{Values: []string{v}}, nil + } + + q, err := s.db.ChunkQuerier(ctx, r.Start, r.End) if err != nil { return nil, status.Error(codes.Internal, err.Error()) } + defer runutil.CloseWithLogOnErr(s.logger, q, "close tsdb querier label values") + res, _, err := q.LabelValues(r.Label, matchers...) if err != nil { return nil, status.Error(codes.Internal, err.Error()) } + return &storepb.LabelValuesResponse{Values: res}, nil } diff --git a/pkg/store/tsdb_test.go b/pkg/store/tsdb_test.go index 0ba2ac9329..0a644033a6 100644 --- a/pkg/store/tsdb_test.go +++ b/pkg/store/tsdb_test.go @@ -13,11 +13,10 @@ import ( "os" "sort" "testing" - "time" "github.com/go-kit/kit/log" "github.com/prometheus/prometheus/pkg/labels" - "github.com/prometheus/prometheus/pkg/timestamp" + "github.com/prometheus/prometheus/storage" "github.com/prometheus/prometheus/tsdb" "github.com/thanos-io/thanos/pkg/component" "github.com/thanos-io/thanos/pkg/store/labelpb" @@ -188,240 +187,18 @@ func TestTSDBStore_Series(t *testing.T) { } } -func TestTSDBStore_LabelNames(t *testing.T) { - defer testutil.TolerantVerifyLeak(t) - - var err error - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - - db, err := e2eutil.NewTSDB() - defer func() { testutil.Ok(t, db.Close()) }() - testutil.Ok(t, err) - - appender := db.Appender(context.Background()) - addLabels := func(lbs []string, timestamp int64) { - if len(lbs) > 0 { - _, err = appender.Append(0, labels.FromStrings(lbs...), timestamp, 1) - testutil.Ok(t, err) - } - } - - tsdbStore := NewTSDBStore(nil, db, component.Rule, labels.FromStrings("region", "eu-west")) - - now := time.Now() - head := db.Head() - for _, tc := range []struct { - title string - labels []string - expectedNames []string - timestamp int64 - start func() int64 - end func() int64 - }{ - { - title: "no label in tsdb", - labels: []string{}, - timestamp: now.Unix(), - start: func() int64 { - return timestamp.FromTime(minTime) - }, - end: func() int64 { - return timestamp.FromTime(maxTime) - }, - }, - { - title: "add one label", - labels: []string{"foo", "foo"}, - expectedNames: []string{"foo", "region"}, - timestamp: now.Unix(), - start: func() int64 { - return timestamp.FromTime(minTime) - }, - end: func() int64 { - return timestamp.FromTime(maxTime) - }, - }, - { - title: "add another label", - labels: []string{"bar", "bar"}, - // We will get two labels here. - expectedNames: []string{"bar", "foo", "region"}, - timestamp: now.Unix(), - start: func() int64 { - return timestamp.FromTime(minTime) - }, - end: func() int64 { - return timestamp.FromTime(maxTime) - }, - }, - { - title: "query range outside tsdb head", - labels: []string{}, - timestamp: now.Unix(), - start: func() int64 { - return timestamp.FromTime(minTime) - }, - end: func() int64 { - return head.MinTime() - 1 - }, - }, - { - title: "get all labels", - labels: []string{"buz", "buz"}, - expectedNames: []string{"bar", "buz", "foo", "region"}, - timestamp: now.Unix(), - start: func() int64 { - return timestamp.FromTime(minTime) - }, - end: func() int64 { - return timestamp.FromTime(maxTime) - }, - }, - } { - if ok := t.Run(tc.title, func(t *testing.T) { - addLabels(tc.labels, tc.timestamp) - resp, err := tsdbStore.LabelNames(ctx, &storepb.LabelNamesRequest{ - Start: tc.start(), - End: tc.end(), - }) - testutil.Ok(t, err) - testutil.Equals(t, tc.expectedNames, resp.Names) - testutil.Equals(t, 0, len(resp.Warnings)) - }); !ok { - return - } - } -} - -func TestTSDBStore_LabelValues(t *testing.T) { - defer testutil.TolerantVerifyLeak(t) - - var err error - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() +func TestTSDBStore_LabelAPIs(t *testing.T) { + t.Cleanup(func() { testutil.TolerantVerifyLeak(t) }) + testLabelAPIs(t, func(extLset labels.Labels, appendFn func(app storage.Appender)) storepb.StoreServer { + db, err := e2eutil.NewTSDB() + testutil.Ok(t, err) + t.Cleanup(func() { testutil.Ok(t, db.Close()) }) - db, err := e2eutil.NewTSDB() - defer func() { testutil.Ok(t, db.Close()) }() - testutil.Ok(t, err) + tsdbStore := NewTSDBStore(nil, db, component.Rule, extLset) - appender := db.Appender(context.Background()) - addLabels := func(lbs []string, timestamp int64) { - if len(lbs) > 0 { - _, err = appender.Append(0, labels.FromStrings(lbs...), timestamp, 1) - testutil.Ok(t, err) - } - } - - tsdbStore := NewTSDBStore(nil, db, component.Rule, labels.FromStrings("region", "eu-west")) - now := time.Now() - head := db.Head() - for _, tc := range []struct { - title string - addedLabels []string - queryLabel string - expectedValues []string - timestamp int64 - start func() int64 - end func() int64 - Matchers []storepb.LabelMatcher - }{ - { - title: "no label in tsdb", - addedLabels: []string{}, - queryLabel: "foo", - timestamp: now.Unix(), - start: func() int64 { - return timestamp.FromTime(minTime) - }, - end: func() int64 { - return timestamp.FromTime(maxTime) - }, - }, - { - title: "add one label value", - addedLabels: []string{"foo", "test"}, - queryLabel: "foo", - expectedValues: []string{"test"}, - timestamp: now.Unix(), - start: func() int64 { - return timestamp.FromTime(minTime) - }, - end: func() int64 { - return timestamp.FromTime(maxTime) - }, - }, - { - title: "add another label value", - addedLabels: []string{"foo", "test1"}, - queryLabel: "foo", - expectedValues: []string{"test", "test1"}, - timestamp: now.Unix(), - start: func() int64 { - return timestamp.FromTime(minTime) - }, - end: func() int64 { - return timestamp.FromTime(maxTime) - }, - }, - { - title: "check label value matcher", - queryLabel: "foo", - expectedValues: []string{"test1"}, - timestamp: now.Unix(), - start: func() int64 { - return timestamp.FromTime(minTime) - }, - end: func() int64 { - return timestamp.FromTime(maxTime) - }, - Matchers: []storepb.LabelMatcher{ - {Type: storepb.LabelMatcher_EQ, Name: "foo", Value: "test1"}, - }, - }, - { - title: "check another label value matcher", - queryLabel: "foo", - expectedValues: []string{}, - timestamp: now.Unix(), - start: func() int64 { - return timestamp.FromTime(minTime) - }, - end: func() int64 { - return timestamp.FromTime(maxTime) - }, - Matchers: []storepb.LabelMatcher{ - {Type: storepb.LabelMatcher_EQ, Name: "foo", Value: "test2"}, - }, - }, - { - title: "query time range outside head", - addedLabels: []string{}, - queryLabel: "foo", - timestamp: now.Unix(), - start: func() int64 { - return timestamp.FromTime(minTime) - }, - end: func() int64 { - return head.MinTime() - 1 - }, - }, - } { - if ok := t.Run(tc.title, func(t *testing.T) { - addLabels(tc.addedLabels, tc.timestamp) - resp, err := tsdbStore.LabelValues(ctx, &storepb.LabelValuesRequest{ - Label: tc.queryLabel, - Start: tc.start(), - End: tc.end(), - Matchers: tc.Matchers, - }) - testutil.Ok(t, err) - testutil.Equals(t, tc.expectedValues, resp.Values) - testutil.Equals(t, 0, len(resp.Warnings)) - }); !ok { - return - } - } + appendFn(db.Appender(context.Background())) + return tsdbStore + }) } // Regression test for https://github.com/thanos-io/thanos/issues/1038. diff --git a/test/e2e/query_test.go b/test/e2e/query_test.go index eca3af1b20..8c34a28283 100644 --- a/test/e2e/query_test.go +++ b/test/e2e/query_test.go @@ -31,7 +31,6 @@ import ( "github.com/thanos-io/thanos/pkg/exemplars/exemplarspb" "github.com/thanos-io/thanos/pkg/promclient" "github.com/thanos-io/thanos/pkg/runutil" - "github.com/thanos-io/thanos/pkg/store/storepb" "github.com/thanos-io/thanos/pkg/testutil" "github.com/thanos-io/thanos/test/e2e/e2ethanos" ) @@ -282,7 +281,7 @@ func TestQueryLabelNames(t *testing.T) { return len(res) == 0 }) - labelNames(t, ctx, q.HTTPEndpoint(), []storepb.LabelMatcher{{Type: storepb.LabelMatcher_EQ, Name: "__name__", Value: "up"}}, + labelNames(t, ctx, q.HTTPEndpoint(), []*labels.Matcher{{Type: labels.MatchEqual, Name: "__name__", Value: "up"}}, timestamp.FromTime(now.Add(-time.Hour)), timestamp.FromTime(now.Add(time.Hour)), func(res []string) bool { // Expected result: [__name__, instance, job, prometheus, replica, receive, tenant_id] // Pre-labelnames pushdown we've done Select() over all series and picked out the label names hence they all had external labels. @@ -292,7 +291,7 @@ func TestQueryLabelNames(t *testing.T) { ) // There is no matched series. - labelNames(t, ctx, q.HTTPEndpoint(), []storepb.LabelMatcher{{Type: storepb.LabelMatcher_EQ, Name: "__name__", Value: "foobar"}}, + labelNames(t, ctx, q.HTTPEndpoint(), []*labels.Matcher{{Type: labels.MatchEqual, Name: "__name__", Value: "foobar"}}, timestamp.FromTime(now.Add(-time.Hour)), timestamp.FromTime(now.Add(time.Hour)), func(res []string) bool { return len(res) == 0 }, @@ -333,13 +332,13 @@ func TestQueryLabelValues(t *testing.T) { return len(res) == 0 }) - labelValues(t, ctx, q.HTTPEndpoint(), "__name__", []storepb.LabelMatcher{{Type: storepb.LabelMatcher_EQ, Name: "__name__", Value: "up"}}, + labelValues(t, ctx, q.HTTPEndpoint(), "__name__", []*labels.Matcher{{Type: labels.MatchEqual, Name: "__name__", Value: "up"}}, timestamp.FromTime(now.Add(-time.Hour)), timestamp.FromTime(now.Add(time.Hour)), func(res []string) bool { return len(res) == 1 && res[0] == "up" }, ) - labelValues(t, ctx, q.HTTPEndpoint(), "__name__", []storepb.LabelMatcher{{Type: storepb.LabelMatcher_EQ, Name: "__name__", Value: "foobar"}}, + labelValues(t, ctx, q.HTTPEndpoint(), "__name__", []*labels.Matcher{{Type: labels.MatchEqual, Name: "__name__", Value: "foobar"}}, timestamp.FromTime(now.Add(-time.Hour)), timestamp.FromTime(now.Add(time.Hour)), func(res []string) bool { return len(res) == 0 }, @@ -616,7 +615,7 @@ func queryAndAssert(t *testing.T, ctx context.Context, addr, q string, opts prom testutil.Equals(t, expected, result) } -func labelNames(t *testing.T, ctx context.Context, addr string, matchers []storepb.LabelMatcher, start, end int64, check func(res []string) bool) { +func labelNames(t *testing.T, ctx context.Context, addr string, matchers []*labels.Matcher, start, end int64, check func(res []string) bool) { t.Helper() logger := log.NewLogfmtLogger(os.Stdout) @@ -635,7 +634,7 @@ func labelNames(t *testing.T, ctx context.Context, addr string, matchers []store } //nolint:unparam -func labelValues(t *testing.T, ctx context.Context, addr, label string, matchers []storepb.LabelMatcher, start, end int64, check func(res []string) bool) { +func labelValues(t *testing.T, ctx context.Context, addr, label string, matchers []*labels.Matcher, start, end int64, check func(res []string) bool) { t.Helper() logger := log.NewLogfmtLogger(os.Stdout)