From 8f9a57f41d95981b1d81e1c55597c5230434dc87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Giedrius=20Statkevi=C4=8Dius?= Date: Mon, 30 Aug 2021 18:04:03 +0300 Subject: [PATCH] store: fix e2e labelname tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Giedrius Statkevičius --- pkg/promclient/promclient.go | 2 +- pkg/query/querier.go | 3 ++- pkg/store/multitsdb.go | 2 +- pkg/store/storepb/rpc.pb.go | 6 ++---- pkg/store/storepb/rpc.proto | 3 +-- pkg/store/tsdb.go | 11 ++++++++++- pkg/store/tsdb_test.go | 6 +++--- test/e2e/query_test.go | 6 ++++-- 8 files changed, 24 insertions(+), 15 deletions(-) diff --git a/pkg/promclient/promclient.go b/pkg/promclient/promclient.go index 614bf9df68..cf5ca3ee3e 100644 --- a/pkg/promclient/promclient.go +++ b/pkg/promclient/promclient.go @@ -702,7 +702,7 @@ 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. It uses gRPC errors. +// LabelNames 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) { u := *base diff --git a/pkg/query/querier.go b/pkg/query/querier.go index bfff5f24fc..964b2c9439 100644 --- a/pkg/query/querier.go +++ b/pkg/query/querier.go @@ -365,7 +365,8 @@ func (q *querier) LabelValues(name string, matchers ...*labels.Matcher) ([]strin return resp.Values, warns, nil } -// LabelNames returns all the unique label names present in the block in sorted order. +// LabelNames returns all the unique label names present in the block in sorted order constrained +// by the given matchers. func (q *querier) LabelNames(matchers ...*labels.Matcher) ([]string, storage.Warnings, error) { span, ctx := tracing.StartSpan(q.ctx, "querier_label_names") defer span.Finish() diff --git a/pkg/store/multitsdb.go b/pkg/store/multitsdb.go index d57753eb6a..0069b32908 100644 --- a/pkg/store/multitsdb.go +++ b/pkg/store/multitsdb.go @@ -269,7 +269,7 @@ func (s *MultiTSDBStore) Series(r *storepb.SeriesRequest, srv storepb.Store_Seri } -// LabelNames returns all known label names. +// LabelNames returns all known label names constrained by the matchers. func (s *MultiTSDBStore) LabelNames(ctx context.Context, req *storepb.LabelNamesRequest) (*storepb.LabelNamesResponse, error) { span, ctx := tracing.StartSpan(ctx, "multitsdb_label_names") defer span.Finish() diff --git a/pkg/store/storepb/rpc.pb.go b/pkg/store/storepb/rpc.pb.go index 9c0ddf4ae8..969578c0ee 100644 --- a/pkg/store/storepb/rpc.pb.go +++ b/pkg/store/storepb/rpc.pb.go @@ -703,8 +703,7 @@ type StoreClient interface { /// There is no requirements on chunk sorting, however it is recommended to have chunk sorted by chunk min time. /// This heavily optimizes the resource usage on Querier / Federated Queries. Series(ctx context.Context, in *SeriesRequest, opts ...grpc.CallOption) (Store_SeriesClient, error) - /// LabelNames returns all label names that is available. - /// Currently unimplemented in all Thanos implementations, because Query API does not implement this either. + /// LabelNames returns all label names constrained by the given matchers. LabelNames(ctx context.Context, in *LabelNamesRequest, opts ...grpc.CallOption) (*LabelNamesResponse, error) /// LabelValues returns all label values for given label name. LabelValues(ctx context.Context, in *LabelValuesRequest, opts ...grpc.CallOption) (*LabelValuesResponse, error) @@ -792,8 +791,7 @@ type StoreServer interface { /// There is no requirements on chunk sorting, however it is recommended to have chunk sorted by chunk min time. /// This heavily optimizes the resource usage on Querier / Federated Queries. Series(*SeriesRequest, Store_SeriesServer) error - /// LabelNames returns all label names that is available. - /// Currently unimplemented in all Thanos implementations, because Query API does not implement this either. + /// LabelNames returns all label names constrained by the given matchers. LabelNames(context.Context, *LabelNamesRequest) (*LabelNamesResponse, error) /// LabelValues returns all label values for given label name. LabelValues(context.Context, *LabelValuesRequest) (*LabelValuesResponse, error) diff --git a/pkg/store/storepb/rpc.proto b/pkg/store/storepb/rpc.proto index 3ef0185af8..f60661a32a 100644 --- a/pkg/store/storepb/rpc.proto +++ b/pkg/store/storepb/rpc.proto @@ -40,8 +40,7 @@ service Store { /// This heavily optimizes the resource usage on Querier / Federated Queries. rpc Series(SeriesRequest) returns (stream SeriesResponse); - /// LabelNames returns all label names that is available. - /// Currently unimplemented in all Thanos implementations, because Query API does not implement this either. + /// LabelNames returns all label names constrained by the given matchers. rpc LabelNames(LabelNamesRequest) returns (LabelNamesResponse); /// LabelValues returns all label values for given label name. diff --git a/pkg/store/tsdb.go b/pkg/store/tsdb.go index 30a2af29fc..09de901be3 100644 --- a/pkg/store/tsdb.go +++ b/pkg/store/tsdb.go @@ -7,6 +7,7 @@ import ( "context" "io" "math" + "sort" "github.com/go-kit/kit/log" "github.com/pkg/errors" @@ -194,7 +195,7 @@ func (s *TSDBStore) Series(r *storepb.SeriesRequest, srv storepb.Store_SeriesSer return nil } -// LabelNames returns all known label names. +// LabelNames returns all known label names constrained with the given matchers. func (s *TSDBStore) LabelNames(ctx context.Context, r *storepb.LabelNamesRequest) ( *storepb.LabelNamesResponse, error, ) { @@ -213,6 +214,14 @@ func (s *TSDBStore) LabelNames(ctx context.Context, r *storepb.LabelNamesRequest if err != nil { return nil, status.Error(codes.Internal, err.Error()) } + + if len(res) > 0 { + for _, lbl := range s.extLset { + res = append(res, lbl.Name) + } + sort.Strings(res) + } + return &storepb.LabelNamesResponse{Names: res}, nil } diff --git a/pkg/store/tsdb_test.go b/pkg/store/tsdb_test.go index c3d29807ad..0ba2ac9329 100644 --- a/pkg/store/tsdb_test.go +++ b/pkg/store/tsdb_test.go @@ -233,7 +233,7 @@ func TestTSDBStore_LabelNames(t *testing.T) { { title: "add one label", labels: []string{"foo", "foo"}, - expectedNames: []string{"foo"}, + expectedNames: []string{"foo", "region"}, timestamp: now.Unix(), start: func() int64 { return timestamp.FromTime(minTime) @@ -246,7 +246,7 @@ func TestTSDBStore_LabelNames(t *testing.T) { title: "add another label", labels: []string{"bar", "bar"}, // We will get two labels here. - expectedNames: []string{"bar", "foo"}, + expectedNames: []string{"bar", "foo", "region"}, timestamp: now.Unix(), start: func() int64 { return timestamp.FromTime(minTime) @@ -269,7 +269,7 @@ func TestTSDBStore_LabelNames(t *testing.T) { { title: "get all labels", labels: []string{"buz", "buz"}, - expectedNames: []string{"bar", "buz", "foo"}, + expectedNames: []string{"bar", "buz", "foo", "region"}, timestamp: now.Unix(), start: func() int64 { return timestamp.FromTime(minTime) diff --git a/test/e2e/query_test.go b/test/e2e/query_test.go index e903959e59..63a4331579 100644 --- a/test/e2e/query_test.go +++ b/test/e2e/query_test.go @@ -280,8 +280,10 @@ func TestQueryLabelNames(t *testing.T) { labelNames(t, ctx, q.HTTPEndpoint(), []storepb.LabelMatcher{{Type: storepb.LabelMatcher_EQ, 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] - return len(res) == 5 + // 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. + // With labelnames pushdown we had to extend the LabelNames() call to enrich the response with the external labelset with there are more than one label. + return len(res) == 7 }, )