From 0b3917e28b5e8799f399cd5d9b70fdc1d0e26821 Mon Sep 17 00:00:00 2001 From: Xabier Larrakoetxea Date: Sat, 14 Dec 2019 11:52:10 +0100 Subject: [PATCH 1/4] Use a Notification on the notifiers instead of the alert manager group directly Signed-off-by: Xabier Larrakoetxea --- internal/forward/forward.go | 2 +- internal/forward/forward_test.go | 20 +++++++----- internal/forward/metrics.go | 4 +-- internal/forward/notify.go | 12 +++++++- internal/mocks/forward/notifier.go | 13 ++++---- internal/notify/notify.go | 11 +++---- internal/notify/telegram/telegram.go | 7 +++-- internal/notify/telegram/telegram_test.go | 37 ++++++++++++++--------- 8 files changed, 63 insertions(+), 43 deletions(-) diff --git a/internal/forward/forward.go b/internal/forward/forward.go index 288149c..85ec286 100644 --- a/internal/forward/forward.go +++ b/internal/forward/forward.go @@ -41,7 +41,7 @@ func (s service) Forward(ctx context.Context, alertGroup *model.AlertGroup) erro // TODO(slok): Add concurrency using workers. for _, not := range s.notifiers { - err := not.Notify(ctx, alertGroup) + err := not.Notify(ctx, Notification{AlertGroup: *alertGroup}) if err != nil { s.logger.WithValues(log.KV{"notifier": not.Type(), "alertGroupID": alertGroup.ID}). Errorf("could not notify alert group: %s", err) diff --git a/internal/forward/forward_test.go b/internal/forward/forward_test.go index 4902e00..ff32ab6 100644 --- a/internal/forward/forward_test.go +++ b/internal/forward/forward_test.go @@ -33,12 +33,14 @@ func TestServiceForward(t *testing.T) { Alerts: []model.Alert{model.Alert{Name: "test"}}, }, mock: func(ns []*forwardmock.Notifier) { - expAlertGroup := &model.AlertGroup{ - ID: "test-group", - Alerts: []model.Alert{model.Alert{Name: "test"}}, + expNotification := forward.Notification{ + AlertGroup: model.AlertGroup{ + ID: "test-group", + Alerts: []model.Alert{model.Alert{Name: "test"}}, + }, } for _, n := range ns { - n.On("Notify", mock.Anything, expAlertGroup).Once().Return(nil) + n.On("Notify", mock.Anything, expNotification).Once().Return(nil) } }, }, @@ -49,9 +51,11 @@ func TestServiceForward(t *testing.T) { Alerts: []model.Alert{model.Alert{Name: "test"}}, }, mock: func(ns []*forwardmock.Notifier) { - expAlertGroup := &model.AlertGroup{ - ID: "test-group", - Alerts: []model.Alert{model.Alert{Name: "test"}}, + expNotification := forward.Notification{ + AlertGroup: model.AlertGroup{ + ID: "test-group", + Alerts: []model.Alert{model.Alert{Name: "test"}}, + }, } for i, n := range ns { err := errTest @@ -59,7 +63,7 @@ func TestServiceForward(t *testing.T) { if i != 0 { err = nil } - n.On("Notify", mock.Anything, expAlertGroup).Once().Return(err) + n.On("Notify", mock.Anything, expNotification).Once().Return(err) n.On("Type").Maybe().Return("") } }, diff --git a/internal/forward/metrics.go b/internal/forward/metrics.go index d97bcb4..3c1880c 100644 --- a/internal/forward/metrics.go +++ b/internal/forward/metrics.go @@ -52,11 +52,11 @@ func NewMeasureNotifier(rec NotifierMetricsRecorder, next Notifier) Notifier { } } -func (m measureNotifier) Notify(ctx context.Context, ag *model.AlertGroup) (err error) { +func (m measureNotifier) Notify(ctx context.Context, n Notification) (err error) { defer func(t0 time.Time) { m.rec.ObserveForwardNotifierOpDuration(ctx, m.notifierType, "Notify", err == nil, time.Since(t0)) }(time.Now()) - return m.next.Notify(ctx, ag) + return m.next.Notify(ctx, n) } func (m measureNotifier) Type() string { diff --git a/internal/forward/notify.go b/internal/forward/notify.go index 8ecb356..15af7b9 100644 --- a/internal/forward/notify.go +++ b/internal/forward/notify.go @@ -6,8 +6,18 @@ import ( "github.com/slok/alertgram/internal/model" ) +// Notification is the notification that wants to be sed +// via a notifier. +type Notification struct { + // ChatID is an ID to send the notification. In + // Telegram could be a channel/group ID, in Slack + // a room or a user. + ChatID string + AlertGroup model.AlertGroup +} + // Notifier knows how to notify alerts to different backends. type Notifier interface { - Notify(ctx context.Context, alertGroup *model.AlertGroup) error + Notify(ctx context.Context, notification Notification) error Type() string } diff --git a/internal/mocks/forward/notifier.go b/internal/mocks/forward/notifier.go index 3e37417..8d4ca30 100644 --- a/internal/mocks/forward/notifier.go +++ b/internal/mocks/forward/notifier.go @@ -5,9 +5,8 @@ package mocks import ( context "context" + forward "github.com/slok/alertgram/internal/forward" mock "github.com/stretchr/testify/mock" - - model "github.com/slok/alertgram/internal/model" ) // Notifier is an autogenerated mock type for the Notifier type @@ -15,13 +14,13 @@ type Notifier struct { mock.Mock } -// Notify provides a mock function with given fields: ctx, alertGroup -func (_m *Notifier) Notify(ctx context.Context, alertGroup *model.AlertGroup) error { - ret := _m.Called(ctx, alertGroup) +// Notify provides a mock function with given fields: ctx, notification +func (_m *Notifier) Notify(ctx context.Context, notification forward.Notification) error { + ret := _m.Called(ctx, notification) var r0 error - if rf, ok := ret.Get(0).(func(context.Context, *model.AlertGroup) error); ok { - r0 = rf(ctx, alertGroup) + if rf, ok := ret.Get(0).(func(context.Context, forward.Notification) error); ok { + r0 = rf(ctx, notification) } else { r0 = ret.Error(0) } diff --git a/internal/notify/notify.go b/internal/notify/notify.go index bf2510e..e0a49d8 100644 --- a/internal/notify/notify.go +++ b/internal/notify/notify.go @@ -5,7 +5,6 @@ import ( "github.com/slok/alertgram/internal/forward" "github.com/slok/alertgram/internal/log" - "github.com/slok/alertgram/internal/model" ) type dummy int @@ -13,8 +12,8 @@ type dummy int // Dummy is a dummy notifier. const Dummy = dummy(0) -func (dummy) Notify(ctx context.Context, alertGroup *model.AlertGroup) error { return nil } -func (dummy) Type() string { return "dummy" } +func (dummy) Notify(ctx context.Context, notification forward.Notification) error { return nil } +func (dummy) Type() string { return "dummy" } type logger struct { renderer TemplateRenderer @@ -30,10 +29,10 @@ func NewLogger(r TemplateRenderer, l log.Logger) forward.Notifier { } } -func (l logger) Notify(ctx context.Context, alertGroup *model.AlertGroup) error { - logger := l.logger.WithValues(log.KV{"alertGroup": alertGroup.ID, "alertsNumber": len(alertGroup.Alerts)}) +func (l logger) Notify(ctx context.Context, n forward.Notification) error { + logger := l.logger.WithValues(log.KV{"alertGroup": n.AlertGroup.ID, "alertsNumber": len(n.AlertGroup.Alerts)}) - alertText, err := l.renderer.Render(ctx, alertGroup) + alertText, err := l.renderer.Render(ctx, &n.AlertGroup) if err != nil { return err } diff --git a/internal/notify/telegram/telegram.go b/internal/notify/telegram/telegram.go index a6f504c..a05b3a4 100644 --- a/internal/notify/telegram/telegram.go +++ b/internal/notify/telegram/telegram.go @@ -79,8 +79,9 @@ func NewNotifier(cfg Config) (forward.Notifier, error) { }, nil } -func (n notifier) Notify(ctx context.Context, alertGroup *model.AlertGroup) error { - logger := n.logger.WithValues(log.KV{"alertGroup": alertGroup.ID, "alertsNumber": len(alertGroup.Alerts)}) +func (n notifier) Notify(ctx context.Context, notification forward.Notification) error { + ag := notification.AlertGroup + logger := n.logger.WithValues(log.KV{"alertGroup": ag.ID, "alertsNumber": len(ag.Alerts)}) select { case <-ctx.Done(): logger.Infof("context cancelled, not notifying alerts") @@ -88,7 +89,7 @@ func (n notifier) Notify(ctx context.Context, alertGroup *model.AlertGroup) erro default: } - msg, err := n.alertGroupToMessage(ctx, alertGroup) + msg, err := n.alertGroupToMessage(ctx, &ag) if err != nil { return fmt.Errorf("could not format the alerts to message: %w", err) } diff --git a/internal/notify/telegram/telegram_test.go b/internal/notify/telegram/telegram_test.go index fea1372..1f10f77 100644 --- a/internal/notify/telegram/telegram_test.go +++ b/internal/notify/telegram/telegram_test.go @@ -10,14 +10,15 @@ import ( "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" + "github.com/slok/alertgram/internal/forward" notifymock "github.com/slok/alertgram/internal/mocks/notify" telegrammock "github.com/slok/alertgram/internal/mocks/notify/telegram" "github.com/slok/alertgram/internal/model" "github.com/slok/alertgram/internal/notify/telegram" ) -func GetBaseAlertGroup() *model.AlertGroup { - return &model.AlertGroup{ +func GetBaseAlertGroup() model.AlertGroup { + return model.AlertGroup{ ID: "test-alert", Alerts: []model.Alert{ { @@ -46,10 +47,10 @@ var errTest = errors.New("whatever") func TestNotify(t *testing.T) { tests := map[string]struct { - cfg telegram.Config - mocks func(t *testing.T, mcli *telegrammock.Client, mr *notifymock.TemplateRenderer) - alertGroup *model.AlertGroup - expErr error + cfg telegram.Config + mocks func(t *testing.T, mcli *telegrammock.Client, mr *notifymock.TemplateRenderer) + notification forward.Notification + expErr error }{ "A alertGroup should be rendered and send the message to telegram.": { cfg: telegram.Config{ @@ -58,7 +59,7 @@ func TestNotify(t *testing.T) { mocks: func(t *testing.T, mcli *telegrammock.Client, mr *notifymock.TemplateRenderer) { expMsgData := "rendered template" expAlertGroup := GetBaseAlertGroup() - mr.On("Render", mock.Anything, expAlertGroup).Once().Return(expMsgData, nil) + mr.On("Render", mock.Anything, &expAlertGroup).Once().Return(expMsgData, nil) expMsg := tgbotapi.MessageConfig{ BaseChat: tgbotapi.BaseChat{ChatID: 1234}, @@ -68,7 +69,9 @@ func TestNotify(t *testing.T) { } mcli.On("Send", expMsg).Once().Return(tgbotapi.Message{}, nil) }, - alertGroup: GetBaseAlertGroup(), + notification: forward.Notification{ + AlertGroup: GetBaseAlertGroup(), + }, }, "A error in the template rendering process should be processed.": { @@ -77,10 +80,12 @@ func TestNotify(t *testing.T) { }, mocks: func(t *testing.T, mcli *telegrammock.Client, mr *notifymock.TemplateRenderer) { expAlertGroup := GetBaseAlertGroup() - mr.On("Render", mock.Anything, expAlertGroup).Once().Return("", errTest) + mr.On("Render", mock.Anything, &expAlertGroup).Once().Return("", errTest) + }, + notification: forward.Notification{ + AlertGroup: GetBaseAlertGroup(), }, - alertGroup: GetBaseAlertGroup(), - expErr: errTest, + expErr: errTest, }, "A error in the notification send process should be processed with communication error.": { @@ -90,7 +95,7 @@ func TestNotify(t *testing.T) { mocks: func(t *testing.T, mcli *telegrammock.Client, mr *notifymock.TemplateRenderer) { expMsgData := "rendered template" expAlertGroup := GetBaseAlertGroup() - mr.On("Render", mock.Anything, expAlertGroup).Once().Return(expMsgData, nil) + mr.On("Render", mock.Anything, &expAlertGroup).Once().Return(expMsgData, nil) expMsg := tgbotapi.MessageConfig{ BaseChat: tgbotapi.BaseChat{ChatID: 1234}, @@ -100,8 +105,10 @@ func TestNotify(t *testing.T) { } mcli.On("Send", expMsg).Once().Return(tgbotapi.Message{}, errTest) }, - alertGroup: GetBaseAlertGroup(), - expErr: telegram.ErrComm, + notification: forward.Notification{ + AlertGroup: GetBaseAlertGroup(), + }, + expErr: telegram.ErrComm, }, } @@ -120,7 +127,7 @@ func TestNotify(t *testing.T) { // Execute. n, err := telegram.NewNotifier(test.cfg) require.NoError(err) - err = n.Notify(context.TODO(), test.alertGroup) + err = n.Notify(context.TODO(), test.notification) // Check. if test.expErr != nil && assert.Error(err) { From 87801d05c4d25b96acf2cca19fe5fc57401a7aa1 Mon Sep 17 00:00:00 2001 From: Xabier Larrakoetxea Date: Sat, 14 Dec 2019 18:59:21 +0100 Subject: [PATCH 2/4] Add notificaton chat ID customization option at URL level for alertmanager Signed-off-by: Xabier Larrakoetxea --- README.md | 42 +++++++- cmd/alertgram/config.go | 24 +++-- internal/forward/forward.go | 19 +++- internal/forward/forward_test.go | 7 +- internal/forward/metrics.go | 4 +- internal/http/alertmanager/alertmanager.go | 15 ++- .../http/alertmanager/alertmanager_test.go | 97 +++++++++++++++++-- internal/http/alertmanager/handler.go | 14 ++- internal/mocks/forward/service.go | 11 ++- internal/notify/telegram/telegram.go | 30 +++++- internal/notify/telegram/telegram_test.go | 36 +++++++ 11 files changed, 258 insertions(+), 41 deletions(-) diff --git a/README.md b/README.md index d65899e..e3f718c 100644 --- a/README.md +++ b/README.md @@ -6,9 +6,25 @@ Alertgram is the easiest way to forward alerts to [Telegram] (Supports [Promethe alertgram

