apitech
/
grafana
mirror of https://github.com/grafana/grafana
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Alerting NG: update API to expect UIDs instead of IDs (#29896)

Browse Source 
* Change API to expect UIDs instead of ID

* Remove unnecessary transactions

When only one query is executed

* Modify API responses

* Cleanup tests

* Use globally orgID and UID for identifying alert definitions
pull/30116/head
Sofia Papagiannaki 5 years ago committed by GitHub
parent a6aa0024a2
commit 5560be73bf
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 214 additions and 213 deletions
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Show Stats Download Patch File Download Diff File
  1. 53
      pkg/services/ngalert/api.go
  2. 2
      pkg/services/ngalert/common_test.go
  3. 100
      pkg/services/ngalert/database.go
  4. 86
      pkg/services/ngalert/database_test.go
  5. 6
      pkg/services/ngalert/middleware.go
  6. 29
      pkg/services/ngalert/models.go
  7. 8
      pkg/services/ngalert/ngalert.go
  8. 90
      pkg/services/ngalert/schedule.go
  9. 53
      pkg/services/ngalert/schedule_test.go

53
pkg/services/ngalert/api.go
Unescape View File

@ -15,12 +15,12 @@ import (
func (ng *AlertNG) registerAPIEndpoints() {
ng.RouteRegister.Group("/api/alert-definitions", func(alertDefinitions routing.RouteRegister) {
alertDefinitions.Get("", middleware.ReqSignedIn, api.Wrap(ng.listAlertDefinitions))
alertDefinitions.Get("/eval/:alertDefinitionId", ng.validateOrgAlertDefinition, api.Wrap(ng.alertDefinitionEvalEndpoint))
alertDefinitions.Get("/eval/:alertDefinitionUID", ng.validateOrgAlertDefinition, api.Wrap(ng.alertDefinitionEvalEndpoint))
alertDefinitions.Post("/eval", middleware.ReqSignedIn, binding.Bind(evalAlertConditionCommand{}), api.Wrap(ng.conditionEvalEndpoint))
alertDefinitions.Get("/:alertDefinitionId", ng.validateOrgAlertDefinition, api.Wrap(ng.getAlertDefinitionEndpoint))
alertDefinitions.Delete("/:alertDefinitionId", ng.validateOrgAlertDefinition, api.Wrap(ng.deleteAlertDefinitionEndpoint))
alertDefinitions.Get("/:alertDefinitionUID", ng.validateOrgAlertDefinition, api.Wrap(ng.getAlertDefinitionEndpoint))
alertDefinitions.Delete("/:alertDefinitionUID", ng.validateOrgAlertDefinition, api.Wrap(ng.deleteAlertDefinitionEndpoint))
alertDefinitions.Post("/", middleware.ReqSignedIn, binding.Bind(saveAlertDefinitionCommand{}), api.Wrap(ng.createAlertDefinitionEndpoint))
alertDefinitions.Put("/:alertDefinitionId", ng.validateOrgAlertDefinition, binding.Bind(updateAlertDefinitionCommand{}), api.Wrap(ng.updateAlertDefinitionEndpoint))
alertDefinitions.Put("/:alertDefinitionUID", ng.validateOrgAlertDefinition, binding.Bind(updateAlertDefinitionCommand{}), api.Wrap(ng.updateAlertDefinitionEndpoint))
})
ng.RouteRegister.Group("/api/ngalert/", func(schedulerRouter routing.RouteRegister) {
@ -52,11 +52,11 @@ func (ng *AlertNG) conditionEvalEndpoint(c *models.ReqContext, dto evalAlertCond
})
}
// alertDefinitionEvalEndpoint handles GET /api/alert-definitions/eval/:dashboardId/:panelId/:refId".
// alertDefinitionEvalEndpoint handles GET /api/alert-definitions/eval/:alertDefinitionUID.
func (ng *AlertNG) alertDefinitionEvalEndpoint(c *models.ReqContext) api.Response {
alertDefinitionID := c.ParamsInt64(":alertDefinitionId")
alertDefinitionUID := c.ParamsEscape(":alertDefinitionUID")
condition, err := ng.LoadAlertCondition(alertDefinitionID)
condition, err := ng.LoadAlertCondition(alertDefinitionUID, c.SignedInUser.OrgId)
if err != nil {
return api.Error(400, "Failed to load alert definition conditions", err)
}
@ -67,7 +67,7 @@ func (ng *AlertNG) alertDefinitionEvalEndpoint(c *models.ReqContext) api.Respons
evalResults, err := eval.ConditionEval(condition, timeNow())
if err != nil {
return api.Error(400, "Failed to evaludate alert", err)
return api.Error(400, "Failed to evaluate alert", err)
}
frame := evalResults.AsDataFrame()
@ -85,44 +85,41 @@ func (ng *AlertNG) alertDefinitionEvalEndpoint(c *models.ReqContext) api.Respons
})
}
// getAlertDefinitionEndpoint handles GET /api/alert-definitions/:alertDefinitionId.
// getAlertDefinitionEndpoint handles GET /api/alert-definitions/:alertDefinitionUID.
func (ng *AlertNG) getAlertDefinitionEndpoint(c *models.ReqContext) api.Response {
alertDefinitionID := c.ParamsInt64(":alertDefinitionId")
alertDefinitionUID := c.ParamsEscape(":alertDefinitionUID")
query := getAlertDefinitionByIDQuery{
ID: alertDefinitionID,
query := getAlertDefinitionByUIDQuery{
UID: alertDefinitionUID,
OrgID: c.SignedInUser.OrgId,
}
if err := ng.getAlertDefinitionByID(&query); err != nil {
if err := ng.getAlertDefinitionByUID(&query); err != nil {
return api.Error(500, "Failed to get alert definition", err)
}
return api.JSON(200, &query.Result)
}
// deleteAlertDefinitionEndpoint handles DELETE /api/alert-definitions/:alertDefinitionId.
// deleteAlertDefinitionEndpoint handles DELETE /api/alert-definitions/:alertDefinitionUID.
func (ng *AlertNG) deleteAlertDefinitionEndpoint(c *models.ReqContext) api.Response {
alertDefinitionID := c.ParamsInt64(":alertDefinitionId")
alertDefinitionUID := c.ParamsEscape(":alertDefinitionUID")
cmd := deleteAlertDefinitionByIDCommand{
ID: alertDefinitionID,
cmd := deleteAlertDefinitionByUIDCommand{
UID: alertDefinitionUID,
OrgID: c.SignedInUser.OrgId,
}
if err := ng.deleteAlertDefinitionByID(&cmd); err != nil {
if err := ng.deleteAlertDefinitionByUID(&cmd); err != nil {
return api.Error(500, "Failed to delete alert definition", err)
}
if cmd.RowsAffected != 1 {
ng.log.Warn("unexpected number of rows affected on alert definition delete", "definitionID", alertDefinitionID, "rowsAffected", cmd.RowsAffected)
}
return api.Success("Alert definition deleted")
}
// updateAlertDefinitionEndpoint handles PUT /api/alert-definitions/:alertDefinitionId.
// updateAlertDefinitionEndpoint handles PUT /api/alert-definitions/:alertDefinitionUID.
func (ng *AlertNG) updateAlertDefinitionEndpoint(c *models.ReqContext, cmd updateAlertDefinitionCommand) api.Response {
cmd.ID = c.ParamsInt64(":alertDefinitionId")
cmd.UID = c.ParamsEscape(":alertDefinitionUID")
cmd.OrgID = c.SignedInUser.OrgId
if err := ng.validateCondition(cmd.Condition, c.SignedInUser); err != nil {
@ -133,11 +130,7 @@ func (ng *AlertNG) updateAlertDefinitionEndpoint(c *models.ReqContext, cmd updat
return api.Error(500, "Failed to update alert definition", err)
}
if cmd.RowsAffected != 1 {
ng.log.Warn("unexpected number of rows affected on alert definition update", "definitionID", cmd.ID, "rowsAffected", cmd.RowsAffected)
}
return api.Success("Alert definition updated")
return api.JSON(200, cmd.Result)
}
// createAlertDefinitionEndpoint handles POST /api/alert-definitions.
@ -152,7 +145,7 @@ func (ng *AlertNG) createAlertDefinitionEndpoint(c *models.ReqContext, cmd saveA
return api.Error(500, "Failed to create alert definition", err)
}
return api.JSON(200, util.DynMap{"id": cmd.Result.ID})
return api.JSON(200, cmd.Result)
}
// listAlertDefinitions handles GET /api/alert-definitions.

2
pkg/services/ngalert/common_test.go
Unescape View File

@ -80,6 +80,6 @@ func createTestAlertDefinition(t *testing.T, ng *AlertNG, intervalSeconds int64)
}
err := ng.saveAlertDefinition(&cmd)
require.NoError(t, err)
t.Logf("alert definition: %d with interval: %d created", cmd.Result.ID, intervalSeconds)
t.Logf("alert definition: %v with interval: %d created", cmd.Result.getKey(), intervalSeconds)
return cmd.Result
}

100
pkg/services/ngalert/database.go
Unescape View File

@ -9,9 +9,10 @@ import (
"github.com/grafana/grafana/pkg/util"
)
func getAlertDefinitionByID(alertDefinitionID int64, sess *sqlstore.DBSession) (*AlertDefinition, error) {
alertDefinition := AlertDefinition{}
has, err := sess.ID(alertDefinitionID).Get(&alertDefinition)
func getAlertDefinitionByUID(sess *sqlstore.DBSession, alertDefinitionUID string, orgID int64) (*AlertDefinition, error) {
// we consider optionally enabling some caching
alertDefinition := AlertDefinition{OrgID: orgID, UID: alertDefinitionUID}
has, err := sess.Get(&alertDefinition)
if !has {
return nil, errAlertDefinitionNotFound
}
@ -23,20 +24,19 @@ func getAlertDefinitionByID(alertDefinitionID int64, sess *sqlstore.DBSession) (
// deleteAlertDefinitionByID is a handler for deleting an alert definition.
// It returns models.ErrAlertDefinitionNotFound if no alert definition is found for the provided ID.
func (ng *AlertNG) deleteAlertDefinitionByID(cmd *deleteAlertDefinitionByIDCommand) error {
func (ng *AlertNG) deleteAlertDefinitionByUID(cmd *deleteAlertDefinitionByUIDCommand) error {
return ng.SQLStore.WithTransactionalDbSession(context.Background(), func(sess *sqlstore.DBSession) error {
res, err := sess.Exec("DELETE FROM alert_definition WHERE id = ?", cmd.ID)
res, err := sess.Exec("DELETE FROM alert_definition WHERE uid = ? AND org_id = ?", cmd.UID, cmd.OrgID)
if err != nil {
return err
}
rowsAffected, err := res.RowsAffected()
_, err = res.RowsAffected()
if err != nil {
return err
}
cmd.RowsAffected = rowsAffected
_, err = sess.Exec("DELETE FROM alert_definition_version WHERE alert_definition_id = ?", cmd.ID)
_, err = sess.Exec("DELETE FROM alert_definition_version WHERE alert_definition_uid = ?", cmd.UID)
if err != nil {
return err
}
@ -45,11 +45,11 @@ func (ng *AlertNG) deleteAlertDefinitionByID(cmd *deleteAlertDefinitionByIDComma
})
}
// getAlertDefinitionByID is a handler for retrieving an alert definition from that database by its ID.
// getAlertDefinitionByUID is a handler for retrieving an alert definition from that database by its UID and organisation ID.
// It returns models.ErrAlertDefinitionNotFound if no alert definition is found for the provided ID.
func (ng *AlertNG) getAlertDefinitionByID(query *getAlertDefinitionByIDQuery) error {
return ng.SQLStore.WithTransactionalDbSession(context.Background(), func(sess *sqlstore.DBSession) error {
alertDefinition, err := getAlertDefinitionByID(query.ID, sess)
func (ng *AlertNG) getAlertDefinitionByUID(query *getAlertDefinitionByUIDQuery) error {
return ng.SQLStore.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error {
alertDefinition, err := getAlertDefinitionByUID(sess, query.UID, query.OrgID)
if err != nil {
return err
}
@ -118,42 +118,14 @@ func (ng *AlertNG) saveAlertDefinition(cmd *saveAlertDefinitionCommand) error {
// It returns models.ErrAlertDefinitionNotFound if no alert definition is found for the provided ID.
func (ng *AlertNG) updateAlertDefinition(cmd *updateAlertDefinitionCommand) error {
return ng.SQLStore.WithTransactionalDbSession(context.Background(), func(sess *sqlstore.DBSession) error {
alertDefinition := &AlertDefinition{
ID: cmd.ID,
Title: cmd.Title,
Condition: cmd.Condition.RefID,
Data: cmd.Condition.QueriesAndExpressions,
OrgID: cmd.OrgID,
}
if cmd.IntervalSeconds != nil {
alertDefinition.IntervalSeconds = *cmd.IntervalSeconds
}
if err := ng.validateAlertDefinition(alertDefinition, true); err != nil {
return err
}
if err := alertDefinition.preSave(); err != nil {
return err
}
existingAlertDefinition, err := getAlertDefinitionByID(alertDefinition.ID, sess)
existingAlertDefinition, err := getAlertDefinitionByUID(sess, cmd.UID, cmd.OrgID)
if err != nil {
if errors.Is(err, errAlertDefinitionNotFound) {
cmd.Result = alertDefinition
cmd.RowsAffected = 0
return nil
}
return err
}
alertDefinition.Version = existingAlertDefinition.Version + 1
affectedRows, err := sess.ID(cmd.ID).Update(alertDefinition)
if err != nil {
return err
}
title := cmd.Title
if title == "" {
title = existingAlertDefinition.Title
@ -171,30 +143,55 @@ func (ng *AlertNG) updateAlertDefinition(cmd *updateAlertDefinitionCommand) erro
intervalSeconds = &existingAlertDefinition.IntervalSeconds
}
// explicitly set all fields regardless of being provided or not
alertDefinition := &AlertDefinition{
ID: existingAlertDefinition.ID,
Title: title,
Condition: condition,
Data: data,
OrgID: existingAlertDefinition.OrgID,
IntervalSeconds: *intervalSeconds,
UID: existingAlertDefinition.UID,
}
if err := ng.validateAlertDefinition(alertDefinition, true); err != nil {
return err
}
if err := alertDefinition.preSave(); err != nil {
return err
}
alertDefinition.Version = existingAlertDefinition.Version + 1
_, err = sess.ID(existingAlertDefinition.ID).Update(alertDefinition)
if err != nil {
return err
}
alertDefVersion := AlertDefinitionVersion{
AlertDefinitionID: alertDefinition.ID,
AlertDefinitionUID: existingAlertDefinition.UID,
ParentVersion: existingAlertDefinition.Version,
AlertDefinitionUID: alertDefinition.UID,
ParentVersion: alertDefinition.Version,
Version: alertDefinition.Version,
Condition: condition,
Condition: alertDefinition.Condition,
Created: alertDefinition.Updated,
Title: title,
Data: data,
IntervalSeconds: *intervalSeconds,
Title: alertDefinition.Title,
Data: alertDefinition.Data,
IntervalSeconds: alertDefinition.IntervalSeconds,
}
if _, err := sess.Insert(alertDefVersion); err != nil {
return err
}
cmd.Result = alertDefinition
cmd.RowsAffected = affectedRows
return nil
})
}
// getOrgAlertDefinitions is a handler for retrieving alert definitions of specific organisation.
func (ng *AlertNG) getOrgAlertDefinitions(query *listAlertDefinitionsQuery) error {
return ng.SQLStore.WithTransactionalDbSession(context.Background(), func(sess *sqlstore.DBSession) error {
return ng.SQLStore.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error {
alertDefinitions := make([]*AlertDefinition, 0)
q := "SELECT * FROM alert_definition WHERE org_id = ?"
if err := sess.SQL(q, query.OrgID).Find(&alertDefinitions); err != nil {
@ -207,9 +204,9 @@ func (ng *AlertNG) getOrgAlertDefinitions(query *listAlertDefinitionsQuery) erro
}
func (ng *AlertNG) getAlertDefinitions(query *listAlertDefinitionsQuery) error {
return ng.SQLStore.WithTransactionalDbSession(context.Background(), func(sess *sqlstore.DBSession) error {
return ng.SQLStore.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error {
alerts := make([]*AlertDefinition, 0)
q := "SELECT id, interval_seconds, version FROM alert_definition"
q := "SELECT uid, org_id, interval_seconds, version FROM alert_definition"
if err := sess.SQL(q).Find(&alerts); err != nil {
return err
}
@ -218,7 +215,6 @@ func (ng *AlertNG) getAlertDefinitions(query *listAlertDefinitionsQuery) error {
return nil
})
}
func generateNewAlertDefinitionUID(sess *sqlstore.DBSession, orgID int64) (string, error) {
for i := 0; i < 3; i++ {
uid := util.GenerateShortUID()

86
pkg/services/ngalert/database_test.go
Unescape View File

@ -8,6 +8,7 @@ import (
"testing"
"time"
"github.com/grafana/grafana/pkg/registry"
"github.com/grafana/grafana/pkg/services/ngalert/eval"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
@ -63,6 +64,8 @@ func TestCreatingAlertDefinition(t *testing.T) {
for _, tc := range testCases {
t.Run(tc.desc, func(t *testing.T) {
ng := setupTestEnv(t)
t.Cleanup(registry.ClearOverrides)
q := saveAlertDefinitionCommand{
OrgID: 1,
Title: tc.inputTitle,
@ -103,14 +106,15 @@ func TestCreatingAlertDefinition(t *testing.T) {
}
func TestUpdatingAlertDefinition(t *testing.T) {
mockTimeNow()
defer resetTimeNow()
t.Run("zero rows affected when updating unknown alert", func(t *testing.T) {
mockTimeNow()
defer resetTimeNow()
ng := setupTestEnv(t)
t.Cleanup(registry.ClearOverrides)
q := updateAlertDefinitionCommand{
ID: 1,
UID: "unknown",
OrgID: 1,
Title: "something completely different",
Condition: eval.Condition{
@ -134,11 +138,15 @@ func TestUpdatingAlertDefinition(t *testing.T) {
err := ng.updateAlertDefinition(&q)
require.NoError(t, err)
assert.Equal(t, int64(0), q.RowsAffected)
})
t.Run("updating existing alert", func(t *testing.T) {
mockTimeNow()
defer resetTimeNow()
ng := setupTestEnv(t)
t.Cleanup(registry.ClearOverrides)
var initialInterval int64 = 120
alertDefinition := createTestAlertDefinition(t, ng, initialInterval)
created := alertDefinition.Updated
@ -159,7 +167,7 @@ func TestUpdatingAlertDefinition(t *testing.T) {
inputOrgID: alertDefinition.OrgID,
inputTitle: "something completely different",
expectedIntervalSeconds: initialInterval,
expectedUpdated: time.Unix(2, 0).UTC(),
expectedUpdated: time.Unix(1, 0).UTC(),
},
{
desc: "should update interval if it's provided",
@ -167,7 +175,7 @@ func TestUpdatingAlertDefinition(t *testing.T) {
inputOrgID: alertDefinition.OrgID,
inputTitle: "something completely different",
expectedIntervalSeconds: customInterval,
expectedUpdated: time.Unix(3, 0).UTC(),
expectedUpdated: time.Unix(2, 0).UTC(),
},
{
desc: "should not update organisation if it's provided",
@ -175,7 +183,7 @@ func TestUpdatingAlertDefinition(t *testing.T) {
inputOrgID: 0,
inputTitle: "something completely different",
expectedIntervalSeconds: customInterval,
expectedUpdated: time.Unix(4, 0).UTC(),
expectedUpdated: time.Unix(3, 0).UTC(),
},
{
desc: "should not update alert definition if the name it's too big",
@ -187,7 +195,7 @@ func TestUpdatingAlertDefinition(t *testing.T) {
}
q := updateAlertDefinitionCommand{
ID: (*alertDefinition).ID,
UID: (*alertDefinition).UID,
Title: "something completely different",
Condition: eval.Condition{
RefID: "B",
@ -224,20 +232,16 @@ func TestUpdatingAlertDefinition(t *testing.T) {
case tc.expectedError != nil:
require.Error(t, err)
getAlertDefinitionByIDQuery := getAlertDefinitionByIDQuery{ID: (*alertDefinition).ID}
err = ng.getAlertDefinitionByID(&getAlertDefinitionByIDQuery)
require.NoError(t, err)
assert.Equal(t, previousAlertDefinition.Title, getAlertDefinitionByIDQuery.Result.Title)
assert.Equal(t, previousAlertDefinition.Condition, getAlertDefinitionByIDQuery.Result.Condition)
assert.Equal(t, len(previousAlertDefinition.Data), len(getAlertDefinitionByIDQuery.Result.Data))
assert.Equal(t, previousAlertDefinition.IntervalSeconds, getAlertDefinitionByIDQuery.Result.IntervalSeconds)
assert.Equal(t, previousAlertDefinition.Updated, getAlertDefinitionByIDQuery.Result.Updated)
assert.Equal(t, previousAlertDefinition.Version, getAlertDefinitionByIDQuery.Result.Version)
assert.Equal(t, previousAlertDefinition.OrgID, getAlertDefinitionByIDQuery.Result.OrgID)
assert.Equal(t, previousAlertDefinition.UID, getAlertDefinitionByIDQuery.Result.UID)
assert.Equal(t, previousAlertDefinition.Title, q.Result.Title)
assert.Equal(t, previousAlertDefinition.Condition, q.Result.Condition)
assert.Equal(t, len(previousAlertDefinition.Data), len(q.Result.Data))
assert.Equal(t, previousAlertDefinition.IntervalSeconds, q.Result.IntervalSeconds)
assert.Equal(t, previousAlertDefinition.Updated, q.Result.Updated)
assert.Equal(t, previousAlertDefinition.Version, q.Result.Version)
assert.Equal(t, previousAlertDefinition.OrgID, q.Result.OrgID)
assert.Equal(t, previousAlertDefinition.UID, q.Result.UID)
default:
require.NoError(t, err)
assert.Equal(t, int64(1), q.RowsAffected)
assert.Equal(t, int64(1), q.Result.ID)
assert.True(t, q.Result.Updated.After(lastUpdated))
assert.Equal(t, tc.expectedUpdated, q.Result.Updated)
@ -245,19 +249,16 @@ func TestUpdatingAlertDefinition(t *testing.T) {
assert.Equal(t, alertDefinition.OrgID, q.Result.OrgID)
getAlertDefinitionByIDQuery := getAlertDefinitionByIDQuery{ID: (*alertDefinition).ID}
err = ng.getAlertDefinitionByID(&getAlertDefinitionByIDQuery)
require.NoError(t, err)
assert.Equal(t, "something completely different", getAlertDefinitionByIDQuery.Result.Title)
assert.Equal(t, "B", getAlertDefinitionByIDQuery.Result.Condition)
assert.Equal(t, 1, len(getAlertDefinitionByIDQuery.Result.Data))
assert.Equal(t, tc.expectedUpdated, getAlertDefinitionByIDQuery.Result.Updated)
assert.Equal(t, tc.expectedIntervalSeconds, getAlertDefinitionByIDQuery.Result.IntervalSeconds)
assert.Equal(t, previousAlertDefinition.Version+1, getAlertDefinitionByIDQuery.Result.Version)
assert.Equal(t, alertDefinition.OrgID, getAlertDefinitionByIDQuery.Result.OrgID)
assert.Equal(t, alertDefinition.UID, getAlertDefinitionByIDQuery.Result.UID)
previousAlertDefinition = getAlertDefinitionByIDQuery.Result
assert.Equal(t, "something completely different", q.Result.Title)
assert.Equal(t, "B", q.Result.Condition)
assert.Equal(t, 1, len(q.Result.Data))
assert.Equal(t, tc.expectedUpdated, q.Result.Updated)
assert.Equal(t, tc.expectedIntervalSeconds, q.Result.IntervalSeconds)
assert.Equal(t, previousAlertDefinition.Version+1, q.Result.Version)
assert.Equal(t, alertDefinition.OrgID, q.Result.OrgID)
assert.Equal(t, alertDefinition.UID, q.Result.UID)
previousAlertDefinition = q.Result
}
})
@ -269,29 +270,30 @@ func TestUpdatingAlertDefinition(t *testing.T) {
func TestDeletingAlertDefinition(t *testing.T) {
t.Run("zero rows affected when deleting unknown alert", func(t *testing.T) {
ng := setupTestEnv(t)
t.Cleanup(registry.ClearOverrides)
q := deleteAlertDefinitionByIDCommand{
ID: 1,
q := deleteAlertDefinitionByUIDCommand{
UID: "unknown",
OrgID: 1,
}
err := ng.deleteAlertDefinitionByID(&q)
err := ng.deleteAlertDefinitionByUID(&q)
require.NoError(t, err)
assert.Equal(t, int64(0), q.RowsAffected)
})
t.Run("deleting successfully existing alert", func(t *testing.T) {
ng := setupTestEnv(t)
t.Cleanup(registry.ClearOverrides)
alertDefinition := createTestAlertDefinition(t, ng, 60)
q := deleteAlertDefinitionByIDCommand{
ID: (*alertDefinition).ID,
q := deleteAlertDefinitionByUIDCommand{
UID: (*alertDefinition).UID,
OrgID: 1,
}
err := ng.deleteAlertDefinitionByID(&q)
err := ng.deleteAlertDefinitionByUID(&q)
require.NoError(t, err)
assert.Equal(t, int64(1), q.RowsAffected)
})
}

6
pkg/services/ngalert/middleware.go
Unescape View File

@ -5,10 +5,10 @@ import (
)
func (ng *AlertNG) validateOrgAlertDefinition(c *models.ReqContext) {
id := c.ParamsInt64(":alertDefinitionId")
query := getAlertDefinitionByIDQuery{ID: id}
uid := c.ParamsEscape(":alertDefinitionUID")
query := getAlertDefinitionByUIDQuery{UID: uid, OrgID: c.SignedInUser.OrgId}
if err := ng.getAlertDefinitionByID(&query); err != nil {
if err := ng.getAlertDefinitionByUID(&query); err != nil {
c.JsonApiErr(404, "Alert definition not found", nil)
return
}

29
pkg/services/ngalert/models.go
Unescape View File

@ -23,6 +23,19 @@ type AlertDefinition struct {
UID string `xorm:"uid"`
}
type alertDefinitionKey struct {
orgID int64
definitionUID string
}
func (k alertDefinitionKey) String() string {
return fmt.Sprintf("{orgID: %d, definitionUID: %s}", k.orgID, k.definitionUID)
}
func (alertDefinition *AlertDefinition) getKey() alertDefinitionKey {
return alertDefinitionKey{orgID: alertDefinition.OrgID, definitionUID: alertDefinition.UID}
}
// AlertDefinitionVersion is the model for alert definition versions in Alerting NG.
type AlertDefinitionVersion struct {
ID int64 `xorm:"pk autoincr 'id'"`
@ -44,19 +57,17 @@ var (
errAlertDefinitionNotFound = fmt.Errorf("could not find alert definition")
)
// getAlertDefinitionByIDQuery is the query for retrieving/deleting an alert definition by ID.
type getAlertDefinitionByIDQuery struct {
ID int64
// getAlertDefinitionByUIDQuery is the query for retrieving/deleting an alert definition by UID and organisation ID.
type getAlertDefinitionByUIDQuery struct {
UID string
OrgID int64
Result *AlertDefinition
}
type deleteAlertDefinitionByIDCommand struct {
ID int64
type deleteAlertDefinitionByUIDCommand struct {
UID string
OrgID int64
RowsAffected int64
}
// saveAlertDefinitionCommand is the query for saving a new alert definition.
@ -71,15 +82,13 @@ type saveAlertDefinitionCommand struct {
// updateAlertDefinitionCommand is the query for updating an existing alert definition.
type updateAlertDefinitionCommand struct {
ID int64 `json:"-"`
Title string `json:"title"`
OrgID int64 `json:"-"`
Condition eval.Condition `json:"condition"`
IntervalSeconds *int64 `json:"interval_seconds"`
UID string `json:"-"`
RowsAffected int64
Result *AlertDefinition
Result *AlertDefinition
}
type evalAlertConditionCommand struct {

8
pkg/services/ngalert/ngalert.go
Unescape View File

@ -77,12 +77,12 @@ func (ng *AlertNG) AddMigration(mg *migrator.Migrator) {
}
// LoadAlertCondition returns a Condition object for the given alertDefinitionID.
func (ng *AlertNG) LoadAlertCondition(alertDefinitionID int64) (*eval.Condition, error) {
getAlertDefinitionByIDQuery := getAlertDefinitionByIDQuery{ID: alertDefinitionID}
if err := ng.getAlertDefinitionByID(&getAlertDefinitionByIDQuery); err != nil {
func (ng *AlertNG) LoadAlertCondition(alertDefinitionUID string, orgID int64) (*eval.Condition, error) {
q := getAlertDefinitionByUIDQuery{UID: alertDefinitionUID, OrgID: orgID}
if err := ng.getAlertDefinitionByUID(&q); err != nil {
return nil, err
}
alertDefinition := getAlertDefinitionByIDQuery.Result
alertDefinition := q.Result
err := ng.validateAlertDefinition(alertDefinition, true)
if err != nil {

90
pkg/services/ngalert/schedule.go
Unescape View File

@ -13,8 +13,8 @@ import (
"golang.org/x/sync/errgroup"
)
func (ng *AlertNG) definitionRoutine(grafanaCtx context.Context, definitionID int64, evalCh <-chan *evalContext) error {
ng.log.Debug("alert definition routine started", "definitionID", definitionID)
func (ng *AlertNG) definitionRoutine(grafanaCtx context.Context, key alertDefinitionKey, evalCh <-chan *evalContext) error {
ng.log.Debug("alert definition routine started", "key", key)
evalRunning := false
var start, end time.Time
@ -32,14 +32,14 @@ func (ng *AlertNG) definitionRoutine(grafanaCtx context.Context, definitionID in
// fetch latest alert definition version
if alertDefinition == nil || alertDefinition.Version < ctx.version {
q := getAlertDefinitionByIDQuery{ID: definitionID}
err := ng.getAlertDefinitionByID(&q)
q := getAlertDefinitionByUIDQuery{OrgID: key.orgID, UID: key.definitionUID}
err := ng.getAlertDefinitionByUID(&q)
if err != nil {
ng.schedule.log.Error("failed to fetch alert definition", "alertDefinitionID", alertDefinition.ID)
ng.schedule.log.Error("failed to fetch alert definition", "key", key)
return err
}
alertDefinition = q.Result
ng.schedule.log.Debug("new alert definition version fetched", "alertDefinitionID", alertDefinition.ID, "version", alertDefinition.Version)
ng.schedule.log.Debug("new alert definition version fetched", "key", key, "version", alertDefinition.Version)
}
condition := eval.Condition{
@ -50,11 +50,11 @@ func (ng *AlertNG) definitionRoutine(grafanaCtx context.Context, definitionID in
results, err := eval.ConditionEval(&condition, ctx.now)
end = timeNow()
if err != nil {
ng.schedule.log.Error("failed to evaluate alert definition", "definitionID", definitionID, "attempt", attempt, "now", ctx.now, "duration", end.Sub(start), "error", err)
ng.schedule.log.Error("failed to evaluate alert definition", "key", key, "attempt", attempt, "now", ctx.now, "duration", end.Sub(start), "error", err)
return err
}
for _, r := range results {
ng.schedule.log.Info("alert definition result", "definitionID", definitionID, "attempt", attempt, "now", ctx.now, "duration", end.Sub(start), "instance", r.Instance, "state", r.State.String())
ng.schedule.log.Info("alert definition result", "key", key, "attempt", attempt, "now", ctx.now, "duration", end.Sub(start), "instance", r.Instance, "state", r.State.String())
}
return nil
}
@ -64,7 +64,7 @@ func (ng *AlertNG) definitionRoutine(grafanaCtx context.Context, definitionID in
defer func() {
evalRunning = false
if ng.schedule.evalApplied != nil {
ng.schedule.evalApplied(definitionID, ctx.now)
ng.schedule.evalApplied(key, ctx.now)
}
}()
@ -75,9 +75,9 @@ func (ng *AlertNG) definitionRoutine(grafanaCtx context.Context, definitionID in
}
}
}()
case id := <-ng.schedule.stop:
if id == definitionID {
ng.schedule.log.Debug("stopping alert definition routine", "definitionID", definitionID)
case k := <-ng.schedule.stop:
if k == key {
ng.schedule.log.Debug("stopping alert definition routine", "key", key)
// interrupt evaluation if it's running
return nil
}
@ -95,7 +95,7 @@ type schedule struct {
registry alertDefinitionRegistry
// broadcast channel for stopping definition routines
stop chan int64
stop chan alertDefinitionKey
maxAttempts int64
@ -106,17 +106,17 @@ type schedule struct {
// evalApplied is only used for tests: test code can set it to non-nil
// function, and then it'll be called from the event loop whenever the
// message from evalApplied is handled.
evalApplied func(int64, time.Time)
evalApplied func(alertDefinitionKey, time.Time)
log log.Logger
}
// newScheduler returns a new schedule.
func newScheduler(c clock.Clock, baseInterval time.Duration, logger log.Logger, evalApplied func(int64, time.Time)) *schedule {
func newScheduler(c clock.Clock, baseInterval time.Duration, logger log.Logger, evalApplied func(alertDefinitionKey, time.Time)) *schedule {
ticker := alerting.NewTicker(c.Now(), time.Second*0, c, int64(baseInterval.Seconds()))
sch := schedule{
registry: alertDefinitionRegistry{alertDefinitionInfo: make(map[int64]alertDefinitionInfo)},
stop: make(chan int64),
registry: alertDefinitionRegistry{alertDefinitionInfo: make(map[alertDefinitionKey]alertDefinitionInfo)},
stop: make(chan alertDefinitionKey),
maxAttempts: maxAttempts,
clock: c,
baseInterval: baseInterval,
@ -161,37 +161,39 @@ func (ng *AlertNG) alertingTicker(grafanaCtx context.Context) error {
registeredDefinitions := ng.schedule.registry.keyMap()
type readyToRunItem struct {
id int64
key alertDefinitionKey
definitionInfo alertDefinitionInfo
}
readyToRun := make([]readyToRunItem, 0)
for _, item := range alertDefinitions {
itemID := item.ID
itemUID := item.UID
itemOrgID := item.OrgID
key := item.getKey()
itemVersion := item.Version
newRoutine := !ng.schedule.registry.exists(itemID)
definitionInfo := ng.schedule.registry.getOrCreateInfo(itemID, itemVersion)
newRoutine := !ng.schedule.registry.exists(key)
definitionInfo := ng.schedule.registry.getOrCreateInfo(key, itemVersion)
invalidInterval := item.IntervalSeconds%int64(ng.schedule.baseInterval.Seconds()) != 0
if newRoutine && !invalidInterval {
dispatcherGroup.Go(func() error {
return ng.definitionRoutine(ctx, itemID, definitionInfo.ch)
return ng.definitionRoutine(ctx, key, definitionInfo.ch)
})
}
if invalidInterval {
// this is expected to be always false
// give that we validate interval during alert definition updates
ng.schedule.log.Debug("alert definition with invalid interval will be ignored: interval should be divided exactly by scheduler interval", "definitionID", itemID, "interval", time.Duration(item.IntervalSeconds)*time.Second, "scheduler interval", ng.schedule.baseInterval)
ng.schedule.log.Debug("alert definition with invalid interval will be ignored: interval should be divided exactly by scheduler interval", "definitionUID", itemUID, "orgID", itemOrgID, "interval", time.Duration(item.IntervalSeconds)*time.Second, "scheduler interval", ng.schedule.baseInterval)
continue
}
itemFrequency := item.IntervalSeconds / int64(ng.schedule.baseInterval.Seconds())
if item.IntervalSeconds != 0 && tickNum%itemFrequency == 0 {
readyToRun = append(readyToRun, readyToRunItem{id: itemID, definitionInfo: definitionInfo})
readyToRun = append(readyToRun, readyToRunItem{key: key, definitionInfo: definitionInfo})
}
// remove the alert definition from the registered alert definitions
delete(registeredDefinitions, itemID)
delete(registeredDefinitions, key)
}
var step int64 = 0
@ -208,9 +210,9 @@ func (ng *AlertNG) alertingTicker(grafanaCtx context.Context) error {
}
// unregister and stop routines of the deleted alert definitions
for id := range registeredDefinitions {
ng.schedule.stop <- id
ng.schedule.registry.del(id)
for key := range registeredDefinitions {
ng.schedule.stop <- key
ng.schedule.registry.del(key)
}
case <-grafanaCtx.Done():
err := dispatcherGroup.Wait()
@ -221,42 +223,42 @@ func (ng *AlertNG) alertingTicker(grafanaCtx context.Context) error {
type alertDefinitionRegistry struct {
mu sync.Mutex
alertDefinitionInfo map[int64]alertDefinitionInfo
alertDefinitionInfo map[alertDefinitionKey]alertDefinitionInfo
}
// getOrCreateInfo returns the channel for the specific alert definition
// if it does not exists creates one and returns it
func (r *alertDefinitionRegistry) getOrCreateInfo(definitionID int64, definitionVersion int64) alertDefinitionInfo {
func (r *alertDefinitionRegistry) getOrCreateInfo(key alertDefinitionKey, definitionVersion int64) alertDefinitionInfo {
r.mu.Lock()
defer r.mu.Unlock()
info, ok := r.alertDefinitionInfo[definitionID]
info, ok := r.alertDefinitionInfo[key]
if !ok {
r.alertDefinitionInfo[definitionID] = alertDefinitionInfo{ch: make(chan *evalContext), version: definitionVersion}
return r.alertDefinitionInfo[definitionID]
r.alertDefinitionInfo[key] = alertDefinitionInfo{ch: make(chan *evalContext), version: definitionVersion}
return r.alertDefinitionInfo[key]
}
info.version = definitionVersion
r.alertDefinitionInfo[definitionID] = info
r.alertDefinitionInfo[key] = info
return info
}
func (r *alertDefinitionRegistry) exists(definitionID int64) bool {
func (r *alertDefinitionRegistry) exists(key alertDefinitionKey) bool {
r.mu.Lock()
defer r.mu.Unlock()
_, ok := r.alertDefinitionInfo[definitionID]
_, ok := r.alertDefinitionInfo[key]
return ok
}
func (r *alertDefinitionRegistry) del(definitionID int64) {
func (r *alertDefinitionRegistry) del(key alertDefinitionKey) {
r.mu.Lock()
defer r.mu.Unlock()
delete(r.alertDefinitionInfo, definitionID)
delete(r.alertDefinitionInfo, key)
}
func (r *alertDefinitionRegistry) iter() <-chan int64 {
c := make(chan int64)
func (r *alertDefinitionRegistry) iter() <-chan alertDefinitionKey {
c := make(chan alertDefinitionKey)
f := func() {
r.mu.Lock()
@ -272,10 +274,10 @@ func (r *alertDefinitionRegistry) iter() <-chan int64 {
return c
}
func (r *alertDefinitionRegistry) keyMap() map[int64]struct{} {
definitionsIDs := make(map[int64]struct{})
for definitionID := range r.iter() {
definitionsIDs[definitionID] = struct{}{}
func (r *alertDefinitionRegistry) keyMap() map[alertDefinitionKey]struct{} {
definitionsIDs := make(map[alertDefinitionKey]struct{})
for k := range r.iter() {
definitionsIDs[k] = struct{}{}
}
return definitionsIDs
}

53
pkg/services/ngalert/schedule_test.go
Unescape View File

@ -4,7 +4,6 @@ import (
"context"
"fmt"
"runtime"
"strconv"
"strings"
"testing"
"time"
@ -18,8 +17,8 @@ import (
)
type evalAppliedInfo struct {
alertDefID int64
now time.Time
alertDefKey alertDefinitionKey
now time.Time
}
func TestAlertingTicker(t *testing.T) {
@ -39,8 +38,8 @@ func TestAlertingTicker(t *testing.T) {
evalAppliedCh := make(chan evalAppliedInfo, len(alerts))
ng.schedule.evalApplied = func(alertDefID int64, now time.Time) {
evalAppliedCh <- evalAppliedInfo{alertDefID: alertDefID, now: now}
ng.schedule.evalApplied = func(alertDefKey alertDefinitionKey, now time.Time) {
evalAppliedCh <- evalAppliedInfo{alertDefKey: alertDefKey, now: now}
}
ctx := context.Background()
@ -50,7 +49,7 @@ func TestAlertingTicker(t *testing.T) {
}()
runtime.Gosched()
expectedAlertDefinitionsEvaluated := []int64{alerts[1].ID}
expectedAlertDefinitionsEvaluated := []alertDefinitionKey{alerts[1].getKey()}
t.Run(fmt.Sprintf("on 1st tick alert definitions: %s should be evaluated", concatenate(expectedAlertDefinitionsEvaluated)), func(t *testing.T) {
tick := advanceClock(t, mockedClock)
assertEvalRun(t, evalAppliedCh, tick, expectedAlertDefinitionsEvaluated...)
@ -59,42 +58,42 @@ func TestAlertingTicker(t *testing.T) {
// change alert definition interval to three seconds
var threeSecInterval int64 = 3
err := ng.updateAlertDefinition(&updateAlertDefinitionCommand{
ID: alerts[0].ID,
UID: alerts[0].UID,
IntervalSeconds: &threeSecInterval,
OrgID: alerts[0].OrgID,
})
require.NoError(t, err)
t.Logf("alert definition: %d interval reset to: %d", alerts[0].ID, threeSecInterval)
t.Logf("alert definition: %v interval reset to: %d", alerts[0].getKey(), threeSecInterval)
expectedAlertDefinitionsEvaluated = []int64{alerts[1].ID}
expectedAlertDefinitionsEvaluated = []alertDefinitionKey{alerts[1].getKey()}
t.Run(fmt.Sprintf("on 2nd tick alert definition: %s should be evaluated", concatenate(expectedAlertDefinitionsEvaluated)), func(t *testing.T) {
tick := advanceClock(t, mockedClock)
assertEvalRun(t, evalAppliedCh, tick, expectedAlertDefinitionsEvaluated...)
})
expectedAlertDefinitionsEvaluated = []int64{alerts[1].ID, alerts[0].ID}
expectedAlertDefinitionsEvaluated = []alertDefinitionKey{alerts[1].getKey(), alerts[0].getKey()}
t.Run(fmt.Sprintf("on 3rd tick alert definitions: %s should be evaluated", concatenate(expectedAlertDefinitionsEvaluated)), func(t *testing.T) {
tick := advanceClock(t, mockedClock)
assertEvalRun(t, evalAppliedCh, tick, expectedAlertDefinitionsEvaluated...)
})
expectedAlertDefinitionsEvaluated = []int64{alerts[1].ID}
expectedAlertDefinitionsEvaluated = []alertDefinitionKey{alerts[1].getKey()}
t.Run(fmt.Sprintf("on 4th tick alert definitions: %s should be evaluated", concatenate(expectedAlertDefinitionsEvaluated)), func(t *testing.T) {
tick := advanceClock(t, mockedClock)
assertEvalRun(t, evalAppliedCh, tick, expectedAlertDefinitionsEvaluated...)
})
err = ng.deleteAlertDefinitionByID(&deleteAlertDefinitionByIDCommand{ID: alerts[1].ID})
err = ng.deleteAlertDefinitionByUID(&deleteAlertDefinitionByUIDCommand{UID: alerts[1].UID, OrgID: alerts[1].OrgID})
require.NoError(t, err)
t.Logf("alert definition: %d deleted", alerts[1].ID)
t.Logf("alert definition: %v deleted", alerts[1].getKey())
expectedAlertDefinitionsEvaluated = []int64{}
expectedAlertDefinitionsEvaluated = []alertDefinitionKey{}
t.Run(fmt.Sprintf("on 5th tick alert definitions: %s should be evaluated", concatenate(expectedAlertDefinitionsEvaluated)), func(t *testing.T) {
tick := advanceClock(t, mockedClock)
assertEvalRun(t, evalAppliedCh, tick, expectedAlertDefinitionsEvaluated...)
})
expectedAlertDefinitionsEvaluated = []int64{alerts[0].ID}
expectedAlertDefinitionsEvaluated = []alertDefinitionKey{alerts[0].getKey()}
t.Run(fmt.Sprintf("on 6th tick alert definitions: %s should be evaluated", concatenate(expectedAlertDefinitionsEvaluated)), func(t *testing.T) {
tick := advanceClock(t, mockedClock)
assertEvalRun(t, evalAppliedCh, tick, expectedAlertDefinitionsEvaluated...)
@ -103,29 +102,29 @@ func TestAlertingTicker(t *testing.T) {
// create alert definition with one second interval
alerts = append(alerts, createTestAlertDefinition(t, ng, 1))
expectedAlertDefinitionsEvaluated = []int64{alerts[2].ID}
expectedAlertDefinitionsEvaluated = []alertDefinitionKey{alerts[2].getKey()}
t.Run(fmt.Sprintf("on 7th tick alert definitions: %s should be evaluated", concatenate(expectedAlertDefinitionsEvaluated)), func(t *testing.T) {
tick := advanceClock(t, mockedClock)
assertEvalRun(t, evalAppliedCh, tick, expectedAlertDefinitionsEvaluated...)
})
}
func assertEvalRun(t *testing.T, ch <-chan evalAppliedInfo, tick time.Time, ids ...int64) {
func assertEvalRun(t *testing.T, ch <-chan evalAppliedInfo, tick time.Time, keys ...alertDefinitionKey) {
timeout := time.After(time.Second)
expected := make(map[int64]struct{}, len(ids))
for _, id := range ids {
expected[id] = struct{}{}
expected := make(map[alertDefinitionKey]struct{}, len(keys))
for _, k := range keys {
expected[k] = struct{}{}
}
for {
select {
case info := <-ch:
_, ok := expected[info.alertDefID]
t.Logf("alert definition: %d evaluated at: %v", info.alertDefID, info.now)
_, ok := expected[info.alertDefKey]
t.Logf("alert definition: %v evaluated at: %v", info.alertDefKey, info.now)
assert.True(t, ok)
assert.Equal(t, tick, info.now)
delete(expected, info.alertDefID)
delete(expected, info.alertDefKey)
if len(expected) == 0 {
return
}
@ -144,10 +143,10 @@ func advanceClock(t *testing.T, mockedClock *clock.Mock) time.Time {
// t.Logf("Tick: %v", mockedClock.Now())
}
func concatenate(ids []int64) string {
s := make([]string, len(ids))
for _, id := range ids {
s = append(s, strconv.FormatInt(id, 10))
func concatenate(keys []alertDefinitionKey) string {
s := make([]string, len(keys))
for _, k := range keys {
s = append(s, k.String())
}
return fmt.Sprintf("[%s]", strings.TrimLeft(strings.Join(s, ","), ","))
}

Write Preview
Loading…
Cancel
Save