Skip to content

Commit

Permalink
Pyroscope: Improve label suggestions in query editor (grafana#78861)
Browse files Browse the repository at this point in the history
* Send sanitized selectors to the Pyroscope backend for LabelNames and LabelValues

* Clean LabelNames response to remove already used labels

* Improve performance after major changes

* Fix import order

* Further improve rendering performance

* Fix frontend tests

* Fix fake pyroscope client signature

* Bump pyroscope/api dependency to include start/end in LabelNames/LabelValues

* Fix issue with old queries running when using the run button

* Add generated file

* Make code more readable, add a few comments

* Format with prettier

* Fix error when assigning data

* Revert "Add generated file"

This reverts commit c4f3372.

* Remove leftover code

* Simplify query editor internal state objects

* Move label selector validation up, improve label filtering

* Simplify query editor state, switch to debounce to reduce rerenders

* Revert cosmetic change
  • Loading branch information
aleks-p authored Dec 7, 2023
1 parent 2d66d0d commit 1c53561
Show file tree
Hide file tree
Showing 10 changed files with 131 additions and 50 deletions.
8 changes: 4 additions & 4 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ require (
github.com/pkg/errors v0.9.1 // indirect
github.com/prometheus/alertmanager v0.25.0 // @grafana/alerting-squad-backend
github.com/prometheus/client_golang v1.17.0 // @grafana/alerting-squad-backend
github.com/prometheus/client_model v0.4.1-0.20230718164431-9a2bf3000d16 // @grafana/backend-platform
github.com/prometheus/client_model v0.5.0 // @grafana/backend-platform
github.com/prometheus/common v0.45.0 // @grafana/alerting-squad-backend
github.com/prometheus/prometheus v1.8.2-0.20221021121301-51a44e6657c3 // @grafana/alerting-squad-backend
github.com/robfig/cron/v3 v3.0.1 // @grafana/backend-platform
Expand Down Expand Up @@ -222,7 +222,7 @@ require (
golang.org/x/text v0.14.0 // @grafana/backend-platform
golang.org/x/xerrors v0.0.0-20220907171357-04be3eba64a2 // indirect
google.golang.org/appengine v1.6.7 // indirect
google.golang.org/genproto v0.0.0-20231002182017-d307bd883b97 // indirect; @grafana/backend-platform
google.golang.org/genproto v0.0.0-20231012201019-e917dd12ba7a // indirect; @grafana/backend-platform
)

require (
Expand Down Expand Up @@ -290,7 +290,7 @@ require (

require github.com/grafana/gofpdf v0.0.0-20231002120153-857cc45be447 // @grafana/sharing-squad

require github.com/grafana/pyroscope/api v0.2.1 // @grafana/observability-traces-and-profiling
require github.com/grafana/pyroscope/api v0.3.0 // @grafana/observability-traces-and-profiling

require github.com/apache/arrow/go/v13 v13.0.0 // @grafana/observability-metrics

Expand Down Expand Up @@ -409,7 +409,7 @@ require (
go.uber.org/zap v1.24.0 // indirect
golang.org/x/term v0.15.0 // indirect
google.golang.org/genproto/googleapis/api v0.0.0-20231002182017-d307bd883b97 // indirect
google.golang.org/genproto/googleapis/rpc v0.0.0-20231012201019-e917dd12ba7a // indirect
google.golang.org/genproto/googleapis/rpc v0.0.0-20231016165738-49dd2c1f3d0b // indirect
gopkg.in/fsnotify/fsnotify.v1 v1.4.7 // indirect
gopkg.in/inf.v0 v0.9.1 // indirect
gopkg.in/natefinch/lumberjack.v2 v2.2.1 // indirect
Expand Down
15 changes: 8 additions & 7 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -1820,8 +1820,8 @@ github.com/grafana/kindsys v0.0.0-20230508162304-452481b63482 h1:1YNoeIhii4UIIQp
github.com/grafana/kindsys v0.0.0-20230508162304-452481b63482/go.mod h1:GNcfpy5+SY6RVbNGQW264gC0r336Dm+0zgQ5vt6+M8Y=
github.com/grafana/prometheus-alertmanager v0.25.1-0.20231027171310-70c52bf65758 h1:ATUhvJSJwzdzhnmzUI92fxVFqyqmcnzJ47wtHTK3LW4=
github.com/grafana/prometheus-alertmanager v0.25.1-0.20231027171310-70c52bf65758/go.mod h1:MmLemcsGjpbOwEeT3k7K+gnvIImXgkatCfVX6sOtx80=
github.com/grafana/pyroscope/api v0.2.1 h1:V/GSrwSN5HgA4Ijf/2SN9Sib55E/xObswaCMkdOOsxs=
github.com/grafana/pyroscope/api v0.2.1/go.mod h1:vNO/Rym3pwNIN4y/f0ACrk5iR7DdWlsdfZGSZE+XChU=
github.com/grafana/pyroscope/api v0.3.0 h1:WcVKNZ8JlriJnD28wTkZray0wGo8dGkizSJXnbG7Gd8=
github.com/grafana/pyroscope/api v0.3.0/go.mod h1:JggA80ToAAUACYGfwL49XoFk5aN5ecHp4pNIZhlk9Uc=
github.com/grafana/regexp v0.0.0-20221122212121-6b5c0a4cb7fd/go.mod h1:M5qHK+eWfAv8VR/265dIuEpL3fNfeC21tXXp9itM24A=
github.com/grafana/regexp v0.0.0-20221123153739-15dc172cd2db h1:7aN5cccjIqCLTzedH7MZzRZt5/lsAHch6Z3L2ZGn5FA=
github.com/grafana/regexp v0.0.0-20221123153739-15dc172cd2db/go.mod h1:M5qHK+eWfAv8VR/265dIuEpL3fNfeC21tXXp9itM24A=
Expand Down Expand Up @@ -2557,8 +2557,9 @@ github.com/prometheus/client_model v0.1.0/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6T
github.com/prometheus/client_model v0.2.0/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA=
github.com/prometheus/client_model v0.3.0/go.mod h1:LDGWKZIo7rky3hgvBe+caln+Dr3dPggB5dvjtD7w9+w=
github.com/prometheus/client_model v0.4.0/go.mod h1:oMQmHW1/JoDwqLtg57MGgP/Fb1CJEYF2imWWhWtMkYU=
github.com/prometheus/client_model v0.4.1-0.20230718164431-9a2bf3000d16 h1:v7DLqVdK4VrYkVD5diGdl4sxJurKJEMnODWRJlxV9oM=
github.com/prometheus/client_model v0.4.1-0.20230718164431-9a2bf3000d16/go.mod h1:oMQmHW1/JoDwqLtg57MGgP/Fb1CJEYF2imWWhWtMkYU=
github.com/prometheus/client_model v0.5.0 h1:VQw1hfvPvk3Uv6Qf29VrPF32JB6rtbgI6cYPYQjL0Qw=
github.com/prometheus/client_model v0.5.0/go.mod h1:dTiFglRmd66nLR9Pv9f0mZi7B7fk5Pm3gvsjB5tr+kI=
github.com/prometheus/common v0.0.0-20181113130724-41aa239b4cce/go.mod h1:daVV7qP5qjZbuso7PdcryaAu0sAZbrN9i7WWcTMWvro=
github.com/prometheus/common v0.0.0-20181126121408-4724e9255275/go.mod h1:daVV7qP5qjZbuso7PdcryaAu0sAZbrN9i7WWcTMWvro=
github.com/prometheus/common v0.2.0/go.mod h1:TNfzLD0ON7rHzMJeJkieUDPYmFC7Snx/y86RQel1bk4=
Expand Down Expand Up @@ -3970,12 +3971,12 @@ google.golang.org/genproto v0.0.0-20230216225411-c8e22ba71e44/go.mod h1:8B0gmkoR
google.golang.org/genproto v0.0.0-20230222225845-10f96fb3dbec/go.mod h1:3Dl5ZL0q0isWJt+FVcfpQyirqemEuLAK/iFvg1UP1Hw=
google.golang.org/genproto v0.0.0-20230223222841-637eb2293923/go.mod h1:3Dl5ZL0q0isWJt+FVcfpQyirqemEuLAK/iFvg1UP1Hw=
google.golang.org/genproto v0.0.0-20230306155012-7f2fa6fef1f4/go.mod h1:NWraEVixdDnqcqQ30jipen1STv2r/n24Wb7twVTGR4s=
google.golang.org/genproto v0.0.0-20231002182017-d307bd883b97 h1:SeZZZx0cP0fqUyA+oRzP9k7cSwJlvDFiROO72uwD6i0=
google.golang.org/genproto v0.0.0-20231002182017-d307bd883b97/go.mod h1:t1VqOqqvce95G3hIDCT5FeO3YUc6Q4Oe24L/+rNMxRk=
google.golang.org/genproto v0.0.0-20231012201019-e917dd12ba7a h1:fwgW9j3vHirt4ObdHoYNwuO24BEZjSzbh+zPaNWoiY8=
google.golang.org/genproto v0.0.0-20231012201019-e917dd12ba7a/go.mod h1:EMfReVxb80Dq1hhioy0sOsY9jCE46YDgHlJ7fWVUWRE=
google.golang.org/genproto/googleapis/api v0.0.0-20231002182017-d307bd883b97 h1:W18sezcAYs+3tDZX4F80yctqa12jcP1PUS2gQu1zTPU=
google.golang.org/genproto/googleapis/api v0.0.0-20231002182017-d307bd883b97/go.mod h1:iargEX0SFPm3xcfMI0d1domjg0ZF4Aa0p2awqyxhvF0=
google.golang.org/genproto/googleapis/rpc v0.0.0-20231012201019-e917dd12ba7a h1:a2MQQVoTo96JC9PMGtGBymLp7+/RzpFc2yX/9WfFg1c=
google.golang.org/genproto/googleapis/rpc v0.0.0-20231012201019-e917dd12ba7a/go.mod h1:4cYg8o5yUbm77w8ZX00LhMVNl/YVBFJRYWDc0uYWMs0=
google.golang.org/genproto/googleapis/rpc v0.0.0-20231016165738-49dd2c1f3d0b h1:ZlWIi1wSK56/8hn4QcBp/j9M7Gt3U/3hZw3mC7vDICo=
google.golang.org/genproto/googleapis/rpc v0.0.0-20231016165738-49dd2c1f3d0b/go.mod h1:swOH3j0KzcDDgGUWr+SNpyTen5YrXjS3eyPzFYKc6lc=
google.golang.org/grpc v1.8.0/go.mod h1:yo6s7OP7yaDglbqo1J04qKzAhqBH6lvTonzMVmEdcZw=
google.golang.org/grpc v1.12.0/go.mod h1:yo6s7OP7yaDglbqo1J04qKzAhqBH6lvTonzMVmEdcZw=
google.golang.org/grpc v1.17.0/go.mod h1:6QZJwpn2B+Zp71q/5VxRsJ6NXXVCE5NRUHRo+f3cWCs=
Expand Down
48 changes: 42 additions & 6 deletions pkg/tsdb/grafana-pyroscope-datasource/instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import (
"fmt"
"net/http"
"net/url"
"slices"
"strconv"
"time"

"github.com/grafana/grafana-plugin-sdk-go/backend"
Expand All @@ -14,6 +16,8 @@ import (
"github.com/grafana/grafana-plugin-sdk-go/data"
"github.com/grafana/grafana/pkg/infra/httpclient"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/prometheus/prometheus/model/labels"
"github.com/prometheus/prometheus/promql/parser"
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/trace"
)
Expand All @@ -27,8 +31,8 @@ var (

type ProfilingClient interface {
ProfileTypes(context.Context) ([]*ProfileType, error)
LabelNames(ctx context.Context) ([]string, error)
LabelValues(ctx context.Context, label string) ([]string, error)
LabelNames(ctx context.Context, labelSelector string, start int64, end int64) ([]string, error)
LabelValues(ctx context.Context, label string, labelSelector string, start int64, end int64) ([]string, error)
GetSeries(ctx context.Context, profileTypeID string, labelSelector string, start int64, end int64, groupBy []string, step float64) (*SeriesResponse, error)
GetProfile(ctx context.Context, profileTypeID string, labelSelector string, start int64, end int64, maxNodes *int64) (*ProfileResponse, error)
GetSpanProfile(ctx context.Context, profileTypeID string, labelSelector string, spanSelector []string, start int64, end int64, maxNodes *int64) (*ProfileResponse, error)
Expand Down Expand Up @@ -105,17 +109,45 @@ func (d *PyroscopeDatasource) profileTypes(ctx context.Context, req *backend.Cal

func (d *PyroscopeDatasource) labelNames(ctx context.Context, req *backend.CallResourceRequest, sender backend.CallResourceResponseSender) error {
ctxLogger := logger.FromContext(ctx)
res, err := d.client.LabelNames(ctx)

u, err := url.Parse(req.URL)
if err != nil {
ctxLogger.Error("Failed to parse URL", "error", err, "function", logEntrypoint())
return err
}
query := u.Query()

start, _ := strconv.ParseInt(query.Get("start"), 10, 64)
end, _ := strconv.ParseInt(query.Get("end"), 10, 64)
labelSelector := query.Get("query")
matchers, err := parser.ParseMetricSelector(labelSelector)
if err != nil {
ctxLogger.Error("Could not parse label selector", "error", err, "function", logEntrypoint())
return fmt.Errorf("failed parsing label selector: %v", err)
}

labelNames, err := d.client.LabelNames(ctx, labelSelector, start, end)
if err != nil {
ctxLogger.Error("Received error from client", "error", err, "function", logEntrypoint())
return fmt.Errorf("error calling LabelNames: %v", err)
}
data, err := json.Marshal(res)

finalLabels := make([]string, 0)
for _, label := range labelNames {
if slices.ContainsFunc(matchers, func(m *labels.Matcher) bool {
return m.Name == label
}) {
continue
}
finalLabels = append(finalLabels, label)
}

jsonResponse, err := json.Marshal(finalLabels)
if err != nil {
ctxLogger.Error("Failed to marshal response", "error", err, "function", logEntrypoint())
return err
}
err = sender.Send(&backend.CallResourceResponse{Body: data, Headers: req.Headers, Status: 200})
err = sender.Send(&backend.CallResourceResponse{Body: jsonResponse, Headers: req.Headers, Status: 200})
if err != nil {
ctxLogger.Error("Failed to send response", "error", err, "function", logEntrypoint())
return err
Expand All @@ -139,7 +171,11 @@ func (d *PyroscopeDatasource) labelValues(ctx context.Context, req *backend.Call
}
query := u.Query()

res, err := d.client.LabelValues(ctx, query["label"][0])
start, _ := strconv.ParseInt(query.Get("start"), 10, 64)
end, _ := strconv.ParseInt(query.Get("end"), 10, 64)
label := query.Get("label")

res, err := d.client.LabelValues(ctx, label, query.Get("query"), start, end)
if err != nil {
ctxLogger.Error("Received error from client", "error", err, "function", logEntrypoint())
return fmt.Errorf("error calling LabelValues: %v", err)
Expand Down
17 changes: 13 additions & 4 deletions pkg/tsdb/grafana-pyroscope-datasource/pyroscopeClient.go
Original file line number Diff line number Diff line change
Expand Up @@ -238,10 +238,14 @@ func getUnits(profileTypeID string) string {
return unit
}

func (c *PyroscopeClient) LabelNames(ctx context.Context) ([]string, error) {
func (c *PyroscopeClient) LabelNames(ctx context.Context, labelSelector string, start int64, end int64) ([]string, error) {
ctx, span := tracing.DefaultTracer().Start(ctx, "datasource.pyroscope.LabelNames")
defer span.End()
resp, err := c.connectClient.LabelNames(ctx, connect.NewRequest(&typesv1.LabelNamesRequest{}))
resp, err := c.connectClient.LabelNames(ctx, connect.NewRequest(&typesv1.LabelNamesRequest{
Matchers: []string{labelSelector},
Start: start,
End: end,
}))
if err != nil {
logger.Error("Received error from client", "error", err, "function", logEntrypoint())
span.RecordError(err)
Expand All @@ -259,10 +263,15 @@ func (c *PyroscopeClient) LabelNames(ctx context.Context) ([]string, error) {
return filtered, nil
}

func (c *PyroscopeClient) LabelValues(ctx context.Context, label string) ([]string, error) {
func (c *PyroscopeClient) LabelValues(ctx context.Context, label string, labelSelector string, start int64, end int64) ([]string, error) {
ctx, span := tracing.DefaultTracer().Start(ctx, "datasource.pyroscope.LabelValues")
defer span.End()
resp, err := c.connectClient.LabelValues(ctx, connect.NewRequest(&typesv1.LabelValuesRequest{Name: label}))
resp, err := c.connectClient.LabelValues(ctx, connect.NewRequest(&typesv1.LabelValuesRequest{
Name: label,
Matchers: []string{labelSelector},
Start: start,
End: end,
}))
if err != nil {
logger.Error("Received error from client", "error", err, "function", logEntrypoint())
span.RecordError(err)
Expand Down
4 changes: 2 additions & 2 deletions pkg/tsdb/grafana-pyroscope-datasource/query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -288,11 +288,11 @@ func (f *FakeClient) ProfileTypes(ctx context.Context) ([]*ProfileType, error) {
}, nil
}

func (f *FakeClient) LabelValues(ctx context.Context, label string) ([]string, error) {
func (f *FakeClient) LabelValues(ctx context.Context, label string, labelSelector string, start int64, end int64) ([]string, error) {
panic("implement me")
}

func (f *FakeClient) LabelNames(ctx context.Context) ([]string, error) {
func (f *FakeClient) LabelNames(ctx context.Context, labelSelector string, start int64, end int64) ([]string, error) {
panic("implement me")
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export function LabelsEditor(props: Props) {
<CodeEditor
value={props.value}
language={langId}
onBlur={props.onChange}
onChange={props.onChange}
containerStyles={styles.queryField}
monacoOptions={{
folding: false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,8 @@ function setupDs() {
},
] as ProfileTypeMessage[]);

ds.getLabelNames = jest.fn().mockResolvedValue(['label_one']);

return ds;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import deepEqual from 'fast-deep-equal';
import React, { useCallback, useEffect } from 'react';
import { useAsync } from 'react-use';
import { debounce } from 'lodash';
import React, { useCallback, useEffect, useState } from 'react';

import { CoreApp, QueryEditorProps, TimeRange } from '@grafana/data';
import { LoadingPlaceholder } from '@grafana/ui';

import { normalizeQuery, PyroscopeDataSource } from '../datasource';
import { PyroscopeDataSourceOptions, ProfileTypeMessage, Query } from '../types';
import { ProfileTypeMessage, PyroscopeDataSourceOptions, Query } from '../types';

import { EditorRow } from './EditorRow';
import { EditorRows } from './EditorRows';
Expand All @@ -17,6 +17,8 @@ import { QueryOptions } from './QueryOptions';

export type Props = QueryEditorProps<PyroscopeDataSource, Query, PyroscopeDataSourceOptions>;

const labelSelectorRegex = /(\w+)\s*=\s*("[^,"]+")/g;

export function QueryEditor(props: Props) {
const { onChange, onRunQuery, datasource, query, range, app } = props;

Expand Down Expand Up @@ -115,33 +117,62 @@ function useLabels(
// Round to nearest 5 seconds. If the range is something like last 1h then every render the range values change slightly
// and what ever has range as dependency is rerun. So this effectively debounces the queries.
const unpreciseRange = {
to: Math.ceil((range?.to.valueOf() || 0) / 5000) * 5000,
from: Math.floor((range?.from.valueOf() || 0) / 5000) * 5000,
to: Math.ceil((range?.to.valueOf() || 0) / 10000) * 10000,
from: Math.floor((range?.from.valueOf() || 0) / 10000) * 10000,
};

const labelsResult = useAsync(() => {
return datasource.getLabelNames(query.profileTypeId + query.labelSelector, unpreciseRange.from, unpreciseRange.to);
}, [datasource, query.profileTypeId, query.labelSelector, unpreciseRange.to, unpreciseRange.from]);
// Transforms user input into a valid label selector including the profile type.
// It can optionally remove a label, used to support editing existing label values.
const createSelector = (rawInput: string, profileTypeId: string, labelToRemove: string): string => {
let labels: string[] = [`__profile_type__=\"${profileTypeId}\"`];
let match;
while ((match = labelSelectorRegex.exec(rawInput)) !== null) {
if (match[1] && match[2]) {
if (match[1] === labelToRemove) {
continue;
}
labels.push(`${match[1]}=${match[2]}`);
}
}
return `{${labels.join(',')}}`;
};

// Create a function with range and query already baked in so we don't have to send those everywhere
const getLabelValues = useCallback(
(label: string) => {
return datasource.getLabelValues(
query.profileTypeId + query.labelSelector,
label,
const [labels, setLabels] = useState(() => ['']);

useEffect(() => {
const fetchData = async () => {
const labels = await datasource.getLabelNames(
createSelector(query.labelSelector, query.profileTypeId, ''),
unpreciseRange.from,
unpreciseRange.to
);

setLabels(labels);
};
fetchData();
}, [query, unpreciseRange.from, unpreciseRange.to, datasource, setLabels]);

// Create a function with range and query already baked in, so we don't have to send those everywhere
const getLabelValues = useCallback(
(label: string) => {
let labelSelector = createSelector(query.labelSelector, query.profileTypeId, label);
return datasource.getLabelValues(labelSelector, label, unpreciseRange.from, unpreciseRange.to);
},
[query, datasource, unpreciseRange.to, unpreciseRange.from]
[datasource, query.labelSelector, query.profileTypeId, unpreciseRange.to, unpreciseRange.from]
);

const onChangeDebounced = debounce((value: string) => {
if (onChange) {
onChange({ ...query, labelSelector: value });
}
}, 200);

const onLabelSelectorChange = useCallback(
(value: string) => {
onChange({ ...query, labelSelector: value });
onChangeDebounced(value);
},
[onChange, query]
[onChangeDebounced]
);

return { labels: labelsResult.value, getLabelValues, onLabelSelectorChange };
return { labels, getLabelValues, onLabelSelectorChange };
}
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,15 @@ export class CompletionProvider implements monacoTypes.languages.CompletionItemP
});
case 'IN_LABEL_VALUE':
let values = await this.getLabelValues(situation.labelName);
return values.map((key) => {
return {
label: key,
insertText: situation.betweenQuotes ? key : `"${key}"`,
type: 'LABEL_VALUE',
};
});
return values
? values.map((key) => {
return {
label: key,
insertText: situation.betweenQuotes ? key : `"${key}"`,
type: 'LABEL_VALUE',
};
})
: [];
default:
throw new Error(`Unexpected situation ${situation}`);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import type { languages } from 'monaco-editor';

export const languageConfiguration: languages.LanguageConfiguration = {
// the default separators except `@$`
wordPattern: /(-?\d*\.\d\w*)|([^`~!#%^&*()\-=+\[{\]}\\|;:'",.<>\/?\s]+)/g,
wordPattern: /(-?\d*\.\d\w*)|([^`~!#%^&*()=+\[{\]}\\|;:'",<>\/?\s]+)/g,
brackets: [['{', '}']],
autoClosingPairs: [
{ open: '{', close: '}' },
Expand Down

0 comments on commit 1c53561

Please sign in to comment.