-
Notifications
You must be signed in to change notification settings - Fork 297
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow fx.Module-specific custom loggers #947
Changes from 5 commits
0da6884
fdd16e1
d26835c
43bba89
1f525ac
fc72859
94e06e3
432c2dd
d18ea0c
2e4bbbb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -26,6 +26,7 @@ import ( | |||||
"go.uber.org/dig" | ||||||
"go.uber.org/fx/fxevent" | ||||||
"go.uber.org/fx/internal/fxreflect" | ||||||
"go.uber.org/multierr" | ||||||
) | ||||||
|
||||||
// A container represents a set of constructors to provide | ||||||
|
@@ -79,14 +80,16 @@ func (o moduleOption) apply(mod *module) { | |||||
} | ||||||
|
||||||
type module struct { | ||||||
parent *module | ||||||
name string | ||||||
scope scope | ||||||
provides []provide | ||||||
invokes []invoke | ||||||
decorators []decorator | ||||||
modules []*module | ||||||
app *App | ||||||
parent *module | ||||||
name string | ||||||
scope scope | ||||||
provides []provide | ||||||
invokes []invoke | ||||||
decorators []decorator | ||||||
modules []*module | ||||||
app *App | ||||||
log fxevent.Logger | ||||||
logConstructor *provide | ||||||
} | ||||||
|
||||||
// scope is a private wrapper interface for dig.Container and dig.Scope. | ||||||
|
@@ -107,9 +110,20 @@ type scope interface { | |||||
func (m *module) build(app *App, root *dig.Container) { | ||||||
if m.parent == nil { | ||||||
m.scope = root | ||||||
m.log = app.log | ||||||
} else { | ||||||
parentScope := m.parent.scope | ||||||
m.scope = parentScope.Scope(m.name) | ||||||
// use parent module's logger by default | ||||||
m.log = m.parent.log | ||||||
} | ||||||
|
||||||
if m.logConstructor != nil { | ||||||
// Since user supplied a custom logger, use a buffered logger | ||||||
// to hold all messages until user supplied logger is | ||||||
// instantiated. Then we flush those messages after fully | ||||||
// constructing the custom logger. | ||||||
m.log = new(logBuffer) | ||||||
} | ||||||
|
||||||
for _, mod := range m.modules { | ||||||
|
@@ -158,7 +172,50 @@ func (m *module) provide(p provide) { | |||||
Err: m.app.err, | ||||||
} | ||||||
} | ||||||
m.app.log.LogEvent(ev) | ||||||
m.log.LogEvent(ev) | ||||||
} | ||||||
|
||||||
// Constructs custom loggers for all modules in the tree | ||||||
func (m *module) constructAllCustomLoggers() { | ||||||
if m.logConstructor != nil { | ||||||
if buffer, ok := m.log.(*logBuffer); ok { | ||||||
// default to app logger if custom logger constructor fails | ||||||
if err := m.constructCustomLogger(buffer); err != nil { | ||||||
m.app.err = multierr.Append(m.app.err, err) | ||||||
m.log = m.parent.log | ||||||
buffer.Connect(m.log) | ||||||
} | ||||||
} | ||||||
} | ||||||
|
||||||
for _, mod := range m.modules { | ||||||
mod.constructAllCustomLoggers() | ||||||
} | ||||||
} | ||||||
|
||||||
// Mirroring the behavior of app.constructCustomLogger | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We're copy-pasting a lot of code here. Looking at app.constructCustomLogger, we can reuse this method and remove the duplication. i.e. the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To do this, I would need to refactor the whole initialization process of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can file a follow-up task to refactor it if you believe this PR is getting too large. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. GO-1690 created |
||||||
func (m *module) constructCustomLogger(buffer *logBuffer) (err error) { | ||||||
p := m.logConstructor | ||||||
fname := fxreflect.FuncName(p.Target) | ||||||
defer func() { | ||||||
m.log.LogEvent(&fxevent.LoggerInitialized{ | ||||||
Err: err, | ||||||
ConstructorName: fname, | ||||||
}) | ||||||
}() | ||||||
|
||||||
if err := m.scope.Provide(p.Target); err != nil { | ||||||
return fmt.Errorf("fx.WithLogger(%v) from:\n%+v\nin Module: %v\nFailed: %w", | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use
Suggested change
|
||||||
fname, p.Stack, m.name, err) | ||||||
} | ||||||
|
||||||
// TODO: Use dig.FillProvideInfo to inspect the provided constructor | ||||||
// and fail the application if its signature didn't match. | ||||||
|
||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe this comment is misplaced. It should be placed above the |
||||||
return m.scope.Invoke(func(log fxevent.Logger) { | ||||||
m.log = log | ||||||
buffer.Connect(log) | ||||||
}) | ||||||
} | ||||||
|
||||||
func (m *module) executeInvokes() error { | ||||||
|
@@ -179,12 +236,12 @@ func (m *module) executeInvokes() error { | |||||
|
||||||
func (m *module) executeInvoke(i invoke) (err error) { | ||||||
fnName := fxreflect.FuncName(i.Target) | ||||||
m.app.log.LogEvent(&fxevent.Invoking{ | ||||||
m.log.LogEvent(&fxevent.Invoking{ | ||||||
FunctionName: fnName, | ||||||
ModuleName: m.name, | ||||||
}) | ||||||
err = runInvoke(m.scope, i) | ||||||
m.app.log.LogEvent(&fxevent.Invoked{ | ||||||
m.log.LogEvent(&fxevent.Invoked{ | ||||||
FunctionName: fnName, | ||||||
ModuleName: m.name, | ||||||
Err: err, | ||||||
|
@@ -203,14 +260,14 @@ func (m *module) decorate() (err error) { | |||||
} | ||||||
|
||||||
if decorator.IsReplace { | ||||||
m.app.log.LogEvent(&fxevent.Replaced{ | ||||||
m.log.LogEvent(&fxevent.Replaced{ | ||||||
ModuleName: m.name, | ||||||
OutputTypeNames: outputNames, | ||||||
Err: err, | ||||||
}) | ||||||
} else { | ||||||
|
||||||
m.app.log.LogEvent(&fxevent.Decorated{ | ||||||
m.log.LogEvent(&fxevent.Decorated{ | ||||||
DecoratorName: fxreflect.FuncName(decorator.Target), | ||||||
ModuleName: m.name, | ||||||
OutputTypeNames: outputNames, | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's add a comment here clarifying that this will build the custom loggers for all the modules in the tree.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a comment