Alerting: Fix Annotation Creation when the alerting state changes (#42479)

* Fix Annotation creation
- Remove validation of panelID, now annotations are created irrespective on whether they're attached to a panel or not.
- Alwasy attach the annotation to an AlertID

* Fix annotation creation

* fix tests
This commit is contained in:
gotjosh 2021-12-01 11:04:54 +00:00 committed by GitHub
parent 55ce03bd0c
commit 357e9ed1ea
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 113 additions and 36 deletions

View File

@ -19,6 +19,7 @@ import (
"github.com/grafana/grafana/pkg/expr"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/services/annotations"
apimodels "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions"
"github.com/grafana/grafana/pkg/services/ngalert/eval"
"github.com/grafana/grafana/pkg/services/ngalert/metrics"
@ -576,6 +577,8 @@ func TestSchedule_ruleRoutine(t *testing.T) {
func setupScheduler(t *testing.T, rs store.RuleStore, is store.InstanceStore, acs store.AdminConfigurationStore, registry *prometheus.Registry) (*schedule, *clock.Mock) {
t.Helper()
fakeAnnoRepo := NewFakeAnnotationsRepo()
annotations.SetRepository(fakeAnnoRepo)
mockedClock := clock.NewMock()
logger := log.New("ngalert schedule test")
if registry == nil {

View File

@ -10,6 +10,8 @@ import (
"testing"
"time"
"github.com/grafana/grafana/pkg/services/annotations"
models2 "github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/ngalert/models"
"github.com/grafana/grafana/pkg/services/ngalert/store"
@ -379,3 +381,47 @@ func (am *FakeExternalAlertmanager) Handler() func(w http.ResponseWriter, r *htt
func (am *FakeExternalAlertmanager) Close() {
am.server.Close()
}
type FakeAnnotationsRepo struct {
mtx sync.Mutex
items []*annotations.Item
}
func NewFakeAnnotationsRepo() *FakeAnnotationsRepo {
return &FakeAnnotationsRepo{
items: make([]*annotations.Item, 0),
}
}
func (repo *FakeAnnotationsRepo) Len() int {
repo.mtx.Lock()
defer repo.mtx.Unlock()
return len(repo.items)
}
func (repo *FakeAnnotationsRepo) Delete(params *annotations.DeleteParams) error {
return nil
}
func (repo *FakeAnnotationsRepo) Save(item *annotations.Item) error {
repo.mtx.Lock()
defer repo.mtx.Unlock()
repo.items = append(repo.items, item)
return nil
}
func (repo *FakeAnnotationsRepo) Update(item *annotations.Item) error {
return nil
}
func (repo *FakeAnnotationsRepo) Find(query *annotations.ItemQuery) ([]*annotations.ItemDTO, error) {
annotations := []*annotations.ItemDTO{{Id: 1}}
return annotations, nil
}
func (repo *FakeAnnotationsRepo) FindTags(query *annotations.TagsQuery) (annotations.FindTagsResult, error) {
result := annotations.FindTagsResult{
Tags: []*annotations.TagsDTO{},
}
return result, nil
}

View File

@ -234,45 +234,46 @@ func translateInstanceState(state ngModels.InstanceStateType) eval.State {
}
func (st *Manager) createAlertAnnotation(ctx context.Context, new eval.State, alertRule *ngModels.AlertRule, result eval.Result, oldState eval.State) {
st.log.Debug("alert state changed creating annotation", "alertRuleUID", alertRule.UID, "newState", new.String())
dashUid, ok := alertRule.Annotations[ngModels.DashboardUIDAnnotation]
if !ok {
return
}
panelUid := alertRule.Annotations[ngModels.PanelIDAnnotation]
panelId, err := strconv.ParseInt(panelUid, 10, 64)
if err != nil {
st.log.Error("error parsing panelUID for alert annotation", "panelUID", panelUid, "alertRuleUID", alertRule.UID, "error", err.Error())
return
}
query := &models.GetDashboardQuery{
Uid: dashUid,
OrgId: alertRule.OrgID,
}
err = sqlstore.GetDashboard(ctx, query)
if err != nil {
st.log.Error("error getting dashboard for alert annotation", "dashboardUID", dashUid, "alertRuleUID", alertRule.UID, "error", err.Error())
return
}
st.log.Debug("alert state changed creating annotation", "alertRuleUID", alertRule.UID, "newState", new.String(), "oldState", oldState.String())
annotationText := fmt.Sprintf("%s {%s} - %s", alertRule.Title, result.Instance.String(), new.String())
item := &annotations.Item{
OrgId: alertRule.OrgID,
DashboardId: query.Result.Id,
PanelId: panelId,
PrevState: oldState.String(),
NewState: new.String(),
Text: annotationText,
Epoch: result.EvaluatedAt.UnixNano() / int64(time.Millisecond),
AlertId: alertRule.ID,
OrgId: alertRule.OrgID,
PrevState: oldState.String(),
NewState: new.String(),
Text: annotationText,
Epoch: result.EvaluatedAt.UnixNano() / int64(time.Millisecond),
}
dashUid, ok := alertRule.Annotations[ngModels.DashboardUIDAnnotation]
if ok {
panelUid := alertRule.Annotations[ngModels.PanelIDAnnotation]
panelId, err := strconv.ParseInt(panelUid, 10, 64)
if err != nil {
st.log.Error("error parsing panelUID for alert annotation", "panelUID", panelUid, "alertRuleUID", alertRule.UID, "error", err.Error())
return
}
query := &models.GetDashboardQuery{
Uid: dashUid,
OrgId: alertRule.OrgID,
}
err = sqlstore.GetDashboard(ctx, query)
if err != nil {
st.log.Error("error getting dashboard for alert annotation", "dashboardUID", dashUid, "alertRuleUID", alertRule.UID, "error", err.Error())
return
}
item.PanelId = panelId
item.DashboardId = query.Result.Id
}
annotationRepo := annotations.GetRepository()
if err = annotationRepo.Save(item); err != nil {
if err := annotationRepo.Save(item); err != nil {
st.log.Error("error saving alert annotation", "alertRuleUID", alertRule.UID, "error", err.Error())
return
}

View File

@ -7,6 +7,8 @@ import (
"testing"
"time"
"github.com/grafana/grafana/pkg/services/annotations"
"github.com/grafana/grafana-plugin-sdk-go/data"
"github.com/grafana/grafana/pkg/expr"
@ -33,10 +35,11 @@ func TestProcessEvalResults(t *testing.T) {
evaluationDuration := 10 * time.Millisecond
testCases := []struct {
desc string
alertRule *models.AlertRule
evalResults []eval.Results
expectedStates map[string]*state.State
desc string
alertRule *models.AlertRule
evalResults []eval.Results
expectedStates map[string]*state.State
expectedAnnotations int
}{
{
desc: "a cache entry is correctly created",
@ -112,6 +115,7 @@ func TestProcessEvalResults(t *testing.T) {
},
},
},
expectedAnnotations: 1,
expectedStates: map[string]*state.State{
`[["__alert_rule_namespace_uid__","test_namespace_uid"],["__alert_rule_uid__","test_alert_rule_uid"],["alertname","test_title"],["instance_label_1","test"],["label","test"]]`: {
AlertRuleUID: "test_alert_rule_uid",
@ -252,6 +256,7 @@ func TestProcessEvalResults(t *testing.T) {
},
},
},
expectedAnnotations: 1,
expectedStates: map[string]*state.State{
`[["__alert_rule_namespace_uid__","test_namespace_uid"],["__alert_rule_uid__","test_alert_rule_uid_2"],["alertname","test_title"],["instance_label","test"],["label","test"]]`: {
AlertRuleUID: "test_alert_rule_uid_2",
@ -323,6 +328,7 @@ func TestProcessEvalResults(t *testing.T) {
},
},
},
expectedAnnotations: 2,
expectedStates: map[string]*state.State{
`[["__alert_rule_namespace_uid__","test_namespace_uid"],["__alert_rule_uid__","test_alert_rule_uid_2"],["alertname","test_title"],["instance_label","test"],["label","test"]]`: {
AlertRuleUID: "test_alert_rule_uid_2",
@ -416,6 +422,7 @@ func TestProcessEvalResults(t *testing.T) {
},
},
},
expectedAnnotations: 3,
expectedStates: map[string]*state.State{
`[["__alert_rule_namespace_uid__","test_namespace_uid"],["__alert_rule_uid__","test_alert_rule_uid_2"],["alertname","test_title"],["instance_label","test"],["label","test"]]`: {
AlertRuleUID: "test_alert_rule_uid_2",
@ -506,6 +513,7 @@ func TestProcessEvalResults(t *testing.T) {
},
},
},
expectedAnnotations: 2,
expectedStates: map[string]*state.State{
`[["__alert_rule_namespace_uid__","test_namespace_uid"],["__alert_rule_uid__","test_alert_rule_uid_2"],["alertname","test_title"],["instance_label","test"],["label","test"]]`: {
AlertRuleUID: "test_alert_rule_uid_2",
@ -579,6 +587,7 @@ func TestProcessEvalResults(t *testing.T) {
},
},
},
expectedAnnotations: 1,
expectedStates: map[string]*state.State{
`[["__alert_rule_namespace_uid__","test_namespace_uid"],["__alert_rule_uid__","test_alert_rule_uid_2"],["alertname","test_title"],["instance_label","test"],["label","test"]]`: {
AlertRuleUID: "test_alert_rule_uid_2",
@ -642,6 +651,7 @@ func TestProcessEvalResults(t *testing.T) {
},
},
},
expectedAnnotations: 1,
expectedStates: map[string]*state.State{
`[["__alert_rule_namespace_uid__","test_namespace_uid"],["__alert_rule_uid__","test_alert_rule_uid_2"],["alertname","test_title"],["instance_label","test"],["label","test"]]`: {
AlertRuleUID: "test_alert_rule_uid_2",
@ -705,6 +715,7 @@ func TestProcessEvalResults(t *testing.T) {
},
},
},
expectedAnnotations: 1,
expectedStates: map[string]*state.State{
`[["__alert_rule_namespace_uid__","test_namespace_uid"],["__alert_rule_uid__","test_alert_rule_uid_2"],["alertname","test_title"],["instance_label","test"],["label","test"]]`: {
AlertRuleUID: "test_alert_rule_uid_2",
@ -768,6 +779,7 @@ func TestProcessEvalResults(t *testing.T) {
},
},
},
expectedAnnotations: 1,
expectedStates: map[string]*state.State{
`[["__alert_rule_namespace_uid__","test_namespace_uid"],["__alert_rule_uid__","test_alert_rule_uid_2"],["alertname","test_title"],["instance_label","test"],["label","test"]]`: {
AlertRuleUID: "test_alert_rule_uid_2",
@ -831,6 +843,7 @@ func TestProcessEvalResults(t *testing.T) {
},
},
},
expectedAnnotations: 1,
expectedStates: map[string]*state.State{
`[["__alert_rule_namespace_uid__","test_namespace_uid"],["__alert_rule_uid__","test_alert_rule_uid_2"],["alertname","test_title"],["label","test"]]`: {
AlertRuleUID: "test_alert_rule_uid_2",
@ -894,6 +907,7 @@ func TestProcessEvalResults(t *testing.T) {
},
},
},
expectedAnnotations: 1,
expectedStates: map[string]*state.State{
`[["__alert_rule_namespace_uid__","test_namespace_uid"],["__alert_rule_uid__","test_alert_rule_uid_2"],["alertname","test_title"],["label","test"]]`: {
AlertRuleUID: "test_alert_rule_uid_2",
@ -959,6 +973,7 @@ func TestProcessEvalResults(t *testing.T) {
},
},
},
expectedAnnotations: 1,
expectedStates: map[string]*state.State{
`[["__alert_rule_namespace_uid__","test_namespace_uid"],["__alert_rule_uid__","test_alert_rule_uid_2"],["alertname","test_title"],["instance_label","test"],["label","test"]]`: {
AlertRuleUID: "test_alert_rule_uid_2",
@ -1081,6 +1096,7 @@ func TestProcessEvalResults(t *testing.T) {
},
},
},
expectedAnnotations: 1,
expectedStates: map[string]*state.State{
`[["__alert_rule_namespace_uid__","test_namespace_uid"],["__alert_rule_uid__","test_alert_rule_uid_2"],["alertname","test_title"],["instance_label","test"],["label","test"]]`: {
AlertRuleUID: "test_alert_rule_uid_2",
@ -1145,6 +1161,7 @@ func TestProcessEvalResults(t *testing.T) {
},
},
},
expectedAnnotations: 1,
expectedStates: map[string]*state.State{
`[["__alert_rule_namespace_uid__","test_namespace_uid"],["__alert_rule_uid__","test_alert_rule_uid_2"],["alertname","test_title"],["instance_label","test"],["label","test"]]`: {
AlertRuleUID: "test_alert_rule_uid_2",
@ -1217,6 +1234,7 @@ func TestProcessEvalResults(t *testing.T) {
},
},
},
expectedAnnotations: 1,
expectedStates: map[string]*state.State{
`[["__alert_rule_namespace_uid__","test_namespace_uid"],["__alert_rule_uid__","test_alert_rule_uid_2"],["alertname","test_title"],["instance_label","test"],["label","test"]]`: {
AlertRuleUID: "test_alert_rule_uid_2",
@ -1302,6 +1320,7 @@ func TestProcessEvalResults(t *testing.T) {
},
},
},
expectedAnnotations: 2,
expectedStates: map[string]*state.State{
`[["__alert_rule_namespace_uid__","test_namespace_uid"],["__alert_rule_uid__","test_alert_rule_uid_2"],["alertname","test_title"],["instance_label","test"],["label","test"]]`: {
AlertRuleUID: "test_alert_rule_uid_2",
@ -1393,6 +1412,7 @@ func TestProcessEvalResults(t *testing.T) {
},
},
},
expectedAnnotations: 2,
expectedStates: map[string]*state.State{
`[["__alert_rule_namespace_uid__","test_namespace_uid"],["__alert_rule_uid__","test_alert_rule_uid_2"],["alertname","test_title"],["instance_label","test"],["label","test"]]`: {
AlertRuleUID: "test_alert_rule_uid_2",
@ -1491,6 +1511,9 @@ func TestProcessEvalResults(t *testing.T) {
for _, tc := range testCases {
st := state.NewManager(log.New("test_state_manager"), testMetrics.GetStateMetrics(), nil, nil, &schedule.FakeInstanceStore{})
t.Run(tc.desc, func(t *testing.T) {
fakeAnnoRepo := schedule.NewFakeAnnotationsRepo()
annotations.SetRepository(fakeAnnoRepo)
for _, res := range tc.evalResults {
_ = st.ProcessEvalResults(context.Background(), tc.alertRule, res)
}
@ -1503,6 +1526,10 @@ func TestProcessEvalResults(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, s, cachedState)
}
require.Eventuallyf(t, func() bool {
return tc.expectedAnnotations == fakeAnnoRepo.Len()
}, time.Second, 100*time.Millisecond, "only %d annotations are present", fakeAnnoRepo.Len())
})
}
}