From 8958cecad68cda49a637c79d6b056d5953b557a7 Mon Sep 17 00:00:00 2001 From: Zhu Jiekun Date: Tue, 20 Aug 2024 04:28:49 +0800 Subject: [PATCH] lib/promrelabel: stop adding default port 80/433 to address label * It was necessary to add default ports for fasthttp client. After migration to the std.httpclient it's no longer needed. * An additional configuration is required at proxy servers with implicitly set 80/443 ports to the host header (such as HA proxy. It's expected that after upgrade __address_ label may change. But it should be rare case. 80/443 ports are not widely used at monitoring ecosystem. And it shouldn't have much impact. Related issue https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6792 Co-authored-by: Nikolay --- docs/CHANGELOG.md | 3 ++ lib/promrelabel/scrape_url.go | 22 ------------- lib/promrelabel/scrape_url_test.go | 28 +++++++++------- lib/promscrape/config_test.go | 52 +++++++++++++++--------------- 4 files changed, 45 insertions(+), 60 deletions(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 52f508b6a..4f7e0f06e 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -30,11 +30,14 @@ See also [LTS releases](https://docs.victoriametrics.com/lts-releases/). ## tip **Update note 1: The `external_labels` field in vmalert-tool [test file](https://docs.victoriametrics.com/vmalert-tool/#test-file-format) will be deprecated soon. Please use `-external.label` command-line flag instead, in the same way as vmalert uses it. This change is done for the sake of consistency between vmalert and vmalert-tool configuration. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6735).** +**Update note 2: starting from this release, [vmagent](https://docs.victoriametrics.com/vmagent/) and [Single-node VictoriaMetrics](https://docs.victoriametrics.com/) will no longer add the default port 80/443 for scrape URLs without a specified port. +The value of `instance` label for those scrape targets will be changed from `
:<80|443>` to `
`, generating new time series. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6792) for details.** * FEATURE: add `/influx/health` health-check handler for Influx endpoints. This is needed as some clients use the health endpoint to determine if the server is healthy and ready for data ingestion. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6653) for the details. * FEATURE: [vmctl](https://docs.victoriametrics.com/vmctl/): add `--vm-backoff-retries`, `--vm-backoff-factor`, `--vm-backoff-min-duration` and `--vm-native-backoff-retries`, `--vm-native-backoff-factor`, `--vm-native-backoff-min-duration` command-line flags. These flags allow to change backoff policy config for import requests to VictoriaMetrics. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6622). * FEATURE: [vmagent](https://docs.victoriametrics.com/vmagent/): allow overriding the `sample_limit` option at [scrape_configs](https://docs.victoriametrics.com/sd_configs/#scrape_configs) when a label `__sample_limit__` is specified for target. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6665). Thanks to @zoglam for the [pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/6666). * FEATURE: [vmagent](https://docs.victoriametrics.com/vmagent/): reduce memory usage when scraping targets with big response body. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6759). +* FEATURE: [vmagent](https://docs.victoriametrics.com/vmagent/) and [Single-node VictoriaMetrics](https://docs.victoriametrics.com/): stop adding default port 80/443 for scrape URLs without a port. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6792). * FEATURE: [vmalert](https://docs.victoriametrics.com/vmalert/): add command-line flag `-notifier.headers` to allow configuring additional headers for all requests sent to the corresponding `-notifier.url`. * FEATURE: [vmalert-tool](https://docs.victoriametrics.com/vmalert-tool/): add `-external.label` and `-external.url` command-line flags, in the same way as these flags are supported by vmalert. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6735). * FEATURE: [vmbackup](https://docs.victoriametrics.com/vmbackup/), [vmrestore](https://docs.victoriametrics.com/vmrestore/), [vmbackupmanager](https://docs.victoriametrics.com/vmbackupmanager/): use exponential backoff for retries when uploading or downloading data from S3. This should reduce the number of failed uploads and downloads when S3 is temporarily unavailable. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6732). diff --git a/lib/promrelabel/scrape_url.go b/lib/promrelabel/scrape_url.go index 55d7d5d5d..7533d7547 100644 --- a/lib/promrelabel/scrape_url.go +++ b/lib/promrelabel/scrape_url.go @@ -37,7 +37,6 @@ func GetScrapeURL(labels *promutils.Labels, extraParams map[string][]string) (st metricsPath = address[n:] address = address[:n] } - address = addMissingPort(address, scheme == "https") if !strings.HasPrefix(metricsPath, "/") { metricsPath = "/" + metricsPath @@ -90,25 +89,4 @@ func buildScrapeURL(scheme, address, metricsPath, optionalQuestion, paramsStr st return s } -func addMissingPort(addr string, isTLS bool) string { - if strings.Contains(addr, ":") { - return addr - } - if isTLS { - return concatTwoStrings(addr, ":443") - } - return concatTwoStrings(addr, ":80") -} - -func concatTwoStrings(x, y string) string { - bb := bbPool.Get() - b := bb.B[:0] - b = append(b, x...) - b = append(b, y...) - s := bytesutil.InternBytes(b) - bb.B = b - bbPool.Put(bb) - return s -} - var bbPool bytesutil.ByteBufferPool diff --git a/lib/promrelabel/scrape_url_test.go b/lib/promrelabel/scrape_url_test.go index de8da3a32..7bdbdb5a1 100644 --- a/lib/promrelabel/scrape_url_test.go +++ b/lib/promrelabel/scrape_url_test.go @@ -24,31 +24,35 @@ func TestGetScrapeURL(t *testing.T) { f(`{foo="bar"}`, "", "") // __address__ without port - f(`{__address__="foo"}`, "http://foo:80/metrics", "foo:80") + f(`{__address__="foo"}`, "http://foo/metrics", "foo") // __address__ with explicit port f(`{__address__="foo:1234"}`, "http://foo:1234/metrics", "foo:1234") // explicit __scheme__ - f(`{__address__="foo",__scheme__="https"}`, "https://foo:443/metrics", "foo:443") + f(`{__address__="foo",__scheme__="https"}`, "https://foo/metrics", "foo") f(`{__address__="foo:1234",__scheme__="https"}`, "https://foo:1234/metrics", "foo:1234") // explicit __metrics_path__ - f(`{__address__="foo",__metrics_path__="abc"}`, "http://foo:80/abc", "foo:80") - f(`{__address__="foo",__metrics_path__="/abc"}`, "http://foo:80/abc", "foo:80") - f(`{__address__="foo",__metrics_path__="/ab/c?d=ef&aa=bb"}`, "http://foo:80/ab/c?d=ef&aa=bb", "foo:80") + f(`{__address__="foo",__metrics_path__="abc"}`, "http://foo/abc", "foo") + f(`{__address__="foo",__metrics_path__="/abc"}`, "http://foo/abc", "foo") + f(`{__address__="foo",__metrics_path__="/ab/c?d=ef&aa=bb"}`, "http://foo/ab/c?d=ef&aa=bb", "foo") // explitit __param_* - f(`{__address__="foo",__param_x="y"}`, "http://foo:80/metrics?x=y", "foo:80") - f(`{__address__="foo",__param_x="y",__param_y="aa"}`, "http://foo:80/metrics?x=y&y=aa", "foo:80") - f(`{__address__="foo",__param_x="y",__metrics_path__="?abc=de"}`, "http://foo:80/?abc=de&x=y", "foo:80") - f(`{__address__="foo",__param_abc="y",__metrics_path__="?abc=de"}`, "http://foo:80/?abc=de&abc=y", "foo:80") + f(`{__address__="foo",__param_x="y"}`, "http://foo/metrics?x=y", "foo") + f(`{__address__="foo",__param_x="y",__param_y="aa"}`, "http://foo/metrics?x=y&y=aa", "foo") + f(`{__address__="foo",__param_x="y",__metrics_path__="?abc=de"}`, "http://foo/?abc=de&x=y", "foo") + f(`{__address__="foo",__param_abc="y",__metrics_path__="?abc=de"}`, "http://foo/?abc=de&abc=y", "foo") // __address__ with metrics path and/or scheme - f(`{__address__="foo/bar/baz?abc=de"}`, "http://foo:80/bar/baz?abc=de", "foo:80") + f(`{__address__="foo/bar/baz?abc=de"}`, "http://foo/bar/baz?abc=de", "foo") f(`{__address__="foo:784/bar/baz?abc=de"}`, "http://foo:784/bar/baz?abc=de", "foo:784") f(`{__address__="foo:784/bar/baz?abc=de",__param_xx="yy"}`, "http://foo:784/bar/baz?abc=de&xx=yy", "foo:784") f(`{__address__="foo:784/bar/baz?abc=de",__param_xx="yy",__scheme__="https"}`, "https://foo:784/bar/baz?abc=de&xx=yy", "foo:784") - f(`{__address__="http://foo/bar/baz?abc=de",__param_xx="yy"}`, "http://foo:80/bar/baz?abc=de&xx=yy", "foo:80") - f(`{__address__="https://foo/bar/baz?abc=de",__param_xx="yy"}`, "https://foo:443/bar/baz?abc=de&xx=yy", "foo:443") + f(`{__address__="http://foo/bar/baz?abc=de",__param_xx="yy"}`, "http://foo/bar/baz?abc=de&xx=yy", "foo") + f(`{__address__="https://foo/bar/baz?abc=de",__param_xx="yy"}`, "https://foo/bar/baz?abc=de&xx=yy", "foo") + + // __address__ with 80 or 443 as port + f(`{__address__="http://foo:80"}`, "http://foo:80/metrics", "foo:80") + f(`{__address__="https://foo:443"}`, "https://foo:443/metrics", "foo:443") } diff --git a/lib/promscrape/config_test.go b/lib/promscrape/config_test.go index 29930d380..a73c5b5fd 100644 --- a/lib/promscrape/config_test.go +++ b/lib/promscrape/config_test.go @@ -224,23 +224,23 @@ scrape_configs: sws := cfg.getStaticScrapeWork() swsExpected := []*ScrapeWork{ { - ScrapeURL: "http://host1:80/metric/path1?x=y", + ScrapeURL: "http://host1/metric/path1?x=y", ScrapeInterval: defaultScrapeInterval, ScrapeTimeout: defaultScrapeTimeout, MaxScrapeSize: maxScrapeSize.N, Labels: promutils.NewLabelsFromMap(map[string]string{ - "instance": "host1:80", + "instance": "host1", "job": "abc", }), jobNameOriginal: "abc", }, { - ScrapeURL: "https://host2:443/metric/path2?x=y", + ScrapeURL: "https://host2/metric/path2?x=y", ScrapeInterval: defaultScrapeInterval, ScrapeTimeout: defaultScrapeTimeout, MaxScrapeSize: maxScrapeSize.N, Labels: promutils.NewLabelsFromMap(map[string]string{ - "instance": "host2:443", + "instance": "host2", "job": "abc", }), jobNameOriginal: "abc", @@ -459,24 +459,24 @@ scrape_configs: - files: ["testdata/file_sd.json", "testdata/file_sd*.yml"] `, []*ScrapeWork{ { - ScrapeURL: "http://host1:80/abc/de", + ScrapeURL: "http://host1/abc/de", ScrapeInterval: defaultScrapeInterval, ScrapeTimeout: defaultScrapeTimeout, MaxScrapeSize: maxScrapeSize.N, Labels: promutils.NewLabelsFromMap(map[string]string{ - "instance": "host1:80", + "instance": "host1", "job": "foo", "qwe": "rty", }), jobNameOriginal: "foo", }, { - ScrapeURL: "http://host2:80/abc/de", + ScrapeURL: "http://host2/abc/de", ScrapeInterval: defaultScrapeInterval, ScrapeTimeout: defaultScrapeTimeout, MaxScrapeSize: maxScrapeSize.N, Labels: promutils.NewLabelsFromMap(map[string]string{ - "instance": "host2:80", + "instance": "host2", "job": "foo", "qwe": "rty", }), @@ -690,12 +690,12 @@ scrape_configs: - targets: ["s"] `, []*ScrapeWork{ { - ScrapeURL: "http://s:80/metrics", + ScrapeURL: "http://s/metrics", ScrapeInterval: defaultScrapeInterval, ScrapeTimeout: defaultScrapeTimeout, MaxScrapeSize: maxScrapeSize.N, Labels: promutils.NewLabelsFromMap(map[string]string{ - "instance": "s:80", + "instance": "s", "job": "aa", }), jobNameOriginal: "aa", @@ -712,12 +712,12 @@ scrape_configs: - targets: ["s"] `, []*ScrapeWork{ { - ScrapeURL: "http://s:80/metrics", + ScrapeURL: "http://s/metrics", ScrapeInterval: defaultScrapeInterval, ScrapeTimeout: defaultScrapeTimeout, MaxScrapeSize: maxScrapeSize.N, Labels: promutils.NewLabelsFromMap(map[string]string{ - "instance": "s:80", + "instance": "s", "job": "aa", }), jobNameOriginal: "aa", @@ -734,12 +734,12 @@ scrape_configs: - targets: ["s"] `, []*ScrapeWork{ { - ScrapeURL: "http://s:80/metrics", + ScrapeURL: "http://s/metrics", ScrapeInterval: defaultScrapeInterval, ScrapeTimeout: defaultScrapeTimeout, MaxScrapeSize: maxScrapeSize.N, Labels: promutils.NewLabelsFromMap(map[string]string{ - "instance": "s:80", + "instance": "s", "job": "aa", }), jobNameOriginal: "aa", @@ -756,12 +756,12 @@ scrape_configs: - targets: ["s"] `, []*ScrapeWork{ { - ScrapeURL: "http://s:80/metrics", + ScrapeURL: "http://s/metrics", ScrapeInterval: defaultScrapeInterval, ScrapeTimeout: defaultScrapeTimeout, MaxScrapeSize: maxScrapeSize.N, Labels: promutils.NewLabelsFromMap(map[string]string{ - "instance": "s:80", + "instance": "s", "job": "aa", }), jobNameOriginal: "aa", @@ -845,7 +845,7 @@ scrape_configs: - targets: [foobar] `, []*ScrapeWork{ { - ScrapeURL: "https://foo.bar:443/foo/bar?p=x%26y&p=%3D", + ScrapeURL: "https://foo.bar/foo/bar?p=x%26y&p=%3D", ScrapeInterval: 54 * time.Second, ScrapeTimeout: 5 * time.Second, MaxScrapeSize: maxScrapeSize.N, @@ -853,7 +853,7 @@ scrape_configs: HonorTimestamps: true, DenyRedirects: true, Labels: promutils.NewLabelsFromMap(map[string]string{ - "instance": "foo.bar:443", + "instance": "foo.bar", "job": "foo", "x": "y", }), @@ -861,7 +861,7 @@ scrape_configs: jobNameOriginal: "foo", }, { - ScrapeURL: "https://aaa:443/foo/bar?p=x%26y&p=%3D", + ScrapeURL: "https://aaa/foo/bar?p=x%26y&p=%3D", ScrapeInterval: 54 * time.Second, ScrapeTimeout: 5 * time.Second, MaxScrapeSize: maxScrapeSize.N, @@ -869,7 +869,7 @@ scrape_configs: HonorTimestamps: true, DenyRedirects: true, Labels: promutils.NewLabelsFromMap(map[string]string{ - "instance": "aaa:443", + "instance": "aaa", "job": "foo", "x": "y", }), @@ -877,23 +877,23 @@ scrape_configs: jobNameOriginal: "foo", }, { - ScrapeURL: "http://1.2.3.4:80/metrics", + ScrapeURL: "http://1.2.3.4/metrics", ScrapeInterval: 8 * time.Second, ScrapeTimeout: 8 * time.Second, MaxScrapeSize: maxScrapeSize.N, Labels: promutils.NewLabelsFromMap(map[string]string{ - "instance": "1.2.3.4:80", + "instance": "1.2.3.4", "job": "qwer", }), jobNameOriginal: "qwer", }, { - ScrapeURL: "http://foobar:80/metrics", + ScrapeURL: "http://foobar/metrics", ScrapeInterval: 8 * time.Second, ScrapeTimeout: 8 * time.Second, MaxScrapeSize: maxScrapeSize.N, Labels: promutils.NewLabelsFromMap(map[string]string{ - "instance": "foobar:80", + "instance": "foobar", "job": "asdf", }), jobNameOriginal: "asdf", @@ -1103,13 +1103,13 @@ scrape_configs: job: yyy `, []*ScrapeWork{ { - ScrapeURL: "http://pp:80/metrics?a=c&a=xy", + ScrapeURL: "http://pp/metrics?a=c&a=xy", ScrapeInterval: defaultScrapeInterval, ScrapeTimeout: defaultScrapeTimeout, MaxScrapeSize: maxScrapeSize.N, Labels: promutils.NewLabelsFromMap(map[string]string{ "foo": "bar", - "instance": "pp:80", + "instance": "pp", "job": "yyy", }), ExternalLabels: promutils.NewLabelsFromMap(map[string]string{