From 1f0e8fdc0de5f84f63cd15454674f63463835353 Mon Sep 17 00:00:00 2001 From: Aliaksandr Valialkin Date: Sun, 3 May 2020 16:41:33 +0300 Subject: [PATCH] lib/promscrape: fix tests after the commit 658a8742ac7b1ce50d68d6a3ecaa8947ec605715 The original commit copies `__address__` label to `instance` label when generating per-target labels as Prometheus does. See https://www.robustperception.io/life-of-a-label for details. Updates https://github.com/VictoriaMetrics/VictoriaMetrics/issues/453 --- lib/promrelabel/relabel.go | 24 ++------- lib/promrelabel/relabel_test.go | 6 +-- lib/promscrape/config.go | 21 ++++---- lib/promscrape/config_test.go | 60 +++++++++++++++++++++++ lib/promscrape/scrapework_test.go | 53 ++++++++++++++++++-- lib/protoparser/prometheus/parser.go | 2 +- lib/protoparser/prometheus/parser_test.go | 4 ++ 7 files changed, 132 insertions(+), 38 deletions(-) diff --git a/lib/promrelabel/relabel.go b/lib/promrelabel/relabel.go index 72cf8bd3aa..c03bc234dc 100644 --- a/lib/promrelabel/relabel.go +++ b/lib/promrelabel/relabel.go @@ -45,12 +45,10 @@ func ApplyRelabelConfigs(labels []prompbmarshal.Label, labelsOffset int, prcs [] } labels = tmp } + labels = removeEmptyLabels(labels, labelsOffset) if isFinalize { labels = FinalizeLabels(labels[:labelsOffset], labels[labelsOffset:]) } - // remove empty empty labels after finalize, or we can't tell if the label value - // is null character or not set when finalize labels - labels = removeEmptyLabels(labels, labelsOffset) SortLabels(labels[labelsOffset:]) return labels } @@ -92,29 +90,15 @@ func RemoveMetaLabels(dst, src []prompbmarshal.Label) []prompbmarshal.Label { return dst } -// FinalizeLabels finalizes labels according to relabel_config rules. -// -// It renames `__address__` to `instance` and removes labels with "__" in the beginning. -// -// See https://prometheus.io/docs/prometheus/latest/configuration/configuration/#relabel_config +// FinalizeLabels removes labels with "__" in the beginning (except of "__name__"). func FinalizeLabels(dst, src []prompbmarshal.Label) []prompbmarshal.Label { for i := range src { label := &src[i] name := label.Name - if !strings.HasPrefix(name, "__") || name == "__name__" { - dst = append(dst, *label) + if strings.HasPrefix(name, "__") && name != "__name__" { continue } - if name == "__address__" { - if GetLabelByName(src, "instance") != nil { - // The `instance` label is already set. Skip `__address__` label. - continue - } - // Rename `__address__` label to `instance`. - labelCopy := *label - labelCopy.Name = "instance" - dst = append(dst, labelCopy) - } + dst = append(dst, *label) } return dst } diff --git a/lib/promrelabel/relabel_test.go b/lib/promrelabel/relabel_test.go index dedbebd992..a2ea01eaa2 100644 --- a/lib/promrelabel/relabel_test.go +++ b/lib/promrelabel/relabel_test.go @@ -16,7 +16,7 @@ func TestApplyRelabelConfigs(t *testing.T) { t.Fatalf("unexpected result; got\n%v\nwant\n%v", result, resultExpected) } } - t.Run("empty_replabel_configs", func(t *testing.T) { + t.Run("empty_relabel_configs", func(t *testing.T) { f(nil, nil, false, nil) f(nil, nil, true, nil) f(nil, []prompbmarshal.Label{ @@ -238,7 +238,7 @@ func TestApplyRelabelConfigs(t *testing.T) { Value: "yyy", }, { - Name: "__address__", + Name: "instance", Value: "a.bc", }, }, true, []prompbmarshal.Label{ @@ -591,7 +591,7 @@ func TestFinalizeLabels(t *testing.T) { Value: "ass", }, { - Name: "__address__", + Name: "instance", Value: "foo.com", }, }, []prompbmarshal.Label{ diff --git a/lib/promscrape/config.go b/lib/promscrape/config.go index a316d62c05..7899f40c6c 100644 --- a/lib/promscrape/config.go +++ b/lib/promscrape/config.go @@ -430,16 +430,7 @@ func appendScrapeWork(dst []ScrapeWork, swc *scrapeWorkConfig, target string, ex // Drop target with '/' return dst, nil } - instanceRelabeled := promrelabel.GetLabelValueByName(labels, "instance") - if instanceRelabeled == "" { - // After relabeling, the instance label is set to the value of __address__ by default - // if it was not set during relabeling. - labels = append(labels, prompbmarshal.Label{ - Name: "instance", - Value: addressRelabeled, - }) - } - + addressRelabeled = addMissingPort(schemeRelabeled, addressRelabeled) metricsPathRelabeled := promrelabel.GetLabelValueByName(labels, "__metrics_path__") if metricsPathRelabeled == "" { metricsPathRelabeled = "/metrics" @@ -455,6 +446,14 @@ func appendScrapeWork(dst []ScrapeWork, swc *scrapeWorkConfig, target string, ex return dst, fmt.Errorf("invalid url %q for scheme=%q (%q), target=%q (%q), metrics_path=%q (%q) for `job_name` %q: %s", scrapeURL, swc.scheme, schemeRelabeled, target, addressRelabeled, swc.metricsPath, metricsPathRelabeled, swc.jobName, err) } + // Set missing "instance" label according to https://www.robustperception.io/life-of-a-label + if promrelabel.GetLabelByName(labels, "instance") == nil { + labels = append(labels, prompbmarshal.Label{ + Name: "instance", + Value: addressRelabeled, + }) + promrelabel.SortLabels(labels) + } dst = append(dst, ScrapeWork{ ID: atomic.AddUint64(&nextScrapeWorkID, 1), ScrapeURL: scrapeURL, @@ -498,7 +497,7 @@ func mergeLabels(job, scheme, target, metricsPath string, extraLabels, externalL m[k] = v } m["job"] = job - m["__address__"] = addMissingPort(scheme, target) + m["__address__"] = target m["__scheme__"] = scheme m["__metrics_path__"] = metricsPath for k, args := range params { diff --git a/lib/promscrape/config_test.go b/lib/promscrape/config_test.go index e6a2484b3e..596047d0de 100644 --- a/lib/promscrape/config_test.go +++ b/lib/promscrape/config_test.go @@ -438,6 +438,10 @@ scrape_configs: Name: "__vm_filepath", Value: "", }, + { + Name: "instance", + Value: "host1:80", + }, { Name: "job", Value: "foo", @@ -472,6 +476,10 @@ scrape_configs: Name: "__vm_filepath", Value: "", }, + { + Name: "instance", + Value: "host2:80", + }, { Name: "job", Value: "foo", @@ -506,6 +514,10 @@ scrape_configs: Name: "__vm_filepath", Value: "", }, + { + Name: "instance", + Value: "localhost:9090", + }, { Name: "job", Value: "foo", @@ -558,6 +570,10 @@ scrape_configs: Name: "__scheme__", Value: "http", }, + { + Name: "instance", + Value: "foo.bar:1234", + }, { Name: "job", Value: "foo", @@ -599,6 +615,10 @@ scrape_configs: Name: "datacenter", Value: "foobar", }, + { + Name: "instance", + Value: "foo.bar:1234", + }, { Name: "job", Value: "foo", @@ -664,6 +684,10 @@ scrape_configs: Name: "__scheme__", Value: "https", }, + { + Name: "instance", + Value: "foo.bar:443", + }, { Name: "job", Value: "foo", @@ -700,6 +724,10 @@ scrape_configs: Name: "__scheme__", Value: "https", }, + { + Name: "instance", + Value: "aaa:443", + }, { Name: "job", Value: "foo", @@ -732,6 +760,10 @@ scrape_configs: Name: "__scheme__", Value: "http", }, + { + Name: "instance", + Value: "1.2.3.4:80", + }, { Name: "job", Value: "qwer", @@ -805,6 +837,10 @@ scrape_configs: Name: "hash", Value: "82", }, + { + Name: "instance", + Value: "foo.bar:1234", + }, { Name: "prefix:url", Value: "http://foo.bar:1234/metrics", @@ -904,6 +940,10 @@ scrape_configs: Name: "__address__", Value: "foo.bar:1234", }, + { + Name: "instance", + Value: "foo.bar:1234", + }, { Name: "job", Value: "3", @@ -938,6 +978,10 @@ scrape_configs: Name: "__scheme__", Value: "http", }, + { + Name: "instance", + Value: "foo.bar:1234", + }, { Name: "job", Value: "foo", @@ -982,6 +1026,10 @@ scrape_configs: Name: "__scheme__", Value: "http", }, + { + Name: "instance", + Value: "foo.bar:1234", + }, { Name: "job", Value: "foo", @@ -1016,6 +1064,10 @@ scrape_configs: Name: "__scheme__", Value: "http", }, + { + Name: "instance", + Value: "foo.bar:1234", + }, { Name: "job", Value: "foo", @@ -1056,6 +1108,10 @@ scrape_configs: Name: "__scheme__", Value: "http", }, + { + Name: "instance", + Value: "foo.bar:1234", + }, { Name: "job", Value: "foo", @@ -1111,6 +1167,10 @@ scrape_configs: Name: "foo", Value: "bar", }, + { + Name: "instance", + Value: "pp:80", + }, { Name: "job", Value: "yyy", diff --git a/lib/promscrape/scrapework_test.go b/lib/promscrape/scrapework_test.go index 7f20cae4c1..d3b4294f8c 100644 --- a/lib/promscrape/scrapework_test.go +++ b/lib/promscrape/scrapework_test.go @@ -99,7 +99,7 @@ func TestScrapeWorkScrapeInternalSuccess(t *testing.T) { scrape_samples_post_metric_relabeling 0 123 `) f(` - foo{bar="baz"} 34.45 3 + foo{bar="baz",empty_label=""} 34.45 3 abc -2 `, &ScrapeWork{}, ` foo{bar="baz"} 34.45 123 @@ -147,6 +147,53 @@ func TestScrapeWorkScrapeInternalSuccess(t *testing.T) { scrape_duration_seconds{job="override"} 0 123 scrape_samples_post_metric_relabeling{job="override"} 2 123 `) + // Empty instance override. See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/453 + f(` + no_instance{instance="",job="some_job",label="val1",test=""} 5555 + test_with_instance{instance="some_instance",job="some_job",label="val2",test=""} 1555 + `, &ScrapeWork{ + HonorLabels: true, + Labels: []prompbmarshal.Label{ + { + Name: "instance", + Value: "foobar", + }, + { + Name: "job", + Value: "xxx", + }, + }, + }, ` + no_instance{job="some_job",label="val1"} 5555 123 + test_with_instance{instance="some_instance",job="some_job",label="val2"} 1555 123 + up{instance="foobar",job="xxx"} 1 123 + scrape_samples_scraped{instance="foobar",job="xxx"} 2 123 + scrape_duration_seconds{instance="foobar",job="xxx"} 0 123 + scrape_samples_post_metric_relabeling{instance="foobar",job="xxx"} 2 123 + `) + f(` + no_instance{instance="",job="some_job",label="val1",test=""} 5555 + test_with_instance{instance="some_instance",job="some_job",label="val2",test=""} 1555 + `, &ScrapeWork{ + HonorLabels: false, + Labels: []prompbmarshal.Label{ + { + Name: "instance", + Value: "foobar", + }, + { + Name: "job", + Value: "xxx", + }, + }, + }, ` + no_instance{exported_job="some_job",instance="foobar",job="xxx",label="val1"} 5555 123 + test_with_instance{exported_instance="some_instance",exported_job="some_job",instance="foobar",job="xxx",label="val2"} 1555 123 + up{instance="foobar",job="xxx"} 1 123 + scrape_samples_scraped{instance="foobar",job="xxx"} 2 123 + scrape_duration_seconds{instance="foobar",job="xxx"} 0 123 + scrape_samples_post_metric_relabeling{instance="foobar",job="xxx"} 2 123 + `) f(` foo{job="orig",bar="baz"} 34.45 bar{job="aa",a="b",job="bb"} -3e4 2345 @@ -205,7 +252,7 @@ func TestScrapeWorkScrapeInternalSuccess(t *testing.T) { `) f(` foo{bar="baz"} 34.44 - bar{a="b",c="d"} -3e4 + bar{a="b",c="d",} -3e4 dropme{foo="bar"} 334 dropme{xxx="yy",ss="dsf"} 843 `, &ScrapeWork{ @@ -216,7 +263,7 @@ func TestScrapeWorkScrapeInternalSuccess(t *testing.T) { Value: "xx", }, { - Name: "__address__", + Name: "instance", Value: "foo.com", }, }, diff --git a/lib/protoparser/prometheus/parser.go b/lib/protoparser/prometheus/parser.go index e2e5767473..6a8d720977 100644 --- a/lib/protoparser/prometheus/parser.go +++ b/lib/protoparser/prometheus/parser.go @@ -230,8 +230,8 @@ func unmarshalTags(dst []Tag, s string, noEscapes bool) (string, []Tag, error) { } s = s[n+1:] } - // keep null character label value if len(key) > 0 { + // Allow empty values (len(value)==0) - see https://github.com/VictoriaMetrics/VictoriaMetrics/issues/453 if cap(dst) > len(dst) { dst = dst[:len(dst)+1] } else { diff --git a/lib/protoparser/prometheus/parser_test.go b/lib/protoparser/prometheus/parser_test.go index ce945bf39f..d86a19fb42 100644 --- a/lib/protoparser/prometheus/parser_test.go +++ b/lib/protoparser/prometheus/parser_test.go @@ -221,6 +221,10 @@ func TestRowsUnmarshalSuccess(t *testing.T) { Key: "bar", Value: "baz", }, + { + Key: "aa", + Value: "", + }, { Key: "x", Value: "y",