From 244c18fa3830f2e71f7dec383c449a488d246703 Mon Sep 17 00:00:00 2001 From: Dmytro Kozlov Date: Fri, 7 Apr 2023 01:06:52 +0300 Subject: [PATCH] app/vmctl: add multiple filters defined in `--vm-native-filter-match` flag to discovered metric names (#4063) * app/vmctl: add multiple filters defined in `--vm-native-filter-match` flag to discovered metric names * app/vmctl: fix comments * app/vmctl: move function buildMatchWithFilter to the correct place * app/vmctl: update CHANGELOG.md * app/vmctl: fix CI, remove error wrapping * app/vmctl: fix CI, simplify `Set()` --- app/vmctl/vm_native.go | 20 ++++++++++- app/vmctl/vm_native_test.go | 65 ++++++++++++++++++++++++++++++++++++ docs/CHANGELOG.md | 1 + lib/promutils/labels.go | 18 +++++++++- lib/promutils/labels_test.go | 19 +++++++++++ 5 files changed, 121 insertions(+), 2 deletions(-) diff --git a/app/vmctl/vm_native.go b/app/vmctl/vm_native.go index 999fd3b63..da76d4a05 100644 --- a/app/vmctl/vm_native.go +++ b/app/vmctl/vm_native.go @@ -14,6 +14,7 @@ import ( "github.com/VictoriaMetrics/VictoriaMetrics/app/vmctl/stepper" "github.com/VictoriaMetrics/VictoriaMetrics/app/vmctl/vm" "github.com/VictoriaMetrics/VictoriaMetrics/lib/logger" + "github.com/VictoriaMetrics/VictoriaMetrics/lib/promutils" "github.com/cheggaaa/pb/v3" ) @@ -232,6 +233,13 @@ func (p *vmNativeProcessor) runBackfilling(ctx context.Context, tenantID string, // any error breaks the import for s := range metrics { + + match, err := buildMatchWithFilter(p.filter.Match, s) + if err != nil { + logger.Errorf("failed to build export filters: %s", err) + continue + } + for _, times := range ranges { select { case <-ctx.Done(): @@ -239,7 +247,7 @@ func (p *vmNativeProcessor) runBackfilling(ctx context.Context, tenantID string, case infErr := <-errCh: return fmt.Errorf("native error: %s", infErr) case filterCh <- native.Filter{ - Match: fmt.Sprintf("{%s=%q}", nameLabel, s), + Match: match, TimeStart: times[0].Format(time.RFC3339), TimeEnd: times[1].Format(time.RFC3339), }: @@ -303,3 +311,13 @@ func byteCountSI(b int64) string { return fmt.Sprintf("%.1f %cB", float64(b)/float64(div), "kMGTPE"[exp]) } + +func buildMatchWithFilter(filter string, metricName string) (string, error) { + labels, err := promutils.NewLabelsFromString(filter) + if err != nil { + return "", err + } + labels.Set("__name__", metricName) + + return labels.String(), nil +} diff --git a/app/vmctl/vm_native_test.go b/app/vmctl/vm_native_test.go index 82038e352..173972c0f 100644 --- a/app/vmctl/vm_native_test.go +++ b/app/vmctl/vm_native_test.go @@ -294,3 +294,68 @@ func deleteSeries(name, value string) (int, error) { } return vmstorage.DeleteSeries(nil, []*storage.TagFilters{tfs}) } + +func Test_buildMatchWithFilter(t *testing.T) { + tests := []struct { + name string + filter string + metricName string + want string + wantErr bool + }{ + { + name: "parsed metric with label", + filter: `{__name__="http_request_count_total",cluster="kube1"}`, + metricName: "http_request_count_total", + want: `{__name__="http_request_count_total",cluster="kube1"}`, + wantErr: false, + }, + { + name: "metric name with label", + filter: `http_request_count_total{cluster="kube1"}`, + metricName: "http_request_count_total", + want: `{__name__="http_request_count_total",cluster="kube1"}`, + wantErr: false, + }, + { + name: "parsed metric with regexp value", + filter: `{__name__="http_request_count_total",cluster~="kube.*"}`, + metricName: "http_request_count_total", + want: `{__name__="http_request_count_total",cluster~="kube.*"}`, + wantErr: false, + }, + { + name: "only label with regexp", + filter: `{cluster~=".*"}`, + metricName: "http_request_count_total", + want: `{cluster~=".*",__name__="http_request_count_total"}`, + wantErr: false, + }, + { + name: "many labels in filter with regexp", + filter: `{cluster~=".*",job!=""}`, + metricName: "http_request_count_total", + want: `{cluster~=".*",job!="",__name__="http_request_count_total"}`, + wantErr: false, + }, + { + name: "match with error", + filter: `{cluster=~".*"}`, + metricName: "http_request_count_total", + want: ``, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := buildMatchWithFilter(tt.filter, tt.metricName) + if (err != nil) != tt.wantErr { + t.Errorf("buildMatchWithFilter() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.want { + t.Errorf("buildMatchWithFilter() got = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 0e41f4ebb..0ccfaa4e6 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -55,6 +55,7 @@ created by v1.90.0 or newer versions. The solution is to upgrade to v1.90.0 or n * BUGFIX: properly support comma-separated filters inside [retention filters](https://docs.victoriametrics.com/#retention-filters). See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3915). * BUGFIX: verify response code when fetching configuration files via HTTP. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4034). * BUGFIX: [vmalert](https://docs.victoriametrics.com/vmalert.html): replace empty labels with "" instead of "" during templating, as Prometheus does. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4012). +* BUGFIX: [vmctl](https://docs.victoriametrics.com/vmctl.html): properly pass multiple filters from `--vm-native-filter-match` command-line flag to the data source. Previously filters from `--vm-native-filter-match` were only used to discover the metric names, and the metric names like `__name__="metric_name"` has been taken into account, while the remaining filters were ignored. For example `--vm-native-src-addr={foo="bar",baz="abc"}` may found `metric_name{foo="bar",baz="abc"}` and filter was treated as `--vm-native-src-addr={__name__="metrics_name"}`, e.g. `foo="bar",baz="abc"` filter was ignored. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4062). ## [v1.89.1](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.89.1) diff --git a/lib/promutils/labels.go b/lib/promutils/labels.go index f22c1552f..b4f9216eb 100644 --- a/lib/promutils/labels.go +++ b/lib/promutils/labels.go @@ -188,6 +188,22 @@ func (x *Labels) Get(name string) string { return "" } +// Set label value for label with given name +// If the label with the given name doesn't exist, it adds as the new label +func (x *Labels) Set(name, value string) { + if name == "" || value == "" { + return + } + labels := x.GetLabels() + for i, label := range labels { + if label.Name == name { + labels[i].Value = value + return + } + } + x.Add(name, value) +} + // InternStrings interns all the strings used in x labels. func (x *Labels) InternStrings() { labels := x.GetLabels() @@ -306,7 +322,7 @@ func MustNewLabelsFromString(metricWithLabels string) *Labels { // NewLabelsFromString creates labels from s, which can have the form `metric{labels}`. // -// This function must be used only in tests +// This function must be used only in non performance-critical code, since it allocates too much func NewLabelsFromString(metricWithLabels string) (*Labels, error) { stripDummyMetric := false if strings.HasPrefix(metricWithLabels, "{") { diff --git a/lib/promutils/labels_test.go b/lib/promutils/labels_test.go index 83e79040c..18bdfee4f 100644 --- a/lib/promutils/labels_test.go +++ b/lib/promutils/labels_test.go @@ -175,3 +175,22 @@ func TestLabelsRemoveLabelsWithDoubleUnderscorePrefix(t *testing.T) { f(`{__meta_foo="bar",a="b",__name__="foo",__vm_filepath="aa"}`, `{a="b"}`) f(`{__meta_foo="bdffr",foo="bar",__meta_xxx="basd"}`, `{foo="bar"}`) } + +func TestLabels_Set(t *testing.T) { + f := func(metric, name, value, resultExpected string) { + t.Helper() + labels := MustNewLabelsFromString(metric) + labels.Set(name, value) + result := labels.String() + if result != resultExpected { + t.Fatalf("unexpected result of RemoveLabelsWithDoubleUnderscorePrefix;\ngot\n%s\nwant\n%s", result, resultExpected) + } + } + f(`{}`, ``, ``, `{}`) + f(`{foo="bar"}`, `bar`, `baz`, `{foo="bar",bar="baz"}`) + f(`{__meta_foo="bar",a="b",__name__="foo",__vm_filepath="aa"}`, `__name__`, `bar`, `{__meta_foo="bar",a="b",__name__="bar",__vm_filepath="aa"}`) + f(`{__meta_foo="bdffr",foo="bar",__meta_xxx="basd"}`, `__name__`, `baz`, `{__meta_foo="bdffr",foo="bar",__meta_xxx="basd",__name__="baz"}`) + f(`http_request_total{a="b"}`, `__name__`, `metric`, `{__name__="metric",a="b"}`) + f(`http_request_total{a="b"}`, `a`, `c`, `{__name__="http_request_total",a="c"}`) + f(`http_request_total{a="b"}`, `ip`, `127.0.0.1`, `{__name__="http_request_total",a="b",ip="127.0.0.1"}`) +}