Skip to content

Commit

Permalink
fix ReplicaLabelRemover panic when replicaLabels are not specified
Browse files Browse the repository at this point in the history
Signed-off-by: Ben Ye <yb532204897@gmail.com>
  • Loading branch information
yeya24 committed Jul 24, 2020
1 parent 9b2343f commit 413bf07
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 8 deletions.
4 changes: 4 additions & 0 deletions pkg/block/fetcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -683,6 +683,10 @@ func NewReplicaLabelRemover(logger log.Logger, replicaLabels []string) *ReplicaL

// Modify modifies external labels of existing blocks, it removes given replica labels from the metadata of blocks that have it.
func (r *ReplicaLabelRemover) Modify(_ context.Context, metas map[ulid.ULID]*metadata.Meta, modified *extprom.TxGaugeVec) error {
if len(r.replicaLabels) == 0 {
return nil
}

for u, meta := range metas {
l := meta.Thanos.Labels
for _, replicaLabel := range r.replicaLabels {
Expand Down
33 changes: 25 additions & 8 deletions pkg/block/fetcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -878,13 +878,13 @@ func TestDeduplicateFilter_Filter(t *testing.T) {
func TestReplicaLabelRemover_Modify(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second)
defer cancel()
rm := NewReplicaLabelRemover(log.NewNopLogger(), []string{"replica", "rule_replica"})

for _, tcase := range []struct {
name string
input map[ulid.ULID]*metadata.Meta
expected map[ulid.ULID]*metadata.Meta
modified float64
name string
input map[ulid.ULID]*metadata.Meta
expected map[ulid.ULID]*metadata.Meta
modified float64
replicaLabelRemover *ReplicaLabelRemover
}{
{
name: "without replica labels",
Expand All @@ -898,7 +898,8 @@ func TestReplicaLabelRemover_Modify(t *testing.T) {
ULID(2): {Thanos: metadata.Thanos{Labels: map[string]string{"message": "something"}}},
ULID(3): {Thanos: metadata.Thanos{Labels: map[string]string{"message": "something1"}}},
},
modified: 0,
modified: 0,
replicaLabelRemover: NewReplicaLabelRemover(log.NewNopLogger(), []string{"replica", "rule_replica"}),
},
{
name: "with replica labels",
Expand All @@ -914,11 +915,27 @@ func TestReplicaLabelRemover_Modify(t *testing.T) {
ULID(3): {Thanos: metadata.Thanos{Labels: map[string]string{"message": "something"}}},
ULID(4): {Thanos: metadata.Thanos{Labels: map[string]string{"replica": "deduped"}}},
},
modified: 5.0,
modified: 5.0,
replicaLabelRemover: NewReplicaLabelRemover(log.NewNopLogger(), []string{"replica", "rule_replica"}),
},
{
name: "no replica label specified in the ReplicaLabelRemover",
input: map[ulid.ULID]*metadata.Meta{
ULID(1): {Thanos: metadata.Thanos{Labels: map[string]string{"message": "something"}}},
ULID(2): {Thanos: metadata.Thanos{Labels: map[string]string{"message": "something"}}},
ULID(3): {Thanos: metadata.Thanos{Labels: map[string]string{"message": "something1"}}},
},
expected: map[ulid.ULID]*metadata.Meta{
ULID(1): {Thanos: metadata.Thanos{Labels: map[string]string{"message": "something"}}},
ULID(2): {Thanos: metadata.Thanos{Labels: map[string]string{"message": "something"}}},
ULID(3): {Thanos: metadata.Thanos{Labels: map[string]string{"message": "something1"}}},
},
modified: 0,
replicaLabelRemover: NewReplicaLabelRemover(log.NewNopLogger(), []string{}),
},
} {
m := newTestFetcherMetrics()
testutil.Ok(t, rm.Modify(ctx, tcase.input, m.modified))
testutil.Ok(t, tcase.replicaLabelRemover.Modify(ctx, tcase.input, m.modified))

testutil.Equals(t, tcase.modified, promtest.ToFloat64(m.modified.WithLabelValues(replicaRemovedMeta)))
testutil.Equals(t, tcase.expected, tcase.input)
Expand Down

0 comments on commit 413bf07

Please sign in to comment.