Skip to content

Commit

Permalink
Replace notify.Factory with a build function that accepts typed confi…
Browse files Browse the repository at this point in the history
…guration (#63)

* replace constructors for each notifier to accept only arguments that are needed.
* replace Factory with function BuildReceiverIntegrations

* remove NotificationChannel interface (internalized in the function)
* remove receivers/factory.go.
* move GetDecryptedValueFn to notify package 
* add notifier uid to log context
* update LoggerFactory type to distinguish between logger name and additional context.

---------

Co-authored-by: Santiago <[email protected]>
  • Loading branch information
yuri-tceretian and santihernandezc authored Mar 28, 2023
1 parent d1e3c68 commit c7c80f3
Show file tree
Hide file tree
Showing 28 changed files with 401 additions and 385 deletions.
2 changes: 1 addition & 1 deletion logging/log.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package logging

type LoggerFactory func(ctx ...interface{}) Logger
type LoggerFactory func(loggerName string, ctx ...interface{}) Logger

type Logger interface {
// New returns a new contextual Logger that has this logger's context plus the given context.
Expand Down
156 changes: 105 additions & 51 deletions notify/factory.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
package notify

import (
"strings"
"fmt"

"github.com/prometheus/alertmanager/notify"
"github.com/prometheus/alertmanager/template"
"github.com/prometheus/alertmanager/types"

"github.com/grafana/alerting/images"
"github.com/grafana/alerting/logging"
"github.com/grafana/alerting/receivers"
"github.com/grafana/alerting/receivers/alertmanager"
"github.com/grafana/alerting/receivers/dinding"
Expand All @@ -27,57 +31,107 @@ import (
"github.com/grafana/alerting/receivers/wecom"
)

var receiverFactories = map[string]func(receivers.FactoryConfig) (NotificationChannel, error){
"prometheus-alertmanager": wrap(alertmanager.New),
"dingding": wrap(dinding.New),
"discord": wrap(discord.New),
"email": wrap(email.New),
"googlechat": wrap(googlechat.New),
"kafka": wrap(kafka.New),
"line": wrap(line.New),
"opsgenie": wrap(opsgenie.New),
"pagerduty": wrap(pagerduty.New),
"pushover": wrap(pushover.New),
"sensugo": wrap(sensugo.New),
"slack": wrap(slack.New),
"teams": wrap(teams.New),
"telegram": wrap(telegram.New),
"threema": wrap(threema.New),
"victorops": wrap(victorops.New),
"webhook": wrap(webhook.New),
"wecom": wrap(wecom.New),
"webex": wrap(webex.New),
}

type NotificationChannel interface {
notify.Notifier
notify.ResolvedSender
}

func Factory(receiverType string) (func(receivers.FactoryConfig) (NotificationChannel, error), bool) {
receiverType = strings.ToLower(receiverType)
factory, exists := receiverFactories[receiverType]
return factory, exists
}

// wrap wraps the notifier's factory errors with receivers.ReceiverValidationError
func wrap[T NotificationChannel](f func(fc receivers.FactoryConfig) (T, error)) func(receivers.FactoryConfig) (NotificationChannel, error) {
return func(fc receivers.FactoryConfig) (NotificationChannel, error) {
ch, err := f(fc)
if err != nil {
return nil, ReceiverValidationError{
Err: err,
// TODO it will be removed in the next PR
Cfg: &GrafanaReceiver{
UID: fc.Config.UID,
Name: fc.Config.Name,
Type: fc.Config.Type,
DisableResolveMessage: fc.Config.DisableResolveMessage,
Settings: fc.Config.Settings,
SecureSettings: nil,
},
// BuildReceiverIntegrations creates integrations for each configured notification channel in GrafanaReceiverConfig.
// It returns a slice of Integration objects, one for each notification channel, along with any errors that occurred.
func BuildReceiverIntegrations(
receiver GrafanaReceiverConfig,
tmpl *template.Template,
img images.ImageStore,
logger logging.LoggerFactory,
newWebhookSender func(n receivers.Metadata) (receivers.WebhookSender, error),
newEmailSender func(n receivers.Metadata) (receivers.EmailSender, error),
orgID int64,
version string,
) ([]*Integration, error) {
type notificationChannel interface {
notify.Notifier
notify.ResolvedSender
}
var (
integrations []*Integration
errors types.MultiError
nl = func(meta receivers.Metadata) logging.Logger {
return logger("ngalert.notifier."+meta.Type, "notifierUID", meta.UID)
}
ci = func(idx int, cfg receivers.Metadata, n notificationChannel) {
i := NewIntegration(n, n, cfg.Type, idx)
integrations = append(integrations, i)
}
nw = func(cfg receivers.Metadata) receivers.WebhookSender {
w, e := newWebhookSender(cfg)
if e != nil {
errors.Add(fmt.Errorf("unable to build webhook client for %s notifier %s (UID: %s): %w ", cfg.Type, cfg.Name, cfg.UID, e))
return nil // return nil to simplify the construction code. This works because constructor in notifiers do not check the argument for nil.
// This does not cause misconfigured notifiers because it populates `errors`, which causes the function to return nil integrations and non-nil error.
}
return w
}
return ch, nil
)
// Range through each notification channel in the receiver and create an integration for it.
for i, cfg := range receiver.AlertmanagerConfigs {
ci(i, cfg.Metadata, alertmanager.New(cfg.Settings, cfg.Metadata, img, nl(cfg.Metadata)))
}
for i, cfg := range receiver.DingdingConfigs {
ci(i, cfg.Metadata, dinding.New(cfg.Settings, cfg.Metadata, tmpl, nw(cfg.Metadata), nl(cfg.Metadata)))
}
for i, cfg := range receiver.DiscordConfigs {
ci(i, cfg.Metadata, discord.New(cfg.Settings, cfg.Metadata, tmpl, nw(cfg.Metadata), img, nl(cfg.Metadata), version))
}
for i, cfg := range receiver.EmailConfigs {
mailCli, e := newEmailSender(cfg.Metadata)
if e != nil {
errors.Add(fmt.Errorf("unable to build email client for %s notifier %s (UID: %s): %w ", cfg.Type, cfg.Name, cfg.UID, e))
continue
}
ci(i, cfg.Metadata, email.New(cfg.Settings, cfg.Metadata, tmpl, mailCli, img, nl(cfg.Metadata)))
}
for i, cfg := range receiver.GooglechatConfigs {
ci(i, cfg.Metadata, googlechat.New(cfg.Settings, cfg.Metadata, tmpl, nw(cfg.Metadata), img, nl(cfg.Metadata), version))
}
for i, cfg := range receiver.KafkaConfigs {
ci(i, cfg.Metadata, kafka.New(cfg.Settings, cfg.Metadata, tmpl, nw(cfg.Metadata), img, nl(cfg.Metadata)))
}
for i, cfg := range receiver.LineConfigs {
ci(i, cfg.Metadata, line.New(cfg.Settings, cfg.Metadata, tmpl, nw(cfg.Metadata), nl(cfg.Metadata)))
}
for i, cfg := range receiver.OpsgenieConfigs {
ci(i, cfg.Metadata, opsgenie.New(cfg.Settings, cfg.Metadata, tmpl, nw(cfg.Metadata), img, nl(cfg.Metadata)))
}
for i, cfg := range receiver.PagerdutyConfigs {
ci(i, cfg.Metadata, pagerduty.New(cfg.Settings, cfg.Metadata, tmpl, nw(cfg.Metadata), img, nl(cfg.Metadata)))
}
for i, cfg := range receiver.PushoverConfigs {
ci(i, cfg.Metadata, pushover.New(cfg.Settings, cfg.Metadata, tmpl, nw(cfg.Metadata), img, nl(cfg.Metadata)))
}
for i, cfg := range receiver.SensugoConfigs {
ci(i, cfg.Metadata, sensugo.New(cfg.Settings, cfg.Metadata, tmpl, nw(cfg.Metadata), img, nl(cfg.Metadata)))
}
for i, cfg := range receiver.SlackConfigs {
ci(i, cfg.Metadata, slack.New(cfg.Settings, cfg.Metadata, tmpl, nw(cfg.Metadata), img, nl(cfg.Metadata), version))
}
for i, cfg := range receiver.TeamsConfigs {
ci(i, cfg.Metadata, teams.New(cfg.Settings, cfg.Metadata, tmpl, nw(cfg.Metadata), img, nl(cfg.Metadata)))
}
for i, cfg := range receiver.TelegramConfigs {
ci(i, cfg.Metadata, telegram.New(cfg.Settings, cfg.Metadata, tmpl, nw(cfg.Metadata), img, nl(cfg.Metadata)))
}
for i, cfg := range receiver.ThreemaConfigs {
ci(i, cfg.Metadata, threema.New(cfg.Settings, cfg.Metadata, tmpl, nw(cfg.Metadata), img, nl(cfg.Metadata)))
}
for i, cfg := range receiver.VictoropsConfigs {
ci(i, cfg.Metadata, victorops.New(cfg.Settings, cfg.Metadata, tmpl, nw(cfg.Metadata), img, nl(cfg.Metadata), version))
}
for i, cfg := range receiver.WebhookConfigs {
ci(i, cfg.Metadata, webhook.New(cfg.Settings, cfg.Metadata, tmpl, nw(cfg.Metadata), img, nl(cfg.Metadata), orgID))
}
for i, cfg := range receiver.WecomConfigs {
ci(i, cfg.Metadata, wecom.New(cfg.Settings, cfg.Metadata, tmpl, nw(cfg.Metadata), nl(cfg.Metadata)))
}
for i, cfg := range receiver.WebexConfigs {
ci(i, cfg.Metadata, webex.New(cfg.Settings, cfg.Metadata, tmpl, nw(cfg.Metadata), img, nl(cfg.Metadata), orgID))
}
if errors.Len() > 0 {
return nil, &errors
}
return integrations, nil
}
118 changes: 118 additions & 0 deletions notify/factory_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
package notify

import (
"context"
"errors"
"fmt"
"math/rand"
"testing"

"github.com/stretchr/testify/require"

"github.com/grafana/alerting/images"
"github.com/grafana/alerting/logging"
"github.com/grafana/alerting/receivers"
"github.com/grafana/alerting/templates"
)

func TestBuildReceiverIntegrations(t *testing.T) {
var orgID = rand.Int63()
var version = fmt.Sprintf("Grafana v%d", rand.Uint32())
imageStore := &images.FakeImageStore{}
tmpl := templates.ForTests(t)

webhookFactory := func(n receivers.Metadata) (receivers.WebhookSender, error) {
return receivers.MockNotificationService(), nil
}
emailFactory := func(n receivers.Metadata) (receivers.EmailSender, error) {
return receivers.MockNotificationService(), nil
}
loggerFactory := func(_ string, _ ...interface{}) logging.Logger {
return &logging.FakeLogger{}
}

getFullConfig := func(t *testing.T) (GrafanaReceiverConfig, int) {
recCfg := &APIReceiver{ConfigReceiver: ConfigReceiver{Name: "test-receiver"}}
for notifierType, cfg := range allKnownConfigs {
recCfg.Receivers = append(recCfg.Receivers, cfg.getRawNotifierConfig(notifierType))
}
parsed, err := BuildReceiverConfiguration(context.Background(), recCfg, GetDecryptedValueFnForTesting)
require.NoError(t, err)
return parsed, len(recCfg.Receivers)
}

t.Run("should build all supported notifiers", func(t *testing.T) {
fullCfg, qty := getFullConfig(t)

loggerNames := make(map[string]struct{}, qty)
logger := func(name string, _ ...interface{}) logging.Logger {
loggerNames[name] = struct{}{}
return &logging.FakeLogger{}
}

webhooks := make(map[receivers.Metadata]struct{}, qty)
wh := func(n receivers.Metadata) (receivers.WebhookSender, error) {
webhooks[n] = struct{}{}
return webhookFactory(n)
}

emails := make(map[receivers.Metadata]struct{}, qty)
em := func(n receivers.Metadata) (receivers.EmailSender, error) {
emails[n] = struct{}{}
return emailFactory(n)
}

integrations, err := BuildReceiverIntegrations(fullCfg, tmpl, imageStore, logger, wh, em, orgID, version)

require.NoError(t, err)
require.Len(t, integrations, qty)

t.Run("should call logger factory for each config", func(t *testing.T) {
require.Len(t, loggerNames, qty)
})
t.Run("should call webhook factory for each config that needs it", func(t *testing.T) {
require.Len(t, webhooks, 17) // we have 17 notifiers that support webhook
})
t.Run("should call email factory for each config that needs it", func(t *testing.T) {
require.Len(t, emails, 1) // we have only email notifier that needs sender
})
})
t.Run("should return errors if webhook factory fails", func(t *testing.T) {
fullCfg, _ := getFullConfig(t)
calls := 0
failingFactory := func(n receivers.Metadata) (receivers.WebhookSender, error) {
calls++
return nil, errors.New("bad-test")
}

integrations, err := BuildReceiverIntegrations(fullCfg, tmpl, imageStore, loggerFactory, failingFactory, emailFactory, orgID, version)

require.Empty(t, integrations)
require.NotNil(t, err)
require.ErrorContains(t, err, "bad-test")
require.Greater(t, calls, 0)
})
t.Run("should return errors if email factory fails", func(t *testing.T) {
fullCfg, _ := getFullConfig(t)
calls := 0
failingFactory := func(n receivers.Metadata) (receivers.EmailSender, error) {
calls++
return nil, errors.New("bad-test")
}

integrations, err := BuildReceiverIntegrations(fullCfg, tmpl, imageStore, loggerFactory, webhookFactory, failingFactory, orgID, version)

require.Empty(t, integrations)
require.NotNil(t, err)
require.ErrorContains(t, err, "bad-test")
require.Greater(t, calls, 0)
})
t.Run("should not produce any integration if config is empty", func(t *testing.T) {
cfg := GrafanaReceiverConfig{Name: "test"}

integrations, err := BuildReceiverIntegrations(cfg, tmpl, imageStore, loggerFactory, webhookFactory, emailFactory, orgID, version)

require.NoError(t, err)
require.Empty(t, integrations)
})
}
8 changes: 6 additions & 2 deletions notify/receivers.go
Original file line number Diff line number Diff line change
Expand Up @@ -347,8 +347,12 @@ type NotifierConfig[T interface{}] struct {
Settings T
}

// GetDecryptedValueFn is a function that returns the decrypted value of
// the given key. If the key is not present, then it returns the fallback value.
type GetDecryptedValueFn func(ctx context.Context, sjd map[string][]byte, key string, fallback string) string

// BuildReceiverConfiguration parses, decrypts and validates the APIReceiver.
func BuildReceiverConfiguration(ctx context.Context, api *APIReceiver, decrypt receivers.GetDecryptedValueFn) (GrafanaReceiverConfig, error) {
func BuildReceiverConfiguration(ctx context.Context, api *APIReceiver, decrypt GetDecryptedValueFn) (GrafanaReceiverConfig, error) {
result := GrafanaReceiverConfig{
Name: api.Name,
}
Expand All @@ -365,7 +369,7 @@ func BuildReceiverConfiguration(ctx context.Context, api *APIReceiver, decrypt r
}

// parseNotifier parses receivers and populates the corresponding field in GrafanaReceiverConfig. Returns an error if the configuration cannot be parsed.
func parseNotifier(ctx context.Context, result *GrafanaReceiverConfig, receiver *GrafanaReceiver, decrypt receivers.GetDecryptedValueFn) error {
func parseNotifier(ctx context.Context, result *GrafanaReceiverConfig, receiver *GrafanaReceiver, decrypt GetDecryptedValueFn) error {
secureSettings, err := decodeSecretsFromBase64(receiver.SecureSettings)
if err != nil {
return err
Expand Down
9 changes: 1 addition & 8 deletions notify/receivers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,14 +106,7 @@ func TestProcessNotifierError(t *testing.T) {
}

func TestBuildReceiverConfiguration(t *testing.T) {
var decrypt receivers.GetDecryptedValueFn = func(ctx context.Context, sjd map[string][]byte, key string, fallback string) string {
v, ok := sjd[key]
if !ok {
return fallback
}
return string(v)
}

decrypt := GetDecryptedValueFnForTesting
t.Run("should decode secrets from base64", func(t *testing.T) {
recCfg := &APIReceiver{ConfigReceiver: ConfigReceiver{Name: "test-receiver"}}
for notifierType, cfg := range allKnownConfigs {
Expand Down
6 changes: 6 additions & 0 deletions notify/testing.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import (
"time"

"github.com/prometheus/alertmanager/types"

receiversTesting "github.com/grafana/alerting/receivers/testing"
)

func newFakeMaintanenceOptions(t *testing.T) *fakeMaintenanceOptions {
Expand Down Expand Up @@ -84,3 +86,7 @@ func (f *fakeNotifier) Notify(_ context.Context, _ ...*types.Alert) (bool, error
func (f *fakeNotifier) SendResolved() bool {
return true
}

func GetDecryptedValueFnForTesting(_ context.Context, sjd map[string][]byte, key string, fallback string) string {
return receiversTesting.DecryptForTesting(sjd)(key, fallback)
}
17 changes: 6 additions & 11 deletions receivers/alertmanager/alertmanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,13 @@ import (
"github.com/grafana/alerting/receivers"
)

func New(fc receivers.FactoryConfig) (*Notifier, error) {
settings, err := NewConfig(fc.Config.Settings, fc.Decrypt)
if err != nil {
return nil, err
}

func New(cfg Config, meta receivers.Metadata, images images.ImageStore, logger logging.Logger) *Notifier {
return &Notifier{
Base: receivers.NewBase(fc.Config),
images: fc.ImageStore,
settings: settings,
logger: fc.Logger,
}, nil
Base: receivers.NewBase(meta),
images: images,
settings: cfg,
logger: logger,
}
}

// Notifier sends alert notifications to the alert manager
Expand Down
10 changes: 9 additions & 1 deletion receivers/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,15 @@ func (n *Base) GetDisableResolveMessage() bool {
return n.DisableResolveMessage
}

func NewBase(cfg *NotificationChannelConfig) *Base {
// Metadata contains the metadata of the notifier.
type Metadata struct {
UID string
Name string
Type string
DisableResolveMessage bool
}

func NewBase(cfg Metadata) *Base {
return &Base{
UID: cfg.UID,
Name: cfg.Name,
Expand Down
Loading

0 comments on commit c7c80f3

Please sign in to comment.