Chore: Refactor usage of legacy data contracts (#41218)

Refactor usage of legacy data contracts. Moves legacy data contracts 
to pkg/tsdb/legacydata package.
Refactor pkg/expr to be a proper service/dependency that can be provided 
to wire to remove some unneeded dependencies to SSE in ngalert and other places.
Refactor pkg/expr to not use the legacydata,RequestHandler and use 
backend.QueryDataHandler instead.
This commit is contained in:
Marcus Efraimsson
2021-11-10 11:52:16 +01:00
committed by GitHub
parent d6ed5d295e
commit baab021fec
54 changed files with 732 additions and 951 deletions

View File

@@ -6,6 +6,7 @@ import (
"time"
"github.com/grafana/grafana/pkg/api/routing"
"github.com/grafana/grafana/pkg/expr"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/services/datasourceproxy"
"github.com/grafana/grafana/pkg/services/datasources"
@@ -18,7 +19,6 @@ import (
"github.com/grafana/grafana/pkg/services/quota"
"github.com/grafana/grafana/pkg/services/secrets"
"github.com/grafana/grafana/pkg/setting"
"github.com/grafana/grafana/pkg/tsdb"
)
// timeNow makes it possible to test usage of time
@@ -54,7 +54,7 @@ type API struct {
Cfg *setting.Cfg
DatasourceCache datasources.CacheService
RouteRegister routing.RouteRegister
DataService *tsdb.Service
ExpressionService *expr.Service
QuotaService *quota.QuotaService
Schedule schedule.ScheduleService
RuleStore store.RuleStore
@@ -93,11 +93,11 @@ func (api *API) RegisterAPIEndpoints(m *metrics.API) {
RulerSrv{DatasourceCache: api.DatasourceCache, QuotaService: api.QuotaService, manager: api.StateManager, store: api.RuleStore, log: logger},
), m)
api.RegisterTestingApiEndpoints(TestingApiSrv{
AlertingProxy: proxy,
Cfg: api.Cfg,
DataService: api.DataService,
DatasourceCache: api.DatasourceCache,
log: logger,
AlertingProxy: proxy,
Cfg: api.Cfg,
ExpressionService: api.ExpressionService,
DatasourceCache: api.DatasourceCache,
log: logger,
}, m)
api.RegisterConfigurationApiEndpoints(AdminSrv{
store: api.AdminConfigStore,

View File

@@ -8,22 +8,22 @@ import (
"strconv"
"github.com/grafana/grafana/pkg/api/response"
"github.com/grafana/grafana/pkg/expr"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/datasources"
apimodels "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions"
"github.com/grafana/grafana/pkg/services/ngalert/eval"
"github.com/grafana/grafana/pkg/setting"
"github.com/grafana/grafana/pkg/tsdb"
"github.com/grafana/grafana/pkg/web"
)
type TestingApiSrv struct {
*AlertingProxy
Cfg *setting.Cfg
DataService *tsdb.Service
DatasourceCache datasources.CacheService
log log.Logger
Cfg *setting.Cfg
ExpressionService *expr.Service
DatasourceCache datasources.CacheService
log log.Logger
}
func (srv TestingApiSrv) RouteTestRuleConfig(c *models.ReqContext, body apimodels.TestRulePayload) response.Response {
@@ -32,7 +32,7 @@ func (srv TestingApiSrv) RouteTestRuleConfig(c *models.ReqContext, body apimodel
if body.Type() != apimodels.GrafanaBackend || body.GrafanaManagedCondition == nil {
return ErrResp(http.StatusBadRequest, errors.New("unexpected payload"), "")
}
return conditionEval(c, *body.GrafanaManagedCondition, srv.DatasourceCache, srv.DataService, srv.Cfg, srv.log)
return conditionEval(c, *body.GrafanaManagedCondition, srv.DatasourceCache, srv.ExpressionService, srv.Cfg, srv.log)
}
if body.Type() != apimodels.LoTexRulerBackend {
@@ -86,7 +86,7 @@ func (srv TestingApiSrv) RouteEvalQueries(c *models.ReqContext, cmd apimodels.Ev
}
evaluator := eval.Evaluator{Cfg: srv.Cfg, Log: srv.log}
evalResults, err := evaluator.QueriesAndExpressionsEval(c.SignedInUser.OrgId, cmd.Data, now, srv.DataService)
evalResults, err := evaluator.QueriesAndExpressionsEval(c.SignedInUser.OrgId, cmd.Data, now, srv.ExpressionService)
if err != nil {
return ErrResp(http.StatusBadRequest, err, "Failed to evaluate queries and expressions")
}

View File

@@ -13,6 +13,7 @@ import (
"github.com/grafana/grafana-plugin-sdk-go/data"
"github.com/grafana/grafana/pkg/api/response"
"github.com/grafana/grafana/pkg/expr"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/datasourceproxy"
@@ -21,7 +22,6 @@ import (
"github.com/grafana/grafana/pkg/services/ngalert/eval"
ngmodels "github.com/grafana/grafana/pkg/services/ngalert/models"
"github.com/grafana/grafana/pkg/setting"
"github.com/grafana/grafana/pkg/tsdb"
"github.com/grafana/grafana/pkg/util"
"github.com/grafana/grafana/pkg/web"
"github.com/pkg/errors"
@@ -223,7 +223,7 @@ func validateQueriesAndExpressions(data []ngmodels.AlertQuery, user *models.Sign
return refIDs, nil
}
func conditionEval(c *models.ReqContext, cmd ngmodels.EvalAlertConditionCommand, datasourceCache datasources.CacheService, dataService *tsdb.Service, cfg *setting.Cfg, log log.Logger) response.Response {
func conditionEval(c *models.ReqContext, cmd ngmodels.EvalAlertConditionCommand, datasourceCache datasources.CacheService, expressionService *expr.Service, cfg *setting.Cfg, log log.Logger) response.Response {
evalCond := ngmodels.Condition{
Condition: cmd.Condition,
OrgID: c.SignedInUser.OrgId,
@@ -239,7 +239,7 @@ func conditionEval(c *models.ReqContext, cmd ngmodels.EvalAlertConditionCommand,
}
evaluator := eval.Evaluator{Cfg: cfg, Log: log}
evalResults, err := evaluator.ConditionEval(&evalCond, now, dataService)
evalResults, err := evaluator.ConditionEval(&evalCond, now, expressionService)
if err != nil {
return ErrResp(http.StatusBadRequest, err, "Failed to evaluate conditions")
}

View File

@@ -14,7 +14,6 @@ import (
"github.com/grafana/grafana/pkg/services/ngalert/models"
"github.com/grafana/grafana/pkg/setting"
"github.com/grafana/grafana/pkg/tsdb"
"github.com/grafana/grafana-plugin-sdk-go/backend"
"github.com/grafana/grafana-plugin-sdk-go/data"
@@ -165,10 +164,10 @@ type NumberValueCapture struct {
Value *float64
}
func executeCondition(ctx AlertExecCtx, c *models.Condition, now time.Time, dataService *tsdb.Service) ExecutionResults {
func executeCondition(ctx AlertExecCtx, c *models.Condition, now time.Time, exprService *expr.Service) ExecutionResults {
result := ExecutionResults{}
execResp, err := executeQueriesAndExpressions(ctx, c.Data, now, dataService)
execResp, err := executeQueriesAndExpressions(ctx, c.Data, now, exprService)
if err != nil {
return ExecutionResults{Error: err}
@@ -232,7 +231,7 @@ func executeCondition(ctx AlertExecCtx, c *models.Condition, now time.Time, data
return result
}
func executeQueriesAndExpressions(ctx AlertExecCtx, data []models.AlertQuery, now time.Time, dataService *tsdb.Service) (resp *backend.QueryDataResponse, err error) {
func executeQueriesAndExpressions(ctx AlertExecCtx, data []models.AlertQuery, now time.Time, exprService *expr.Service) (resp *backend.QueryDataResponse, err error) {
defer func() {
if e := recover(); e != nil {
ctx.Log.Error("alert rule panic", "error", e, "stack", string(debug.Stack()))
@@ -250,10 +249,6 @@ func executeQueriesAndExpressions(ctx AlertExecCtx, data []models.AlertQuery, no
return nil, err
}
exprService := expr.Service{
Cfg: &setting.Cfg{ExpressionsEnabled: ctx.ExpressionsEnabled},
DataService: dataService,
}
return exprService.TransformData(ctx.Ctx, queryDataReq)
}
@@ -431,26 +426,26 @@ func (evalResults Results) AsDataFrame() data.Frame {
}
// ConditionEval executes conditions and evaluates the result.
func (e *Evaluator) ConditionEval(condition *models.Condition, now time.Time, dataService *tsdb.Service) (Results, error) {
func (e *Evaluator) ConditionEval(condition *models.Condition, now time.Time, expressionService *expr.Service) (Results, error) {
alertCtx, cancelFn := context.WithTimeout(context.Background(), e.Cfg.UnifiedAlerting.EvaluationTimeout)
defer cancelFn()
alertExecCtx := AlertExecCtx{OrgID: condition.OrgID, Ctx: alertCtx, ExpressionsEnabled: e.Cfg.ExpressionsEnabled, Log: e.Log}
execResult := executeCondition(alertExecCtx, condition, now, dataService)
execResult := executeCondition(alertExecCtx, condition, now, expressionService)
evalResults := evaluateExecutionResult(execResult, now)
return evalResults, nil
}
// QueriesAndExpressionsEval executes queries and expressions and returns the result.
func (e *Evaluator) QueriesAndExpressionsEval(orgID int64, data []models.AlertQuery, now time.Time, dataService *tsdb.Service) (*backend.QueryDataResponse, error) {
func (e *Evaluator) QueriesAndExpressionsEval(orgID int64, data []models.AlertQuery, now time.Time, expressionService *expr.Service) (*backend.QueryDataResponse, error) {
alertCtx, cancelFn := context.WithTimeout(context.Background(), e.Cfg.UnifiedAlerting.EvaluationTimeout)
defer cancelFn()
alertExecCtx := AlertExecCtx{OrgID: orgID, Ctx: alertCtx, ExpressionsEnabled: e.Cfg.ExpressionsEnabled, Log: e.Log}
execResult, err := executeQueriesAndExpressions(alertExecCtx, data, now, dataService)
execResult, err := executeQueriesAndExpressions(alertExecCtx, data, now, expressionService)
if err != nil {
return nil, fmt.Errorf("failed to execute conditions: %w", err)
}

View File

@@ -7,6 +7,7 @@ import (
"github.com/benbjohnson/clock"
"github.com/grafana/grafana/pkg/api/routing"
"github.com/grafana/grafana/pkg/expr"
"github.com/grafana/grafana/pkg/infra/kvstore"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/services/datasourceproxy"
@@ -22,7 +23,6 @@ import (
"github.com/grafana/grafana/pkg/services/secrets"
"github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/grafana/grafana/pkg/setting"
"github.com/grafana/grafana/pkg/tsdb"
"golang.org/x/sync/errgroup"
)
@@ -38,20 +38,20 @@ const (
)
func ProvideService(cfg *setting.Cfg, dataSourceCache datasources.CacheService, routeRegister routing.RouteRegister,
sqlStore *sqlstore.SQLStore, kvStore kvstore.KVStore, dataService *tsdb.Service, dataProxy *datasourceproxy.DataSourceProxyService,
sqlStore *sqlstore.SQLStore, kvStore kvstore.KVStore, expressionService *expr.Service, dataProxy *datasourceproxy.DataSourceProxyService,
quotaService *quota.QuotaService, secretsService secrets.Service, m *metrics.NGAlert) (*AlertNG, error) {
ng := &AlertNG{
Cfg: cfg,
DataSourceCache: dataSourceCache,
RouteRegister: routeRegister,
SQLStore: sqlStore,
KVStore: kvStore,
DataService: dataService,
DataProxy: dataProxy,
QuotaService: quotaService,
SecretsService: secretsService,
Metrics: m,
Log: log.New("ngalert"),
Cfg: cfg,
DataSourceCache: dataSourceCache,
RouteRegister: routeRegister,
SQLStore: sqlStore,
KVStore: kvStore,
ExpressionService: expressionService,
DataProxy: dataProxy,
QuotaService: quotaService,
SecretsService: secretsService,
Metrics: m,
Log: log.New("ngalert"),
}
if ng.IsDisabled() {
@@ -67,19 +67,19 @@ func ProvideService(cfg *setting.Cfg, dataSourceCache datasources.CacheService,
// AlertNG is the service for evaluating the condition of an alert definition.
type AlertNG struct {
Cfg *setting.Cfg
DataSourceCache datasources.CacheService
RouteRegister routing.RouteRegister
SQLStore *sqlstore.SQLStore
KVStore kvstore.KVStore
DataService *tsdb.Service
DataProxy *datasourceproxy.DataSourceProxyService
QuotaService *quota.QuotaService
SecretsService secrets.Service
Metrics *metrics.NGAlert
Log log.Logger
schedule schedule.ScheduleService
stateManager *state.Manager
Cfg *setting.Cfg
DataSourceCache datasources.CacheService
RouteRegister routing.RouteRegister
SQLStore *sqlstore.SQLStore
KVStore kvstore.KVStore
ExpressionService *expr.Service
DataProxy *datasourceproxy.DataSourceProxyService
QuotaService *quota.QuotaService
SecretsService secrets.Service
Metrics *metrics.NGAlert
Log log.Logger
schedule schedule.ScheduleService
stateManager *state.Manager
// Alerting notification services
MultiOrgAlertmanager *notifier.MultiOrgAlertmanager
@@ -136,7 +136,7 @@ func (ng *AlertNG) init() error {
appUrl = nil
}
stateManager := state.NewManager(ng.Log, ng.Metrics.GetStateMetrics(), appUrl, store, store)
scheduler := schedule.NewScheduler(schedCfg, ng.DataService, appUrl, stateManager)
scheduler := schedule.NewScheduler(schedCfg, ng.ExpressionService, appUrl, stateManager)
ng.stateManager = stateManager
ng.schedule = scheduler
@@ -145,7 +145,7 @@ func (ng *AlertNG) init() error {
Cfg: ng.Cfg,
DatasourceCache: ng.DataSourceCache,
RouteRegister: ng.RouteRegister,
DataService: ng.DataService,
ExpressionService: ng.ExpressionService,
Schedule: ng.schedule,
DataProxy: ng.DataProxy,
QuotaService: ng.QuotaService,

View File

@@ -8,6 +8,7 @@ import (
"sync"
"time"
"github.com/grafana/grafana/pkg/expr"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/services/alerting"
"github.com/grafana/grafana/pkg/services/ngalert/eval"
@@ -17,7 +18,6 @@ import (
"github.com/grafana/grafana/pkg/services/ngalert/sender"
"github.com/grafana/grafana/pkg/services/ngalert/state"
"github.com/grafana/grafana/pkg/services/ngalert/store"
"github.com/grafana/grafana/pkg/tsdb"
"github.com/benbjohnson/clock"
"golang.org/x/sync/errgroup"
@@ -64,11 +64,11 @@ type schedule struct {
evaluator eval.Evaluator
ruleStore store.RuleStore
instanceStore store.InstanceStore
adminConfigStore store.AdminConfigurationStore
orgStore store.OrgStore
dataService *tsdb.Service
ruleStore store.RuleStore
instanceStore store.InstanceStore
adminConfigStore store.AdminConfigurationStore
orgStore store.OrgStore
expressionService *expr.Service
stateManager *state.Manager
@@ -107,7 +107,7 @@ type SchedulerCfg struct {
}
// NewScheduler returns a new schedule.
func NewScheduler(cfg SchedulerCfg, dataService *tsdb.Service, appURL *url.URL, stateManager *state.Manager) *schedule {
func NewScheduler(cfg SchedulerCfg, expressionService *expr.Service, appURL *url.URL, stateManager *state.Manager) *schedule {
ticker := alerting.NewTicker(cfg.C.Now(), time.Second*0, cfg.C, int64(cfg.BaseInterval.Seconds()))
sch := schedule{
@@ -123,7 +123,7 @@ func NewScheduler(cfg SchedulerCfg, dataService *tsdb.Service, appURL *url.URL,
ruleStore: cfg.RuleStore,
instanceStore: cfg.InstanceStore,
orgStore: cfg.OrgStore,
dataService: dataService,
expressionService: expressionService,
adminConfigStore: cfg.AdminConfigStore,
multiOrgNotifier: cfg.MultiOrgNotifier,
metrics: cfg.Metrics,
@@ -449,7 +449,7 @@ func (sch *schedule) ruleRoutine(grafanaCtx context.Context, key models.AlertRul
OrgID: alertRule.OrgID,
Data: alertRule.Data,
}
results, err := sch.evaluator.ConditionEval(&condition, ctx.now, sch.dataService)
results, err := sch.evaluator.ConditionEval(&condition, ctx.now, sch.expressionService)
dur := sch.clock.Now().Sub(start)
evalTotal.Inc()
evalDuration.Observe(dur.Seconds())

View File

@@ -17,6 +17,7 @@ import (
"github.com/prometheus/common/model"
"github.com/stretchr/testify/require"
"github.com/grafana/grafana/pkg/expr"
"github.com/grafana/grafana/pkg/infra/log"
apimodels "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions"
"github.com/grafana/grafana/pkg/services/ngalert/eval"
@@ -604,7 +605,7 @@ func setupScheduler(t *testing.T, rs store.RuleStore, is store.InstanceStore, ac
Scheme: "http",
Host: "localhost",
}
return NewScheduler(schedCfg, nil, appUrl, st), mockedClock
return NewScheduler(schedCfg, expr.ProvideService(&setting.Cfg{ExpressionsEnabled: true}, nil, nil), appUrl, st), mockedClock
}
// createTestAlertRule creates a dummy alert definition to be used by the tests.