From 3a7c51ab70fc7615cd318204d3aa7c078b7c5b20 Mon Sep 17 00:00:00 2001 From: Brian Brazil Date: Mon, 23 Oct 2017 15:12:22 +0100 Subject: [PATCH] Remote read endpoint should handle matchers for external labels. (#3325) If the other Prometheus has an external label that matches that of the Prometheus being read from, then we need to remove that matcher from the request as it's not actually stored in the database - it's only added for alerts, federation and on the output of the remote read endpoint. Instead we check for that label being empty, in case there is a time series with a different label value for that external label. --- CHANGELOG.md | 2 +- web/api/v1/api.go | 22 ++++++++++++++++++++-- web/api/v1/api_test.go | 11 ++++++++--- 3 files changed, 29 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 483cf9792..47fd50286 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ ## 1.8.1 / 2017-10-19 -* [BUGFIX] Apply external labels to remote read endpoint +* [BUGFIX] Correctly handle external labels on remote read endpoint ## 1.8.0 / 2017-10-06 diff --git a/web/api/v1/api.go b/web/api/v1/api.go index b8de72ebc..649c08f33 100644 --- a/web/api/v1/api.go +++ b/web/api/v1/api.go @@ -486,7 +486,26 @@ func (api *API) remoteRead(w http.ResponseWriter, r *http.Request) { http.Error(w, err.Error(), http.StatusBadRequest) return } - iters, err := querier.QueryRange(r.Context(), from, through, matchers...) + // Change equality matchers which match external labels + // to a matcher that looks for an empty label, + // as that label should not be present in the storage. + externalLabels := api.config().GlobalConfig.ExternalLabels.Clone() + filteredMatchers := make([]*metric.LabelMatcher, 0, len(matchers)) + for _, m := range matchers { + value := externalLabels[m.Name] + if m.Type == metric.Equal && value == m.Value { + matcher, err := metric.NewLabelMatcher(metric.Equal, m.Name, "") + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + filteredMatchers = append(filteredMatchers, matcher) + } else { + filteredMatchers = append(filteredMatchers, m) + } + } + + iters, err := querier.QueryRange(r.Context(), from, through, filteredMatchers...) if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) return @@ -496,7 +515,6 @@ func (api *API) remoteRead(w http.ResponseWriter, r *http.Request) { OldestInclusive: from, NewestInclusive: through, })) - externalLabels := api.config().GlobalConfig.ExternalLabels.Clone() for _, ts := range resp.Results[i].Timeseries { globalUsed := map[string]struct{}{} for _, l := range ts.Labels { diff --git a/web/api/v1/api_test.go b/web/api/v1/api_test.go index 6b5e9b033..8be5e8496 100644 --- a/web/api/v1/api_test.go +++ b/web/api/v1/api_test.go @@ -560,6 +560,7 @@ func TestReadEndpoint(t *testing.T) { ExternalLabels: model.LabelSet{ "baz": "a", "b": "c", + "d": "e", }, }, } @@ -567,11 +568,15 @@ func TestReadEndpoint(t *testing.T) { } // Encode the request. - matcher, err := metric.NewLabelMatcher(metric.Equal, "__name__", "test_metric1") + matcher1, err := metric.NewLabelMatcher(metric.Equal, "__name__", "test_metric1") if err != nil { t.Fatal(err) } - query, err := remote.ToQuery(0, 1, metric.LabelMatchers{matcher}) + matcher2, err := metric.NewLabelMatcher(metric.Equal, "d", "e") + if err != nil { + t.Fatal(err) + } + query, err := remote.ToQuery(0, 1, metric.LabelMatchers{matcher1, matcher2}) if err != nil { t.Fatal(err) } @@ -611,7 +616,7 @@ func TestReadEndpoint(t *testing.T) { result := remote.FromQueryResult(resp.Results[0]) expected := &model.Matrix{ &model.SampleStream{ - Metric: model.Metric{"__name__": "test_metric1", "b": "c", "baz": "qux", "foo": "bar"}, + Metric: model.Metric{"__name__": "test_metric1", "b": "c", "d": "e", "baz": "qux", "foo": "bar"}, Values: []model.SamplePair{model.SamplePair{Value: 1, Timestamp: 0}}, }, }