Alerting: Optimize external Loki queries (grafana#73014)

Nortech-ai · Aug 8, 2023 · 2c6cf66 · 2c6cf66
1 parent 8d79d45
commit 2c6cf66
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 29 deletions.
diff --git a/pkg/services/ngalert/store/loki_range_to_instant.go b/pkg/services/ngalert/store/loki_range_to_instant.go
@@ -3,14 +3,21 @@ package store
 import (
 	"encoding/json"
 
+	"github.com/grafana/grafana/pkg/expr"
+	"github.com/grafana/grafana/pkg/services/datasources"
 	"github.com/grafana/grafana/pkg/services/ngalert/models"
 )
 
-const (
-	grafanaCloudLogs          = "grafanacloud-logs"
-	grafanaCloudUsageInsights = "grafanacloud-usage-insights"
-	grafanaCloudStateHistory  = "grafanacloud-loki-alert-state-history"
-)
+// DSType can be used to check the datasource type if it's set in the model.
+type dsType struct {
+	DS struct {
+		Type string `json:"type"`
+	} `json:"datasource"`
+}
+
+func (t dsType) isLoki() bool {
+	return t.DS.Type == datasources.DS_LOKI
+}
 
 func canBeInstant(r *models.AlertRule) bool {
 	if len(r.Data) < 2 {
@@ -20,20 +27,25 @@ func canBeInstant(r *models.AlertRule) bool {
 	if r.Data[0].QueryType != "range" {
 		return false
 	}
-	// First query part should go to cloud logs or insights.
-	if r.Data[0].DatasourceUID != grafanaCloudLogs &&
-		r.Data[0].DatasourceUID != grafanaCloudUsageInsights &&
-		r.Data[0].DatasourceUID != grafanaCloudStateHistory {
-		return false
-	}
-	// Second query part should be and expression, '-100' is the legacy way to define it.
-	if r.Data[1].DatasourceUID != "__expr__" && r.Data[1].DatasourceUID != "-100" {
+
+	var t dsType
+	// We can ignore the error here, the query just won't be optimized.
+	_ = json.Unmarshal(r.Data[0].Model, &t)
+
+	if !t.isLoki() {
 		return false
 	}
+
 	exprRaw := make(map[string]interface{})
 	if err := json.Unmarshal(r.Data[1].Model, &exprRaw); err != nil {
 		return false
 	}
+
+	// Second query part should be and expression.
+	if !expr.IsDataSource(r.Data[1].DatasourceUID) {
+		return false
+	}
+
 	// Second query part should be "last()"
 	if val, ok := exprRaw["reducer"].(string); !ok || val != "last" {
 		return false

diff --git a/pkg/services/ngalert/store/loki_range_to_instant_test.go b/pkg/services/ngalert/store/loki_range_to_instant_test.go
@@ -4,8 +4,9 @@ import (
 	"encoding/json"
 	"testing"
 
-	"github.com/grafana/grafana/pkg/services/ngalert/models"
 	"github.com/stretchr/testify/require"
+
+	"github.com/grafana/grafana/pkg/services/ngalert/models"
 )
 
 func TestCanBeInstant(t *testing.T) {
@@ -20,31 +21,24 @@ func TestCanBeInstant(t *testing.T) {
 			rule:     createMigrateableLokiRule(t),
 		},
 		{
-			name:     "invalid rule where the data array is too short to be migrateable",
-			expected: false,
-			rule: createMigrateableLokiRule(t, func(r *models.AlertRule) {
-				r.Data = []models.AlertQuery{r.Data[0]}
-			}),
-		},
-		{
-			name:     "invalid rule that is not a range query",
-			expected: false,
+			name:     "valid rule with external loki datasource",
+			expected: true,
 			rule: createMigrateableLokiRule(t, func(r *models.AlertRule) {
-				r.Data[0].QueryType = "something-else"
+				r.Data[0].DatasourceUID = "something-external"
 			}),
 		},
 		{
-			name:     "invalid rule that does not use a cloud datasource",
+			name:     "invalid rule where the data array is too short to be migrateable",
 			expected: false,
 			rule: createMigrateableLokiRule(t, func(r *models.AlertRule) {
-				r.Data[0].DatasourceUID = "something-else"
+				r.Data = []models.AlertQuery{r.Data[0]}
 			}),
 		},
 		{
-			name:     "invalid rule that has no aggregation as second item",
+			name:     "invalid rule that is not a range query",
 			expected: false,
 			rule: createMigrateableLokiRule(t, func(r *models.AlertRule) {
-				r.Data[1].DatasourceUID = "something-else"
+				r.Data[0].QueryType = "something-else"
 			}),
 		},
 		{
@@ -59,6 +53,13 @@ func TestCanBeInstant(t *testing.T) {
 				require.NoError(t, err)
 			}),
 		},
+		{
+			name:     "invalid rule that has no aggregation as second item",
+			expected: false,
+			rule: createMigrateableLokiRule(t, func(r *models.AlertRule) {
+				r.Data[1].DatasourceUID = "something-else"
+			}),
+		},
 		{
 			name:     "invalid rule that has not last() pointing to range query",
 			expected: false,
@@ -120,7 +121,7 @@ func createMigrateableLokiRule(t *testing.T, muts ...func(*models.AlertRule)) *m
 			{
 				RefID:         "A",
 				QueryType:     "range",
-				DatasourceUID: grafanaCloudLogs,
+				DatasourceUID: "grafanacloud-logs",
 				Model: []byte(`{
 					"datasource": {
 						"type": "loki",