+## Table of contents + +- [Introduction](#introduction) +- [Input alerts](#input-alerts) +- [Options](#options) +- [Run](#run) + - [Simple example](#simple-example) + - [Production](#production) +- [Metrics](#metrics) +- [Development and debugging](#development-and-debugging) +- [FAQ](#faq) + - [Only alertmanager alerts are supported?](#only-alertmanager-alerts-are-supported-) + - [Where does alertgram listen to alertmanager alerts?](#where-does-alertgram-listen-to-alertmanager-alerts-) + - [Can I notify to different chats?](#can-i-notify-to-different-chats-) + - [Can I use custom templates?](#can-i-use-custom-templates-) + ## Introduction -Everything started as a way of forwarding [Prometheus alertmanager] alerts to [Telegram] because the solutions that I found where too complex, I just wanted to forward alerts to channels without trouble. And Alertgram is just that, a simple app that forwards alerts to Telegram groups and channels. +Everything started as a way of forwarding [Prometheus alertmanager] alerts to [Telegram] because the solutions that I found were too complex, I just wanted to forward alerts to channels without trouble. And Alertgram is just that, a simple app that forwards alerts to Telegram groups and channels. ## Input alerts @@ -58,7 +74,28 @@ Also remember that you can use `--debug` flag. ## FAQ -### Can I use custom template? +### Only alertmanager alerts are supported? + +At this moment yes, but we can add more input alert systems if you want, create an issue +so we can discuss and implement. + +### Where does alertgram listen to alertmanager alerts? + +By default in `0.0.0.0:8080/alerts`, but you can use `--alertmanager.listen-address` and +`--alertmanager.webhook-path` to customize. + +### Can I notify to different chats? + +There are 3 levels where you could customize the notification chat: + +- By default: Using the required `--telegram.chat-id` flag. +- At URL level: using [query string] parameter, e.g. `0.0.0.0:8080/alerts?chat-id=-1009876543210`. + This query param can be customized with `--alertmanager.chat-id-query-string` flag. +- At alert level: TODO + +The preference is in order from the lowest to the highest: Default, URL, Alert. + +### Can I use custom templates? Yes!, use the flag `--notify.template-path`. You can check [testdata/templates](testdata/templates) for examples. @@ -93,3 +130,4 @@ curl -i http://127.0.0.1:8080/alerts -d @./testdata/alerts/base.json [kubernetes-deployment]: docs/kubernetes [html go templates]: https://golang.org/pkg/html/template/ [sprig]: http://masterminds.github.io/sprig +[query string]: https://en.wikipedia.org/wiki/Query_string diff --git a/cmd/alertgram/config.go b/cmd/alertgram/config.go index 25a17bb..f7a968a 100644 --- a/cmd/alertgram/config.go +++ b/cmd/alertgram/config.go @@ -15,6 +15,7 @@ var ( const ( descAMListenAddr = "The listen address where the server will be listening to alertmanager's webhook request." descAMWebhookPath = "The path where the server will be handling the alertmanager webhook alert requests." + descAMChatIDQS = "The optional query string key used to customize the chat id of the notification." descTelegramAPIToken = "The token that will be used to use the telegram API to send the alerts." descTelegramDefChatID = "The default ID of the chat (group/channel) in telegram where the alerts will be sent." descMetricsListenAddr = "The listen address where the metrics will be being served." @@ -28,6 +29,7 @@ const ( const ( defAMListenAddr = ":8080" defAMWebhookPath = "/alerts" + defAMChatIDQS = "chat-id" defMetricsListenAddr = ":8081" defMetricsPath = "/metrics" defMetricsHCPath = "/status" @@ -35,16 +37,17 @@ const ( // Config has the configuration of the application. type Config struct { - AlertmanagerListenAddr string - AlertmanagerWebhookPath string - TeletramAPIToken string - TelegramChatID int64 - MetricsListenAddr string - MetricsPath string - MetricsHCPath string - NotifyTemplate *os.File - DebugMode bool - NotifyDryRun bool + AlertmanagerListenAddr string + AlertmanagerWebhookPath string + AlertmanagerChatIDQQueryString string + TeletramAPIToken string + TelegramChatID int64 + MetricsListenAddr string + MetricsPath string + MetricsHCPath string + NotifyTemplate *os.File + DebugMode bool + NotifyDryRun bool app *kingpin.Application } @@ -71,6 +74,7 @@ func NewConfig() (*Config, error) { func (c *Config) registerFlags() { c.app.Flag("alertmanager.listen-address", descAMListenAddr).Default(defAMListenAddr).StringVar(&c.AlertmanagerListenAddr) c.app.Flag("alertmanager.webhook-path", descAMWebhookPath).Default(defAMWebhookPath).StringVar(&c.AlertmanagerWebhookPath) + c.app.Flag("alertmanager.chat-id-query-string", descAMChatIDQS).Default(defAMChatIDQS).StringVar(&c.AlertmanagerChatIDQQueryString) c.app.Flag("telegram.api-token", descTelegramAPIToken).Required().StringVar(&c.TeletramAPIToken) c.app.Flag("telegram.chat-id", descTelegramDefChatID).Required().Int64Var(&c.TelegramChatID) c.app.Flag("metrics.listen-address", descMetricsListenAddr).Default(defMetricsListenAddr).StringVar(&c.MetricsListenAddr) diff --git a/internal/forward/forward.go b/internal/forward/forward.go index 85ec286..0342cee 100644 --- a/internal/forward/forward.go +++ b/internal/forward/forward.go @@ -9,10 +9,19 @@ import ( "github.com/slok/alertgram/internal/model" ) +// Properties are the properties an AlertGroup can have +// when the forwarding process is done. +type Properties struct { + // CustomChatID can be used when the forward should be done + // to a different target (chat, group, channel, user...) + // instead of using the default one. + CustomChatID string +} + // Service is the domain service that forwards alerts type Service interface { // Forward knows how to forward alerts from an input to an output. - Forward(ctx context.Context, alertGroup *model.AlertGroup) error + Forward(ctx context.Context, props Properties, alertGroup *model.AlertGroup) error } type service struct { @@ -33,15 +42,19 @@ var ( ErrInvalidAlertGroup = errors.New("invalid alert group") ) -func (s service) Forward(ctx context.Context, alertGroup *model.AlertGroup) error { +func (s service) Forward(ctx context.Context, props Properties, alertGroup *model.AlertGroup) error { // TODO(slok): Add better validation. if alertGroup == nil { return fmt.Errorf("alertgroup can't be empty: %w", ErrInvalidAlertGroup) } // TODO(slok): Add concurrency using workers. + notification := Notification{ + AlertGroup: *alertGroup, + ChatID: props.CustomChatID, + } for _, not := range s.notifiers { - err := not.Notify(ctx, Notification{AlertGroup: *alertGroup}) + err := not.Notify(ctx, notification) if err != nil { s.logger.WithValues(log.KV{"notifier": not.Type(), "alertGroupID": alertGroup.ID}). Errorf("could not notify alert group: %s", err) diff --git a/internal/forward/forward_test.go b/internal/forward/forward_test.go index ff32ab6..e5a8cd7 100644 --- a/internal/forward/forward_test.go +++ b/internal/forward/forward_test.go @@ -18,6 +18,7 @@ var errTest = errors.New("whatever") func TestServiceForward(t *testing.T) { tests := map[string]struct { + props forward.Properties alertGroup *model.AlertGroup mock func(ns []*forwardmock.Notifier) expErr error @@ -28,12 +29,16 @@ func TestServiceForward(t *testing.T) { }, "A forwarded alerts should be send to all notifiers.": { + props: forward.Properties{ + CustomChatID: "-1001234567890", + }, alertGroup: &model.AlertGroup{ ID: "test-group", Alerts: []model.Alert{model.Alert{Name: "test"}}, }, mock: func(ns []*forwardmock.Notifier) { expNotification := forward.Notification{ + ChatID: "-1001234567890", AlertGroup: model.AlertGroup{ ID: "test-group", Alerts: []model.Alert{model.Alert{Name: "test"}}, @@ -79,7 +84,7 @@ func TestServiceForward(t *testing.T) { test.mock([]*forwardmock.Notifier{mn1, mn2}) svc := forward.NewService([]forward.Notifier{mn1, mn2}, log.Dummy) - err := svc.Forward(context.TODO(), test.alertGroup) + err := svc.Forward(context.TODO(), test.props, test.alertGroup) if test.expErr != nil && assert.Error(err) { assert.True(errors.Is(err, test.expErr)) diff --git a/internal/forward/metrics.go b/internal/forward/metrics.go index 3c1880c..df2574c 100644 --- a/internal/forward/metrics.go +++ b/internal/forward/metrics.go @@ -25,11 +25,11 @@ func NewMeasureService(rec ServiceMetricsRecorder, next Service) Service { } } -func (m measureService) Forward(ctx context.Context, ag *model.AlertGroup) (err error) { +func (m measureService) Forward(ctx context.Context, props Properties, ag *model.AlertGroup) (err error) { defer func(t0 time.Time) { m.rec.ObserveForwardServiceOpDuration(ctx, "Forward", err == nil, time.Since(t0)) }(time.Now()) - return m.next.Forward(ctx, ag) + return m.next.Forward(ctx, props, ag) } // NotifierMetricsRecorder knows how to record metrics on forward.Notifier. diff --git a/internal/http/alertmanager/alertmanager.go b/internal/http/alertmanager/alertmanager.go index 773074d..cf1bdab 100644 --- a/internal/http/alertmanager/alertmanager.go +++ b/internal/http/alertmanager/alertmanager.go @@ -15,11 +15,12 @@ import ( // Config is the configuration of the WebhookHandler. type Config struct { - MetricsRecorder metrics.Recorder - WebhookPath string - Forwarder forward.Service - Debug bool - Logger log.Logger + MetricsRecorder metrics.Recorder + WebhookPath string + ChatIDQueryString string + Forwarder forward.Service + Debug bool + Logger log.Logger } func (c *Config) defaults() error { @@ -31,6 +32,10 @@ func (c *Config) defaults() error { return fmt.Errorf("forward can't be nil") } + if c.ChatIDQueryString == "" { + c.ChatIDQueryString = "chat-id" + } + if c.Logger == nil { c.Logger = log.Dummy } diff --git a/internal/http/alertmanager/alertmanager_test.go b/internal/http/alertmanager/alertmanager_test.go index 4f82eb3..2d554ef 100644 --- a/internal/http/alertmanager/alertmanager_test.go +++ b/internal/http/alertmanager/alertmanager_test.go @@ -2,6 +2,8 @@ package alertmanager_test import ( "encoding/json" + "errors" + "fmt" "net/http" "net/http/httptest" "strings" @@ -15,7 +17,9 @@ import ( "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" + "github.com/slok/alertgram/internal/forward" "github.com/slok/alertgram/internal/http/alertmanager" + "github.com/slok/alertgram/internal/internalerrors" forwardmock "github.com/slok/alertgram/internal/mocks/forward" "github.com/slok/alertgram/internal/model" ) @@ -83,11 +87,14 @@ func getBaseAlertmanagerAlerts() webhook.Message { func TestHandleAlerts(t *testing.T) { tests := map[string]struct { + config alertmanager.Config + urlPath string webhookAlertJSON func(t *testing.T) string mock func(t *testing.T, msvc *forwardmock.Service) expCode int }{ - "Alertmanager webhook alerts request should be handled correctly.": { + "Alertmanager webhook alerts request should be handled correctly (with defaults).": { + urlPath: "/alerts", webhookAlertJSON: func(t *testing.T) string { wa := getBaseAlertmanagerAlerts() body, err := json.Marshal(wa) @@ -96,10 +103,88 @@ func TestHandleAlerts(t *testing.T) { }, mock: func(t *testing.T, msvc *forwardmock.Service) { expAlerts := getBaseAlerts() - msvc.On("Forward", mock.Anything, expAlerts).Once().Return(nil) + expProps := forward.Properties{} + msvc.On("Forward", mock.Anything, expProps, expAlerts).Once().Return(nil) }, expCode: http.StatusOK, }, + + "Alertmanager webhook alerts request should be handled correctly (with custom params).": { + config: alertmanager.Config{ + WebhookPath: "/test-alerts", + ChatIDQueryString: "custom-telegram-chat-id", + }, + urlPath: "/test-alerts?custom-telegram-chat-id=-1009876543210", + webhookAlertJSON: func(t *testing.T) string { + wa := getBaseAlertmanagerAlerts() + body, err := json.Marshal(wa) + require.NoError(t, err) + return string(body) + }, + mock: func(t *testing.T, msvc *forwardmock.Service) { + expAlerts := getBaseAlerts() + expProps := forward.Properties{ + CustomChatID: "-1009876543210", + } + msvc.On("Forward", mock.Anything, expProps, expAlerts).Once().Return(nil) + }, + expCode: http.StatusOK, + }, + + "Internal errors should be propagated to clients (forwarding).": { + urlPath: "/alerts", + webhookAlertJSON: func(t *testing.T) string { + wa := getBaseAlertmanagerAlerts() + body, err := json.Marshal(wa) + require.NoError(t, err) + return string(body) + }, + mock: func(t *testing.T, msvc *forwardmock.Service) { + expAlerts := getBaseAlerts() + expProps := forward.Properties{} + msvc.On("Forward", mock.Anything, expProps, expAlerts).Once().Return(errors.New("whatever")) + }, + expCode: http.StatusInternalServerError, + }, + + "Configuration errors should be propagated to clients (forwarding).": { + urlPath: "/alerts", + webhookAlertJSON: func(t *testing.T) string { + wa := getBaseAlertmanagerAlerts() + body, err := json.Marshal(wa) + require.NoError(t, err) + return string(body) + }, + mock: func(t *testing.T, msvc *forwardmock.Service) { + expAlerts := getBaseAlerts() + expProps := forward.Properties{} + err := fmt.Errorf("custom error: %w", internalerrors.ErrInvalidConfiguration) + msvc.On("Forward", mock.Anything, expProps, expAlerts).Once().Return(err) + }, + expCode: http.StatusBadRequest, + }, + + "Configuration errors on notification should be propagated to clients (alert mapping).": { + urlPath: "/alerts", + webhookAlertJSON: func(t *testing.T) string { + wa := getBaseAlertmanagerAlerts() + wa.Version = "v3" + body, err := json.Marshal(wa) + require.NoError(t, err) + return string(body) + }, + mock: func(t *testing.T, msvc *forwardmock.Service) {}, + expCode: http.StatusBadRequest, + }, + + "Configuration errors on notification should be propagated to clients (JSON formatting).": { + urlPath: "/alerts", + webhookAlertJSON: func(t *testing.T) string { + return "{" + }, + mock: func(t *testing.T, msvc *forwardmock.Service) {}, + expCode: http.StatusBadRequest, + }, } for name, test := range tests { @@ -112,13 +197,11 @@ func TestHandleAlerts(t *testing.T) { test.mock(t, msvc) // Execute. - h, _ := alertmanager.NewHandler(alertmanager.Config{ - WebhookPath: "/test-alerts", - Forwarder: msvc, - }) + test.config.Forwarder = msvc + h, _ := alertmanager.NewHandler(test.config) srv := httptest.NewServer(h) defer srv.Close() - req, err := http.NewRequest(http.MethodPost, srv.URL+"/test-alerts", strings.NewReader(test.webhookAlertJSON(t))) + req, err := http.NewRequest(http.MethodPost, srv.URL+test.urlPath, strings.NewReader(test.webhookAlertJSON(t))) require.NoError(err) resp, err := http.DefaultClient.Do(req) require.NoError(err) diff --git a/internal/http/alertmanager/handler.go b/internal/http/alertmanager/handler.go index d058a7b..6c50a63 100644 --- a/internal/http/alertmanager/handler.go +++ b/internal/http/alertmanager/handler.go @@ -1,9 +1,12 @@ package alertmanager import ( + "errors" "net/http" "github.com/gin-gonic/gin" + "github.com/slok/alertgram/internal/forward" + "github.com/slok/alertgram/internal/internalerrors" ) func (w webhookHandler) HandleAlerts() gin.HandlerFunc { @@ -23,9 +26,18 @@ func (w webhookHandler) HandleAlerts() gin.HandlerFunc { return } - err = w.forwarder.Forward(ctx.Request.Context(), model) + props := forward.Properties{ + CustomChatID: ctx.Query(w.cfg.ChatIDQueryString), + } + err = w.forwarder.Forward(ctx.Request.Context(), props, model) if err != nil { w.logger.Errorf("error forwarding alert: %s", err) + + if errors.Is(err, internalerrors.ErrInvalidConfiguration) { + _ = ctx.AbortWithError(http.StatusBadRequest, err).SetType(gin.ErrorTypePublic) + return + } + _ = ctx.AbortWithError(http.StatusInternalServerError, err).SetType(gin.ErrorTypePublic) return } diff --git a/internal/mocks/forward/service.go b/internal/mocks/forward/service.go index 9706ede..acf18f8 100644 --- a/internal/mocks/forward/service.go +++ b/internal/mocks/forward/service.go @@ -5,6 +5,7 @@ package mocks import ( context "context" + forward "github.com/slok/alertgram/internal/forward" mock "github.com/stretchr/testify/mock" model "github.com/slok/alertgram/internal/model" @@ -15,13 +16,13 @@ type Service struct { mock.Mock } -// Forward provides a mock function with given fields: ctx, alertGroup -func (_m *Service) Forward(ctx context.Context, alertGroup *model.AlertGroup) error { - ret := _m.Called(ctx, alertGroup) +// Forward provides a mock function with given fields: ctx, props, alertGroup +func (_m *Service) Forward(ctx context.Context, props forward.Properties, alertGroup *model.AlertGroup) error { + ret := _m.Called(ctx, props, alertGroup) var r0 error - if rf, ok := ret.Get(0).(func(context.Context, *model.AlertGroup) error); ok { - r0 = rf(ctx, alertGroup) + if rf, ok := ret.Get(0).(func(context.Context, forward.Properties, *model.AlertGroup) error); ok { + r0 = rf(ctx, props, alertGroup) } else { r0 = ret.Error(0) } diff --git a/internal/notify/telegram/telegram.go b/internal/notify/telegram/telegram.go index a05b3a4..b2f464d 100644 --- a/internal/notify/telegram/telegram.go +++ b/internal/notify/telegram/telegram.go @@ -4,13 +4,13 @@ import ( "context" "errors" "fmt" + "strconv" tgbotapi "github.com/go-telegram-bot-api/telegram-bot-api" "github.com/slok/alertgram/internal/forward" "github.com/slok/alertgram/internal/internalerrors" "github.com/slok/alertgram/internal/log" - "github.com/slok/alertgram/internal/model" "github.com/slok/alertgram/internal/notify" ) @@ -81,6 +81,7 @@ func NewNotifier(cfg Config) (forward.Notifier, error) { func (n notifier) Notify(ctx context.Context, notification forward.Notification) error { ag := notification.AlertGroup + logger := n.logger.WithValues(log.KV{"alertGroup": ag.ID, "alertsNumber": len(ag.Alerts)}) select { case <-ctx.Done(): @@ -89,7 +90,7 @@ func (n notifier) Notify(ctx context.Context, notification forward.Notification) default: } - msg, err := n.alertGroupToMessage(ctx, &ag) + msg, err := n.createMessage(ctx, notification) if err != nil { return fmt.Errorf("could not format the alerts to message: %w", err) } @@ -105,15 +106,34 @@ func (n notifier) Notify(ctx context.Context, notification forward.Notification) return nil } -func (n notifier) alertGroupToMessage(ctx context.Context, a *model.AlertGroup) (tgbotapi.Chattable, error) { - data, err := n.tplRenderer.Render(ctx, a) +func (n notifier) getChatID(notification forward.Notification) (int64, error) { + if notification.ChatID == "" { + return n.cfg.DefaultTelegramChatID, nil + } + + chatID, err := strconv.ParseInt(notification.ChatID, 10, 64) + if err != nil { + return 0, fmt.Errorf("%w: %s", internalerrors.ErrInvalidConfiguration, err) + } + + return chatID, nil +} + +func (n notifier) createMessage(ctx context.Context, notification forward.Notification) (tgbotapi.Chattable, error) { + chatID, err := n.getChatID(notification) + if err != nil { + return nil, fmt.Errorf("could not get a valid telegran chat ID: %w", err) + } + + data, err := n.tplRenderer.Render(ctx, ¬ification.AlertGroup) if err != nil { return nil, fmt.Errorf("error rendering alerts to template: %w", err) } - msg := tgbotapi.NewMessage(n.cfg.DefaultTelegramChatID, data) + msg := tgbotapi.NewMessage(chatID, data) msg.ParseMode = "HTML" msg.DisableWebPagePreview = true // TODO(slok): Make it configurable? + return msg, nil } diff --git a/internal/notify/telegram/telegram_test.go b/internal/notify/telegram/telegram_test.go index 1f10f77..530553e 100644 --- a/internal/notify/telegram/telegram_test.go +++ b/internal/notify/telegram/telegram_test.go @@ -11,6 +11,7 @@ import ( "github.com/stretchr/testify/require" "github.com/slok/alertgram/internal/forward" + "github.com/slok/alertgram/internal/internalerrors" notifymock "github.com/slok/alertgram/internal/mocks/notify" telegrammock "github.com/slok/alertgram/internal/mocks/notify/telegram" "github.com/slok/alertgram/internal/model" @@ -74,6 +75,29 @@ func TestNotify(t *testing.T) { }, }, + "If using a custom chat ID based on notificaiton it should send to that chat.": { + cfg: telegram.Config{ + DefaultTelegramChatID: 1234, + }, + mocks: func(t *testing.T, mcli *telegrammock.Client, mr *notifymock.TemplateRenderer) { + expMsgData := "rendered template" + expAlertGroup := GetBaseAlertGroup() + mr.On("Render", mock.Anything, &expAlertGroup).Once().Return(expMsgData, nil) + + expMsg := tgbotapi.MessageConfig{ + BaseChat: tgbotapi.BaseChat{ChatID: -1009876543210}, + ParseMode: "HTML", + DisableWebPagePreview: true, + Text: expMsgData, + } + mcli.On("Send", expMsg).Once().Return(tgbotapi.Message{}, nil) + }, + notification: forward.Notification{ + ChatID: "-1009876543210", + AlertGroup: GetBaseAlertGroup(), + }, + }, + "A error in the template rendering process should be processed.": { cfg: telegram.Config{ DefaultTelegramChatID: 1234, @@ -88,6 +112,18 @@ func TestNotify(t *testing.T) { expErr: errTest, }, + "A error with an invalid custom Chat ID should be propagated.": { + cfg: telegram.Config{ + DefaultTelegramChatID: 1234, + }, + mocks: func(t *testing.T, mcli *telegrammock.Client, mr *notifymock.TemplateRenderer) {}, + notification: forward.Notification{ + ChatID: "notAnInt64", + AlertGroup: GetBaseAlertGroup(), + }, + expErr: internalerrors.ErrInvalidConfiguration, + }, + "A error in the notification send process should be processed with communication error.": { cfg: telegram.Config{ DefaultTelegramChatID: 1234, From 8b67b16fd6a94e465acae13b66a4b1b5d0a0dca4 Mon Sep 17 00:00:00 2001 From: Xabier Larrakoetxea Date: Sat, 14 Dec 2019 19:34:16 +0100 Subject: [PATCH 3/4] Improve alertmaneger documentation Signed-off-by: Xabier Larrakoetxea --- docs/alertmanager/README.md | 13 +++++++++++-- docs/kubernetes/alertmanager-cfg.yaml | 28 +++++++++++++++++++++++++-- internal/forward/notify.go | 2 +- 3 files changed, 38 insertions(+), 5 deletions(-) diff --git a/docs/alertmanager/README.md b/docs/alertmanager/README.md index 1da4cbf..951428b 100644 --- a/docs/alertmanager/README.md +++ b/docs/alertmanager/README.md @@ -26,11 +26,20 @@ route: group_wait: 30s group_interval: 5m repeat_interval: 3h - receiver: 'telegram' + receiver: telegram + routes: + # Only important alerts. + - match_re: + severity: ^(oncall|critical)$ + receiver: telegram-oncall receivers: -- name: 'telegram' +- name: telegram webhook_configs: - url: 'http://alertgram:8080/alerts' send_resolved: false + +- name: telegram-oncall + webhook_configs: + - url: 'http://alertgram:8080/alerts?chat-id=-1001111111111' ``` diff --git a/docs/kubernetes/alertmanager-cfg.yaml b/docs/kubernetes/alertmanager-cfg.yaml index 5848414..efec953 100644 --- a/docs/kubernetes/alertmanager-cfg.yaml +++ b/docs/kubernetes/alertmanager-cfg.yaml @@ -15,9 +15,33 @@ stringData: group_wait: 30s group_interval: 5m repeat_interval: 12h - receiver: 'telegram' + receiver: 'telegram-default' + routes: + # Warning alerts not important. + - match: + severity: warning + receiver: telegram-warning + + # Only important alerts. + - match_re: + severity: ^(oncall|critical)$ + receiver: telegram-oncall + receivers: - - name: 'telegram' + # Use default alert channel for the default alerts. + - name: telegram-default webhook_configs: - url: 'http://alertgram:8080/alerts' send_resolved: false + + # Critical and oncall alerts to special channel. + - name: telegram-oncall + webhook_configs: + - url: 'http://alertgram:8080/alerts?chat-id=-1001111111111' + send_resolved: false + + # Warning alerts to a more public informative channel. + - name: telegram-warning + webhook_configs: + - url: 'http://alertgram:8080/alerts?chat-id=-1002222222222' + send_resolved: false diff --git a/internal/forward/notify.go b/internal/forward/notify.go index 15af7b9..5667f93 100644 --- a/internal/forward/notify.go +++ b/internal/forward/notify.go @@ -6,7 +6,7 @@ import ( "github.com/slok/alertgram/internal/model" ) -// Notification is the notification that wants to be sed +// Notification is the notification that wants to be send // via a notifier. type Notification struct { // ChatID is an ID to send the notification. In From 59184fc20eb1e01a1006359da45c5ce894207f54 Mon Sep 17 00:00:00 2001 From: Xabier Larrakoetxea Date: Sat, 14 Dec 2019 19:45:45 +0100 Subject: [PATCH 4/4] Update changelog Signed-off-by: Xabier Larrakoetxea --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5b27c92..3dacefe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ ## [unreleased] - YYYY-MM-DD +### Added + +- Alertmanager API accepts a query string param with a custom chat ID. +- Telegram notifier can send to customized chats. + ## [0.1.0] - 2019-12-13 ### Added