From 3c5967f05144f1e30735bcef1aca0234631ea743 Mon Sep 17 00:00:00 2001
From: Cristian Greco <cristian@regolo.cc>
Date: Mon, 6 May 2024 16:27:28 +0200
Subject: [PATCH] mimirtool: use strings replacer in dashboards queries
 analyzer (#8062)

* mimirtool: use strings replacer in dashboards queries analyzer

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.

* restore regexps
---
 CHANGELOG.md                          |  1 +
 pkg/mimirtool/analyze/grafana.go      | 16 +++++++-
 pkg/mimirtool/analyze/grafana_test.go | 59 ++++++++++++++++++++++++++-
 3 files changed, 73 insertions(+), 3 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1b3224907b7..11940f80a91 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -103,6 +103,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
 
diff --git a/pkg/mimirtool/analyze/grafana.go b/pkg/mimirtool/analyze/grafana.go
index 0e8176df70c..a9ca94bc2d3 100644
--- a/pkg/mimirtool/analyze/grafana.go
+++ b/pkg/mimirtool/analyze/grafana.go
@@ -8,6 +8,7 @@ package analyze
 import (
 	"encoding/json"
 	"fmt"
+	"strings"
 
 	"github.com/grafana/regexp"
 	"github.com/pkg/errors"
@@ -27,6 +28,16 @@ var (
 	validMetricName              = regexp.MustCompile(`^[a-zA-Z_:][a-zA-Z0-9_:]*$`)
 	variableRangeQueryRangeRegex = regexp.MustCompile(`\[\$?\w+?]`)
 	variableSubqueryRangeRegex   = regexp.MustCompile(`\[\$?\w+:\$?\w+?]`)
+	variableReplacer             = 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 {
@@ -205,14 +216,15 @@ func metricsFromPanel(panel minisdk.Panel, metrics map[string]struct{}) []error
 	return parseErrors
 }
 
-func removeVariablesFromRanges(query string) string {
+func replaceVariables(query string) string {
+	query = variableReplacer.Replace(query)
 	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(replaceVariables(query))
 	if err != nil {
 		return err
 	}
diff --git a/pkg/mimirtool/analyze/grafana_test.go b/pkg/mimirtool/analyze/grafana_test.go
index 22ead855bc6..a34a023995d 100644
--- a/pkg/mimirtool/analyze/grafana_test.go
+++ b/pkg/mimirtool/analyze/grafana_test.go
@@ -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{
@@ -166,4 +166,61 @@ 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)
+	})
+
+	t.Run(`query contains range with other variables`, func(t *testing.T) {
+		metrics := make(map[string]struct{})
+		in := minisdk.Templating{
+			List: []minisdk.TemplateVar{
+				{
+					Name:       "variable",
+					Type:       "query",
+					Datasource: nil,
+					Query:      `myapp_metric_foo[$__from:$__to]`,
+				},
+			},
+		}
+
+		errs := metricsFromTemplating(in, metrics)
+		require.Empty(t, errs)
+		require.Len(t, metrics, 1)
+		require.Equal(t, map[string]struct{}{"myapp_metric_foo": {}}, metrics)
+	})
 }