diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f5dd2d4f8..3140315ed8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,8 @@ We use *breaking :warning:* to mark changes that are not backward compatible (re ### Fixed +- [#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. + ### Added ### Changed diff --git a/pkg/store/acceptance_test.go b/pkg/store/acceptance_test.go index 9b1e8d49be..2498d571f2 100644 --- a/pkg/store/acceptance_test.go +++ b/pkg/store/acceptance_test.go @@ -53,9 +53,10 @@ type labelValuesCallCase struct { } type seriesCallCase struct { - matchers []storepb.LabelMatcher - start int64 - end int64 + matchers []storepb.LabelMatcher + start int64 + end int64 + skipChunks bool expectedLabels []labels.Labels expectErr error @@ -219,6 +220,28 @@ func testStoreAPIsAcceptance(t *testing.T, startStore func(t *testing.T, extLset }, }, }, + { + desc: "conflicting internal and external labels when skipping chunks", + appendFn: func(app storage.Appender) { + _, err := app.Append(0, labels.FromStrings("foo", "bar", "region", "somewhere"), 0, 0) + testutil.Ok(t, err) + + testutil.Ok(t, app.Commit()) + }, + seriesCalls: []seriesCallCase{ + { + start: timestamp.FromTime(minTime), + end: timestamp.FromTime(maxTime), + matchers: []storepb.LabelMatcher{ + {Type: storepb.LabelMatcher_EQ, Name: "foo", Value: "bar"}, + }, + skipChunks: true, + expectedLabels: []labels.Labels{ + labels.FromStrings("foo", "bar", "region", "eu-west"), + }, + }, + }, + }, { // Testcases taken from https://github.com/prometheus/prometheus/blob/95e705612c1d557f1681bd081a841b78f93ee158/tsdb/querier_test.go#L1898 desc: "matching behavior", @@ -701,9 +724,10 @@ func testStoreAPIsAcceptance(t *testing.T, startStore func(t *testing.T, extLset t.Run("series", func(t *testing.T) { srv := newStoreSeriesServer(context.Background()) err := store.Series(&storepb.SeriesRequest{ - MinTime: c.start, - MaxTime: c.end, - Matchers: c.matchers, + MinTime: c.start, + MaxTime: c.end, + Matchers: c.matchers, + SkipChunks: c.skipChunks, }, srv) if c.expectErr != nil { testutil.NotOk(t, err) diff --git a/pkg/store/prometheus.go b/pkg/store/prometheus.go index f9e836aca9..05af3b5d93 100644 --- a/pkg/store/prometheus.go +++ b/pkg/store/prometheus.go @@ -189,15 +189,17 @@ func (p *PrometheusStore) Series(r *storepb.SeriesRequest, seriesSrv storepb.Sto if err != nil { return err } + var b labels.Builder for _, lbm := range labelMaps { - lset := make([]labelpb.ZLabel, 0, len(lbm)+finalExtLset.Len()) + b.Reset(labels.EmptyLabels()) for k, v := range lbm { - lset = append(lset, labelpb.ZLabel{Name: k, Value: v}) + b.Set(k, v) } - lset = append(lset, labelpb.ZLabelsFromPromLabels(finalExtLset)...) - sort.Slice(lset, func(i, j int) bool { - return lset[i].Name < lset[j].Name + // external labels should take precedence + finalExtLset.Range(func(l labels.Label) { + b.Set(l.Name, l.Value) }) + lset := labelpb.ZLabelsFromPromLabels(b.Labels()) if err = s.Send(storepb.NewSeriesResponse(&storepb.Series{Labels: lset})); err != nil { return err }