diff --git a/CHANGELOG.md b/CHANGELOG.md index 41b07e4ad7a..30fa0cd0b94 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,18 +1,25 @@ # 5.3.0 (unreleased) +### New Major Features + +* **Alerting**: Notification reminders [#7330](https://github.com/grafana/grafana/issues/7330), thx [@jbaublitz](https://github.com/jbaublitz) * **Dashboard**: TV & Kiosk mode changes, new cycle view mode button in dashboard toolbar [#13025](https://github.com/grafana/grafana/pull/13025) * **OAuth**: Gitlab OAuth with support for filter by groups [#5623](https://github.com/grafana/grafana/issues/5623), thx [@BenoitKnecht](https://github.com/BenoitKnecht) -* **Dataproxy**: Pass configured/auth headers to a Datasource [#10971](https://github.com/grafana/grafana/issues/10971), thx [@mrsiano](https://github.com/mrsiano) -* **Cleanup**: Make temp file time to live configurable [#11607](https://github.com/grafana/grafana/issues/11607), thx [@xapon](https://github.com/xapon) + +### New Features + * **LDAP**: Define Grafana Admin permission in ldap group mappings [#2469](https://github.com/grafana/grafana/issues/2496), PR [#12622](https://github.com/grafana/grafana/issues/12622) -* **Cloudwatch**: CloudWatch GetMetricData support [#11487](https://github.com/grafana/grafana/issues/11487), thx [@mtanda](https://github.com/mtanda) -* **Configuration**: Allow auto-assigning users to specific organization (other than Main. Org) [#1823](https://github.com/grafana/grafana/issues/1823) [#12801](https://github.com/grafana/grafana/issues/12801), thx [@gzzo](https://github.com/gzzo) and [@ofosos](https://github.com/ofosos) -* **Profile**: List teams that the user is member of in current/active organization [#12476](https://github.com/grafana/grafana/issues/12476) * **LDAP**: Client certificates support [#12805](https://github.com/grafana/grafana/issues/12805), thx [@nyxi](https://github.com/nyxi) +* **Profile**: List teams that the user is member of in current/active organization [#12476](https://github.com/grafana/grafana/issues/12476) +* **Configuration**: Allow auto-assigning users to specific organization (other than Main. Org) [#1823](https://github.com/grafana/grafana/issues/1823) [#12801](https://github.com/grafana/grafana/issues/12801), thx [@gzzo](https://github.com/gzzo) and [@ofosos](https://github.com/ofosos) +* **Dataproxy**: Pass configured/auth headers to a Datasource [#10971](https://github.com/grafana/grafana/issues/10971), thx [@mrsiano](https://github.com/mrsiano) +* **Cloudwatch**: CloudWatch GetMetricData support [#11487](https://github.com/grafana/grafana/issues/11487), thx [@mtanda](https://github.com/mtanda) * **Postgres**: TimescaleDB support, e.g. use `time_bucket` for grouping by time when option enabled [#12680](https://github.com/grafana/grafana/pull/12680), thx [svenklemm](https://github.com/svenklemm) +* **Cleanup**: Make temp file time to live configurable [#11607](https://github.com/grafana/grafana/issues/11607), thx [@xapon](https://github.com/xapon) ### Minor +* **GrafanaCli**: Fixed issue with grafana-cli install plugin resulting in corrupt http response from source error. Fixes [#13079](https://github.com/grafana/grafana/issues/13079) * **Api**: Delete nonexistent datasource should return 404 [#12313](https://github.com/grafana/grafana/issues/12313), thx [@AustinWinstanley](https://github.com/AustinWinstanley) * **Dashboard**: Fix selecting current dashboard from search should not reload dashboard [#12248](https://github.com/grafana/grafana/issues/12248) * **Dashboard**: Use uid when linking to dashboards internally in a dashboard [#10705](https://github.com/grafana/grafana/issues/10705) @@ -42,7 +49,6 @@ * **Cloudwatch**: Add new Redshift metrics and dimensions [#12063](https://github.com/grafana/grafana/pulls/12063), thx [@A21z](https://github.com/A21z) * **Table**: Adjust header contrast for the light theme [#12668](https://github.com/grafana/grafana/issues/12668) * **Table**: Fix link color when using light theme and thresholds in use [#12766](https://github.com/grafana/grafana/issues/12766) -om/grafana/grafana/issues/12668) * **Table**: Fix for useless horizontal scrollbar for table panel [#9964](https://github.com/grafana/grafana/issues/9964) * **Table**: Make table sorting stable when null values exist [#12362](https://github.com/grafana/grafana/pull/12362), thx [@bz2](https://github.com/bz2) * **Elasticsearch**: For alerting/backend, support having index name to the right of pattern in index pattern [#12731](https://github.com/grafana/grafana/issues/12731) diff --git a/docs/sources/alerting/notifications.md b/docs/sources/alerting/notifications.md index 58046cafae4..a5b7f4264e0 100644 --- a/docs/sources/alerting/notifications.md +++ b/docs/sources/alerting/notifications.md @@ -16,12 +16,11 @@ weight = 2 When an alert changes state, it sends out notifications. Each alert rule can have multiple notifications. In order to add a notification to an alert rule you first need -to add and configure a `notification` channel (can be email, PagerDuty or other integration). This is done from the Notification Channels page. +to add and configure a `notification` channel (can be email, PagerDuty or other integration). +This is done from the Notification Channels page. ## Notification Channel Setup -{{< imgbox max-width="30%" img="/img/docs/v50/alerts_notifications_menu.png" caption="Alerting Notification Channels" >}} - On the Notification Channels page hit the `New Channel` button to go the page where you can configure and setup a new Notification Channel. @@ -30,7 +29,31 @@ sure it's setup correctly. ### Send on all alerts -When checked, this option will nofity for all alert rules - existing and new. +When checked, this option will notify for all alert rules - existing and new. + +### Send reminders + +> Only available in Grafana v5.3 and above. + +{{< docs-imagebox max-width="600px" img="/img/docs/v53/alerting_notification_reminders.png" class="docs-image--right" caption="Alerting notification reminders setup" >}} + +When this option is checked additional notifications (reminders) will be sent for triggered alerts. You can specify how often reminders +should be sent using number of seconds (s), minutes (m) or hours (h), for example `30s`, `3m`, `5m` or `1h` etc. + +**Important:** Alert reminders are sent after rules are evaluated. Therefore a reminder can never be sent more frequently than a configured [alert rule evaluation interval](/alerting/rules/#name-evaluation-interval). + +These examples show how often and when reminders are sent for a triggered alert. + +Alert rule evaluation interval | Send reminders every | Reminder sent every (after last alert notification) +---------- | ----------- | ----------- +`30s` | `15s` | ~30 seconds +`1m` | `5m` | ~5 minutes +`5m` | `15m` | ~15 minutes +`6m` | `20m` | ~24 minutes +`1h` | `15m` | ~1 hour +`1h` | `2h` | ~2 hours + +
## Supported Notification Types @@ -132,23 +155,23 @@ Once these two properties are set, you can send the alerts to Kafka for further ### All supported notifiers -Name | Type |Support images ------|------------ | ------ -Slack | `slack` | yes -Pagerduty | `pagerduty` | yes -Email | `email` | yes -Webhook | `webhook` | link -Kafka | `kafka` | no -Hipchat | `hipchat` | yes -VictorOps | `victorops` | yes -Sensu | `sensu` | yes -OpsGenie | `opsgenie` | yes -Threema | `threema` | yes -Pushover | `pushover` | no -Telegram | `telegram` | no -Line | `line` | no -Prometheus Alertmanager | `prometheus-alertmanager` | no -Microsoft Teams | `teams` | yes +Name | Type |Support images | Support reminders +-----|------------ | ------ | ------ | +Slack | `slack` | yes | yes +Pagerduty | `pagerduty` | yes | yes +Email | `email` | yes | yes +Webhook | `webhook` | link | yes +Kafka | `kafka` | no | yes +Hipchat | `hipchat` | yes | yes +VictorOps | `victorops` | yes | yes +Sensu | `sensu` | yes | yes +OpsGenie | `opsgenie` | yes | yes +Threema | `threema` | yes | yes +Pushover | `pushover` | no | yes +Telegram | `telegram` | no | yes +Line | `line` | no | yes +Microsoft Teams | `teams` | yes | yes +Prometheus Alertmanager | `prometheus-alertmanager` | no | no diff --git a/docs/sources/alerting/rules.md b/docs/sources/alerting/rules.md index fa7332e7145..488619055e2 100644 --- a/docs/sources/alerting/rules.md +++ b/docs/sources/alerting/rules.md @@ -88,6 +88,11 @@ So as you can see from the above scenario Grafana will not send out notification to fire if the rule already is in state `Alerting`. To improve support for queries that return multiple series we plan to track state **per series** in a future release. +> Starting with Grafana v5.3 you can configure reminders to be sent for triggered alerts. This will send additional notifications +> when an alert continues to fire. If other series (like server2 in the example above) also cause the alert rule to fire they will +> be included in the reminder notification. Depending on what notification channel you're using you may be able to take advantage +> of this feature for identifying new/existing series causing alert to fire. [Read more about notification reminders here](/alerting/notifications/#send-reminders). + ### No Data / Null values Below your conditions you can configure how the rule evaluation engine should handle queries that return no data or only null values. diff --git a/docs/sources/http_api/alerting.md b/docs/sources/http_api/alerting.md index 80b6e283be3..032fd508dd0 100644 --- a/docs/sources/http_api/alerting.md +++ b/docs/sources/http_api/alerting.md @@ -50,6 +50,7 @@ Authorization: Bearer eyJrIjoiT0tTcG1pUlY2RnVKZTFVaDFsNFZXdE9ZWmNrMkZYbk ```http HTTP/1.1 200 Content-Type: application/json + [ { "id": 1, @@ -86,6 +87,7 @@ Authorization: Bearer eyJrIjoiT0tTcG1pUlY2RnVKZTFVaDFsNFZXdE9ZWmNrMkZYbk ```http HTTP/1.1 200 Content-Type: application/json + { "id": 1, "dashboardId": 1, @@ -146,6 +148,7 @@ JSON Body Schema: ```http HTTP/1.1 200 Content-Type: application/json + { "alertId": 1, "state": "Paused", @@ -177,6 +180,7 @@ JSON Body Schema: ```http HTTP/1.1 200 Content-Type: application/json + { "state": "Paused", "message": "alert paused", @@ -204,14 +208,21 @@ Authorization: Bearer eyJrIjoiT0tTcG1pUlY2RnVKZTFVaDFsNFZXdE9ZWmNrMkZYbk HTTP/1.1 200 Content-Type: application/json -{ - "id": 1, - "name": "Team A", - "type": "email", - "isDefault": true, - "created": "2017-01-01 12:45", - "updated": "2017-01-01 12:45" -} +[ + { + "id": 1, + "name": "Team A", + "type": "email", + "isDefault": false, + "sendReminder": false, + "settings": { + "addresses": "carl@grafana.com;dev@grafana.com" + }, + "created": "2018-04-23T14:44:09+02:00", + "updated": "2018-08-20T15:47:49+02:00" + } +] + ``` ## Create alert notification @@ -232,6 +243,7 @@ Authorization: Bearer eyJrIjoiT0tTcG1pUlY2RnVKZTFVaDFsNFZXdE9ZWmNrMkZYbk "name": "new alert notification", //Required "type": "email", //Required "isDefault": false, + "sendReminder": false, "settings": { "addresses": "carl@grafana.com;dev@grafana.com" } @@ -243,14 +255,18 @@ Authorization: Bearer eyJrIjoiT0tTcG1pUlY2RnVKZTFVaDFsNFZXdE9ZWmNrMkZYbk ```http HTTP/1.1 200 Content-Type: application/json + { "id": 1, "name": "new alert notification", "type": "email", "isDefault": false, - "settings": { addresses: "carl@grafana.com;dev@grafana.com"} } - "created": "2017-01-01 12:34", - "updated": "2017-01-01 12:34" + "sendReminder": false, + "settings": { + "addresses": "carl@grafana.com;dev@grafana.com" + }, + "created": "2018-04-23T14:44:09+02:00", + "updated": "2018-08-20T15:47:49+02:00" } ``` @@ -271,6 +287,8 @@ Authorization: Bearer eyJrIjoiT0tTcG1pUlY2RnVKZTFVaDFsNFZXdE9ZWmNrMkZYbk "name": "new alert notification", //Required "type": "email", //Required "isDefault": false, + "sendReminder": true, + "frequency": "15m", "settings": { "addresses: "carl@grafana.com;dev@grafana.com" } @@ -282,12 +300,17 @@ Authorization: Bearer eyJrIjoiT0tTcG1pUlY2RnVKZTFVaDFsNFZXdE9ZWmNrMkZYbk ```http HTTP/1.1 200 Content-Type: application/json + { "id": 1, "name": "new alert notification", "type": "email", "isDefault": false, - "settings": { addresses: "carl@grafana.com;dev@grafana.com"} } + "sendReminder": true, + "frequency": "15m", + "settings": { + "addresses": "carl@grafana.com;dev@grafana.com" + }, "created": "2017-01-01 12:34", "updated": "2017-01-01 12:34" } @@ -311,6 +334,7 @@ Authorization: Bearer eyJrIjoiT0tTcG1pUlY2RnVKZTFVaDFsNFZXdE9ZWmNrMkZYbk ```http HTTP/1.1 200 Content-Type: application/json + { "message": "Notification deleted" } diff --git a/docs/sources/installation/upgrading.md b/docs/sources/installation/upgrading.md index c72bb4c0921..a476a38c3c5 100644 --- a/docs/sources/installation/upgrading.md +++ b/docs/sources/installation/upgrading.md @@ -109,3 +109,11 @@ positioning system when you load them in v5. Dashboards saved in v5 will not wor external panel plugins might need to be updated to work properly. For more details on the new panel positioning system, [click here]({{< relref "reference/dashboard.md#panel-size-position" >}}) + +## Upgrading to v5.2 + +One of the database migrations included in this release will update all annotation timestamps from second to millisecond precision. If you have a large amount of annotations the database migration may take a long time to complete which may cause problems if you use systemd to run Grafana. + +We've got one report where using systemd, PostgreSQL and a large amount of annotations (table size 1645mb) took 8-20 minutes for the database migration to complete. However, the grafana-server process was killed after 90 seconds by systemd. Any database migration queries in progress when systemd kills the grafana-server process continues to execute in database until finished. + +If you're using systemd and have a large amount of annotations consider temporary adjusting the systemd `TimeoutStartSec` setting to something high like `30m` before upgrading. diff --git a/pkg/api/alerting.go b/pkg/api/alerting.go index 60013fe2b10..a936d696207 100644 --- a/pkg/api/alerting.go +++ b/pkg/api/alerting.go @@ -192,14 +192,7 @@ func GetAlertNotifications(c *m.ReqContext) Response { result := make([]*dtos.AlertNotification, 0) for _, notification := range query.Result { - result = append(result, &dtos.AlertNotification{ - Id: notification.Id, - Name: notification.Name, - Type: notification.Type, - IsDefault: notification.IsDefault, - Created: notification.Created, - Updated: notification.Updated, - }) + result = append(result, dtos.NewAlertNotification(notification)) } return JSON(200, result) @@ -215,7 +208,7 @@ func GetAlertNotificationByID(c *m.ReqContext) Response { return Error(500, "Failed to get alert notifications", err) } - return JSON(200, query.Result) + return JSON(200, dtos.NewAlertNotification(query.Result)) } func CreateAlertNotification(c *m.ReqContext, cmd m.CreateAlertNotificationCommand) Response { @@ -225,7 +218,7 @@ func CreateAlertNotification(c *m.ReqContext, cmd m.CreateAlertNotificationComma return Error(500, "Failed to create alert notification", err) } - return JSON(200, cmd.Result) + return JSON(200, dtos.NewAlertNotification(cmd.Result)) } func UpdateAlertNotification(c *m.ReqContext, cmd m.UpdateAlertNotificationCommand) Response { @@ -235,7 +228,7 @@ func UpdateAlertNotification(c *m.ReqContext, cmd m.UpdateAlertNotificationComma return Error(500, "Failed to update alert notification", err) } - return JSON(200, cmd.Result) + return JSON(200, dtos.NewAlertNotification(cmd.Result)) } func DeleteAlertNotification(c *m.ReqContext) Response { diff --git a/pkg/api/dtos/alerting.go b/pkg/api/dtos/alerting.go index d30f2697f3f..697d0a35a08 100644 --- a/pkg/api/dtos/alerting.go +++ b/pkg/api/dtos/alerting.go @@ -1,35 +1,76 @@ package dtos import ( + "fmt" "time" "github.com/grafana/grafana/pkg/components/null" "github.com/grafana/grafana/pkg/components/simplejson" - m "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/models" ) type AlertRule struct { - Id int64 `json:"id"` - DashboardId int64 `json:"dashboardId"` - PanelId int64 `json:"panelId"` - Name string `json:"name"` - Message string `json:"message"` - State m.AlertStateType `json:"state"` - NewStateDate time.Time `json:"newStateDate"` - EvalDate time.Time `json:"evalDate"` - EvalData *simplejson.Json `json:"evalData"` - ExecutionError string `json:"executionError"` - Url string `json:"url"` - CanEdit bool `json:"canEdit"` + Id int64 `json:"id"` + DashboardId int64 `json:"dashboardId"` + PanelId int64 `json:"panelId"` + Name string `json:"name"` + Message string `json:"message"` + State models.AlertStateType `json:"state"` + NewStateDate time.Time `json:"newStateDate"` + EvalDate time.Time `json:"evalDate"` + EvalData *simplejson.Json `json:"evalData"` + ExecutionError string `json:"executionError"` + Url string `json:"url"` + CanEdit bool `json:"canEdit"` +} + +func formatShort(interval time.Duration) string { + var result string + + hours := interval / time.Hour + if hours > 0 { + result += fmt.Sprintf("%dh", hours) + } + + remaining := interval - (hours * time.Hour) + mins := remaining / time.Minute + if mins > 0 { + result += fmt.Sprintf("%dm", mins) + } + + remaining = remaining - (mins * time.Minute) + seconds := remaining / time.Second + if seconds > 0 { + result += fmt.Sprintf("%ds", seconds) + } + + return result +} + +func NewAlertNotification(notification *models.AlertNotification) *AlertNotification { + return &AlertNotification{ + Id: notification.Id, + Name: notification.Name, + Type: notification.Type, + IsDefault: notification.IsDefault, + Created: notification.Created, + Updated: notification.Updated, + Frequency: formatShort(notification.Frequency), + SendReminder: notification.SendReminder, + Settings: notification.Settings, + } } type AlertNotification struct { - Id int64 `json:"id"` - Name string `json:"name"` - Type string `json:"type"` - IsDefault bool `json:"isDefault"` - Created time.Time `json:"created"` - Updated time.Time `json:"updated"` + Id int64 `json:"id"` + Name string `json:"name"` + Type string `json:"type"` + IsDefault bool `json:"isDefault"` + SendReminder bool `json:"sendReminder"` + Frequency string `json:"frequency"` + Created time.Time `json:"created"` + Updated time.Time `json:"updated"` + Settings *simplejson.Json `json:"settings"` } type AlertTestCommand struct { @@ -39,7 +80,7 @@ type AlertTestCommand struct { type AlertTestResult struct { Firing bool `json:"firing"` - State m.AlertStateType `json:"state"` + State models.AlertStateType `json:"state"` ConditionEvals string `json:"conditionEvals"` TimeMs string `json:"timeMs"` Error string `json:"error,omitempty"` @@ -59,9 +100,11 @@ type EvalMatch struct { } type NotificationTestCommand struct { - Name string `json:"name"` - Type string `json:"type"` - Settings *simplejson.Json `json:"settings"` + Name string `json:"name"` + Type string `json:"type"` + SendReminder bool `json:"sendReminder"` + Frequency string `json:"frequency"` + Settings *simplejson.Json `json:"settings"` } type PauseAlertCommand struct { diff --git a/pkg/api/dtos/alerting_test.go b/pkg/api/dtos/alerting_test.go new file mode 100644 index 00000000000..c38f281be9c --- /dev/null +++ b/pkg/api/dtos/alerting_test.go @@ -0,0 +1,35 @@ +package dtos + +import ( + "testing" + "time" +) + +func TestFormatShort(t *testing.T) { + tcs := []struct { + interval time.Duration + expected string + }{ + {interval: time.Hour, expected: "1h"}, + {interval: time.Hour + time.Minute, expected: "1h1m"}, + {interval: (time.Hour * 10) + time.Minute, expected: "10h1m"}, + {interval: (time.Hour * 10) + (time.Minute * 10) + time.Second, expected: "10h10m1s"}, + {interval: time.Minute * 10, expected: "10m"}, + } + + for _, tc := range tcs { + got := formatShort(tc.interval) + if got != tc.expected { + t.Errorf("expected %s got %s interval: %v", tc.expected, got, tc.interval) + } + + parsed, err := time.ParseDuration(tc.expected) + if err != nil { + t.Fatalf("could not parse expected duration") + } + + if parsed != tc.interval { + t.Errorf("expectes the parsed duration to equal the interval. Got %v expected: %v", parsed, tc.interval) + } + } +} diff --git a/pkg/cmd/grafana-cli/commands/install_command.go b/pkg/cmd/grafana-cli/commands/install_command.go index 9bdb73a5858..5d4969e06af 100644 --- a/pkg/cmd/grafana-cli/commands/install_command.go +++ b/pkg/cmd/grafana-cli/commands/install_command.go @@ -152,7 +152,7 @@ func downloadFile(pluginName, filePath, url string) (err error) { return err } - r, err := zip.NewReader(bytes.NewReader(body), resp.ContentLength) + r, err := zip.NewReader(bytes.NewReader(body), int64(len(body))) if err != nil { return err } diff --git a/pkg/models/alert_notifications.go b/pkg/models/alert_notifications.go index 87b515f370c..42d33d5ed22 100644 --- a/pkg/models/alert_notifications.go +++ b/pkg/models/alert_notifications.go @@ -1,38 +1,50 @@ package models import ( + "errors" "time" "github.com/grafana/grafana/pkg/components/simplejson" ) +var ( + ErrNotificationFrequencyNotFound = errors.New("Notification frequency not specified") + ErrJournalingNotFound = errors.New("alert notification journaling not found") +) + type AlertNotification struct { - Id int64 `json:"id"` - OrgId int64 `json:"-"` - Name string `json:"name"` - Type string `json:"type"` - IsDefault bool `json:"isDefault"` - Settings *simplejson.Json `json:"settings"` - Created time.Time `json:"created"` - Updated time.Time `json:"updated"` + Id int64 `json:"id"` + OrgId int64 `json:"-"` + Name string `json:"name"` + Type string `json:"type"` + SendReminder bool `json:"sendReminder"` + Frequency time.Duration `json:"frequency"` + IsDefault bool `json:"isDefault"` + Settings *simplejson.Json `json:"settings"` + Created time.Time `json:"created"` + Updated time.Time `json:"updated"` } type CreateAlertNotificationCommand struct { - Name string `json:"name" binding:"Required"` - Type string `json:"type" binding:"Required"` - IsDefault bool `json:"isDefault"` - Settings *simplejson.Json `json:"settings"` + Name string `json:"name" binding:"Required"` + Type string `json:"type" binding:"Required"` + SendReminder bool `json:"sendReminder"` + Frequency string `json:"frequency"` + IsDefault bool `json:"isDefault"` + Settings *simplejson.Json `json:"settings"` OrgId int64 `json:"-"` Result *AlertNotification } type UpdateAlertNotificationCommand struct { - Id int64 `json:"id" binding:"Required"` - Name string `json:"name" binding:"Required"` - Type string `json:"type" binding:"Required"` - IsDefault bool `json:"isDefault"` - Settings *simplejson.Json `json:"settings" binding:"Required"` + Id int64 `json:"id" binding:"Required"` + Name string `json:"name" binding:"Required"` + Type string `json:"type" binding:"Required"` + SendReminder bool `json:"sendReminder"` + Frequency string `json:"frequency"` + IsDefault bool `json:"isDefault"` + Settings *simplejson.Json `json:"settings" binding:"Required"` OrgId int64 `json:"-"` Result *AlertNotification @@ -63,3 +75,34 @@ type GetAllAlertNotificationsQuery struct { Result []*AlertNotification } + +type AlertNotificationJournal struct { + Id int64 + OrgId int64 + AlertId int64 + NotifierId int64 + SentAt int64 + Success bool +} + +type RecordNotificationJournalCommand struct { + OrgId int64 + AlertId int64 + NotifierId int64 + SentAt int64 + Success bool +} + +type GetLatestNotificationQuery struct { + OrgId int64 + AlertId int64 + NotifierId int64 + + Result *AlertNotificationJournal +} + +type CleanNotificationJournalCommand struct { + OrgId int64 + AlertId int64 + NotifierId int64 +} diff --git a/pkg/services/alerting/interfaces.go b/pkg/services/alerting/interfaces.go index 18f969ba1b9..46f8b3c769c 100644 --- a/pkg/services/alerting/interfaces.go +++ b/pkg/services/alerting/interfaces.go @@ -1,6 +1,9 @@ package alerting -import "time" +import ( + "context" + "time" +) type EvalHandler interface { Eval(evalContext *EvalContext) @@ -15,10 +18,14 @@ type Notifier interface { Notify(evalContext *EvalContext) error GetType() string NeedsImage() bool - ShouldNotify(evalContext *EvalContext) bool + + // ShouldNotify checks this evaluation should send an alert notification + ShouldNotify(ctx context.Context, evalContext *EvalContext) bool GetNotifierId() int64 GetIsDefault() bool + GetSendReminder() bool + GetFrequency() time.Duration } type NotifierSlice []Notifier diff --git a/pkg/services/alerting/notifier.go b/pkg/services/alerting/notifier.go index f4e0a0f434f..7fbd956f4f9 100644 --- a/pkg/services/alerting/notifier.go +++ b/pkg/services/alerting/notifier.go @@ -1,10 +1,10 @@ package alerting import ( + "context" "errors" "fmt" - - "golang.org/x/sync/errgroup" + "time" "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/components/imguploader" @@ -58,17 +58,47 @@ func (n *notificationService) SendIfNeeded(context *EvalContext) error { return n.sendNotifications(context, notifiers) } -func (n *notificationService) sendNotifications(context *EvalContext, notifiers []Notifier) error { - g, _ := errgroup.WithContext(context.Ctx) - +func (n *notificationService) sendNotifications(evalContext *EvalContext, notifiers []Notifier) error { for _, notifier := range notifiers { - not := notifier //avoid updating scope variable in go routine - n.log.Debug("Sending notification", "type", not.GetType(), "id", not.GetNotifierId(), "isDefault", not.GetIsDefault()) - metrics.M_Alerting_Notification_Sent.WithLabelValues(not.GetType()).Inc() - g.Go(func() error { return not.Notify(context) }) + not := notifier + + err := bus.InTransaction(evalContext.Ctx, func(ctx context.Context) error { + n.log.Debug("trying to send notification", "id", not.GetNotifierId()) + + // Verify that we can send the notification again + // but this time within the same transaction. + if !evalContext.IsTestRun && !not.ShouldNotify(context.Background(), evalContext) { + return nil + } + + n.log.Debug("Sending notification", "type", not.GetType(), "id", not.GetNotifierId(), "isDefault", not.GetIsDefault()) + metrics.M_Alerting_Notification_Sent.WithLabelValues(not.GetType()).Inc() + + //send notification + success := not.Notify(evalContext) == nil + + if evalContext.IsTestRun { + return nil + } + + //write result to db. + cmd := &m.RecordNotificationJournalCommand{ + OrgId: evalContext.Rule.OrgId, + AlertId: evalContext.Rule.Id, + NotifierId: not.GetNotifierId(), + SentAt: time.Now().Unix(), + Success: success, + } + + return bus.DispatchCtx(ctx, cmd) + }) + + if err != nil { + n.log.Error("failed to send notification", "id", not.GetNotifierId()) + } } - return g.Wait() + return nil } func (n *notificationService) uploadImage(context *EvalContext) (err error) { @@ -110,7 +140,7 @@ func (n *notificationService) uploadImage(context *EvalContext) (err error) { return nil } -func (n *notificationService) getNeededNotifiers(orgId int64, notificationIds []int64, context *EvalContext) (NotifierSlice, error) { +func (n *notificationService) getNeededNotifiers(orgId int64, notificationIds []int64, evalContext *EvalContext) (NotifierSlice, error) { query := &m.GetAlertNotificationsToSendQuery{OrgId: orgId, Ids: notificationIds} if err := bus.Dispatch(query); err != nil { @@ -123,7 +153,8 @@ func (n *notificationService) getNeededNotifiers(orgId int64, notificationIds [] if err != nil { return nil, err } - if not.ShouldNotify(context) { + + if not.ShouldNotify(evalContext.Ctx, evalContext) { result = append(result, not) } } diff --git a/pkg/services/alerting/notifiers/alertmanager.go b/pkg/services/alerting/notifiers/alertmanager.go index d449167de13..9826dd1dffb 100644 --- a/pkg/services/alerting/notifiers/alertmanager.go +++ b/pkg/services/alerting/notifiers/alertmanager.go @@ -1,6 +1,7 @@ package notifiers import ( + "context" "time" "github.com/grafana/grafana/pkg/bus" @@ -33,7 +34,7 @@ func NewAlertmanagerNotifier(model *m.AlertNotification) (alerting.Notifier, err } return &AlertmanagerNotifier{ - NotifierBase: NewNotifierBase(model.Id, model.IsDefault, model.Name, model.Type, model.Settings), + NotifierBase: NewNotifierBase(model), Url: url, log: log.New("alerting.notifier.prometheus-alertmanager"), }, nil @@ -45,7 +46,7 @@ type AlertmanagerNotifier struct { log log.Logger } -func (this *AlertmanagerNotifier) ShouldNotify(evalContext *alerting.EvalContext) bool { +func (this *AlertmanagerNotifier) ShouldNotify(ctx context.Context, evalContext *alerting.EvalContext) bool { this.log.Debug("Should notify", "ruleId", evalContext.Rule.Id, "state", evalContext.Rule.State, "previousState", evalContext.PrevAlertState) // Do not notify when we become OK for the first time. diff --git a/pkg/services/alerting/notifiers/base.go b/pkg/services/alerting/notifiers/base.go index 868db3aec79..ca011356247 100644 --- a/pkg/services/alerting/notifiers/base.go +++ b/pkg/services/alerting/notifiers/base.go @@ -1,50 +1,94 @@ package notifiers import ( - "github.com/grafana/grafana/pkg/components/simplejson" - m "github.com/grafana/grafana/pkg/models" + "context" + "time" + + "github.com/grafana/grafana/pkg/bus" + "github.com/grafana/grafana/pkg/log" + "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/alerting" ) type NotifierBase struct { - Name string - Type string - Id int64 - IsDeault bool - UploadImage bool + Name string + Type string + Id int64 + IsDeault bool + UploadImage bool + SendReminder bool + Frequency time.Duration + + log log.Logger } -func NewNotifierBase(id int64, isDefault bool, name, notifierType string, model *simplejson.Json) NotifierBase { +func NewNotifierBase(model *models.AlertNotification) NotifierBase { uploadImage := true - value, exist := model.CheckGet("uploadImage") + value, exist := model.Settings.CheckGet("uploadImage") if exist { uploadImage = value.MustBool() } return NotifierBase{ - Id: id, - Name: name, - IsDeault: isDefault, - Type: notifierType, - UploadImage: uploadImage, + Id: model.Id, + Name: model.Name, + IsDeault: model.IsDefault, + Type: model.Type, + UploadImage: uploadImage, + SendReminder: model.SendReminder, + Frequency: model.Frequency, + log: log.New("alerting.notifier." + model.Name), } } -func defaultShouldNotify(context *alerting.EvalContext) bool { +func defaultShouldNotify(context *alerting.EvalContext, sendReminder bool, frequency time.Duration, lastNotify time.Time) bool { // Only notify on state change. - if context.PrevAlertState == context.Rule.State { + if context.PrevAlertState == context.Rule.State && !sendReminder { return false } + + // Do not notify if interval has not elapsed + if sendReminder && !lastNotify.IsZero() && lastNotify.Add(frequency).After(time.Now()) { + return false + } + + // Do not notify if alert state if OK or pending even on repeated notify + if sendReminder && (context.Rule.State == models.AlertStateOK || context.Rule.State == models.AlertStatePending) { + return false + } + // Do not notify when we become OK for the first time. - if (context.PrevAlertState == m.AlertStatePending) && (context.Rule.State == m.AlertStateOK) { + if (context.PrevAlertState == models.AlertStatePending) && (context.Rule.State == models.AlertStateOK) { return false } + return true } -func (n *NotifierBase) ShouldNotify(context *alerting.EvalContext) bool { - return defaultShouldNotify(context) +// ShouldNotify checks this evaluation should send an alert notification +func (n *NotifierBase) ShouldNotify(ctx context.Context, c *alerting.EvalContext) bool { + cmd := &models.GetLatestNotificationQuery{ + OrgId: c.Rule.OrgId, + AlertId: c.Rule.Id, + NotifierId: n.Id, + } + + err := bus.DispatchCtx(ctx, cmd) + if err == models.ErrJournalingNotFound { + return true + } + + if err != nil { + n.log.Error("Could not determine last time alert notifier fired", "Alert name", c.Rule.Name, "Error", err) + return false + } + + if !cmd.Result.Success { + return true + } + + return defaultShouldNotify(c, n.SendReminder, n.Frequency, time.Unix(cmd.Result.SentAt, 0)) } func (n *NotifierBase) GetType() string { @@ -62,3 +106,11 @@ func (n *NotifierBase) GetNotifierId() int64 { func (n *NotifierBase) GetIsDefault() bool { return n.IsDeault } + +func (n *NotifierBase) GetSendReminder() bool { + return n.SendReminder +} + +func (n *NotifierBase) GetFrequency() time.Duration { + return n.Frequency +} diff --git a/pkg/services/alerting/notifiers/base_test.go b/pkg/services/alerting/notifiers/base_test.go index b7142d144cc..57b82f32466 100644 --- a/pkg/services/alerting/notifiers/base_test.go +++ b/pkg/services/alerting/notifiers/base_test.go @@ -2,7 +2,11 @@ package notifiers import ( "context" + "errors" "testing" + "time" + + "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/components/simplejson" m "github.com/grafana/grafana/pkg/models" @@ -10,47 +14,129 @@ import ( . "github.com/smartystreets/goconvey/convey" ) -func TestBaseNotifier(t *testing.T) { - Convey("Base notifier tests", t, func() { - Convey("default constructor for notifiers", func() { - bJson := simplejson.New() +func TestShouldSendAlertNotification(t *testing.T) { + tcs := []struct { + name string + prevState m.AlertStateType + newState m.AlertStateType + expected bool + sendReminder bool + }{ + { + name: "pending -> ok should not trigger an notification", + newState: m.AlertStatePending, + prevState: m.AlertStateOK, + expected: false, + }, + { + name: "ok -> alerting should trigger an notification", + newState: m.AlertStateOK, + prevState: m.AlertStateAlerting, + expected: true, + }, + { + name: "ok -> pending should not trigger an notification", + newState: m.AlertStateOK, + prevState: m.AlertStatePending, + expected: false, + }, + { + name: "ok -> ok should not trigger an notification", + newState: m.AlertStateOK, + prevState: m.AlertStateOK, + expected: false, + sendReminder: false, + }, + { + name: "ok -> alerting should not trigger an notification", + newState: m.AlertStateOK, + prevState: m.AlertStateAlerting, + expected: true, + sendReminder: true, + }, + { + name: "ok -> ok with reminder should not trigger an notification", + newState: m.AlertStateOK, + prevState: m.AlertStateOK, + expected: false, + sendReminder: true, + }, + } - Convey("can parse false value", func() { - bJson.Set("uploadImage", false) - - base := NewNotifierBase(1, false, "name", "email", bJson) - So(base.UploadImage, ShouldBeFalse) - }) - - Convey("can parse true value", func() { - bJson.Set("uploadImage", true) - - base := NewNotifierBase(1, false, "name", "email", bJson) - So(base.UploadImage, ShouldBeTrue) - }) - - Convey("default value should be true for backwards compatibility", func() { - base := NewNotifierBase(1, false, "name", "email", bJson) - So(base.UploadImage, ShouldBeTrue) - }) + for _, tc := range tcs { + evalContext := alerting.NewEvalContext(context.TODO(), &alerting.Rule{ + State: tc.newState, }) - Convey("should notify", func() { - Convey("pending -> ok", func() { - context := alerting.NewEvalContext(context.TODO(), &alerting.Rule{ - State: m.AlertStatePending, - }) - context.Rule.State = m.AlertStateOK - So(defaultShouldNotify(context), ShouldBeFalse) + evalContext.Rule.State = tc.prevState + if defaultShouldNotify(evalContext, true, 0, time.Now()) != tc.expected { + t.Errorf("failed %s. expected %+v to return %v", tc.name, tc, tc.expected) + } + } +} + +func TestShouldNotifyWhenNoJournalingIsFound(t *testing.T) { + Convey("base notifier", t, func() { + bus.ClearBusHandlers() + + notifier := NewNotifierBase(&m.AlertNotification{ + Id: 1, + Name: "name", + Type: "email", + Settings: simplejson.New(), + }) + evalContext := alerting.NewEvalContext(context.TODO(), &alerting.Rule{}) + + Convey("should notify if no journaling is found", func() { + bus.AddHandlerCtx("", func(ctx context.Context, q *m.GetLatestNotificationQuery) error { + return m.ErrJournalingNotFound }) - Convey("ok -> alerting", func() { - context := alerting.NewEvalContext(context.TODO(), &alerting.Rule{ - State: m.AlertStateOK, - }) - context.Rule.State = m.AlertStateAlerting - So(defaultShouldNotify(context), ShouldBeTrue) + if !notifier.ShouldNotify(context.Background(), evalContext) { + t.Errorf("should send notifications when ErrJournalingNotFound is returned") + } + }) + + Convey("should not notify query returns error", func() { + bus.AddHandlerCtx("", func(ctx context.Context, q *m.GetLatestNotificationQuery) error { + return errors.New("some kind of error unknown error") }) + + if notifier.ShouldNotify(context.Background(), evalContext) { + t.Errorf("should not send notifications when query returns error") + } + }) + }) +} + +func TestBaseNotifier(t *testing.T) { + Convey("default constructor for notifiers", t, func() { + bJson := simplejson.New() + + model := &m.AlertNotification{ + Id: 1, + Name: "name", + Type: "email", + Settings: bJson, + } + + Convey("can parse false value", func() { + bJson.Set("uploadImage", false) + + base := NewNotifierBase(model) + So(base.UploadImage, ShouldBeFalse) + }) + + Convey("can parse true value", func() { + bJson.Set("uploadImage", true) + + base := NewNotifierBase(model) + So(base.UploadImage, ShouldBeTrue) + }) + + Convey("default value should be true for backwards compatibility", func() { + base := NewNotifierBase(model) + So(base.UploadImage, ShouldBeTrue) }) }) } diff --git a/pkg/services/alerting/notifiers/dingding.go b/pkg/services/alerting/notifiers/dingding.go index 14eacef5831..738e43af2d2 100644 --- a/pkg/services/alerting/notifiers/dingding.go +++ b/pkg/services/alerting/notifiers/dingding.go @@ -32,7 +32,7 @@ func NewDingDingNotifier(model *m.AlertNotification) (alerting.Notifier, error) } return &DingDingNotifier{ - NotifierBase: NewNotifierBase(model.Id, model.IsDefault, model.Name, model.Type, model.Settings), + NotifierBase: NewNotifierBase(model), Url: url, log: log.New("alerting.notifier.dingding"), }, nil diff --git a/pkg/services/alerting/notifiers/discord.go b/pkg/services/alerting/notifiers/discord.go index 3ffa7484870..57d9d438fa2 100644 --- a/pkg/services/alerting/notifiers/discord.go +++ b/pkg/services/alerting/notifiers/discord.go @@ -39,7 +39,7 @@ func NewDiscordNotifier(model *m.AlertNotification) (alerting.Notifier, error) { } return &DiscordNotifier{ - NotifierBase: NewNotifierBase(model.Id, model.IsDefault, model.Name, model.Type, model.Settings), + NotifierBase: NewNotifierBase(model), WebhookURL: url, log: log.New("alerting.notifier.discord"), }, nil diff --git a/pkg/services/alerting/notifiers/email.go b/pkg/services/alerting/notifiers/email.go index 562ffbe1269..17b88f7d97f 100644 --- a/pkg/services/alerting/notifiers/email.go +++ b/pkg/services/alerting/notifiers/email.go @@ -52,7 +52,7 @@ func NewEmailNotifier(model *m.AlertNotification) (alerting.Notifier, error) { }) return &EmailNotifier{ - NotifierBase: NewNotifierBase(model.Id, model.IsDefault, model.Name, model.Type, model.Settings), + NotifierBase: NewNotifierBase(model), Addresses: addresses, log: log.New("alerting.notifier.email"), }, nil diff --git a/pkg/services/alerting/notifiers/hipchat.go b/pkg/services/alerting/notifiers/hipchat.go index 58e1b7bd71e..1c284ec3d2b 100644 --- a/pkg/services/alerting/notifiers/hipchat.go +++ b/pkg/services/alerting/notifiers/hipchat.go @@ -59,7 +59,7 @@ func NewHipChatNotifier(model *models.AlertNotification) (alerting.Notifier, err roomId := model.Settings.Get("roomid").MustString() return &HipChatNotifier{ - NotifierBase: NewNotifierBase(model.Id, model.IsDefault, model.Name, model.Type, model.Settings), + NotifierBase: NewNotifierBase(model), Url: url, ApiKey: apikey, RoomId: roomId, diff --git a/pkg/services/alerting/notifiers/kafka.go b/pkg/services/alerting/notifiers/kafka.go index 92f6489106b..d8d19fc5dae 100644 --- a/pkg/services/alerting/notifiers/kafka.go +++ b/pkg/services/alerting/notifiers/kafka.go @@ -43,7 +43,7 @@ func NewKafkaNotifier(model *m.AlertNotification) (alerting.Notifier, error) { } return &KafkaNotifier{ - NotifierBase: NewNotifierBase(model.Id, model.IsDefault, model.Name, model.Type, model.Settings), + NotifierBase: NewNotifierBase(model), Endpoint: endpoint, Topic: topic, log: log.New("alerting.notifier.kafka"), diff --git a/pkg/services/alerting/notifiers/line.go b/pkg/services/alerting/notifiers/line.go index 4814662f3a9..9e3888b8f95 100644 --- a/pkg/services/alerting/notifiers/line.go +++ b/pkg/services/alerting/notifiers/line.go @@ -39,7 +39,7 @@ func NewLINENotifier(model *m.AlertNotification) (alerting.Notifier, error) { } return &LineNotifier{ - NotifierBase: NewNotifierBase(model.Id, model.IsDefault, model.Name, model.Type, model.Settings), + NotifierBase: NewNotifierBase(model), Token: token, log: log.New("alerting.notifier.line"), }, nil diff --git a/pkg/services/alerting/notifiers/opsgenie.go b/pkg/services/alerting/notifiers/opsgenie.go index f0f5142cf05..84148a0d99c 100644 --- a/pkg/services/alerting/notifiers/opsgenie.go +++ b/pkg/services/alerting/notifiers/opsgenie.go @@ -56,7 +56,7 @@ func NewOpsGenieNotifier(model *m.AlertNotification) (alerting.Notifier, error) } return &OpsGenieNotifier{ - NotifierBase: NewNotifierBase(model.Id, model.IsDefault, model.Name, model.Type, model.Settings), + NotifierBase: NewNotifierBase(model), ApiKey: apiKey, ApiUrl: apiUrl, AutoClose: autoClose, diff --git a/pkg/services/alerting/notifiers/pagerduty.go b/pkg/services/alerting/notifiers/pagerduty.go index 02219b2203d..bf85466388f 100644 --- a/pkg/services/alerting/notifiers/pagerduty.go +++ b/pkg/services/alerting/notifiers/pagerduty.go @@ -51,7 +51,7 @@ func NewPagerdutyNotifier(model *m.AlertNotification) (alerting.Notifier, error) } return &PagerdutyNotifier{ - NotifierBase: NewNotifierBase(model.Id, model.IsDefault, model.Name, model.Type, model.Settings), + NotifierBase: NewNotifierBase(model), Key: key, AutoResolve: autoResolve, log: log.New("alerting.notifier.pagerduty"), diff --git a/pkg/services/alerting/notifiers/pushover.go b/pkg/services/alerting/notifiers/pushover.go index cbe9e16801a..55dc02c5f4a 100644 --- a/pkg/services/alerting/notifiers/pushover.go +++ b/pkg/services/alerting/notifiers/pushover.go @@ -99,7 +99,7 @@ func NewPushoverNotifier(model *m.AlertNotification) (alerting.Notifier, error) return nil, alerting.ValidationError{Reason: "API token not given"} } return &PushoverNotifier{ - NotifierBase: NewNotifierBase(model.Id, model.IsDefault, model.Name, model.Type, model.Settings), + NotifierBase: NewNotifierBase(model), UserKey: userKey, ApiToken: apiToken, Priority: priority, diff --git a/pkg/services/alerting/notifiers/sensu.go b/pkg/services/alerting/notifiers/sensu.go index 9f77801d458..21d5d3d9d9e 100644 --- a/pkg/services/alerting/notifiers/sensu.go +++ b/pkg/services/alerting/notifiers/sensu.go @@ -51,7 +51,7 @@ func NewSensuNotifier(model *m.AlertNotification) (alerting.Notifier, error) { } return &SensuNotifier{ - NotifierBase: NewNotifierBase(model.Id, model.IsDefault, model.Name, model.Type, model.Settings), + NotifierBase: NewNotifierBase(model), Url: url, User: model.Settings.Get("username").MustString(), Source: model.Settings.Get("source").MustString(), diff --git a/pkg/services/alerting/notifiers/slack.go b/pkg/services/alerting/notifiers/slack.go index c1dadba414d..374b49ea957 100644 --- a/pkg/services/alerting/notifiers/slack.go +++ b/pkg/services/alerting/notifiers/slack.go @@ -78,7 +78,7 @@ func NewSlackNotifier(model *m.AlertNotification) (alerting.Notifier, error) { uploadImage := model.Settings.Get("uploadImage").MustBool(true) return &SlackNotifier{ - NotifierBase: NewNotifierBase(model.Id, model.IsDefault, model.Name, model.Type, model.Settings), + NotifierBase: NewNotifierBase(model), Url: url, Recipient: recipient, Mention: mention, diff --git a/pkg/services/alerting/notifiers/teams.go b/pkg/services/alerting/notifiers/teams.go index 4e34e16ab51..09bcf600533 100644 --- a/pkg/services/alerting/notifiers/teams.go +++ b/pkg/services/alerting/notifiers/teams.go @@ -33,7 +33,7 @@ func NewTeamsNotifier(model *m.AlertNotification) (alerting.Notifier, error) { } return &TeamsNotifier{ - NotifierBase: NewNotifierBase(model.Id, model.IsDefault, model.Name, model.Type, model.Settings), + NotifierBase: NewNotifierBase(model), Url: url, log: log.New("alerting.notifier.teams"), }, nil diff --git a/pkg/services/alerting/notifiers/telegram.go b/pkg/services/alerting/notifiers/telegram.go index ca24c996914..b03f7ca38c5 100644 --- a/pkg/services/alerting/notifiers/telegram.go +++ b/pkg/services/alerting/notifiers/telegram.go @@ -78,7 +78,7 @@ func NewTelegramNotifier(model *m.AlertNotification) (alerting.Notifier, error) } return &TelegramNotifier{ - NotifierBase: NewNotifierBase(model.Id, model.IsDefault, model.Name, model.Type, model.Settings), + NotifierBase: NewNotifierBase(model), BotToken: botToken, ChatID: chatId, UploadImage: uploadImage, diff --git a/pkg/services/alerting/notifiers/threema.go b/pkg/services/alerting/notifiers/threema.go index e4ffffc9108..28a62fade17 100644 --- a/pkg/services/alerting/notifiers/threema.go +++ b/pkg/services/alerting/notifiers/threema.go @@ -106,7 +106,7 @@ func NewThreemaNotifier(model *m.AlertNotification) (alerting.Notifier, error) { } return &ThreemaNotifier{ - NotifierBase: NewNotifierBase(model.Id, model.IsDefault, model.Name, model.Type, model.Settings), + NotifierBase: NewNotifierBase(model), GatewayID: gatewayID, RecipientID: recipientID, APISecret: apiSecret, diff --git a/pkg/services/alerting/notifiers/victorops.go b/pkg/services/alerting/notifiers/victorops.go index a753ca3cbf6..3093aec9957 100644 --- a/pkg/services/alerting/notifiers/victorops.go +++ b/pkg/services/alerting/notifiers/victorops.go @@ -51,7 +51,7 @@ func NewVictoropsNotifier(model *models.AlertNotification) (alerting.Notifier, e } return &VictoropsNotifier{ - NotifierBase: NewNotifierBase(model.Id, model.IsDefault, model.Name, model.Type, model.Settings), + NotifierBase: NewNotifierBase(model), URL: url, AutoResolve: autoResolve, log: log.New("alerting.notifier.victorops"), diff --git a/pkg/services/alerting/notifiers/webhook.go b/pkg/services/alerting/notifiers/webhook.go index 4c97ed2b75e..4045e496af9 100644 --- a/pkg/services/alerting/notifiers/webhook.go +++ b/pkg/services/alerting/notifiers/webhook.go @@ -47,7 +47,7 @@ func NewWebHookNotifier(model *m.AlertNotification) (alerting.Notifier, error) { } return &WebhookNotifier{ - NotifierBase: NewNotifierBase(model.Id, model.IsDefault, model.Name, model.Type, model.Settings), + NotifierBase: NewNotifierBase(model), Url: url, User: model.Settings.Get("username").MustString(), Password: model.Settings.Get("password").MustString(), diff --git a/pkg/services/alerting/result_handler.go b/pkg/services/alerting/result_handler.go index c57b28c7c3e..363d06d1132 100644 --- a/pkg/services/alerting/result_handler.go +++ b/pkg/services/alerting/result_handler.go @@ -88,6 +88,18 @@ func (handler *DefaultResultHandler) Handle(evalContext *EvalContext) error { } } + if evalContext.Rule.State == m.AlertStateOK && evalContext.PrevAlertState != m.AlertStateOK { + for _, notifierId := range evalContext.Rule.Notifications { + cmd := &m.CleanNotificationJournalCommand{ + AlertId: evalContext.Rule.Id, + NotifierId: notifierId, + OrgId: evalContext.Rule.OrgId, + } + if err := bus.DispatchCtx(evalContext.Ctx, cmd); err != nil { + handler.log.Error("Failed to clean up old notification records", "notifier", notifierId, "alert", evalContext.Rule.Id, "Error", err) + } + } + } handler.notifier.SendIfNeeded(evalContext) return nil diff --git a/pkg/services/sqlstore/alert_notification.go b/pkg/services/sqlstore/alert_notification.go index 651241f7714..8fb1e2212a9 100644 --- a/pkg/services/sqlstore/alert_notification.go +++ b/pkg/services/sqlstore/alert_notification.go @@ -2,6 +2,7 @@ package sqlstore import ( "bytes" + "context" "fmt" "strings" "time" @@ -17,6 +18,9 @@ func init() { bus.AddHandler("sql", DeleteAlertNotification) bus.AddHandler("sql", GetAlertNotificationsToSend) bus.AddHandler("sql", GetAllAlertNotifications) + bus.AddHandlerCtx("sql", RecordNotificationJournal) + bus.AddHandlerCtx("sql", GetLatestNotification) + bus.AddHandlerCtx("sql", CleanNotificationJournal) } func DeleteAlertNotification(cmd *m.DeleteAlertNotificationCommand) error { @@ -53,7 +57,9 @@ func GetAlertNotificationsToSend(query *m.GetAlertNotificationsToSendQuery) erro alert_notification.created, alert_notification.updated, alert_notification.settings, - alert_notification.is_default + alert_notification.is_default, + alert_notification.send_reminder, + alert_notification.frequency FROM alert_notification `) @@ -91,7 +97,9 @@ func getAlertNotificationInternal(query *m.GetAlertNotificationsQuery, sess *DBS alert_notification.created, alert_notification.updated, alert_notification.settings, - alert_notification.is_default + alert_notification.is_default, + alert_notification.send_reminder, + alert_notification.frequency FROM alert_notification `) @@ -137,17 +145,31 @@ func CreateAlertNotificationCommand(cmd *m.CreateAlertNotificationCommand) error return fmt.Errorf("Alert notification name %s already exists", cmd.Name) } - alertNotification := &m.AlertNotification{ - OrgId: cmd.OrgId, - Name: cmd.Name, - Type: cmd.Type, - Settings: cmd.Settings, - Created: time.Now(), - Updated: time.Now(), - IsDefault: cmd.IsDefault, + var frequency time.Duration + if cmd.SendReminder { + if cmd.Frequency == "" { + return m.ErrNotificationFrequencyNotFound + } + + frequency, err = time.ParseDuration(cmd.Frequency) + if err != nil { + return err + } } - if _, err = sess.Insert(alertNotification); err != nil { + alertNotification := &m.AlertNotification{ + OrgId: cmd.OrgId, + Name: cmd.Name, + Type: cmd.Type, + Settings: cmd.Settings, + SendReminder: cmd.SendReminder, + Frequency: frequency, + Created: time.Now(), + Updated: time.Now(), + IsDefault: cmd.IsDefault, + } + + if _, err = sess.MustCols("send_reminder").Insert(alertNotification); err != nil { return err } @@ -179,16 +201,77 @@ func UpdateAlertNotification(cmd *m.UpdateAlertNotificationCommand) error { current.Name = cmd.Name current.Type = cmd.Type current.IsDefault = cmd.IsDefault + current.SendReminder = cmd.SendReminder - sess.UseBool("is_default") + if current.SendReminder { + if cmd.Frequency == "" { + return m.ErrNotificationFrequencyNotFound + } + + frequency, err := time.ParseDuration(cmd.Frequency) + if err != nil { + return err + } + + current.Frequency = frequency + } + + sess.UseBool("is_default", "send_reminder") if affected, err := sess.ID(cmd.Id).Update(current); err != nil { return err } else if affected == 0 { - return fmt.Errorf("Could not find alert notification") + return fmt.Errorf("Could not update alert notification") } cmd.Result = ¤t return nil }) } + +func RecordNotificationJournal(ctx context.Context, cmd *m.RecordNotificationJournalCommand) error { + return inTransactionCtx(ctx, func(sess *DBSession) error { + journalEntry := &m.AlertNotificationJournal{ + OrgId: cmd.OrgId, + AlertId: cmd.AlertId, + NotifierId: cmd.NotifierId, + SentAt: cmd.SentAt, + Success: cmd.Success, + } + + if _, err := sess.Insert(journalEntry); err != nil { + return err + } + + return nil + }) +} + +func GetLatestNotification(ctx context.Context, cmd *m.GetLatestNotificationQuery) error { + return inTransactionCtx(ctx, func(sess *DBSession) error { + nj := &m.AlertNotificationJournal{} + + _, err := sess.Desc("alert_notification_journal.sent_at"). + Limit(1). + Where("alert_notification_journal.org_id = ? AND alert_notification_journal.alert_id = ? AND alert_notification_journal.notifier_id = ?", cmd.OrgId, cmd.AlertId, cmd.NotifierId).Get(nj) + + if err != nil { + return err + } + + if nj.AlertId == 0 && nj.Id == 0 && nj.NotifierId == 0 && nj.OrgId == 0 { + return m.ErrJournalingNotFound + } + + cmd.Result = nj + return nil + }) +} + +func CleanNotificationJournal(ctx context.Context, cmd *m.CleanNotificationJournalCommand) error { + return inTransactionCtx(ctx, func(sess *DBSession) error { + sql := "DELETE FROM alert_notification_journal WHERE alert_notification_journal.org_id = ? AND alert_notification_journal.alert_id = ? AND alert_notification_journal.notifier_id = ?" + _, err := sess.Exec(sql, cmd.OrgId, cmd.AlertId, cmd.NotifierId) + return err + }) +} diff --git a/pkg/services/sqlstore/alert_notification_test.go b/pkg/services/sqlstore/alert_notification_test.go index 2dbf9de5ca8..83fb42db9bb 100644 --- a/pkg/services/sqlstore/alert_notification_test.go +++ b/pkg/services/sqlstore/alert_notification_test.go @@ -1,7 +1,9 @@ package sqlstore import ( + "context" "testing" + "time" "github.com/grafana/grafana/pkg/components/simplejson" m "github.com/grafana/grafana/pkg/models" @@ -11,7 +13,48 @@ import ( func TestAlertNotificationSQLAccess(t *testing.T) { Convey("Testing Alert notification sql access", t, func() { InitTestDB(t) - var err error + + Convey("Alert notification journal", func() { + var alertId int64 = 5 + var orgId int64 = 5 + var notifierId int64 = 5 + + Convey("Getting last journal should raise error if no one exists", func() { + query := &m.GetLatestNotificationQuery{AlertId: alertId, OrgId: orgId, NotifierId: notifierId} + err := GetLatestNotification(context.Background(), query) + So(err, ShouldEqual, m.ErrJournalingNotFound) + + Convey("shoulbe be able to record two journaling events", func() { + createCmd := &m.RecordNotificationJournalCommand{AlertId: alertId, NotifierId: notifierId, OrgId: orgId, Success: true, SentAt: 1} + + err := RecordNotificationJournal(context.Background(), createCmd) + So(err, ShouldBeNil) + + createCmd.SentAt += 1000 //increase epoch + + err = RecordNotificationJournal(context.Background(), createCmd) + So(err, ShouldBeNil) + + Convey("get last journaling event", func() { + err := GetLatestNotification(context.Background(), query) + So(err, ShouldBeNil) + So(query.Result.SentAt, ShouldEqual, 1001) + + Convey("be able to clear all journaling for an notifier", func() { + cmd := &m.CleanNotificationJournalCommand{AlertId: alertId, NotifierId: notifierId, OrgId: orgId} + err := CleanNotificationJournal(context.Background(), cmd) + So(err, ShouldBeNil) + + Convey("querying for last junaling should raise error", func() { + query := &m.GetLatestNotificationQuery{AlertId: alertId, OrgId: orgId, NotifierId: notifierId} + err := GetLatestNotification(context.Background(), query) + So(err, ShouldEqual, m.ErrJournalingNotFound) + }) + }) + }) + }) + }) + }) Convey("Alert notifications should be empty", func() { cmd := &m.GetAlertNotificationsQuery{ @@ -24,19 +67,75 @@ func TestAlertNotificationSQLAccess(t *testing.T) { So(cmd.Result, ShouldBeNil) }) - Convey("Can save Alert Notification", func() { + Convey("Cannot save alert notifier with send reminder = true", func() { cmd := &m.CreateAlertNotificationCommand{ - Name: "ops", - Type: "email", - OrgId: 1, - Settings: simplejson.New(), + Name: "ops", + Type: "email", + OrgId: 1, + SendReminder: true, + Settings: simplejson.New(), } - err = CreateAlertNotificationCommand(cmd) + Convey("and missing frequency", func() { + err := CreateAlertNotificationCommand(cmd) + So(err, ShouldEqual, m.ErrNotificationFrequencyNotFound) + }) + + Convey("invalid frequency", func() { + cmd.Frequency = "invalid duration" + + err := CreateAlertNotificationCommand(cmd) + So(err.Error(), ShouldEqual, "time: invalid duration invalid duration") + }) + }) + + Convey("Cannot update alert notifier with send reminder = false", func() { + cmd := &m.CreateAlertNotificationCommand{ + Name: "ops update", + Type: "email", + OrgId: 1, + SendReminder: false, + Settings: simplejson.New(), + } + + err := CreateAlertNotificationCommand(cmd) + So(err, ShouldBeNil) + + updateCmd := &m.UpdateAlertNotificationCommand{ + Id: cmd.Result.Id, + SendReminder: true, + } + + Convey("and missing frequency", func() { + err := UpdateAlertNotification(updateCmd) + So(err, ShouldEqual, m.ErrNotificationFrequencyNotFound) + }) + + Convey("invalid frequency", func() { + updateCmd.Frequency = "invalid duration" + + err := UpdateAlertNotification(updateCmd) + So(err, ShouldNotBeNil) + So(err.Error(), ShouldEqual, "time: invalid duration invalid duration") + }) + }) + + Convey("Can save Alert Notification", func() { + cmd := &m.CreateAlertNotificationCommand{ + Name: "ops", + Type: "email", + OrgId: 1, + SendReminder: true, + Frequency: "10s", + Settings: simplejson.New(), + } + + err := CreateAlertNotificationCommand(cmd) So(err, ShouldBeNil) So(cmd.Result.Id, ShouldNotEqual, 0) So(cmd.Result.OrgId, ShouldNotEqual, 0) So(cmd.Result.Type, ShouldEqual, "email") + So(cmd.Result.Frequency, ShouldEqual, 10*time.Second) Convey("Cannot save Alert Notification with the same name", func() { err = CreateAlertNotificationCommand(cmd) @@ -45,25 +144,42 @@ func TestAlertNotificationSQLAccess(t *testing.T) { Convey("Can update alert notification", func() { newCmd := &m.UpdateAlertNotificationCommand{ - Name: "NewName", - Type: "webhook", - OrgId: cmd.Result.OrgId, - Settings: simplejson.New(), - Id: cmd.Result.Id, + Name: "NewName", + Type: "webhook", + OrgId: cmd.Result.OrgId, + SendReminder: true, + Frequency: "60s", + Settings: simplejson.New(), + Id: cmd.Result.Id, } err := UpdateAlertNotification(newCmd) So(err, ShouldBeNil) So(newCmd.Result.Name, ShouldEqual, "NewName") + So(newCmd.Result.Frequency, ShouldEqual, 60*time.Second) + }) + + Convey("Can update alert notification to disable sending of reminders", func() { + newCmd := &m.UpdateAlertNotificationCommand{ + Name: "NewName", + Type: "webhook", + OrgId: cmd.Result.OrgId, + SendReminder: false, + Settings: simplejson.New(), + Id: cmd.Result.Id, + } + err := UpdateAlertNotification(newCmd) + So(err, ShouldBeNil) + So(newCmd.Result.SendReminder, ShouldBeFalse) }) }) Convey("Can search using an array of ids", func() { - cmd1 := m.CreateAlertNotificationCommand{Name: "nagios", Type: "webhook", OrgId: 1, Settings: simplejson.New()} - cmd2 := m.CreateAlertNotificationCommand{Name: "slack", Type: "webhook", OrgId: 1, Settings: simplejson.New()} - cmd3 := m.CreateAlertNotificationCommand{Name: "ops2", Type: "email", OrgId: 1, Settings: simplejson.New()} - cmd4 := m.CreateAlertNotificationCommand{IsDefault: true, Name: "default", Type: "email", OrgId: 1, Settings: simplejson.New()} + cmd1 := m.CreateAlertNotificationCommand{Name: "nagios", Type: "webhook", OrgId: 1, SendReminder: true, Frequency: "10s", Settings: simplejson.New()} + cmd2 := m.CreateAlertNotificationCommand{Name: "slack", Type: "webhook", OrgId: 1, SendReminder: true, Frequency: "10s", Settings: simplejson.New()} + cmd3 := m.CreateAlertNotificationCommand{Name: "ops2", Type: "email", OrgId: 1, SendReminder: true, Frequency: "10s", Settings: simplejson.New()} + cmd4 := m.CreateAlertNotificationCommand{IsDefault: true, Name: "default", Type: "email", OrgId: 1, SendReminder: true, Frequency: "10s", Settings: simplejson.New()} - otherOrg := m.CreateAlertNotificationCommand{Name: "default", Type: "email", OrgId: 2, Settings: simplejson.New()} + otherOrg := m.CreateAlertNotificationCommand{Name: "default", Type: "email", OrgId: 2, SendReminder: true, Frequency: "10s", Settings: simplejson.New()} So(CreateAlertNotificationCommand(&cmd1), ShouldBeNil) So(CreateAlertNotificationCommand(&cmd2), ShouldBeNil) diff --git a/pkg/services/sqlstore/migrations/alert_mig.go b/pkg/services/sqlstore/migrations/alert_mig.go index 2a364d5f464..e27e64c6124 100644 --- a/pkg/services/sqlstore/migrations/alert_mig.go +++ b/pkg/services/sqlstore/migrations/alert_mig.go @@ -65,6 +65,13 @@ func addAlertMigrations(mg *Migrator) { mg.AddMigration("Add column is_default", NewAddColumnMigration(alert_notification, &Column{ Name: "is_default", Type: DB_Bool, Nullable: false, Default: "0", })) + mg.AddMigration("Add column frequency", NewAddColumnMigration(alert_notification, &Column{ + Name: "frequency", Type: DB_BigInt, Nullable: true, + })) + mg.AddMigration("Add column send_reminder", NewAddColumnMigration(alert_notification, &Column{ + Name: "send_reminder", Type: DB_Bool, Nullable: true, Default: "0", + })) + mg.AddMigration("add index alert_notification org_id & name", NewAddIndexMigration(alert_notification, alert_notification.Indices[0])) mg.AddMigration("Update alert table charset", NewTableCharsetMigration("alert", []*Column{ @@ -82,4 +89,22 @@ func addAlertMigrations(mg *Migrator) { {Name: "type", Type: DB_NVarchar, Length: 255, Nullable: false}, {Name: "settings", Type: DB_Text, Nullable: false}, })) + + notification_journal := Table{ + Name: "alert_notification_journal", + Columns: []*Column{ + {Name: "id", Type: DB_BigInt, IsPrimaryKey: true, IsAutoIncrement: true}, + {Name: "org_id", Type: DB_BigInt, Nullable: false}, + {Name: "alert_id", Type: DB_BigInt, Nullable: false}, + {Name: "notifier_id", Type: DB_BigInt, Nullable: false}, + {Name: "sent_at", Type: DB_BigInt, Nullable: false}, + {Name: "success", Type: DB_Bool, Nullable: false}, + }, + Indices: []*Index{ + {Cols: []string{"org_id", "alert_id", "notifier_id"}, Type: IndexType}, + }, + } + + mg.AddMigration("create notification_journal table v1", NewAddTableMigration(notification_journal)) + mg.AddMigration("add index notification_journal org_id & alert_id & notifier_id", NewAddIndexMigration(notification_journal, notification_journal.Indices[0])) } diff --git a/public/app/features/alerting/notification_edit_ctrl.ts b/public/app/features/alerting/notification_edit_ctrl.ts index 60942e6ffb4..315a9a619a1 100644 --- a/public/app/features/alerting/notification_edit_ctrl.ts +++ b/public/app/features/alerting/notification_edit_ctrl.ts @@ -11,6 +11,8 @@ export class AlertNotificationEditCtrl { model: any; defaults: any = { type: 'email', + sendReminder: false, + frequency: '15m', settings: { httpMethod: 'POST', autoResolve: true, @@ -18,12 +20,17 @@ export class AlertNotificationEditCtrl { }, isDefault: false, }; + getFrequencySuggestion: any; /** @ngInject */ constructor(private $routeParams, private backendSrv, private $location, private $templateCache, navModelSrv) { this.navModel = navModelSrv.getNav('alerting', 'channels', 0); this.isNew = !this.$routeParams.id; + this.getFrequencySuggestion = () => { + return ['1m', '5m', '10m', '15m', '30m', '1h']; + }; + this.backendSrv .get(`/api/alert-notifiers`) .then(notifiers => { @@ -102,6 +109,7 @@ export class AlertNotificationEditCtrl { const payload = { name: this.model.name, type: this.model.type, + frequency: this.model.frequency, settings: this.model.settings, }; diff --git a/public/app/features/alerting/partials/notification_edit.html b/public/app/features/alerting/partials/notification_edit.html index d20b9031a8f..7b198736b83 100644 --- a/public/app/features/alerting/partials/notification_edit.html +++ b/public/app/features/alerting/partials/notification_edit.html @@ -32,6 +32,29 @@ checked="ctrl.model.settings.uploadImage" tooltip="Captures an image and include it in the notification"> + + +
+
+ Send reminder every + + Specify how often reminders should be sent, e.g. every 30s, 1m, 10m, 30m or 1h etc. + + + +
+
+
+ + Alert reminders are sent after rules are evaluated. Therefore a reminder can never be sent more frequently than a configured alert rule evaluation interval. + +
diff --git a/tslint.json b/tslint.json index f53eb541643..0d525bea3f0 100644 --- a/tslint.json +++ b/tslint.json @@ -29,6 +29,7 @@ "label-position": true, "max-line-length": [true, 150], "member-access": [true, "no-public"], + "new-parens": true, "no-angle-bracket-type-assertion": true, "no-arg": true, "no-bitwise": false,