Skip to content

Commit 81ac414

Browse files
56quarters56quarters
authored
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 <nick.pillitteri@grafana.com>
1 parent 090988c commit 81ac414

File tree

5 files changed

+31
-23
lines changed

5 files changed

+31
-23
lines changed

‎pkg/cortex/modules.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -827,7 +827,7 @@ func (t *Cortex) initQueryScheduler() (services.Service, error) {
827827
}
828828

829829
func (t *Cortex) setupModuleManager() error {
830-
mm := modules.NewManager()
830+
mm := modules.NewManager(util_log.Logger)
831831

832832
// Register all modules here.
833833
// RegisterModule(name string, initFn func()(services.Service, error))

‎pkg/util/module_service.go

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,10 @@ import (
44
"context"
55
"fmt"
66

7+
"github.com/go-kit/kit/log"
78
"github.com/go-kit/kit/log/level"
89
"github.com/pkg/errors"
910

10-
util_log "github.com/cortexproject/cortex/pkg/util/log"
1111
"github.com/cortexproject/cortex/pkg/util/services"
1212
)
1313

@@ -18,6 +18,7 @@ type moduleService struct {
1818

1919
service services.Service
2020
name string
21+
logger log.Logger
2122

2223
// startDeps, stopDeps return map of service names to services
2324
startDeps, stopDeps func(string) map[string]services.Service
@@ -26,9 +27,10 @@ type moduleService struct {
2627
// NewModuleService wraps a module service, and makes sure that dependencies are started/stopped before module service starts or stops.
2728
// If any dependency fails to start, this service fails as well.
2829
// On stop, errors from failed dependencies are ignored.
29-
func NewModuleService(name string, service services.Service, startDeps, stopDeps func(string) map[string]services.Service) services.Service {
30+
func NewModuleService(name string, logger log.Logger, service services.Service, startDeps, stopDeps func(string) map[string]services.Service) services.Service {
3031
w := &moduleService{
3132
name: name,
33+
logger: logger,
3234
service: service,
3335
startDeps: startDeps,
3436
stopDeps: stopDeps,
@@ -46,7 +48,7 @@ func (w *moduleService) start(serviceContext context.Context) error {
4648
continue
4749
}
4850

49-
level.Debug(util_log.Logger).Log("msg", "module waiting for initialization", "module", w.name, "waiting_for", m)
51+
level.Debug(w.logger).Log("msg", "module waiting for initialization", "module", w.name, "waiting_for", m)
5052

5153
err := s.AwaitRunning(serviceContext)
5254
if err != nil {
@@ -56,7 +58,7 @@ func (w *moduleService) start(serviceContext context.Context) error {
5658

5759
// we don't want to let this service to stop until all dependant services are stopped,
5860
// so we use independent context here
59-
level.Info(util_log.Logger).Log("msg", "initialising", "module", w.name)
61+
level.Info(w.logger).Log("msg", "initialising", "module", w.name)
6062
err := w.service.StartAsync(context.Background())
6163
if err != nil {
6264
return errors.Wrapf(err, "error starting module: %s", w.name)
@@ -78,17 +80,17 @@ func (w *moduleService) stop(_ error) error {
7880
// Only wait for other modules, if underlying service is still running.
7981
w.waitForModulesToStop()
8082

81-
level.Debug(util_log.Logger).Log("msg", "stopping", "module", w.name)
83+
level.Debug(w.logger).Log("msg", "stopping", "module", w.name)
8284

8385
err = services.StopAndAwaitTerminated(context.Background(), w.service)
8486
} else {
8587
err = w.service.FailureCase()
8688
}
8789

8890
if err != nil && err != ErrStopProcess {
89-
level.Warn(util_log.Logger).Log("msg", "module failed with error", "module", w.name, "err", err)
91+
level.Warn(w.logger).Log("msg", "module failed with error", "module", w.name, "err", err)
9092
} else {
91-
level.Info(util_log.Logger).Log("msg", "module stopped", "module", w.name)
93+
level.Info(w.logger).Log("msg", "module stopped", "module", w.name)
9294
}
9395
return err
9496
}
@@ -101,7 +103,7 @@ func (w *moduleService) waitForModulesToStop() {
101103
continue
102104
}
103105

104-
level.Debug(util_log.Logger).Log("msg", "module waiting for", "module", w.name, "waiting_for", n)
106+
level.Debug(w.logger).Log("msg", "module waiting for", "module", w.name, "waiting_for", n)
105107
// Passed context isn't canceled, so we can only get error here, if service
106108
// fails. But we don't care *how* service stops, as long as it is done.
107109
_ = s.AwaitTerminated(context.Background())

‎pkg/util/modules/module_service_wrapper.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
package modules
22

33
import (
4+
"github.com/go-kit/kit/log"
5+
46
"github.com/cortexproject/cortex/pkg/util"
57
"github.com/cortexproject/cortex/pkg/util/services"
68
)
79

810
// This function wraps module service, and adds waiting for dependencies to start before starting,
911
// and dependant modules to stop before stopping this module service.
10-
func newModuleServiceWrapper(serviceMap map[string]services.Service, mod string, modServ services.Service, startDeps []string, stopDeps []string) services.Service {
12+
func newModuleServiceWrapper(serviceMap map[string]services.Service, mod string, logger log.Logger, modServ services.Service, startDeps []string, stopDeps []string) services.Service {
1113
getDeps := func(deps []string) map[string]services.Service {
1214
r := map[string]services.Service{}
1315
for _, m := range deps {
@@ -19,7 +21,7 @@ func newModuleServiceWrapper(serviceMap map[string]services.Service, mod string,
1921
return r
2022
}
2123

22-
return util.NewModuleService(mod, modServ,
24+
return util.NewModuleService(mod, logger, modServ,
2325
func(_ string) map[string]services.Service {
2426
return getDeps(startDeps)
2527
},

‎pkg/util/modules/modules.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"fmt"
55
"sort"
66

7+
"github.com/go-kit/kit/log"
78
"github.com/pkg/errors"
89

910
"github.com/cortexproject/cortex/pkg/util/services"
@@ -25,6 +26,7 @@ type module struct {
2526
// in the right order of dependencies.
2627
type Manager struct {
2728
modules map[string]*module
29+
logger log.Logger
2830
}
2931

3032
// 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) {
3335
}
3436

3537
// NewManager creates a new Manager
36-
func NewManager() *Manager {
38+
func NewManager(logger log.Logger) *Manager {
3739
return &Manager{
3840
modules: make(map[string]*module),
41+
logger: logger,
3942
}
4043
}
4144

@@ -108,7 +111,7 @@ func (m *Manager) initModule(name string, initMap map[string]bool, servicesMap m
108111
if s != nil {
109112
// We pass servicesMap, which isn't yet complete. By the time service starts,
110113
// it will be fully built, so there is no need for extra synchronization.
111-
serv = newModuleServiceWrapper(servicesMap, n, s, m.DependenciesForModule(n), m.findInverseDependencies(n, deps[ix+1:]))
114+
serv = newModuleServiceWrapper(servicesMap, n, m.logger, s, m.DependenciesForModule(n), m.findInverseDependencies(n, deps[ix+1:]))
112115
}
113116
}
114117

‎pkg/util/modules/modules_test.go

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"testing"
88
"time"
99

10+
"github.com/go-kit/kit/log"
1011
"github.com/stretchr/testify/assert"
1112
"github.com/stretchr/testify/require"
1213

@@ -36,7 +37,7 @@ func TestDependencies(t *testing.T) {
3637
},
3738
}
3839

39-
mm := NewManager()
40+
mm := NewManager(log.NewNopLogger())
4041
for name, mod := range testModules {
4142
mm.RegisterModule(name, mod.initFn)
4243
}
@@ -75,7 +76,7 @@ func TestDependencies(t *testing.T) {
7576
}
7677

7778
func TestRegisterModuleDefaultsToUserVisible(t *testing.T) {
78-
sut := NewManager()
79+
sut := NewManager(log.NewNopLogger())
7980
sut.RegisterModule("module1", mockInitFunc)
8081

8182
m := sut.modules["module1"]
@@ -88,7 +89,7 @@ func TestFunctionalOptAtTheEndWins(t *testing.T) {
8889
userVisibleMod := func(option *module) {
8990
option.userVisible = true
9091
}
91-
sut := NewManager()
92+
sut := NewManager(log.NewNopLogger())
9293
sut.RegisterModule("mod1", mockInitFunc, UserInvisibleModule, userVisibleMod, UserInvisibleModule)
9394

9495
m := sut.modules["mod1"]
@@ -98,7 +99,7 @@ func TestFunctionalOptAtTheEndWins(t *testing.T) {
9899
}
99100

100101
func TestGetAllUserVisibleModulesNames(t *testing.T) {
101-
sut := NewManager()
102+
sut := NewManager(log.NewNopLogger())
102103
sut.RegisterModule("userVisible3", mockInitFunc)
103104
sut.RegisterModule("userVisible2", mockInitFunc)
104105
sut.RegisterModule("userVisible1", mockInitFunc)
@@ -111,7 +112,7 @@ func TestGetAllUserVisibleModulesNames(t *testing.T) {
111112
}
112113

113114
func TestGetAllUserVisibleModulesNamesHasNoDupWithDependency(t *testing.T) {
114-
sut := NewManager()
115+
sut := NewManager(log.NewNopLogger())
115116
sut.RegisterModule("userVisible1", mockInitFunc)
116117
sut.RegisterModule("userVisible2", mockInitFunc)
117118
sut.RegisterModule("userVisible3", mockInitFunc)
@@ -125,7 +126,7 @@ func TestGetAllUserVisibleModulesNamesHasNoDupWithDependency(t *testing.T) {
125126
}
126127

127128
func TestGetEmptyListWhenThereIsNoUserVisibleModule(t *testing.T) {
128-
sut := NewManager()
129+
sut := NewManager(log.NewNopLogger())
129130
sut.RegisterModule("internal1", mockInitFunc, UserInvisibleModule)
130131
sut.RegisterModule("internal2", mockInitFunc, UserInvisibleModule)
131132
sut.RegisterModule("internal3", mockInitFunc, UserInvisibleModule)
@@ -139,7 +140,7 @@ func TestGetEmptyListWhenThereIsNoUserVisibleModule(t *testing.T) {
139140
func TestIsUserVisibleModule(t *testing.T) {
140141
userVisibleModName := "userVisible"
141142
internalModName := "internal"
142-
sut := NewManager()
143+
sut := NewManager(log.NewNopLogger())
143144
sut.RegisterModule(userVisibleModName, mockInitFunc)
144145
sut.RegisterModule(internalModName, mockInitFunc, UserInvisibleModule)
145146

@@ -157,7 +158,7 @@ func TestIsModuleRegistered(t *testing.T) {
157158
successModule := "successModule"
158159
failureModule := "failureModule"
159160

160-
m := NewManager()
161+
m := NewManager(log.NewNopLogger())
161162
m.RegisterModule(successModule, mockInitFunc)
162163

163164
var result = m.IsModuleRegistered(successModule)
@@ -168,7 +169,7 @@ func TestIsModuleRegistered(t *testing.T) {
168169
}
169170

170171
func TestDependenciesForModule(t *testing.T) {
171-
m := NewManager()
172+
m := NewManager(log.NewNopLogger())
172173
m.RegisterModule("test", nil)
173174
m.RegisterModule("dep1", nil)
174175
m.RegisterModule("dep2", nil)
@@ -206,7 +207,7 @@ func TestModuleWaitsForAllDependencies(t *testing.T) {
206207
}, nil), nil
207208
}
208209

209-
m := NewManager()
210+
m := NewManager(log.NewNopLogger())
210211
m.RegisterModule("A", initA)
211212
m.RegisterModule("B", nil)
212213
m.RegisterModule("C", initC)

0 commit comments

Comments
 (0)