lib/promscrape: properly add exported_ prefix to labels, which clash with target labels if honor_labels: true option isn't set.

The issue was in the `labels := dst[offset:]` line in the beginning of appendExtraLabels() function.
The `dst` may be re-allocated when adding extra labels to it. In this case the addition of `exported_`
prefix to labels inside `labels` slice become invisible in the returned `dst` labels.

While at it, properly handle some corner cases:

- Add additional `exported_` prefix to clashing metric labels with already existing `exported_` prefix.
- Store scraped metric names in `exported___name__` label if scrape target contains `__name__` label.

Updates https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3278

Thanks to @jplanckeel for the initial attempt to fix this issue
at https://github.com/VictoriaMetrics/VictoriaMetrics/pull/3281
This commit is contained in:
Aliaksandr Valialkin 2022-10-28 21:34:07 +03:00
parent ba739c8052
commit ac5528cb46
No known key found for this signature in database
GPG Key ID: A72BEC6CD3D0DED1
3 changed files with 29 additions and 15 deletions

View File

@ -62,6 +62,7 @@ The following tip changes can be tested by building VictoriaMetrics components f
* BUGFIX: [vmalert](https://docs.victoriametrics.com/vmalert.html): fix panic if `vmalert` runs with `-clusterMode` command-line flag in [multitenant mode](https://docs.victoriametrics.com/vmalert.html#multitenancy). The issue has been introduced in [v1.82.0](https://docs.victoriametrics.com/CHANGELOG.html#v1820).
* BUGFIX: [vmalert](https://docs.victoriametrics.com/vmalert.html): properly escape string passed to `quotesEscape` [template function](https://docs.victoriametrics.com/vmalert.html#template-functions), so it can be safely embedded into JSON string. This makes obsolete the `crlfEscape` function. See [this](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3139) and [this](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/890) issue.
* BUGFIX: [vmagent](https://docs.victoriametrics.com/vmagent.html): do not show invalid error message in Kubernetes service discovery: `cannot parse WatchEvent json response: EOF`. The invalid error message has been appeared in [v1.82.0](https://docs.victoriametrics.com/CHANGELOG.html#v1820).
* BUGFIX: [vmagent](https://docs.victoriametrics.com/vmagent.html): properly add `exported_` prefix to metric labels, which clashing with scrape target labels if `honor_labels: true` option isn't set in [scrape_config](https://docs.victoriametrics.com/sd_configs.html#scrape_configs). Previously some `exported_` prefixes were missing in the resulting metric labels. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3278). The issue has been introduced in [v1.82.0](https://docs.victoriametrics.com/CHANGELOG.html#v1820).
* BUGFIX: `vmselect`: expose missing metric `vm_cache_size_max_bytes{type="promql/rollupResult"}` . This metric is used for monitoring rollup cache usage with the query `vm_cache_size_bytes{type="promql/rollupResult"} / vm_cache_size_max_bytes{type="promql/rollupResult"}` in the same way as this is done for other cache types.
## [v1.82.1](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.82.1)

View File

@ -704,7 +704,15 @@ func (wc *writeRequestCtx) reset() {
func (wc *writeRequestCtx) resetNoRows() {
prompbmarshal.ResetWriteRequest(&wc.writeRequest)
labels := wc.labels
for i := range labels {
label := &labels[i]
label.Name = ""
label.Value = ""
}
wc.labels = wc.labels[:0]
wc.samples = wc.samples[:0]
}
@ -740,6 +748,7 @@ func (sw *scrapeWork) applySeriesLimit(wc *writeRequestCtx) int {
}
dstSeries = append(dstSeries, ts)
}
prompbmarshal.ResetTimeSeries(wc.writeRequest.Timeseries[len(dstSeries):])
wc.writeRequest.Timeseries = dstSeries
return samplesDropped
}
@ -880,16 +889,14 @@ func appendLabels(dst []prompbmarshal.Label, metric string, src []parser.Tag, ex
func appendExtraLabels(dst, extraLabels []prompbmarshal.Label, offset int, honorLabels bool) []prompbmarshal.Label {
// Add extraLabels to labels.
// Handle duplicates in the same way as Prometheus does.
if len(dst) > offset && dst[offset].Name == "__name__" {
offset++
}
labels := dst[offset:]
if len(labels) == 0 {
if len(dst) == offset {
// Fast path - add extraLabels to dst without the need to de-duplicate.
dst = append(dst, extraLabels...)
return dst
}
offsetEnd := len(dst)
for _, label := range extraLabels {
labels := dst[offset:offsetEnd]
prevLabel := promrelabel.GetLabelByName(labels, label.Name)
if prevLabel == nil {
// Fast path - the label doesn't exist in labels, so just add it to dst.
@ -904,13 +911,13 @@ func appendExtraLabels(dst, extraLabels []prompbmarshal.Label, offset int, honor
// See https://prometheus.io/docs/prometheus/latest/configuration/configuration/#scrape_config
exportedName := "exported_" + label.Name
exportedLabel := promrelabel.GetLabelByName(labels, exportedName)
if exportedLabel == nil {
prevLabel.Name = exportedName
dst = append(dst, label)
} else {
exportedLabel.Value = prevLabel.Value
prevLabel.Value = label.Value
if exportedLabel != nil {
// The label with the name exported_<label.Name> already exists.
// Add yet another 'exported_' prefix to it.
exportedLabel.Name = "exported_" + exportedName
}
prevLabel.Name = exportedName
dst = append(dst, label)
}
return dst
}

View File

@ -27,16 +27,22 @@ func TestAppendExtraLabels(t *testing.T) {
f("{}", "{}", false, "{}")
f("foo", "{}", true, `{__name__="foo"}`)
f("foo", "{}", false, `{__name__="foo"}`)
f("foo", "bar", true, `{__name__="foo",__name__="bar"}`)
f("foo", "bar", false, `{__name__="foo",__name__="bar"}`)
f("foo", "bar", true, `{__name__="foo"}`)
f("foo", "bar", false, `{exported___name__="foo",__name__="bar"}`)
f(`{a="b"}`, `{c="d"}`, true, `{a="b",c="d"}`)
f(`{a="b"}`, `{c="d"}`, false, `{a="b",c="d"}`)
f(`{a="b"}`, `{a="d"}`, true, `{a="b"}`)
f(`{a="b"}`, `{a="d"}`, false, `{exported_a="b",a="d"}`)
f(`{a="b",exported_a="x"}`, `{a="d"}`, true, `{a="b",exported_a="x"}`)
f(`{a="b",exported_a="x"}`, `{a="d"}`, false, `{a="d",exported_a="b"}`)
f(`{a="b",exported_a="x"}`, `{a="d"}`, false, `{exported_a="b",exported_exported_a="x",a="d"}`)
f(`{a="b"}`, `{a="d",exported_a="x"}`, true, `{a="b",exported_a="x"}`)
f(`{a="b"}`, `{a="d",exported_a="x"}`, false, `{exported_a="b",a="d",exported_a="x"}`)
f(`{a="b"}`, `{a="d",exported_a="x"}`, false, `{exported_exported_a="b",a="d",exported_a="x"}`)
f(`{foo="a",exported_foo="b"}`, `{exported_foo="c"}`, true, `{foo="a",exported_foo="b"}`)
f(`{foo="a",exported_foo="b"}`, `{exported_foo="c"}`, false, `{foo="a",exported_exported_foo="b",exported_foo="c"}`)
f(`{foo="a",exported_foo="b"}`, `{exported_foo="c",bar="x"}`, true, `{foo="a",exported_foo="b",bar="x"}`)
f(`{foo="a",exported_foo="b"}`, `{exported_foo="c",bar="x"}`, false, `{foo="a",exported_exported_foo="b",exported_foo="c",bar="x"}`)
f(`{foo="a",exported_foo="b"}`, `{exported_foo="c",foo="d"}`, true, `{foo="a",exported_foo="b"}`)
f(`{foo="a",exported_foo="b"}`, `{exported_foo="c",foo="d"}`, false, `{exported_foo="a",exported_exported_foo="b",exported_foo="c",foo="d"}`)
}
func TestPromLabelsString(t *testing.T) {