Alerting: Remove bridge between log15 and go-kit logger (#43769)

* remove bridge between log15 and go-kit logger.

* fix tests

pkg/services/ngalert/logging/logging.go

@@ -1,63 +0,0 @@
-package logging
-
-import (
-	"strings"
-
-	"github.com/go-kit/log/level"
-
-	glog "github.com/grafana/grafana/pkg/infra/log"
-)
-
-// GoKitWrapper wraps around the grafana-specific logger to make a compatible logger for go-kit.
-type GoKitWrapper struct {
-	logger glog.Logger
-}
-
-// NewWrapper creates a new go-kit wrapper for a grafana-specific logger
-func NewWrapper(l glog.Logger) *GoKitWrapper {
-	return &GoKitWrapper{logger: l}
-}
-
-// Write implements io.Writer
-func (w *GoKitWrapper) Write(p []byte) (n int, err error) {
-	withoutNewline := strings.TrimSuffix(string(p), "\n")
-	w.logger.Info(withoutNewline)
-	return len(p), nil
-}
-
-// Log implements interface go-kit/log/Logger. It tries to extract level and message from the context and writes to underlying message.
-// To successfully extract the log level, the first pair of elements should be 'lvl' and log level. Otherwise, it falls back to info.
-// The following pair should be 'msg' and message. Otherwise, it uses the empty string as message.
-func (w *GoKitWrapper) Log(keyvals ...interface{}) error {
-	if len(keyvals) == 0 {
-		return nil
-	}
-
-	f := w.logger.Info
-	startwith := 0
-	if keyvals[0] == level.Key() {
-		startwith = 2
-		switch keyvals[1] {
-		case level.DebugValue():
-			f = w.logger.Debug
-		case level.InfoValue():
-			f = w.logger.Info
-		case level.WarnValue():
-			f = w.logger.Warn
-		case level.ErrorValue():
-			f = w.logger.Error
-		}
-	}
-
-	msg := ""
-	if keyvals[startwith] == "msg" {
-		str, ok := keyvals[startwith+1].(string)
-		if ok {
-			msg = str
-			startwith += 2
-		}
-	}
-
-	f(msg, keyvals[startwith:]...)
-	return nil
-}

pkg/services/ngalert/logging/logging_test.go

@@ -1,103 +0,0 @@
-package logging
-
-import (
-	"bytes"
-	"fmt"
-	"io"
-	"testing"
-
-	"github.com/go-kit/log"
-	glog "github.com/grafana/grafana/pkg/infra/log"
-	"github.com/grafana/grafana/pkg/infra/log/level"
-	"github.com/stretchr/testify/require"
-)
-
-func Test_GoKitWrapper(t *testing.T) {
-	getLogger := func(writer io.Writer) log.Logger {
-		gLogger := glog.New()
-		gLogger.AddLogger(log.NewLogfmtLogger(writer), "info", map[string]level.Option{})
-		return NewWrapper(gLogger)
-	}
-
-	tests := []struct {
-		level         level.Value
-		expectedLevel string
-	}{
-		{
-			level.DebugValue(),
-			"dbug",
-		},
-		{
-			level.InfoValue(),
-			"info",
-		},
-		{
-			level.WarnValue(),
-			"warn",
-		},
-		{
-			level.ErrorValue(),
-			"eror",
-		},
-	}
-	for _, test := range tests {
-		t.Run(fmt.Sprintf("when level %s", test.level.String()), func(t *testing.T) {
-			t.Run(fmt.Sprintf("rendered message should contain the level %s", test.expectedLevel), func(t *testing.T) {
-				var data bytes.Buffer
-				gokitLogger := getLogger(&data)
-				_ = log.WithPrefix(gokitLogger, level.Key(), test.level).Log("msg", "test", "some", "more", "context", "data")
-
-				str := data.String()
-				require.Contains(t, str, fmt.Sprintf("lvl=%s msg=test some=more context=data", test.expectedLevel))
-			})
-		})
-	}
-
-	t.Run("use info when level does not exist", func(t *testing.T) {
-		var data bytes.Buffer
-		gokitLogger := getLogger(&data)
-		_ = gokitLogger.Log("msg", "test", "some", "more", "context", "data")
-		str := data.String()
-		require.Contains(t, str, "lvl=info msg=test some=more context=data")
-	})
-	t.Run("use empty msg when context misses msg", func(t *testing.T) {
-		var data bytes.Buffer
-		gokitLogger := getLogger(&data)
-		_ = gokitLogger.Log("message", "test", "some", "more", "context", "data")
-		str := data.String()
-		require.Contains(t, str, "lvl=info msg= message=test some=more context=data")
-	})
-}
-
-func Benchmark_Baseline(t *testing.B) {
-	gLogger := glog.New()
-	var buff bytes.Buffer
-	gLogger.AddLogger(log.NewLogfmtLogger(&buff), "info", map[string]level.Option{})
-
-	for i := 0; i < t.N; i++ {
-		gLogger.Info("test", "some", "more", "context", "data")
-	}
-}
-
-func Benchmark_WrapperLogger(t *testing.B) {
-	gLogger := glog.New()
-	var buff bytes.Buffer
-	gLogger.AddLogger(log.NewLogfmtLogger(&buff), "info", map[string]level.Option{})
-
-	gokit := NewWrapper(gLogger)
-
-	for i := 0; i < t.N; i++ {
-		_ = level.Info(gokit).Log("msg", "test", "some", "more", "context", "data")
-	}
-}
-
-func Benchmark_WrapperWriter(t *testing.B) {
-	gLogger := glog.New()
-	var buff bytes.Buffer
-	gLogger.AddLogger(log.NewLogfmtLogger(&buff), "info", map[string]level.Option{})
-	gokit := NewWrapper(gLogger)
-
-	for i := 0; i < t.N; i++ {
-		_ = level.Info(gokit).Log("msg", "test", "some", "more", "context", "data")
-	}
-}

pkg/services/ngalert/notifier/alertmanager.go

@@ -15,7 +15,6 @@ import (
 	"time"
 	"unicode/utf8"
 
-	gokit_log "github.com/go-kit/kit/log"
 	amv2 "github.com/prometheus/alertmanager/api/v2/models"
 	"github.com/prometheus/alertmanager/cluster"
 	"github.com/prometheus/alertmanager/config"
@@ -37,7 +36,6 @@ import (
 	"github.com/grafana/grafana/pkg/infra/kvstore"
 	"github.com/grafana/grafana/pkg/infra/log"
 	apimodels "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions"
-	"github.com/grafana/grafana/pkg/services/ngalert/logging"
 	"github.com/grafana/grafana/pkg/services/ngalert/metrics"
 	ngmodels "github.com/grafana/grafana/pkg/services/ngalert/models"
 	"github.com/grafana/grafana/pkg/services/ngalert/notifier/channels"
@@ -86,8 +84,7 @@ type ClusterPeer interface {
 }
 
 type Alertmanager struct {
-	logger      log.Logger
+	logger log.Logger
-	gokitLogger gokit_log.Logger
 
 	Settings  *setting.Cfg
 	Store     store.AlertingStore
@@ -144,7 +141,6 @@ func newAlertmanager(ctx context.Context, orgID int64, cfg *setting.Cfg, store s
 		decryptFn:         decryptFn,
 	}
 
-	am.gokitLogger = logging.NewWrapper(am.logger)
 	am.fileStore = NewFileStore(am.orgID, kvStore, am.WorkingDirPath())
 
 	nflogFilepath, err := am.fileStore.FilepathFor(ctx, notificationLogFilename)
@@ -193,7 +189,7 @@ func newAlertmanager(ctx context.Context, orgID int64, cfg *setting.Cfg, store s
 	}()
 
 	// Initialize in-memory alerts
-	am.alerts, err = mem.NewAlerts(context.Background(), am.marker, memoryAlertsGCInterval, nil, am.gokitLogger)
+	am.alerts, err = mem.NewAlerts(context.Background(), am.marker, memoryAlertsGCInterval, nil, am.logger)
 	if err != nil {
 		return nil, fmt.Errorf("unable to initialize the alert provider component of alerting: %w", err)
 	}
@@ -400,9 +396,9 @@ func (am *Alertmanager) applyConfig(cfg *apimodels.PostableUserConfig, rawConfig
 		am.dispatcher.Stop()
 	}
 
-	am.inhibitor = inhibit.NewInhibitor(am.alerts, cfg.AlertmanagerConfig.InhibitRules, am.marker, am.gokitLogger)
+	am.inhibitor = inhibit.NewInhibitor(am.alerts, cfg.AlertmanagerConfig.InhibitRules, am.marker, am.logger)
 	am.muteTimes = am.buildMuteTimesMap(cfg.AlertmanagerConfig.MuteTimeIntervals)
-	am.silencer = silence.NewSilencer(am.silences, am.marker, am.gokitLogger)
+	am.silencer = silence.NewSilencer(am.silences, am.marker, am.logger)
 
 	meshStage := notify.NewGossipSettleStage(am.peer)
 	inhibitionStage := notify.NewMuteStage(am.inhibitor)
@@ -414,7 +410,7 @@ func (am *Alertmanager) applyConfig(cfg *apimodels.PostableUserConfig, rawConfig
 	}
 
 	am.route = dispatch.NewRoute(cfg.AlertmanagerConfig.Route.AsAMRoute(), nil)
-	am.dispatcher = dispatch.NewDispatcher(am.alerts, am.route, routingStage, am.marker, am.timeoutFunc, &nilLimits{}, am.gokitLogger, am.dispatcherMetrics)
+	am.dispatcher = dispatch.NewDispatcher(am.alerts, am.route, routingStage, am.marker, am.timeoutFunc, &nilLimits{}, am.logger, am.dispatcherMetrics)
 
 	am.wg.Add(1)
 	go func() {

pkg/services/ngalert/notifier/alertmanager_test.go

@@ -11,22 +11,21 @@ import (
 
 	"github.com/grafana/grafana/pkg/services/secrets/database"
 
-	gokit_log "github.com/go-kit/kit/log"
 	"github.com/go-openapi/strfmt"
-	"github.com/grafana/grafana/pkg/infra/log"
-	apimodels "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions"
-	"github.com/grafana/grafana/pkg/services/ngalert/logging"
-	"github.com/grafana/grafana/pkg/services/ngalert/metrics"
-	"github.com/grafana/grafana/pkg/services/ngalert/store"
-	secretsManager "github.com/grafana/grafana/pkg/services/secrets/manager"
-	"github.com/grafana/grafana/pkg/services/sqlstore"
-	"github.com/grafana/grafana/pkg/setting"
 	"github.com/prometheus/alertmanager/api/v2/models"
 	"github.com/prometheus/alertmanager/provider/mem"
 	"github.com/prometheus/alertmanager/types"
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/common/model"
 	"github.com/stretchr/testify/require"
+
+	"github.com/grafana/grafana/pkg/infra/log"
+	apimodels "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions"
+	"github.com/grafana/grafana/pkg/services/ngalert/metrics"
+	"github.com/grafana/grafana/pkg/services/ngalert/store"
+	secretsManager "github.com/grafana/grafana/pkg/services/secrets/manager"
+	"github.com/grafana/grafana/pkg/services/sqlstore"
+	"github.com/grafana/grafana/pkg/setting"
 )
 
 func setupAMTest(t *testing.T) *Alertmanager {
@@ -317,7 +316,7 @@ func TestPutAlert(t *testing.T) {
 		t.Run(c.title, func(t *testing.T) {
 			r := prometheus.NewRegistry()
 			am.marker = types.NewMarker(r)
-			am.alerts, err = mem.NewAlerts(context.Background(), am.marker, 15*time.Minute, nil, gokit_log.NewLogfmtLogger(logging.NewWrapper(am.logger)))
+			am.alerts, err = mem.NewAlerts(context.Background(), am.marker, 15*time.Minute, nil, am.logger)
 			require.NoError(t, err)
 
 			alerts := []*types.Alert{}

pkg/services/ngalert/notifier/channels/template_data.go

@@ -8,14 +8,12 @@ import (
 	"strings"
 	"time"
 
-	gokit_log "github.com/go-kit/kit/log"
 	"github.com/prometheus/alertmanager/notify"
 	"github.com/prometheus/alertmanager/template"
 	"github.com/prometheus/alertmanager/types"
 	"github.com/prometheus/common/model"
 
 	"github.com/grafana/grafana/pkg/infra/log"
-	"github.com/grafana/grafana/pkg/services/ngalert/logging"
 	ngmodels "github.com/grafana/grafana/pkg/services/ngalert/models"
 )
 
@@ -129,7 +127,7 @@ func ExtendData(data *template.Data, logger log.Logger) *ExtendedData {
 }
 
 func TmplText(ctx context.Context, tmpl *template.Template, alerts []*types.Alert, l log.Logger, tmplErr *error) (func(string) string, *ExtendedData) {
-	promTmplData := notify.GetTemplateData(ctx, tmpl, alerts, gokit_log.NewLogfmtLogger(logging.NewWrapper(l)))
+	promTmplData := notify.GetTemplateData(ctx, tmpl, alerts, l)
 	data := ExtendData(promTmplData, l)
 
 	return func(name string) (s string) {

pkg/services/ngalert/notifier/multiorg_alertmanager.go

@@ -11,16 +11,15 @@ import (
 
 	"github.com/grafana/grafana/pkg/services/ngalert/notifier/channels"
 
-	gokit_log "github.com/go-kit/kit/log"
+	"github.com/prometheus/alertmanager/cluster"
+	"github.com/prometheus/client_golang/prometheus"
+
 	"github.com/grafana/grafana/pkg/infra/kvstore"
 	"github.com/grafana/grafana/pkg/infra/log"
-	"github.com/grafana/grafana/pkg/services/ngalert/logging"
 	"github.com/grafana/grafana/pkg/services/ngalert/metrics"
 	"github.com/grafana/grafana/pkg/services/ngalert/models"
 	"github.com/grafana/grafana/pkg/services/ngalert/store"
 	"github.com/grafana/grafana/pkg/setting"
-	"github.com/prometheus/alertmanager/cluster"
-	"github.com/prometheus/client_golang/prometheus"
 )
 
 var (
@@ -62,7 +61,7 @@ func NewMultiOrgAlertmanager(cfg *setting.Cfg, configStore store.AlertingStore,
 		metrics:       m,
 	}
 
-	clusterLogger := gokit_log.With(gokit_log.NewLogfmtLogger(logging.NewWrapper(l)), "component", "cluster")
+	clusterLogger := l.New("component", "cluster")
 	moa.peer = &NilPeer{}
 	if len(cfg.UnifiedAlerting.HAPeers) > 0 {
 		peer, err := cluster.Create(

pkg/services/ngalert/sender/sender.go

@@ -9,11 +9,9 @@ import (
 
 	"github.com/grafana/grafana/pkg/infra/log"
 	apimodels "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions"
-	"github.com/grafana/grafana/pkg/services/ngalert/logging"
 	"github.com/grafana/grafana/pkg/services/ngalert/metrics"
 	ngmodels "github.com/grafana/grafana/pkg/services/ngalert/models"
 
-	gokit_log "github.com/go-kit/kit/log"
 	"github.com/prometheus/alertmanager/api/v2/models"
 	"github.com/prometheus/client_golang/prometheus"
 	common_config "github.com/prometheus/common/config"
@@ -31,9 +29,8 @@ const (
 
 // Sender is responsible for dispatching alert notifications to an external Alertmanager service.
 type Sender struct {
-	logger      log.Logger
+	logger log.Logger
-	gokitLogger gokit_log.Logger
+	wg     sync.WaitGroup
-	wg          sync.WaitGroup
 
 	manager *notifier.Manager
 
@@ -45,19 +42,18 @@ func New(_ *metrics.Scheduler) (*Sender, error) {
 	l := log.New("sender")
 	sdCtx, sdCancel := context.WithCancel(context.Background())
 	s := &Sender{
-		logger:      l,
+		logger:   l,
-		gokitLogger: gokit_log.NewLogfmtLogger(logging.NewWrapper(l)),
+		sdCancel: sdCancel,
-		sdCancel:    sdCancel,
 	}
 
 	s.manager = notifier.NewManager(
 		// Injecting a new registry here means these metrics are not exported.
 		// Once we fix the individual Alertmanager metrics we should fix this scenario too.
 		&notifier.Options{QueueCapacity: defaultMaxQueueCapacity, Registerer: prometheus.NewRegistry()},
-		s.gokitLogger,
+		s.logger,
 	)
 
-	s.sdManager = discovery.NewManager(sdCtx, s.gokitLogger)
+	s.sdManager = discovery.NewManager(sdCtx, s.logger)
 
 	return s, nil
 }