From 9f10c63cffdc33ecdac9fd33a3801fb6caf82017 Mon Sep 17 00:00:00 2001 From: Julius Volz Date: Sun, 12 Nov 2017 00:47:47 +0100 Subject: [PATCH] Fix remote read labelset corruption (#3456) The labelsets returned from remote read are mutated in higher levels (like seriesFilter.Labels()) and since the concreteSeriesSet didn't return a copy, the external mutation affected the labelset in the concreteSeries itself. This resulted in bizarre bugs where local and remote series would show with identical label sets in the UI, but not be deduplicated, since internally, a series might come to look like: {__name__="node_load5", instance="192.168.1.202:12090", job="node_exporter", node="odroid", node="odroid"} (note the repetition of the last label) --- storage/remote/codec.go | 2 +- storage/remote/codec_test.go | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/storage/remote/codec.go b/storage/remote/codec.go index 4ed9662c01..5b966acba3 100644 --- a/storage/remote/codec.go +++ b/storage/remote/codec.go @@ -201,7 +201,7 @@ type concreteSeries struct { } func (c *concreteSeries) Labels() labels.Labels { - return c.labels + return labels.New(c.labels...) } func (c *concreteSeries) Iterator() storage.SeriesIterator { diff --git a/storage/remote/codec_test.go b/storage/remote/codec_test.go index 8cb9a2a954..c6b9010e70 100644 --- a/storage/remote/codec_test.go +++ b/storage/remote/codec_test.go @@ -16,6 +16,8 @@ package remote import ( "testing" + "github.com/stretchr/testify/require" + "github.com/prometheus/prometheus/pkg/labels" "github.com/prometheus/prometheus/prompb" "github.com/prometheus/prometheus/storage" @@ -124,3 +126,22 @@ func TestConcreteSeriesSet(t *testing.T) { t.Fatalf("Expected Next() to be false.") } } + +func TestConcreteSeriesClonesLabels(t *testing.T) { + lbls := labels.Labels{ + labels.Label{Name: "a", Value: "b"}, + labels.Label{Name: "c", Value: "d"}, + } + cs := concreteSeries{ + labels: labels.New(lbls...), + } + + gotLabels := cs.Labels() + require.Equal(t, lbls, gotLabels) + + gotLabels[0].Value = "foo" + gotLabels[1].Value = "bar" + + gotLabels = cs.Labels() + require.Equal(t, lbls, gotLabels) +}