diff --git a/CHANGELOG.md b/CHANGELOG.md index 43a273b81c..6abe76c96b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,19 @@ We use *breaking :warning:* to mark changes that are not backward compatible (re ### Fixed +- [#7083](https://github.com/thanos-io/thanos/pull/7083) Store Gateway: Fix lazy expanded postings with 0 length failed to be cached. +- [#7082](https://github.com/thanos-io/thanos/pull/7082) Stores: fix label values edge case when requesting external label values with matchers + +### Added + +### Changed + +### Removed + +## [v0.34.0](https://github.com/thanos-io/thanos/tree/release-0.34) - release in progress + +### Fixed + - [#7011](https://github.com/thanos-io/thanos/pull/7011) Query Frontend: queries with negative offset should check whether it is cacheable or not. - [#6874](https://github.com/thanos-io/thanos/pull/6874) Sidecar: fix labels returned by 'api/v1/series' in presence of conflicting external and inner labels. - [#7009](https://github.com/thanos-io/thanos/pull/7009) Rule: Fix spacing error in URL. diff --git a/pkg/store/acceptance_test.go b/pkg/store/acceptance_test.go index 34abc649ab..dae3e503c1 100644 --- a/pkg/store/acceptance_test.go +++ b/pkg/store/acceptance_test.go @@ -255,6 +255,29 @@ func testStoreAPIsAcceptance(t *testing.T, startStore startStoreFn) { }, }, }, + { + desc: "series matcher on other labels when requesting external labels", + appendFn: func(app storage.Appender) { + _, err := app.Append(0, labels.FromStrings("__name__", "up", "foo", "bar", "job", "C"), 0, 0) + testutil.Ok(t, err) + _, err = app.Append(0, labels.FromStrings("__name__", "up", "foo", "baz", "job", "C"), 0, 0) + testutil.Ok(t, err) + + testutil.Ok(t, app.Commit()) + }, + labelValuesCalls: []labelValuesCallCase{ + { + start: timestamp.FromTime(minTime), + end: timestamp.FromTime(maxTime), + label: "region", + matchers: []storepb.LabelMatcher{ + {Type: storepb.LabelMatcher_EQ, Name: "__name__", Value: "up"}, + {Type: storepb.LabelMatcher_EQ, Name: "job", Value: "C"}, + }, + expectedValues: []string{"eu-west"}, + }, + }, + }, { // Testcases taken from https://github.com/prometheus/prometheus/blob/95e705612c1d557f1681bd081a841b78f93ee158/tsdb/querier_test.go#L1898 desc: "matching behavior", diff --git a/pkg/store/bucket.go b/pkg/store/bucket.go index fd4fb7392c..7bd1647f66 100644 --- a/pkg/store/bucket.go +++ b/pkg/store/bucket.go @@ -1947,8 +1947,9 @@ func (s *BucketStore) LabelValues(ctx context.Context, req *storepb.LabelValuesR continue } - // If we have series matchers, add != "" matcher, to only select series that have given label name. - if len(reqSeriesMatchersNoExtLabels) > 0 { + // If we have series matchers and the Label is not an external one, add != "" matcher + // to only select series that have given label name. + if len(reqSeriesMatchersNoExtLabels) > 0 && !b.extLset.Has(req.Label) { m, err := labels.NewMatcher(labels.MatchNotEqual, req.Label, "") if err != nil { return nil, status.Error(codes.InvalidArgument, err.Error()) diff --git a/pkg/store/prometheus.go b/pkg/store/prometheus.go index 197364ca04..eea0334dd0 100644 --- a/pkg/store/prometheus.go +++ b/pkg/store/prometheus.go @@ -108,6 +108,11 @@ func NewPrometheusStore( return p, nil } +func (p *PrometheusStore) labelCallsSupportMatchers() bool { + version, parseErr := semver.Parse(p.promVersion()) + return parseErr == nil && version.GTE(baseVer) +} + // Info returns store information about the Prometheus instance. // NOTE(bwplotka): MaxTime & MinTime are not accurate nor adjusted dynamically. // This is fine for now, but might be needed in future. @@ -656,8 +661,7 @@ func (p *PrometheusStore) LabelNames(ctx context.Context, r *storepb.LabelNamesR } var lbls []string - version, parseErr := semver.Parse(p.promVersion()) - if len(matchers) == 0 || (parseErr == nil && version.GTE(baseVer)) { + if len(matchers) == 0 || p.labelCallsSupportMatchers() { lbls, err = p.client.LabelNamesInGRPC(ctx, p.base, matchers, r.Start, r.End) if err != nil { return nil, err @@ -707,23 +711,27 @@ func (p *PrometheusStore) LabelValues(ctx context.Context, r *storepb.LabelValue return &storepb.LabelValuesResponse{}, nil } - // First check for matching external label which has priority. - if l := extLset.Get(r.Label); l != "" { - for _, m := range matchers { - if !m.Matches(l) { - return &storepb.LabelValuesResponse{}, nil - } - } - return &storepb.LabelValuesResponse{Values: []string{l}}, nil - } - var ( sers []map[string]string vals []string ) - version, parseErr := semver.Parse(p.promVersion()) - if len(matchers) == 0 || (parseErr == nil && version.GTE(baseVer)) { + // If we request label values for an external label while selecting an additional matcher for other label values + if val := extLset.Get(r.Label); val != "" { + if len(matchers) == 0 { + return &storepb.LabelValuesResponse{Values: []string{val}}, nil + } + sers, err = p.client.SeriesInGRPC(ctx, p.base, matchers, r.Start, r.End) + if err != nil { + return nil, err + } + if len(sers) > 0 { + return &storepb.LabelValuesResponse{Values: []string{val}}, nil + } + return &storepb.LabelValuesResponse{}, nil + } + + if len(matchers) == 0 || p.labelCallsSupportMatchers() { vals, err = p.client.LabelValuesInGRPC(ctx, p.base, r.Label, matchers, r.Start, r.End) if err != nil { return nil, err diff --git a/pkg/store/tsdb.go b/pkg/store/tsdb.go index 8953946a29..0b860b0675 100644 --- a/pkg/store/tsdb.go +++ b/pkg/store/tsdb.go @@ -339,26 +339,35 @@ func (s *TSDBStore) LabelValues(ctx context.Context, r *storepb.LabelValuesReque if err != nil { return nil, status.Error(codes.InvalidArgument, err.Error()) } - if !match { return &storepb.LabelValuesResponse{}, nil } - if v := s.getExtLset().Get(r.Label); v != "" { - for _, m := range matchers { - if !m.Matches(v) { - return &storepb.LabelValuesResponse{}, nil - } - } - return &storepb.LabelValuesResponse{Values: []string{v}}, nil - } - q, err := s.db.ChunkQuerier(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") + // If we request label values for an external label while selecting an additional matcher for other label values + if val := s.getExtLset().Get(r.Label); val != "" { + if len(matchers) == 0 { + return &storepb.LabelValuesResponse{Values: []string{val}}, nil + } + + hints := &storage.SelectHints{ + Start: r.Start, + End: r.End, + Func: "series", + } + set := q.Select(ctx, false, hints, matchers...) + + for set.Next() { + return &storepb.LabelValuesResponse{Values: []string{val}}, nil + } + return &storepb.LabelValuesResponse{}, nil + } + res, _, err := q.LabelValues(ctx, r.Label, matchers...) if err != nil { return nil, status.Error(codes.Internal, err.Error())