Replace use of global loggers in the module subsystem (#4425)

In prep for moving the "modules" package to dskit, replace uses
of the global Cortex logger with an injected one.

Signed-off-by: Nick Pillitteri <[email protected]>

Author: 56quarters

pkg/cortex/modules.go

@@ -827,7 +827,7 @@ func (t *Cortex) initQueryScheduler() (services.Service, error) {
 }
 
 func (t *Cortex) setupModuleManager() error {
-	mm := modules.NewManager()
+	mm := modules.NewManager(util_log.Logger)
 
 	// Register all modules here.
 	// RegisterModule(name string, initFn func()(services.Service, error))

pkg/util/module_service.go

@@ -4,10 +4,10 @@ import (
 	"context"
 	"fmt"
 
+	"github.com/go-kit/kit/log"
 	"github.com/go-kit/kit/log/level"
 	"github.com/pkg/errors"
 
-	util_log "github.com/cortexproject/cortex/pkg/util/log"
 	"github.com/cortexproject/cortex/pkg/util/services"
 )
 
@@ -18,6 +18,7 @@ type moduleService struct {
 
 	service services.Service
 	name    string
+	logger  log.Logger
 
 	// startDeps, stopDeps return map of service names to services
 	startDeps, stopDeps func(string) map[string]services.Service
@@ -26,9 +27,10 @@ type moduleService struct {
 // NewModuleService wraps a module service, and makes sure that dependencies are started/stopped before module service starts or stops.
 // If any dependency fails to start, this service fails as well.
 // On stop, errors from failed dependencies are ignored.
-func NewModuleService(name string, service services.Service, startDeps, stopDeps func(string) map[string]services.Service) services.Service {
+func NewModuleService(name string, logger log.Logger, service services.Service, startDeps, stopDeps func(string) map[string]services.Service) services.Service {
 	w := &moduleService{
 		name:      name,
+		logger:    logger,
 		service:   service,
 		startDeps: startDeps,
 		stopDeps:  stopDeps,
@@ -46,7 +48,7 @@ func (w *moduleService) start(serviceContext context.Context) error {
 			continue
 		}
 
-		level.Debug(util_log.Logger).Log("msg", "module waiting for initialization", "module", w.name, "waiting_for", m)
+		level.Debug(w.logger).Log("msg", "module waiting for initialization", "module", w.name, "waiting_for", m)
 
 		err := s.AwaitRunning(serviceContext)
 		if err != nil {
@@ -56,7 +58,7 @@ func (w *moduleService) start(serviceContext context.Context) error {
 
 	// we don't want to let this service to stop until all dependant services are stopped,
 	// so we use independent context here
-	level.Info(util_log.Logger).Log("msg", "initialising", "module", w.name)
+	level.Info(w.logger).Log("msg", "initialising", "module", w.name)
 	err := w.service.StartAsync(context.Background())
 	if err != nil {
 		return errors.Wrapf(err, "error starting module: %s", w.name)
@@ -78,17 +80,17 @@ func (w *moduleService) stop(_ error) error {
 		// Only wait for other modules, if underlying service is still running.
 		w.waitForModulesToStop()
 
-		level.Debug(util_log.Logger).Log("msg", "stopping", "module", w.name)
+		level.Debug(w.logger).Log("msg", "stopping", "module", w.name)
 
 		err = services.StopAndAwaitTerminated(context.Background(), w.service)
 	} else {
 		err = w.service.FailureCase()
 	}
 
 	if err != nil && err != ErrStopProcess {
-		level.Warn(util_log.Logger).Log("msg", "module failed with error", "module", w.name, "err", err)
+		level.Warn(w.logger).Log("msg", "module failed with error", "module", w.name, "err", err)
 	} else {
-		level.Info(util_log.Logger).Log("msg", "module stopped", "module", w.name)
+		level.Info(w.logger).Log("msg", "module stopped", "module", w.name)
 	}
 	return err
 }
@@ -101,7 +103,7 @@ func (w *moduleService) waitForModulesToStop() {
 			continue
 		}
 
-		level.Debug(util_log.Logger).Log("msg", "module waiting for", "module", w.name, "waiting_for", n)
+		level.Debug(w.logger).Log("msg", "module waiting for", "module", w.name, "waiting_for", n)
 		// Passed context isn't canceled, so we can only get error here, if service
 		// fails. But we don't care *how* service stops, as long as it is done.
 		_ = s.AwaitTerminated(context.Background())

pkg/util/modules/module_service_wrapper.go

@@ -1,13 +1,15 @@
 package modules
 
 import (
+	"github.com/go-kit/kit/log"
+
 	"github.com/cortexproject/cortex/pkg/util"
 	"github.com/cortexproject/cortex/pkg/util/services"
 )
 
 // This function wraps module service, and adds waiting for dependencies to start before starting,
 // and dependant modules to stop before stopping this module service.
-func newModuleServiceWrapper(serviceMap map[string]services.Service, mod string, modServ services.Service, startDeps []string, stopDeps []string) services.Service {
+func newModuleServiceWrapper(serviceMap map[string]services.Service, mod string, logger log.Logger, modServ services.Service, startDeps []string, stopDeps []string) services.Service {
 	getDeps := func(deps []string) map[string]services.Service {
 		r := map[string]services.Service{}
 		for _, m := range deps {
@@ -19,7 +21,7 @@ func newModuleServiceWrapper(serviceMap map[string]services.Service, mod string,
 		return r
 	}
 
-	return util.NewModuleService(mod, modServ,
+	return util.NewModuleService(mod, logger, modServ,
 		func(_ string) map[string]services.Service {
 			return getDeps(startDeps)
 		},

pkg/util/modules/modules.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"sort"
 
+	"github.com/go-kit/kit/log"
 	"github.com/pkg/errors"
 
 	"github.com/cortexproject/cortex/pkg/util/services"
@@ -25,6 +26,7 @@ type module struct {
 // in the right order of dependencies.
 type Manager struct {
 	modules map[string]*module
+	logger  log.Logger
 }
 
 // UserInvisibleModule is an option for `RegisterModule` that marks module not visible to user. Modules are user visible by default.
@@ -33,9 +35,10 @@ func UserInvisibleModule(m *module) {
 }
 
 // NewManager creates a new Manager
-func NewManager() *Manager {
+func NewManager(logger log.Logger) *Manager {
 	return &Manager{
 		modules: make(map[string]*module),
+		logger:  logger,
 	}
 }
 
@@ -108,7 +111,7 @@ func (m *Manager) initModule(name string, initMap map[string]bool, servicesMap m
 			if s != nil {
 				// We pass servicesMap, which isn't yet complete. By the time service starts,
 				// it will be fully built, so there is no need for extra synchronization.
-				serv = newModuleServiceWrapper(servicesMap, n, s, m.DependenciesForModule(n), m.findInverseDependencies(n, deps[ix+1:]))
+				serv = newModuleServiceWrapper(servicesMap, n, m.logger, s, m.DependenciesForModule(n), m.findInverseDependencies(n, deps[ix+1:]))
 			}
 		}
 

pkg/util/modules/modules_test.go

@@ -7,6 +7,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/go-kit/kit/log"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
@@ -36,7 +37,7 @@ func TestDependencies(t *testing.T) {
 		},
 	}
 
-	mm := NewManager()
+	mm := NewManager(log.NewNopLogger())
 	for name, mod := range testModules {
 		mm.RegisterModule(name, mod.initFn)
 	}
@@ -75,7 +76,7 @@ func TestDependencies(t *testing.T) {
 }
 
 func TestRegisterModuleDefaultsToUserVisible(t *testing.T) {
-	sut := NewManager()
+	sut := NewManager(log.NewNopLogger())
 	sut.RegisterModule("module1", mockInitFunc)
 
 	m := sut.modules["module1"]
@@ -88,7 +89,7 @@ func TestFunctionalOptAtTheEndWins(t *testing.T) {
 	userVisibleMod := func(option *module) {
 		option.userVisible = true
 	}
-	sut := NewManager()
+	sut := NewManager(log.NewNopLogger())
 	sut.RegisterModule("mod1", mockInitFunc, UserInvisibleModule, userVisibleMod, UserInvisibleModule)
 
 	m := sut.modules["mod1"]
@@ -98,7 +99,7 @@ func TestFunctionalOptAtTheEndWins(t *testing.T) {
 }
 
 func TestGetAllUserVisibleModulesNames(t *testing.T) {
-	sut := NewManager()
+	sut := NewManager(log.NewNopLogger())
 	sut.RegisterModule("userVisible3", mockInitFunc)
 	sut.RegisterModule("userVisible2", mockInitFunc)
 	sut.RegisterModule("userVisible1", mockInitFunc)
@@ -111,7 +112,7 @@ func TestGetAllUserVisibleModulesNames(t *testing.T) {
 }
 
 func TestGetAllUserVisibleModulesNamesHasNoDupWithDependency(t *testing.T) {
-	sut := NewManager()
+	sut := NewManager(log.NewNopLogger())
 	sut.RegisterModule("userVisible1", mockInitFunc)
 	sut.RegisterModule("userVisible2", mockInitFunc)
 	sut.RegisterModule("userVisible3", mockInitFunc)
@@ -125,7 +126,7 @@ func TestGetAllUserVisibleModulesNamesHasNoDupWithDependency(t *testing.T) {
 }
 
 func TestGetEmptyListWhenThereIsNoUserVisibleModule(t *testing.T) {
-	sut := NewManager()
+	sut := NewManager(log.NewNopLogger())
 	sut.RegisterModule("internal1", mockInitFunc, UserInvisibleModule)
 	sut.RegisterModule("internal2", mockInitFunc, UserInvisibleModule)
 	sut.RegisterModule("internal3", mockInitFunc, UserInvisibleModule)
@@ -139,7 +140,7 @@ func TestGetEmptyListWhenThereIsNoUserVisibleModule(t *testing.T) {
 func TestIsUserVisibleModule(t *testing.T) {
 	userVisibleModName := "userVisible"
 	internalModName := "internal"
-	sut := NewManager()
+	sut := NewManager(log.NewNopLogger())
 	sut.RegisterModule(userVisibleModName, mockInitFunc)
 	sut.RegisterModule(internalModName, mockInitFunc, UserInvisibleModule)
 
@@ -157,7 +158,7 @@ func TestIsModuleRegistered(t *testing.T) {
 	successModule := "successModule"
 	failureModule := "failureModule"
 
-	m := NewManager()
+	m := NewManager(log.NewNopLogger())
 	m.RegisterModule(successModule, mockInitFunc)
 
 	var result = m.IsModuleRegistered(successModule)
@@ -168,7 +169,7 @@ func TestIsModuleRegistered(t *testing.T) {
 }
 
 func TestDependenciesForModule(t *testing.T) {
-	m := NewManager()
+	m := NewManager(log.NewNopLogger())
 	m.RegisterModule("test", nil)
 	m.RegisterModule("dep1", nil)
 	m.RegisterModule("dep2", nil)
@@ -206,7 +207,7 @@ func TestModuleWaitsForAllDependencies(t *testing.T) {
 		}, nil), nil
 	}
 
-	m := NewManager()
+	m := NewManager(log.NewNopLogger())
 	m.RegisterModule("A", initA)
 	m.RegisterModule("B", nil)
 	m.RegisterModule("C", initC)