Skip to content

Commit

Permalink
mimirtool: use strings replacer in dashboards queries analyzer
Browse files Browse the repository at this point in the history
When parsing queries from grafana dashboards, use a replacer with a
list of variables rather than a regex matcher.

The regex matcher works ok with ranges and subqueries, but is not
working when the variables are used in other parts of the query (e.g.
with `offset`).

This PR switches back to using a list of variables to be replaced,
partially reverting the changes from #6657, but uses a
`strings.Replacer` for better readability.
  • Loading branch information
cristiangreco committed May 6, 2024
1 parent 8c4bbd7 commit 6d5ed2b
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 14 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@
* [BUGFIX] Fix panic in `loadgen` subcommand. #7629
* [ENHANCEMENT] `mimirtool promql format`: Format PromQL query with Prometheus' string or pretty-print formatter. #7742
* [BUGFIX] `mimirtool rules prepare`: do not add aggregation label to `on()` clause if already present in `group_left()` or `group_right()`. #7839
* [BUGFIX] Analyze Grafana: fix parsing queries with variables. #8062

### Mimir Continuous Test

Expand Down
29 changes: 16 additions & 13 deletions pkg/mimirtool/analyze/grafana.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ package analyze
import (
"encoding/json"
"fmt"
"strings"

"github.com/grafana/regexp"
"github.com/pkg/errors"
Expand All @@ -21,12 +22,20 @@ import (
)

var (
lvRegexp = regexp.MustCompile(`(?s)label_values\((.+),.+\)`)
lvNoQueryRegexp = regexp.MustCompile(`(?s)label_values\((.+)\)`)
qrRegexp = regexp.MustCompile(`(?s)query_result\((.+)\)`)
validMetricName = regexp.MustCompile(`^[a-zA-Z_:][a-zA-Z0-9_:]*$`)
variableRangeQueryRangeRegex = regexp.MustCompile(`\[\$?\w+?]`)
variableSubqueryRangeRegex = regexp.MustCompile(`\[\$?\w+:\$?\w+?]`)
lvRegexp = regexp.MustCompile(`(?s)label_values\((.+),.+\)`)
lvNoQueryRegexp = regexp.MustCompile(`(?s)label_values\((.+)\)`)
qrRegexp = regexp.MustCompile(`(?s)query_result\((.+)\)`)
validMetricName = regexp.MustCompile(`^[a-zA-Z_:][a-zA-Z0-9_:]*$`)
replacer = strings.NewReplacer(
"$__interval", "5m",
"$interval", "5m",
"$resolution", "5s",
"$__rate_interval", "15s",
"$rate_interval", "15s",
"$__range", "1d",
"${__range_s:glob}", "30",
"${__range_s}", "30",
)
)

type MetricsInGrafana struct {
Expand Down Expand Up @@ -205,14 +214,8 @@ func metricsFromPanel(panel minisdk.Panel, metrics map[string]struct{}) []error
return parseErrors
}

func removeVariablesFromRanges(query string) string {
query = variableRangeQueryRangeRegex.ReplaceAllLiteralString(query, `[5m]`)
query = variableSubqueryRangeRegex.ReplaceAllLiteralString(query, `[5m:1m]`)
return query
}

func parseQuery(query string, metrics map[string]struct{}) error {
expr, err := parser.ParseExpr(removeVariablesFromRanges(query))
expr, err := parser.ParseExpr(replacer.Replace(query))
if err != nil {
return err
}
Expand Down
40 changes: 39 additions & 1 deletion pkg/mimirtool/analyze/grafana_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ func TestMetricsFromTemplating(t *testing.T) {
require.Empty(t, metrics)
})

t.Run(`label_values query with no metric selector multiline `, func(t *testing.T) {
t.Run(`label_values query with no metric selector multiline`, func(t *testing.T) {
metrics := make(map[string]struct{})
in := minisdk.Templating{
List: []minisdk.TemplateVar{
Expand All @@ -166,4 +166,42 @@ func TestMetricsFromTemplating(t *testing.T) {
require.Empty(t, errs)
require.Empty(t, metrics)
})

t.Run(`query contains a subquery with the $__interval variable`, func(t *testing.T) {
metrics := make(map[string]struct{})
in := minisdk.Templating{
List: []minisdk.TemplateVar{
{
Name: "variable",
Type: "query",
Datasource: nil,
Query: `increase(varnish_main_threads_failed{job=~"$job",instance=~"$instance"}[$__interval:])`,
},
},
}

errs := metricsFromTemplating(in, metrics)
require.Empty(t, errs)
require.Len(t, metrics, 1)
require.Equal(t, map[string]struct{}{"varnish_main_threads_failed": {}}, metrics)
})

t.Run(`query uses offset with $__interval variable`, func(t *testing.T) {
metrics := make(map[string]struct{})
in := minisdk.Templating{
List: []minisdk.TemplateVar{
{
Name: "variable",
Type: "query",
Datasource: nil,
Query: `increase(tomcat_session_processingtime_total{job=~"$job", instance=~"$instance", host=~"$host", context=~"$context"}[$__interval:] offset -$__interval)`,
},
},
}

errs := metricsFromTemplating(in, metrics)
require.Empty(t, errs)
require.Len(t, metrics, 1)
require.Equal(t, map[string]struct{}{"tomcat_session_processingtime_total": {}}, metrics)
})
}

0 comments on commit 6d5ed2b

Please sign in to comment.