From 760e92e58c4537dd91b2c502c2946cd29cf6e299 Mon Sep 17 00:00:00 2001 From: Yury Molodov Date: Fri, 15 Sep 2023 15:08:39 +0200 Subject: [PATCH 1/2] fix: fetch metrics to `metrics browser` (#98) (#100) --- CHANGELOG.md | 1 + src/components/PromQueryField.tsx | 39 +++++++++++-------------------- 2 files changed, 15 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d5b47614..c8adc14a 100755 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ * FEATURE: add datasource settings for limiting the number of metrics during discovery. The proper limits should protect users from slowing down the browser when datasource returns big amounts of discovered metrics in response. See [this issue](https://github.com/VictoriaMetrics/grafana-datasource/issues/82). * BUGFIX: correctly handle custom query parameters in annotation queries. See [this issue](https://github.com/VictoriaMetrics/grafana-datasource/issues/95) +* BUGFIX: fix the loading of metrics in the `metrics browser`. See [this issue](https://github.com/VictoriaMetrics/grafana-datasource/issues/98) ## [v0.3.0](https://github.com/VictoriaMetrics/grafana-datasource/releases/tag/v0.3.0) diff --git a/src/components/PromQueryField.tsx b/src/components/PromQueryField.tsx index 35250bdc..81b74ce0 100755 --- a/src/components/PromQueryField.tsx +++ b/src/components/PromQueryField.tsx @@ -22,7 +22,6 @@ import { Plugin } from 'slate'; import { Editor } from 'slate-react'; import { CoreApp, isDataFrame, QueryEditorProps, QueryHint, TimeRange, toLegacyResponseData } from '@grafana/data'; -import { reportInteraction } from '@grafana/runtime'; import { BracesPlugin, Icon, @@ -30,6 +29,7 @@ import { } from '@grafana/ui'; import { PrometheusDatasource } from '../datasource'; +import PromQlLanguageProvider from "../language_provider"; import { roundMsToMin } from '../language_utils'; import { PromOptions, PromQuery } from '../types'; import { @@ -69,6 +69,7 @@ interface PromQueryFieldState { labelBrowserVisible: boolean; syntaxLoaded: boolean; hint: QueryHint | null; + hasMetrics: boolean; } class PromQueryField extends React.PureComponent { @@ -96,6 +97,7 @@ class PromQueryField extends React.PureComponent 0 ? initHints[0] : null; if (!data || data.series.length === 0) { - this.setState({ - hint: initHint, - }); + this.setState({ hint: initHint }); return; } @@ -167,7 +165,7 @@ class PromQueryField extends React.PureComponent { this.setState((state) => ({ labelBrowserVisible: !state.labelBrowserVisible })); - - reportInteraction('user_grafana_prometheus_metrics_browser_clicked', { - editorMode: this.state.labelBrowserVisible ? 'metricViewClosed' : 'metricViewOpen', - app: this.props?.app ?? '', - }); }; onClickHintFix = () => { @@ -226,30 +219,26 @@ class PromQueryField extends React.PureComponent { - const { - datasource: { languageProvider }, - } = this.props; + onUpdateLanguage = (languageProvider: PromQlLanguageProvider) => { const { metrics } = languageProvider; if (!metrics) { return; } - - this.setState({ syntaxLoaded: true }); + this.setState({ + syntaxLoaded: true, + hasMetrics: metrics.length > 0 + }) }; render() { const { datasource, - datasource: { languageProvider }, query, ExtraFieldElement, history = [], } = this.props; - - const { labelBrowserVisible, syntaxLoaded, hint } = this.state; - const hasMetrics = languageProvider.metrics.length > 0; + const { labelBrowserVisible, syntaxLoaded, hint, hasMetrics } = this.state; const chooserText = getChooserText(datasource.lookupsDisabled, syntaxLoaded, hasMetrics); const buttonDisabled = !(syntaxLoaded && hasMetrics); @@ -274,7 +263,7 @@ class PromQueryField extends React.PureComponent Date: Fri, 15 Sep 2023 17:19:18 +0200 Subject: [PATCH 2/2] fix: correct the display of the legend (#97) Co-authored-by: dmitryk-dk --- CHANGELOG.md | 1 + pkg/plugin/datasource.go | 6 +- pkg/plugin/query.go | 78 +++++++++++++++-- pkg/plugin/query_test.go | 168 ++++++++++++++++++++++++++++++++++++ pkg/plugin/response.go | 36 +++----- pkg/plugin/response_test.go | 57 +++++++----- 6 files changed, 294 insertions(+), 52 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c8adc14a..9e1de3ea 100755 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ * FEATURE: add datasource settings for limiting the number of metrics during discovery. The proper limits should protect users from slowing down the browser when datasource returns big amounts of discovered metrics in response. See [this issue](https://github.com/VictoriaMetrics/grafana-datasource/issues/82). * BUGFIX: correctly handle custom query parameters in annotation queries. See [this issue](https://github.com/VictoriaMetrics/grafana-datasource/issues/95) +* BUGFIX: fix the duplication of labels in the legend when using expressions. See [this issue](https://github.com/VictoriaMetrics/grafana-datasource/issues/93) * BUGFIX: fix the loading of metrics in the `metrics browser`. See [this issue](https://github.com/VictoriaMetrics/grafana-datasource/issues/98) ## [v0.3.0](https://github.com/VictoriaMetrics/grafana-datasource/releases/tag/v0.3.0) diff --git a/pkg/plugin/datasource.go b/pkg/plugin/datasource.go index 711fb833..8621dd39 100644 --- a/pkg/plugin/datasource.go +++ b/pkg/plugin/datasource.go @@ -132,12 +132,14 @@ func (d *Datasource) query(ctx context.Context, query backend.DataQuery) backend return newResponseError(err, backend.StatusInternal) } - legend := q.parseLegend() - frames, err := r.getDataFrames(legend) + frames, err := r.getDataFrames() if err != nil { err = fmt.Errorf("failed to prepare data from reponse: %w", err) return newResponseError(err, backend.StatusInternal) } + for i := range frames { + q.addMetadataToMultiFrame(frames[i]) + } return backend.DataResponse{Frames: frames} } diff --git a/pkg/plugin/query.go b/pkg/plugin/query.go index e1b3d5a1..4467881a 100644 --- a/pkg/plugin/query.go +++ b/pkg/plugin/query.go @@ -4,14 +4,20 @@ import ( "fmt" "net/url" "path" + "regexp" + "sort" "strconv" "strings" "time" + + "github.com/grafana/grafana-plugin-sdk-go/data" ) const ( instantQueryPath = "/api/v1/query" rangeQueryPath = "/api/v1/query_range" + legendFormatAuto = "__auto" + metricsName = "__name__" ) // Query represents backend query object @@ -119,13 +125,75 @@ func (q *Query) queryRangeURL(expr string, step time.Duration, queryParams url.V return q.url.String() } -var legendReplacer = strings.NewReplacer("{{", "", "}}", "") +var legendReplacer = regexp.MustCompile(`\{\{\s*(.+?)\s*\}\}`) -func (q *Query) parseLegend() string { - legend := legendReplacer.Replace(q.LegendFormat) +func (q *Query) parseLegend(labels data.Labels) string { - if legend == "{}" { + switch { + case q.LegendFormat == legendFormatAuto: return q.Expr + case q.LegendFormat != "": + result := legendReplacer.ReplaceAllStringFunc(q.LegendFormat, func(in string) string { + labelName := strings.Replace(in, "{{", "", 1) + labelName = strings.Replace(labelName, "}}", "", 1) + labelName = strings.TrimSpace(labelName) + if val, ok := labels[labelName]; ok { + return val + } + return "" + }) + if result == "" { + return q.Expr + } + return result + default: + // If legend is empty brackets, use query expression + legend := labelsToString(labels) + if legend == "{}" { + return q.Expr + } + return legend + } +} + +func (q *Query) addMetadataToMultiFrame(frame *data.Frame) { + if len(frame.Fields) < 2 { + return } - return legend + + customName := q.parseLegend(frame.Fields[1].Labels) + if customName != "" { + frame.Fields[1].Config = &data.FieldConfig{DisplayNameFromDS: customName} + } + + frame.Name = customName +} + +func labelsToString(labels data.Labels) string { + if labels == nil { + return "{}" + } + + var labelStrings []string + for label, value := range labels { + if label == metricsName { + continue + } + labelStrings = append(labelStrings, fmt.Sprintf("%s=%q", label, value)) + } + + var metricName string + mn, ok := labels[metricsName] + if ok { + metricName = mn + } + + if len(labelStrings) < 1 { + return metricName + } + + sort.Strings(labelStrings) + lbs := strings.Join(labelStrings, ",") + + return fmt.Sprintf("%s{%s}", metricName, lbs) } diff --git a/pkg/plugin/query_test.go b/pkg/plugin/query_test.go index 0ed8ef04..dd783f59 100644 --- a/pkg/plugin/query_test.go +++ b/pkg/plugin/query_test.go @@ -4,6 +4,8 @@ import ( "net/url" "testing" "time" + + "github.com/grafana/grafana-plugin-sdk-go/data" ) func TestQuery_getQueryURL(t *testing.T) { @@ -180,3 +182,169 @@ func getTimeRage() TimeRange { to := time.Unix(1670226793, 0) return TimeRange{From: from, To: to} } + +func Test_labelsToString(t *testing.T) { + tests := []struct { + name string + labels data.Labels + want string + }{ + { + name: "empty labels", + labels: nil, + want: "{}", + }, + { + name: "set of labels", + labels: data.Labels{ + "job": "vmstorage-maas", + "instance": "127.0.0.1", + }, + want: `{instance="127.0.0.1",job="vmstorage-maas"}`, + }, + { + name: "has name label", + labels: data.Labels{ + "__name__": "vm_http_requests_total", + "job": "vmstorage-maas", + "instance": "127.0.0.1", + }, + want: `vm_http_requests_total{instance="127.0.0.1",job="vmstorage-maas"}`, + }, + { + name: "name label not from the start", + labels: data.Labels{ + "job": "vmstorage-maas", + "__name__": "vm_http_requests_total", + "instance": "127.0.0.1", + }, + want: `vm_http_requests_total{instance="127.0.0.1",job="vmstorage-maas"}`, + }, + { + name: "has only name label", + labels: data.Labels{ + "__name__": "vm_http_requests_total", + }, + want: `vm_http_requests_total`, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := labelsToString(tt.labels); got != tt.want { + t.Errorf("metricsFromLabels() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestQuery_parseLegend1(t *testing.T) { + tests := []struct { + name string + legendFormat string + expr string + labels data.Labels + want string + }{ + { + name: "empty labels and legend format no expression", + legendFormat: "", + labels: nil, + expr: "", + want: "", + }, + { + name: "empty labels and legend format has expression", + legendFormat: "", + labels: nil, + expr: "sum(vm_http_request_total)", + want: "sum(vm_http_request_total)", + }, + { + name: "empty labels and legend auto has expression", + legendFormat: "__auto", + labels: nil, + expr: "sum(vm_http_request_total)", + want: "sum(vm_http_request_total)", + }, + { + name: "empty labels and legend auto has expression", + legendFormat: "{{job}}", + labels: nil, + expr: "sum(vm_http_request_total)", + want: "sum(vm_http_request_total)", + }, + { + name: "empty labels and legend with metric name", + legendFormat: "{{__name__}}", + labels: nil, + expr: "sum(vm_http_request_total)", + want: "sum(vm_http_request_total)", + }, + { + name: "has labels and legend auto has expression", + legendFormat: "__auto", + labels: data.Labels{ + "job": "vmstorage-maas", + }, + expr: "sum(vm_http_request_total)", + want: "sum(vm_http_request_total)", + }, + { + name: "has labels and legend auto has expression", + legendFormat: "{{job}}", + labels: data.Labels{ + "job": "vmstorage-maas", + }, + expr: "sum(vm_http_request_total)", + want: "vmstorage-maas", + }, + { + name: "do not have label", + legendFormat: "{{job}}", + labels: data.Labels{ + "instance": "127.0.0.1", + }, + expr: "sum(vm_http_request_total)", + want: "sum(vm_http_request_total)", + }, + { + name: "has complex label", + legendFormat: "{{job}} {{instance}}", + labels: data.Labels{ + "job": "vmstorage-maas", + "instance": "127.0.0.1", + }, + expr: "sum(vm_http_request_total)", + want: "vmstorage-maas 127.0.0.1", + }, + { + name: "auto label and only name present", + legendFormat: "__auto", + labels: data.Labels{ + "__name__": "vm_http_request_total", + }, + expr: "sum(vm_http_request_total)", + want: "sum(vm_http_request_total)", + }, + { + name: "use just name in legend format", + legendFormat: "{{__name__}}", + labels: data.Labels{ + "__name__": "vm_http_request_total", + }, + expr: "sum(vm_http_request_total)", + want: "vm_http_request_total", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + q := &Query{ + LegendFormat: tt.legendFormat, + Expr: tt.expr, + } + if got := q.parseLegend(tt.labels); got != tt.want { + t.Errorf("parseLegend() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/pkg/plugin/response.go b/pkg/plugin/response.go index 8b2d3b18..c08b77f2 100644 --- a/pkg/plugin/response.go +++ b/pkg/plugin/response.go @@ -44,21 +44,18 @@ type promInstant struct { Result []Result `json:"result"` } -func (pi promInstant) dataframes(label string) (data.Frames, error) { +func (pi promInstant) dataframes() (data.Frames, error) { frames := make(data.Frames, len(pi.Result)) for i, res := range pi.Result { f, err := strconv.ParseFloat(res.Value[1].(string), 64) if err != nil { return nil, fmt.Errorf("metric %v, unable to parse float64 from %s: %w", res, res.Value[1], err) } - if val, ok := res.Labels[label]; ok { - label = val - } ts := time.Unix(int64(res.Value[0].(float64)), 0) - frames[i] = data.NewFrame(label, - data.NewField("time", nil, []time.Time{ts}), - data.NewField("values", data.Labels(res.Labels), []float64{f})) + frames[i] = data.NewFrame("", + data.NewField(data.TimeSeriesTimeFieldName, nil, []time.Time{ts}), + data.NewField(data.TimeSeriesValueFieldName, data.Labels(res.Labels), []float64{f})) } return frames, nil @@ -68,7 +65,7 @@ type promRange struct { Result []Result `json:"result"` } -func (pr promRange) dataframes(label string) (data.Frames, error) { +func (pr promRange) dataframes() (data.Frames, error) { frames := make(data.Frames, len(pr.Result)) for i, res := range pr.Result { timestamps := make([]time.Time, len(res.Values)) @@ -91,13 +88,9 @@ func (pr promRange) dataframes(label string) (data.Frames, error) { return nil, fmt.Errorf("metric %v contains no values", res) } - if val, ok := res.Labels[label]; ok { - label = val - } - - frames[i] = data.NewFrame(label, - data.NewField("time", nil, timestamps), - data.NewField("values", data.Labels(res.Labels), values)) + frames[i] = data.NewFrame("", + data.NewField(data.TimeSeriesTimeFieldName, nil, timestamps), + data.NewField(data.TimeSeriesValueFieldName, data.Labels(res.Labels), values)) } return frames, nil @@ -111,30 +104,29 @@ func (ps promScalar) dataframes() (data.Frames, error) { if err != nil { return nil, fmt.Errorf("metric %v, unable to parse float64 from %s: %w", ps, ps[1], err) } - label := fmt.Sprintf("%g", f) frames = append(frames, - data.NewFrame(label, - data.NewField("time", nil, []time.Time{time.Unix(int64(ps[0].(float64)), 0)}), - data.NewField("value", nil, []float64{f}))) + data.NewFrame("", + data.NewField(data.TimeSeriesTimeFieldName, nil, []time.Time{time.Unix(int64(ps[0].(float64)), 0)}), + data.NewField(data.TimeSeriesValueFieldName, nil, []float64{f}))) return frames, nil } -func (r *Response) getDataFrames(label string) (data.Frames, error) { +func (r *Response) getDataFrames() (data.Frames, error) { switch r.Data.ResultType { case vector: var pi promInstant if err := json.Unmarshal(r.Data.Result, &pi.Result); err != nil { return nil, fmt.Errorf("umarshal err %s; \n %#v", err, string(r.Data.Result)) } - return pi.dataframes(label) + return pi.dataframes() case matrix: var pr promRange if err := json.Unmarshal(r.Data.Result, &pr.Result); err != nil { return nil, fmt.Errorf("umarshal err %s; \n %#v", err, string(r.Data.Result)) } - return pr.dataframes(label) + return pr.dataframes() case scalar: var ps promScalar if err := json.Unmarshal(r.Data.Result, &ps); err != nil { diff --git a/pkg/plugin/response_test.go b/pkg/plugin/response_test.go index 864e7d06..47201ffb 100644 --- a/pkg/plugin/response_test.go +++ b/pkg/plugin/response_test.go @@ -15,8 +15,8 @@ func TestResponse_getDataFrames(t *testing.T) { } tests := []struct { name string - label string fields fields + query Query want func() data.Frames wantErr bool }{ @@ -29,6 +29,7 @@ func TestResponse_getDataFrames(t *testing.T) { Result: nil, }, }, + query: Query{}, want: func() data.Frames { return nil }, @@ -43,6 +44,7 @@ func TestResponse_getDataFrames(t *testing.T) { Result: nil, }, }, + query: Query{LegendFormat: "legend {{app}}"}, want: func() data.Frames { return nil }, @@ -57,6 +59,7 @@ func TestResponse_getDataFrames(t *testing.T) { Result: []byte("{"), }, }, + query: Query{LegendFormat: "legend {{app}}"}, want: func() data.Frames { return nil }, @@ -71,12 +74,12 @@ func TestResponse_getDataFrames(t *testing.T) { Result: []byte(`[1583786142, "1"]`), }, }, - label: "123", + query: Query{LegendFormat: "legend {{app}}"}, want: func() data.Frames { return []*data.Frame{ - data.NewFrame("1", - data.NewField("time", nil, []time.Time{time.Unix(1583786142, 0)}), - data.NewField("value", nil, []float64{1}), + data.NewFrame("", + data.NewField(data.TimeSeriesTimeFieldName, nil, []time.Time{time.Unix(1583786142, 0)}), + data.NewField(data.TimeSeriesValueFieldName, nil, []float64{1}), ), } }, @@ -91,16 +94,16 @@ func TestResponse_getDataFrames(t *testing.T) { Result: []byte(`[{"metric":{"__name__":"vm_rows"},"value":[1583786142,"13763"]},{"metric":{"__name__":"vm_requests"},"value":[1583786140,"2000"]}]`), }, }, - label: "123", + query: Query{LegendFormat: "legend {{app}}"}, want: func() data.Frames { return []*data.Frame{ - data.NewFrame("123", - data.NewField("time", nil, []time.Time{time.Unix(1583786142, 0)}), - data.NewField("values", data.Labels{"__name__": "vm_rows"}, []float64{13763}), + data.NewFrame("legend ", + data.NewField(data.TimeSeriesTimeFieldName, nil, []time.Time{time.Unix(1583786142, 0)}), + data.NewField(data.TimeSeriesValueFieldName, data.Labels{"__name__": "vm_rows"}, []float64{13763}), ), - data.NewFrame("123", - data.NewField("time", nil, []time.Time{time.Unix(1583786140, 0)}), - data.NewField("values", data.Labels{"__name__": "vm_requests"}, []float64{2000}), + data.NewFrame("legend ", + data.NewField(data.TimeSeriesTimeFieldName, nil, []time.Time{time.Unix(1583786140, 0)}), + data.NewField(data.TimeSeriesValueFieldName, data.Labels{"__name__": "vm_requests"}, []float64{2000}), ), } }, @@ -115,20 +118,20 @@ func TestResponse_getDataFrames(t *testing.T) { Result: []byte(`[{"metric":{"__name__":"ingress_nginx_request_qps","status":"100"},"values":[[1670324477.542,"1"]]}, {"metric":{"__name__":"ingress_nginx_request_qps","status":"500"},"values":[[1670324477.542,"2"]]}, {"metric":{"__name__":"ingress_nginx_request_qps","status":"200"},"values":[[1670324477.542,"3"]]}]`), }, }, - label: "123", + query: Query{LegendFormat: "legend {{app}}"}, want: func() data.Frames { return []*data.Frame{ - data.NewFrame("123", - data.NewField("time", nil, []time.Time{time.Unix(1670324477, 0)}), - data.NewField("values", data.Labels{"__name__": "ingress_nginx_request_qps", "status": "100"}, []float64{1}), + data.NewFrame("legend ", + data.NewField(data.TimeSeriesTimeFieldName, nil, []time.Time{time.Unix(1670324477, 0)}), + data.NewField(data.TimeSeriesValueFieldName, data.Labels{"__name__": "ingress_nginx_request_qps", "status": "100"}, []float64{1}), ), - data.NewFrame("123", - data.NewField("time", nil, []time.Time{time.Unix(1670324477, 0)}), - data.NewField("values", data.Labels{"__name__": "ingress_nginx_request_qps", "status": "500"}, []float64{2}), + data.NewFrame("legend ", + data.NewField(data.TimeSeriesTimeFieldName, nil, []time.Time{time.Unix(1670324477, 0)}), + data.NewField(data.TimeSeriesValueFieldName, data.Labels{"__name__": "ingress_nginx_request_qps", "status": "500"}, []float64{2}), ), - data.NewFrame("123", - data.NewField("time", nil, []time.Time{time.Unix(1670324477, 0)}), - data.NewField("values", data.Labels{"__name__": "ingress_nginx_request_qps", "status": "200"}, []float64{3}), + data.NewFrame("legend ", + data.NewField(data.TimeSeriesTimeFieldName, nil, []time.Time{time.Unix(1670324477, 0)}), + data.NewField(data.TimeSeriesValueFieldName, data.Labels{"__name__": "ingress_nginx_request_qps", "status": "200"}, []float64{3}), ), } }, @@ -141,12 +144,20 @@ func TestResponse_getDataFrames(t *testing.T) { Status: tt.fields.Status, Data: tt.fields.Data, } - got, err := r.getDataFrames(tt.label) + got, err := r.getDataFrames() if (err != nil) != tt.wantErr { t.Errorf("getDataFrames() error = %v, wantErr %v", err, tt.wantErr) return } + w := tt.want() + for i := range w { + tt.query.addMetadataToMultiFrame(w[i]) + } + for i := range got { + tt.query.addMetadataToMultiFrame(got[i]) + } + if !reflect.DeepEqual(got, w) { t.Errorf("getDataFrames() got = %v, want %v", got, w) }