From bd9129117ef642e498b6b0bf8f1c5b516f66c93c Mon Sep 17 00:00:00 2001 From: Julien Pivotto Date: Fri, 15 Jul 2022 15:20:44 +0200 Subject: [PATCH] Target parameter labels should not be overridden by config params The following configuration snippet calls http://127.0.0.1:8000/?foo=value1 instead of http://127.0.0.1:8000/?foo=value2. This is incorrect, the labels of the target should be prefered. ```yaml - job_name: local params: foo: [value1] static_configs: - targets: ['127.0.0.1:8000'] labels: __param_foo: value2 ``` Signed-off-by: Julien Pivotto Signed-off-by: Julien --- scrape/scrape_test.go | 158 ++++++++++++++++++++++++++++++++++++++++++ scrape/target.go | 4 +- 2 files changed, 160 insertions(+), 2 deletions(-) diff --git a/scrape/scrape_test.go b/scrape/scrape_test.go index b703f21d4..15ec71497 100644 --- a/scrape/scrape_test.go +++ b/scrape/scrape_test.go @@ -3838,3 +3838,161 @@ scrape_configs: require.Equal(t, expectedSchema, h.Schema) } } + +func TestTargetScrapeConfigWithLabels(t *testing.T) { + const ( + configTimeout = 1500 * time.Millisecond + expectedTimeout = "1.5" + expectedTimeoutLabel = "1s500ms" + secondTimeout = 500 * time.Millisecond + secondTimeoutLabel = "500ms" + expectedParam = "value1" + secondParam = "value2" + expectedPath = "/metric-ok" + secondPath = "/metric-nok" + httpScheme = "http" + paramLabel = "__param_param" + jobName = "test" + ) + + createTestServer := func(t *testing.T, done chan struct{}) *url.URL { + server := httptest.NewServer( + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + defer close(done) + require.Equal(t, expectedTimeout, r.Header.Get("X-Prometheus-Scrape-Timeout-Seconds")) + require.Equal(t, expectedParam, r.URL.Query().Get("param")) + require.Equal(t, expectedPath, r.URL.Path) + + w.Header().Set("Content-Type", `text/plain; version=0.0.4`) + w.Write([]byte("metric_a 1\nmetric_b 2\n")) + }), + ) + t.Cleanup(server.Close) + serverURL, err := url.Parse(server.URL) + require.NoError(t, err) + return serverURL + } + + run := func(t *testing.T, cfg *config.ScrapeConfig, targets []*targetgroup.Group) chan struct{} { + done := make(chan struct{}) + srvURL := createTestServer(t, done) + + // Update target addresses to use the dynamically created server URL. + for _, target := range targets { + for i := range target.Targets { + target.Targets[i][model.AddressLabel] = model.LabelValue(srvURL.Host) + } + } + + sp, err := newScrapePool(cfg, &nopAppendable{}, 0, nil, nil, &Options{}, newTestScrapeMetrics(t)) + require.NoError(t, err) + t.Cleanup(sp.stop) + + sp.Sync(targets) + return done + } + + cases := []struct { + name string + cfg *config.ScrapeConfig + targets []*targetgroup.Group + }{ + { + name: "Everything in scrape config", + cfg: &config.ScrapeConfig{ + ScrapeInterval: model.Duration(2 * time.Second), + ScrapeTimeout: model.Duration(configTimeout), + Params: url.Values{"param": []string{expectedParam}}, + JobName: jobName, + Scheme: httpScheme, + MetricsPath: expectedPath, + }, + targets: []*targetgroup.Group{ + { + Targets: []model.LabelSet{ + {model.AddressLabel: model.LabelValue("")}, + }, + }, + }, + }, + { + name: "Overridden in target", + cfg: &config.ScrapeConfig{ + ScrapeInterval: model.Duration(2 * time.Second), + ScrapeTimeout: model.Duration(secondTimeout), + JobName: jobName, + Scheme: httpScheme, + MetricsPath: secondPath, + Params: url.Values{"param": []string{secondParam}}, + }, + targets: []*targetgroup.Group{ + { + Targets: []model.LabelSet{ + { + model.AddressLabel: model.LabelValue(""), + model.ScrapeTimeoutLabel: expectedTimeoutLabel, + model.MetricsPathLabel: expectedPath, + paramLabel: expectedParam, + }, + }, + }, + }, + }, + { + name: "Overridden in relabel_config", + cfg: &config.ScrapeConfig{ + ScrapeInterval: model.Duration(2 * time.Second), + ScrapeTimeout: model.Duration(secondTimeout), + JobName: jobName, + Scheme: httpScheme, + MetricsPath: secondPath, + Params: url.Values{"param": []string{secondParam}}, + RelabelConfigs: []*relabel.Config{ + { + Action: relabel.DefaultRelabelConfig.Action, + Regex: relabel.DefaultRelabelConfig.Regex, + SourceLabels: relabel.DefaultRelabelConfig.SourceLabels, + TargetLabel: model.ScrapeTimeoutLabel, + Replacement: expectedTimeoutLabel, + }, + { + Action: relabel.DefaultRelabelConfig.Action, + Regex: relabel.DefaultRelabelConfig.Regex, + SourceLabels: relabel.DefaultRelabelConfig.SourceLabels, + TargetLabel: paramLabel, + Replacement: expectedParam, + }, + { + Action: relabel.DefaultRelabelConfig.Action, + Regex: relabel.DefaultRelabelConfig.Regex, + SourceLabels: relabel.DefaultRelabelConfig.SourceLabels, + TargetLabel: model.MetricsPathLabel, + Replacement: expectedPath, + }, + }, + }, + targets: []*targetgroup.Group{ + { + Targets: []model.LabelSet{ + { + model.AddressLabel: model.LabelValue(""), + model.ScrapeTimeoutLabel: secondTimeoutLabel, + model.MetricsPathLabel: secondPath, + paramLabel: secondParam, + }, + }, + }, + }, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + select { + case <-run(t, c.cfg, c.targets): + case <-time.After(10 * time.Second): + t.Fatal("timeout after 10 seconds") + } + }) + } +} diff --git a/scrape/target.go b/scrape/target.go index 9ef4471fb..375439833 100644 --- a/scrape/target.go +++ b/scrape/target.go @@ -441,8 +441,8 @@ func PopulateLabels(lb *labels.Builder, cfg *config.ScrapeConfig, noDefaultPort } // Encode scrape query parameters as labels. for k, v := range cfg.Params { - if len(v) > 0 { - lb.Set(model.ParamLabelPrefix+k, v[0]) + if name := model.ParamLabelPrefix + k; len(v) > 0 && lb.Get(name) == "" { + lb.Set(name, v[0]) } }