Alerting: Update alert rule migration to use expanded queries (#42493)

* move targetData to target

* use constants instead of literals.

* Update comments and add tests

Co-authored-by: gotjosh <josue.abreu@gmail.com>

pkg/services/sqlstore/migrations/ualert/alert_rule.go

@@ -5,7 +5,9 @@ import (
 	"fmt"
 	"time"
 
+	"github.com/grafana/grafana/pkg/expr"
 	ngmodels "github.com/grafana/grafana/pkg/services/ngalert/models"
+	"github.com/grafana/grafana/pkg/tsdb/graphite"
 	"github.com/grafana/grafana/pkg/util"
 )
 
@@ -95,13 +97,19 @@ func (m *migration) makeAlertRule(cond condition, da dashAlert, folderUID string
 	lbls, annotations := addMigrationInfo(&da)
 	lbls["alertname"] = da.Name
 	annotations["message"] = da.Message
+	var err error
+
+	data, err := migrateAlertRuleQueries(cond.Data)
+	if err != nil {
+		return nil, fmt.Errorf("failed to migrate alert rule queries: %w", err)
+	}
 
 	ar := &alertRule{
 		OrgID:           da.OrgId,
 		Title:           da.Name, // TODO: Make sure all names are unique, make new name on constraint insert error.
 		UID:             util.GenerateShortUID(),
 		Condition:       cond.Condition,
-		Data:            cond.Data,
+		Data:            data,
 		IntervalSeconds: ruleAdjustInterval(da.Frequency),
 		Version:         1,
 		NamespaceUID:    folderUID, // Folder already created, comes from env var.
@@ -112,7 +120,6 @@ func (m *migration) makeAlertRule(cond condition, da dashAlert, folderUID string
 		Labels:          lbls,
 	}
 
-	var err error
 	ar.NoDataState, err = transNoData(da.ParsedSettings.NoDataState)
 	if err != nil {
 		return nil, err
@@ -142,6 +149,43 @@ func (m *migration) makeAlertRule(cond condition, da dashAlert, folderUID string
 	return ar, nil
 }
 
+// migrateAlertRuleQueries attempts to fix alert rule queries so they can work in unified alerting. Queries of some data sources are not compatible with unified alerting.
+func migrateAlertRuleQueries(data []alertQuery) ([]alertQuery, error) {
+	result := make([]alertQuery, 0, len(data))
+	for _, d := range data {
+		// queries that are expression are not relevant, skip them.
+		if d.DatasourceUID == expr.OldDatasourceUID {
+			result = append(result, d)
+			continue
+		}
+		var fixedData map[string]json.RawMessage
+		err := json.Unmarshal(d.Model, &fixedData)
+		if err != nil {
+			return nil, err
+		}
+		fixedData = fixGraphiteReferencedSubQueries(fixedData)
+		updatedModel, err := json.Marshal(fixedData)
+		if err != nil {
+			return nil, err
+		}
+		d.Model = updatedModel
+		result = append(result, d)
+	}
+	return result, nil
+}
+
+// fixGraphiteReferencedSubQueries attempts to fix graphite referenced sub queries, given unified alerting does not support this.
+// targetFull of Graphite data source contains the expanded version of field 'target', so let's copy that.
+func fixGraphiteReferencedSubQueries(queryData map[string]json.RawMessage) map[string]json.RawMessage {
+	fullQuery, ok := queryData[graphite.TargetFullModelField]
+	if ok {
+		delete(queryData, graphite.TargetFullModelField)
+		queryData[graphite.TargetModelField] = fullQuery
+	}
+
+	return queryData
+}
+
 type alertQuery struct {
 	// RefID is the unique identifier of the query, set by the frontend call.
 	RefID string `json:"refId"`

pkg/services/sqlstore/migrations/ualert/alert_rule_test.go

@@ -0,0 +1,47 @@
+package ualert
+
+import (
+	"testing"
+
+	"github.com/grafana/grafana/pkg/components/simplejson"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestMigrateAlertRuleQueries(t *testing.T) {
+	tc := []struct {
+		name     string
+		input    *simplejson.Json
+		expected string
+		err      error
+	}{
+		{
+			name:     "when a query has a sub query - it is extracted",
+			input:    simplejson.NewFromAny(map[string]interface{}{"targetFull": "thisisafullquery", "target": "ahalfquery"}),
+			expected: `{"target":"thisisafullquery"}`,
+		},
+		{
+			name:     "when a query does not have a sub query - it no-ops",
+			input:    simplejson.NewFromAny(map[string]interface{}{"target": "ahalfquery"}),
+			expected: `{"target":"ahalfquery"}`,
+		},
+	}
+
+	for _, tt := range tc {
+		t.Run(tt.name, func(t *testing.T) {
+			model, err := tt.input.Encode()
+			require.NoError(t, err)
+			queries, err := migrateAlertRuleQueries([]alertQuery{{Model: model}})
+			if tt.err != nil {
+				require.Error(t, err)
+				require.EqualError(t, err, tt.err.Error())
+				return
+			}
+
+			require.NoError(t, err)
+			r, err := queries[0].Model.MarshalJSON()
+			require.NoError(t, err)
+			require.JSONEq(t, tt.expected, string(r))
+		})
+	}
+}

pkg/tsdb/graphite/graphite.go

@@ -20,6 +20,8 @@ import (
 	"github.com/grafana/grafana-plugin-sdk-go/backend/datasource"
 	"github.com/grafana/grafana-plugin-sdk-go/backend/instancemgmt"
 	"github.com/grafana/grafana-plugin-sdk-go/data"
+	"github.com/opentracing/opentracing-go"
+
 	"github.com/grafana/grafana/pkg/components/simplejson"
 	"github.com/grafana/grafana/pkg/infra/httpclient"
 	"github.com/grafana/grafana/pkg/infra/log"
@@ -27,7 +29,6 @@ import (
 	"github.com/grafana/grafana/pkg/plugins/backendplugin/coreplugin"
 	"github.com/grafana/grafana/pkg/setting"
 	"github.com/grafana/grafana/pkg/tsdb/legacydata"
-	"github.com/opentracing/opentracing-go"
 )
 
 type Service struct {
@@ -35,6 +36,11 @@ type Service struct {
 	im     instancemgmt.InstanceManager
 }
 
+const (
+	TargetFullModelField = "targetFull"
+	TargetModelField     = "target"
+)
+
 func ProvideService(httpClientProvider httpclient.Provider, registrar plugins.CoreBackendRegistrar) (*Service, error) {
 	s := &Service{
 		logger: log.New("tsdb.graphite"),
@@ -126,10 +132,10 @@ func (s *Service) QueryData(ctx context.Context, req *backend.QueryDataRequest)
 		}
 		s.logger.Debug("graphite", "query", model)
 		currTarget := ""
-		if fullTarget, err := model.Get("targetFull").String(); err == nil {
+		if fullTarget, err := model.Get(TargetFullModelField).String(); err == nil {
 			currTarget = fullTarget
 		} else {
-			currTarget = model.Get("target").MustString()
+			currTarget = model.Get(TargetModelField).MustString()
 		}
 		if currTarget == "" {
 			s.logger.Debug("graphite", "empty query target", model)