Skip to content

Commit

Permalink
Store: fix returned labels on external label conflict when skipping c…
Browse files Browse the repository at this point in the history
…hunks (#6874)

Signed-off-by: Michael Hoffmann <[email protected]>
  • Loading branch information
MichaHoffmann authored Nov 5, 2023
1 parent fe179ac commit c74a050
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 11 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
36 changes: 30 additions & 6 deletions pkg/store/acceptance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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)
Expand Down
12 changes: 7 additions & 5 deletions pkg/store/prometheus.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down

0 comments on commit c74a050

Please sign in to comment.