diff --git a/pkg/store/bucket.go b/pkg/store/bucket.go index 564229ccf0d..bd1cf0fb732 100644 --- a/pkg/store/bucket.go +++ b/pkg/store/bucket.go @@ -282,7 +282,8 @@ type BucketStore struct { // chunksLimiterFactory creates a new limiter used to limit the number of chunks fetched by each Series() call. chunksLimiterFactory ChunksLimiterFactory - // seriesLimiterFactory creates a new limiter used to limit the number of touched series by each Series() call. + // seriesLimiterFactory creates a new limiter used to limit the number of touched series by each Series() call, + // or LabelName and LabelValues calls when used with matchers. seriesLimiterFactory SeriesLimiterFactory partitioner Partitioner @@ -1280,16 +1281,16 @@ func (s *BucketStore) LabelNames(ctx context.Context, req *storepb.LabelNamesReq // LabelValues implements the storepb.StoreServer interface. func (s *BucketStore) LabelValues(ctx context.Context, req *storepb.LabelValuesRequest) (*storepb.LabelValuesResponse, error) { + reqSeriesMatchers, err := storepb.MatchersToPromMatchers(req.Matchers...) + if err != nil { + return nil, status.Error(codes.InvalidArgument, errors.Wrap(err, "translate request labels matchers").Error()) + } + resHints := &hintspb.LabelValuesResponseHints{} g, gctx := errgroup.WithContext(ctx) - s.mtx.RLock() - - var mtx sync.Mutex - var sets [][]string var reqBlockMatchers []*labels.Matcher - if req.Hints != nil { reqHints := &hintspb.LabelValuesRequestHints{} err := types.UnmarshalAny(req.Hints, reqHints) @@ -1303,7 +1304,25 @@ func (s *BucketStore) LabelValues(ctx context.Context, req *storepb.LabelValuesR } } + // If we have series matchers, add != "" matcher, to only select series that have given label name. + if len(reqSeriesMatchers) > 0 { + m, err := labels.NewMatcher(labels.MatchNotEqual, req.Label, "") + if err != nil { + return nil, status.Error(codes.InvalidArgument, err.Error()) + } + + reqSeriesMatchers = append(reqSeriesMatchers, m) + } + + s.mtx.RLock() + + var mtx sync.Mutex + var sets [][]string + var seriesLimiter = s.seriesLimiterFactory(s.metrics.queriesDropped.WithLabelValues("series")) + for _, b := range s.blocks { + b := b + if !b.overlapsClosedInterval(req.Start, req.End) { continue } @@ -1314,24 +1333,52 @@ func (s *BucketStore) LabelValues(ctx context.Context, req *storepb.LabelValuesR resHints.AddQueriedBlock(b.meta.ULID) indexr := b.indexReader(gctx) - extLabels := b.meta.Thanos.Labels g.Go(func() error { defer runutil.CloseWithLogOnErr(s.logger, indexr, "label values") - // Do it via index reader to have pending reader registered correctly. - res, err := indexr.block.indexHeaderReader.LabelValues(req.Label) - if err != nil { - return errors.Wrap(err, "index header label values") - } + var result []string + if len(reqSeriesMatchers) == 0 { + // Do it via index reader to have pending reader registered correctly. + res, err := indexr.block.indexHeaderReader.LabelValues(req.Label) + if err != nil { + return errors.Wrapf(err, "index header label values for block %s", b.meta.ULID) + } + + // Add the external label value as well. + if extLabelValue := b.extLset.Get(req.Label); extLabelValue != "" { + res = strutil.MergeSlices(res, []string{extLabelValue}) + } + result = res + } else { + seriesSet, _, err := blockSeries(b.extLset, indexr, nil, reqSeriesMatchers, nil, seriesLimiter, true, req.Start, req.End, nil) + if err != nil { + return errors.Wrapf(err, "fetch series for block %s", b.meta.ULID) + } - // Add the external label value as well. - if extLabelValue, ok := extLabels[req.Label]; ok { - res = strutil.MergeSlices(res, []string{extLabelValue}) + // Extract given label's value from all series and deduplicate them. + // We don't need to deal with external labels, since they are already added by blockSeries. + values := map[string]struct{}{} + for seriesSet.Next() { + ls, _ := seriesSet.At() + val := ls.Get(req.Label) + if val != "" { // Should never be empty since we added labelName!="" matcher to the list of matchers. + values[val] = struct{}{} + } + } + if seriesSet.Err() != nil { + return errors.Wrapf(seriesSet.Err(), "iterate series for block %s", b.meta.ULID) + } + + result = make([]string, 0, len(values)) + for n := range values { + result = append(result, n) + } + sort.Strings(result) } mtx.Lock() - sets = append(sets, res) + sets = append(sets, result) mtx.Unlock() return nil diff --git a/pkg/store/bucket_e2e_test.go b/pkg/store/bucket_e2e_test.go index ec9097d828a..fda9f6a1f90 100644 --- a/pkg/store/bucket_e2e_test.go +++ b/pkg/store/bucket_e2e_test.go @@ -658,39 +658,6 @@ func TestBucketStore_LabelNames_e2e(t *testing.T) { testutil.Ok(t, err) defer func() { testutil.Ok(t, os.RemoveAll(dir)) }() - s := prepareStoreWithTestBlocks(t, dir, bkt, false, NewChunksLimiterFactory(0), NewSeriesLimiterFactory(0), emptyRelabelConfig, allowAllFilterConf) - - mint, maxt := s.store.TimeRange() - testutil.Equals(t, s.minTime, mint) - testutil.Equals(t, s.maxTime, maxt) - - vals, err := s.store.LabelNames(ctx, &storepb.LabelNamesRequest{ - Start: timestamp.FromTime(minTime), - End: timestamp.FromTime(maxTime), - }) - testutil.Ok(t, err) - // ext2 is added by the prepareStoreWithTestBlocks function. - testutil.Equals(t, []string{"a", "b", "c", "ext1", "ext2"}, vals.Names) - - // Outside the time range. - vals, err = s.store.LabelNames(ctx, &storepb.LabelNamesRequest{ - Start: timestamp.FromTime(time.Now().Add(-24 * time.Hour)), - End: timestamp.FromTime(time.Now().Add(-23 * time.Hour)), - }) - testutil.Ok(t, err) - testutil.Equals(t, []string(nil), vals.Names) - }) -} - -func TestBucketStore_LabelNames_WithMatchers_e2e(t *testing.T) { - objtesting.ForeachStore(t, func(t *testing.T, bkt objstore.Bucket) { - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - - dir, err := ioutil.TempDir("", "test_bucketstore_label_names_with_matchers_e2e") - testutil.Ok(t, err) - defer func() { testutil.Ok(t, os.RemoveAll(dir)) }() - s := prepareStoreWithTestBlocks(t, dir, bkt, false, NewChunksLimiterFactory(0), NewSeriesLimiterFactory(0), emptyRelabelConfig, allowAllFilterConf) s.cache.SwapWith(noopCache{}) @@ -698,70 +665,89 @@ func TestBucketStore_LabelNames_WithMatchers_e2e(t *testing.T) { testutil.Equals(t, s.minTime, mint) testutil.Equals(t, s.maxTime, maxt) - t.Run("matcher matching everything", func(t *testing.T) { - vals, err := s.store.LabelNames(ctx, &storepb.LabelNamesRequest{ - Start: timestamp.FromTime(minTime), - End: timestamp.FromTime(maxTime), - Matchers: []storepb.LabelMatcher{ - { - Type: storepb.LabelMatcher_EQ, - Name: "a", - Value: "1", + for name, tc := range map[string]struct { + req *storepb.LabelNamesRequest + expected []string + }{ + "basic labelNames": { + req: &storepb.LabelNamesRequest{ + Start: timestamp.FromTime(minTime), + End: timestamp.FromTime(maxTime), + }, + expected: []string{"a", "b", "c", "ext1", "ext2"}, // ext2 is added by the prepareStoreWithTestBlocks function. + }, + "outside the time range": { + req: &storepb.LabelNamesRequest{ + Start: timestamp.FromTime(time.Now().Add(-24 * time.Hour)), + End: timestamp.FromTime(time.Now().Add(-23 * time.Hour)), + }, + expected: nil, + }, + "matcher matching everything": { + req: &storepb.LabelNamesRequest{ + Start: timestamp.FromTime(minTime), + End: timestamp.FromTime(maxTime), + Matchers: []storepb.LabelMatcher{ + { + Type: storepb.LabelMatcher_EQ, + Name: "a", + Value: "1", + }, }, }, - }) - testutil.Ok(t, err) - testutil.Equals(t, []string{"a", "b", "c", "ext1", "ext2"}, vals.Names) - }) - - t.Run("b=1 matcher", func(t *testing.T) { - vals, err := s.store.LabelNames(ctx, &storepb.LabelNamesRequest{ - Start: timestamp.FromTime(minTime), - End: timestamp.FromTime(maxTime), - Matchers: []storepb.LabelMatcher{ - { - Type: storepb.LabelMatcher_EQ, - Name: "b", - Value: "1", + expected: []string{"a", "b", "c", "ext1", "ext2"}, + }, + "b=1 matcher": { + req: &storepb.LabelNamesRequest{ + Start: timestamp.FromTime(minTime), + End: timestamp.FromTime(maxTime), + Matchers: []storepb.LabelMatcher{ + { + Type: storepb.LabelMatcher_EQ, + Name: "b", + Value: "1", + }, }, }, - }) - testutil.Ok(t, err) - testutil.Equals(t, []string{"a", "b", "ext1"}, vals.Names) - }) + expected: []string{"a", "b", "ext1"}, + }, - t.Run("b='' matcher", func(t *testing.T) { - vals, err := s.store.LabelNames(ctx, &storepb.LabelNamesRequest{ - Start: timestamp.FromTime(minTime), - End: timestamp.FromTime(maxTime), - Matchers: []storepb.LabelMatcher{ - { - Type: storepb.LabelMatcher_EQ, - Name: "b", - Value: "", + "b='' matcher": { + req: &storepb.LabelNamesRequest{ + Start: timestamp.FromTime(minTime), + End: timestamp.FromTime(maxTime), + Matchers: []storepb.LabelMatcher{ + { + Type: storepb.LabelMatcher_EQ, + Name: "b", + Value: "", + }, }, }, - }) - testutil.Ok(t, err) - testutil.Equals(t, []string{"a", "c", "ext2"}, vals.Names) - }) - - t.Run("outside the time range", func(t *testing.T) { - vals, err := s.store.LabelNames(ctx, &storepb.LabelNamesRequest{ - Start: timestamp.FromTime(time.Now().Add(-24 * time.Hour)), - End: timestamp.FromTime(time.Now().Add(-23 * time.Hour)), - - Matchers: []storepb.LabelMatcher{ - { - Type: storepb.LabelMatcher_EQ, - Name: "a", - Value: "1", + expected: []string{"a", "c", "ext2"}, + }, + "outside the time range, with matcher": { + req: &storepb.LabelNamesRequest{ + Start: timestamp.FromTime(time.Now().Add(-24 * time.Hour)), + End: timestamp.FromTime(time.Now().Add(-23 * time.Hour)), + Matchers: []storepb.LabelMatcher{ + { + Type: storepb.LabelMatcher_EQ, + Name: "a", + Value: "1", + }, }, }, + expected: nil, + }, + } { + t.Run(name, func(t *testing.T) { + vals, err := s.store.LabelNames(ctx, tc.req) + testutil.Ok(t, err) + + testutil.Equals(t, tc.expected, vals.Names) }) - testutil.Ok(t, err) - testutil.Equals(t, []string(nil), vals.Names) - }) + } }) } @@ -775,26 +761,104 @@ func TestBucketStore_LabelValues_e2e(t *testing.T) { defer func() { testutil.Ok(t, os.RemoveAll(dir)) }() s := prepareStoreWithTestBlocks(t, dir, bkt, false, NewChunksLimiterFactory(0), NewSeriesLimiterFactory(0), emptyRelabelConfig, allowAllFilterConf) + s.cache.SwapWith(noopCache{}) mint, maxt := s.store.TimeRange() testutil.Equals(t, s.minTime, mint) testutil.Equals(t, s.maxTime, maxt) - vals, err := s.store.LabelValues(ctx, &storepb.LabelValuesRequest{ - Label: "a", - Start: timestamp.FromTime(minTime), - End: timestamp.FromTime(maxTime), - }) - testutil.Ok(t, err) - testutil.Equals(t, []string{"1", "2"}, vals.Values) + for name, tc := range map[string]struct { + req *storepb.LabelValuesRequest + expected []string + }{ + "label a": { + req: &storepb.LabelValuesRequest{ + Label: "a", + Start: timestamp.FromTime(minTime), + End: timestamp.FromTime(maxTime), + }, + expected: []string{"1", "2"}, + }, + "label a, outside time range": { + req: &storepb.LabelValuesRequest{ + Label: "a", + Start: timestamp.FromTime(time.Now().Add(-24 * time.Hour)), + End: timestamp.FromTime(time.Now().Add(-23 * time.Hour)), + }, + expected: nil, + }, + "label a, a=1": { + req: &storepb.LabelValuesRequest{ + Label: "a", + Start: timestamp.FromTime(minTime), + End: timestamp.FromTime(maxTime), + Matchers: []storepb.LabelMatcher{ + { + Type: storepb.LabelMatcher_EQ, + Name: "a", + Value: "1", + }, + }, + }, + expected: []string{"1"}, + }, + "label a, a=2, c=2": { + req: &storepb.LabelValuesRequest{ + Label: "a", + Start: timestamp.FromTime(minTime), + End: timestamp.FromTime(maxTime), + Matchers: []storepb.LabelMatcher{ + { + Type: storepb.LabelMatcher_EQ, + Name: "a", + Value: "2", + }, + { + Type: storepb.LabelMatcher_EQ, + Name: "c", + Value: "2", + }, + }, + }, + expected: []string{"2"}, + }, + "label ext1": { + req: &storepb.LabelValuesRequest{ + Label: "ext1", + Start: timestamp.FromTime(minTime), + End: timestamp.FromTime(maxTime), + }, + expected: []string{"value1"}, + }, + "label ext1, c=1": { + req: &storepb.LabelValuesRequest{ + Label: "ext1", + Start: timestamp.FromTime(minTime), + End: timestamp.FromTime(maxTime), + Matchers: []storepb.LabelMatcher{ + { + Type: storepb.LabelMatcher_EQ, + Name: "c", + Value: "1", + }, + }, + }, + expected: nil, // ext1 is replaced with ext2 for series with c + }, + } { + t.Run(name, func(t *testing.T) { + vals, err := s.store.LabelValues(ctx, tc.req) + testutil.Ok(t, err) - // Outside the time range. - vals, err = s.store.LabelValues(ctx, &storepb.LabelValuesRequest{ - Label: "a", - Start: timestamp.FromTime(time.Now().Add(-24 * time.Hour)), - End: timestamp.FromTime(time.Now().Add(-23 * time.Hour)), - }) - testutil.Ok(t, err) - testutil.Equals(t, []string(nil), vals.Values) + testutil.Equals(t, tc.expected, emptyToNil(vals.Values)) + }) + } }) } + +func emptyToNil(values []string) []string { + if len(values) == 0 { + return nil + } + return values +}