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

@@ -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
+}

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)
+			}
+		})
+	}
+}

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 "<no value>" 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)

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, "{") {

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"}`)
+}