From 9139f611051d7944da62826b1aff5fe831707bae Mon Sep 17 00:00:00 2001 From: Yuriy Tseretyan Date: Wed, 1 Dec 2021 06:45:27 -0500 Subject: [PATCH] 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 --- .../sqlstore/migrations/ualert/alert_rule.go | 48 ++++++++++++++++++- .../migrations/ualert/alert_rule_test.go | 47 ++++++++++++++++++ pkg/tsdb/graphite/graphite.go | 12 +++-- 3 files changed, 102 insertions(+), 5 deletions(-) create mode 100644 pkg/services/sqlstore/migrations/ualert/alert_rule_test.go diff --git a/pkg/services/sqlstore/migrations/ualert/alert_rule.go b/pkg/services/sqlstore/migrations/ualert/alert_rule.go index 2b962611760..c0e28048d73 100644 --- a/pkg/services/sqlstore/migrations/ualert/alert_rule.go +++ b/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"` diff --git a/pkg/services/sqlstore/migrations/ualert/alert_rule_test.go b/pkg/services/sqlstore/migrations/ualert/alert_rule_test.go new file mode 100644 index 00000000000..533dad039fa --- /dev/null +++ b/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)) + }) + } +} diff --git a/pkg/tsdb/graphite/graphite.go b/pkg/tsdb/graphite/graphite.go index c619c0a39fa..422a1a446fa 100644 --- a/pkg/tsdb/graphite/graphite.go +++ b/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)