diff --git a/pkg/services/sqlstore/migrations/ualert/alert_rule.go b/pkg/services/sqlstore/migrations/ualert/alert_rule.go index f20452442ee..aaae8332280 100644 --- a/pkg/services/sqlstore/migrations/ualert/alert_rule.go +++ b/pkg/services/sqlstore/migrations/ualert/alert_rule.go @@ -278,14 +278,6 @@ func migrateAlertRuleQueries(l log.Logger, ruleID int64, data []alertQuery, pane result = append(result, d) continue } - dsType, ok := dsTypes[d.DatasourceUID] - if !ok { - l.Error("datasource not found", "uid", d.DatasourceUID) - return nil, fmt.Errorf("datasource not found") - } - if dsType.Type != datasources.DS_GRAPHITE { - continue - } var fixedData map[string]json.RawMessage err := json.Unmarshal(d.Model, &fixedData) if err != nil { @@ -293,7 +285,17 @@ func migrateAlertRuleQueries(l log.Logger, ruleID int64, data []alertQuery, pane } // remove hidden tag from the query (if exists) delete(fixedData, "hide") - fixedData = fixGraphiteReferencedSubQueries(l, fixedData, ruleID, panelID, dashboard) + dsType, ok := dsTypes[d.DatasourceUID] + if !ok { + l.Error("datasource not found", "uid", d.DatasourceUID) + return nil, fmt.Errorf("datasource not found") + } + if dsType.Type == datasources.DS_GRAPHITE { + fixedData = fixGraphiteReferencedSubQueries(l, fixedData, ruleID, panelID, dashboard) + } + if dsType.Type == datasources.DS_PROMETHEUS { + fixedData = fixPrometheusBothTypeQuery(l, fixedData) + } updatedModel, err := json.Marshal(fixedData) if err != nil { return nil, err diff --git a/pkg/services/sqlstore/migrations/ualert/alert_rule_prom.go b/pkg/services/sqlstore/migrations/ualert/alert_rule_prom.go new file mode 100644 index 00000000000..cb60c0b17d2 --- /dev/null +++ b/pkg/services/sqlstore/migrations/ualert/alert_rule_prom.go @@ -0,0 +1,78 @@ +package ualert + +import ( + "encoding/json" + "fmt" + + "github.com/grafana/grafana/pkg/infra/log" +) + +// fixPrometheusBothTypeQuery converts Prometheus 'Both' type queries to range queries. +func fixPrometheusBothTypeQuery(l log.Logger, queryData map[string]json.RawMessage) map[string]json.RawMessage { + // There is the possibility to support this functionality by: + // - Splitting the query into two: one for instant and one for range. + // - Splitting the condition into two: one for each query, separated by OR. + // However, relying on a 'Both' query instead of multiple conditions to do this in legacy is likely + // to be unintentional. In addition, this would require more robust operator precedence in classic conditions. + // Given these reasons, we opt to convert them to range queries and log a warning. + + var instant bool + if instantRaw, ok := queryData["instant"]; ok { + if err := json.Unmarshal(instantRaw, &instant); err != nil { + // Nothing to do here, we can't parse the instant field. + if isPrometheus, _ := isPrometheusQuery(queryData); isPrometheus { + l.Info("Failed to parse instant field on Prometheus query", "instant", string(instantRaw), "err", err) + } + return queryData + } + } + var rng bool + if rangeRaw, ok := queryData["range"]; ok { + if err := json.Unmarshal(rangeRaw, &rng); err != nil { + // Nothing to do here, we can't parse the range field. + if isPrometheus, _ := isPrometheusQuery(queryData); isPrometheus { + l.Info("Failed to parse range field on Prometheus query", "range", string(rangeRaw), "err", err) + } + return queryData + } + } + + if !instant || !rng { + // Only apply this fix to 'Both' type queries. + return queryData + } + + isPrometheus, err := isPrometheusQuery(queryData) + if err != nil { + l.Info("Unable to convert alert rule that resembles a Prometheus 'Both' type query to 'Range'", "err", err) + return queryData + } + if !isPrometheus { + // Only apply this fix to Prometheus. + return queryData + } + + // Convert 'Both' type queries to `Range` queries by disabling the `Instant` portion. + l.Info("Prometheus 'Both' type queries are not supported in unified alerting. Converting to range query.") + queryData["instant"] = []byte("false") + + return queryData +} + +// isPrometheusQuery checks if the query is for Prometheus. +func isPrometheusQuery(queryData map[string]json.RawMessage) (bool, error) { + ds, ok := queryData["datasource"] + if !ok { + return false, fmt.Errorf("missing datasource field") + } + var datasource struct { + Type string `json:"type"` + } + if err := json.Unmarshal(ds, &datasource); err != nil { + return false, fmt.Errorf("failed to parse datasource '%s': %w", string(ds), err) + } + if datasource.Type == "" { + return false, fmt.Errorf("missing type field '%s'", string(ds)) + } + return datasource.Type == "prometheus", nil +} diff --git a/pkg/services/sqlstore/migrations/ualert/alert_rule_test.go b/pkg/services/sqlstore/migrations/ualert/alert_rule_test.go index 738df7a1a49..75f0a379fb6 100644 --- a/pkg/services/sqlstore/migrations/ualert/alert_rule_test.go +++ b/pkg/services/sqlstore/migrations/ualert/alert_rule_test.go @@ -15,55 +15,97 @@ import ( func TestMigrateAlertRuleQueries(t *testing.T) { tc := []struct { name string - input *simplejson.Json + input []alertQuery expected string err error + dsMapping map[string]*dsType dashboard *dashboard }{ { name: "when a query has a sub query - it is extracted", - input: simplejson.NewFromAny(map[string]interface{}{ - "targetFull": "thisisafullquery", - "target": "ahalfquery", - }), + input: []alertQuery{ + { + DatasourceUID: "a", + Model: toJson(simplejson.NewFromAny(map[string]interface{}{ + "targetFull": "thisisafullquery", + "target": "ahalfquery", + })), + }, + }, expected: `{"target":"thisisafullquery"}`, + dsMapping: map[string]*dsType{"a": {Type: datasources.DS_GRAPHITE}}, dashboard: &dashboard{}, }, { name: "when a query has a sub query that is not fully unwrapped, it unwraps it", - input: simplejson.NewFromAny(map[string]interface{}{ - "refId": "B", - "targetFull": "alias(xxx, #A)", - "target": "alias(#A, #A)", - }), - expected: `{"refId":"B", "target": "alias(xxx, xxx)"}`, + input: []alertQuery{ + { + DatasourceUID: "a", + Model: toJson(simplejson.NewFromAny(map[string]interface{}{ + "refId": "B", + "targetFull": "alias(xxx, #A)", + "target": "alias(#A, #A)", + })), + }, + }, + expected: `{"refId":"B", "target": "alias(xxx, xxx)"}`, + dsMapping: map[string]*dsType{"a": {Type: datasources.DS_GRAPHITE}}, dashboard: &dashboard{ Data: simplejson.MustJson([]byte(`{"panels":[{"id":0,"targets":[{"refId":"A","target":"xxx"},{"refId":"B","target":"alias(#A, #A)"}]}]}`)), }, }, { name: "when a query does not have a sub query - it no-ops", - input: simplejson.NewFromAny(map[string]interface{}{ - "target": "ahalfquery", - }), + input: []alertQuery{ + { + DatasourceUID: "a", + Model: toJson(simplejson.NewFromAny(map[string]interface{}{ + "target": "ahalfquery", + })), + }, + }, expected: `{"target":"ahalfquery"}`, + dsMapping: map[string]*dsType{"a": {Type: datasources.DS_GRAPHITE}}, dashboard: &dashboard{}, }, { name: "when query was hidden, it removes the flag", - input: simplejson.NewFromAny(map[string]interface{}{ - "hide": true, - }), + input: []alertQuery{ + { + DatasourceUID: "a", + Model: toJson(simplejson.NewFromAny(map[string]interface{}{ + "hide": true, + })), + }, + }, expected: `{}`, + dsMapping: map[string]*dsType{"a": {Type: datasources.DS_GRAPHITE}}, + dashboard: &dashboard{}, + }, + { + name: "a non graphite query should be returned normally", + input: []alertQuery{ + { + DatasourceUID: "a", + Model: toJson(simplejson.NewFromAny(map[string]interface{}{ + "refId": "C", + "model": []byte(`{"expr":"1","hide":false,"interval":"","legendFormat":"{{cluster}} usage","refId":"C"}`), + })), + }, + }, + // The model is base64 encoded, for local testing it can be decoded using "echo | base64 -d" + expected: `{ + "refId": "C", + "model": "eyJleHByIjoiMSIsImhpZGUiOmZhbHNlLCJpbnRlcnZhbCI6IiIsImxlZ2VuZEZvcm1hdCI6Int7Y2x1c3Rlcn19IHVzYWdlIiwicmVmSWQiOiJDIn0=" + }`, + dsMapping: map[string]*dsType{"a": {Type: datasources.DS_PROMETHEUS}}, dashboard: &dashboard{}, }, } for _, tt := range tc { t.Run(tt.name, func(t *testing.T) { - model, err := tt.input.Encode() - require.NoError(t, err) - queries, err := migrateAlertRuleQueries(log.NewNopLogger(), 0, []alertQuery{{Model: model, DatasourceUID: "a"}}, 0, tt.dashboard, map[string]*dsType{"a": {Type: datasources.DS_GRAPHITE}}) + queries, err := migrateAlertRuleQueries(log.NewNopLogger(), 0, tt.input, 0, tt.dashboard, tt.dsMapping) if tt.err != nil { require.Error(t, err) require.EqualError(t, err, tt.err.Error()) @@ -78,6 +120,14 @@ func TestMigrateAlertRuleQueries(t *testing.T) { } } +func toJson(json *simplejson.Json) json.RawMessage { + b, err := json.MarshalJSON() + if err != nil { + panic(err) + } + return b +} + func TestAddMigrationInfo(t *testing.T) { tt := []struct { name string diff --git a/pkg/services/sqlstore/migrations/ualert/ualert.go b/pkg/services/sqlstore/migrations/ualert/ualert.go index af33daebc12..f066fa0a4d9 100644 --- a/pkg/services/sqlstore/migrations/ualert/ualert.go +++ b/pkg/services/sqlstore/migrations/ualert/ualert.go @@ -393,7 +393,6 @@ func (m *migration) Exec(sess *xorm.Session, mg *migrator.Migrator) error { if err != nil { return err } - if _, ok := rulesPerOrg[rule.OrgID]; !ok { rulesPerOrg[rule.OrgID] = make(map[*alertRule][]uidOrID) }