log: Refactor to define proper loggers tree

*container.C is also now passed to module constructor allowing
access to container logger for sublogger initialization. This in
turn required some package movement to break import cycles.

Author: foxcpp

cmd/maddy/maddy.conf

@@ -71,7 +71,7 @@ func LogOutputOption(args []string) (log.Output, error) {
 			}
 			return log.NopOutput{}, nil
 		default:
-			// Log file paths are converted to absolute to make sure
+			// log file paths are converted to absolute to make sure
 			// we will be able to recreate them in right location
 			// after changing working directory to the state dir.
 			absPath, err := filepath.Abs(arg)
@@ -98,7 +98,7 @@ func LogOutputOption(args []string) (log.Output, error) {
 }
 
 func defaultLogOutput() (interface{}, error) {
-	return log.DefaultLogger.Out, nil
+	return nil, nil
 }
 
 func reinitLogging() {
@@ -115,7 +115,7 @@ func reinitLogging() {
 	}
 
 	if err := out.Close(); err != nil {
-		log.Println("Can't close logger:", err)
+		log.Println("Can't close old logger:", err)
 	}
 
 	log.DefaultLogger.Out = newOut

config.go

@@ -36,31 +36,32 @@ import (
 	"github.com/foxcpp/maddy/framework/container"
 	"github.com/foxcpp/maddy/framework/log"
 	"github.com/foxcpp/maddy/framework/module"
+	"github.com/foxcpp/maddy/framework/module/modules"
 )
 
 // createInlineModule is a helper function for config matchers that can create inline modules.
-func createInlineModule(preferredNamespace, modName string) (module.Module, error) {
-	var newMod module.FuncNewModule
+func createInlineModule(c *container.C, preferredNamespace, modName string) (module.Module, error) {
+	var newMod modules.FuncNewModule
 	originalModName := modName
 
 	// First try to extend the name with preferred namespace unless the name
 	// already contains it.
 	if !strings.Contains(modName, ".") && preferredNamespace != "" {
 		modName = preferredNamespace + "." + modName
-		newMod = module.Get(modName)
+		newMod = modules.Get(modName)
 	}
 
 	// Then try global namespace for compatibility and complex modules.
 	if newMod == nil {
-		newMod = module.Get(originalModName)
+		newMod = modules.Get(originalModName)
 	}
 
 	// Bail if both failed.
 	if newMod == nil {
 		return nil, fmt.Errorf("unknown module: %s (namespace: %s)", originalModName, preferredNamespace)
 	}
 
-	return newMod(modName, "")
+	return newMod(c, modName, "")
 }
 
 // configureInlineModule constructs "faked" config tree and passes it to module
@@ -73,7 +74,7 @@ func configureInlineModule(modObj module.Module, args []string, globals map[stri
 		return err
 	}
 
-	if li, ok := modObj.(module.LifetimeModule); ok {
+	if li, ok := modObj.(container.LifetimeModule); ok {
 		container.Global.Lifetime.Add(li)
 	}
 
@@ -115,7 +116,7 @@ func ModuleFromNode(preferredNamespace string, args []string, inlineCfg config.N
 		log.Debugf("%s:%d: reference %s", inlineCfg.File, inlineCfg.Line, args[0])
 	} else {
 		log.Debugf("%s:%d: new module %s %v", inlineCfg.File, inlineCfg.Line, args[0], args[1:])
-		modObj, err = createInlineModule(preferredNamespace, args[0])
+		modObj, err = createInlineModule(container.Global, preferredNamespace, args[0])
 	}
 	if err != nil {
 		return err

framework/config/module/modconfig.go

@@ -20,7 +20,6 @@ package container
 
 import (
 	"github.com/foxcpp/maddy/framework/log"
-	"github.com/foxcpp/maddy/framework/module"
 )
 
 type GlobalConfig struct {
@@ -53,16 +52,17 @@ type GlobalConfig struct {
 
 type C struct {
 	Config        GlobalConfig
-	DefaultLogger log.Logger
-	Modules       *module.Registry
-	Lifetime      *module.LifetimeTracker
+	DefaultLogger *log.Logger
+	Modules       *Registry
+	Lifetime      *LifetimeTracker
 }
 
 func New() *C {
+	rootLog := log.DefaultLogger.Sublogger("")
 	return &C{
-		DefaultLogger: log.DefaultLogger,
-		Modules:       module.NewRegistry(log.DefaultLogger.Sublogger("registry")),
-		Lifetime:      module.NewLifetime(log.DefaultLogger.Sublogger("lifetime")),
+		DefaultLogger: rootLog,
+		Modules:       NewRegistry(rootLog.Sublogger("registry")),
+		Lifetime:      NewLifetime(rootLog.Sublogger("lifetime")),
 	}
 }
 

framework/container/container.go

@@ -16,24 +16,25 @@ You should have received a copy of the GNU General Public License
 along with this program.  If not, see <https://www.gnu.org/licenses/>.
 */
 
-package module
+package container
 
 import (
 	"fmt"
 
 	"github.com/foxcpp/maddy/framework/log"
+	"github.com/foxcpp/maddy/framework/module"
 )
 
 // LifetimeModule is a stateful module that needs to have post-configuration
 // startup and graceful shutdown functionality.
 type LifetimeModule interface {
-	Module
+	module.Module
 	Start() error
 	Stop() error
 }
 
 type ReloadModule interface {
-	Module
+	module.Module
 	Reload() error
 }
 

framework/module/lifetime.go framework/container/lifetime.goframework/module/lifetime.go renamed to framework/container/lifetime.go

@@ -16,12 +16,13 @@ You should have received a copy of the GNU General Public License
 along with this program.  If not, see <https://www.gnu.org/licenses/>.
 */
 
-package module
+package container
 
 import (
 	"errors"
 
 	"github.com/foxcpp/maddy/framework/log"
+	"github.com/foxcpp/maddy/framework/module"
 )
 
 var (
@@ -30,7 +31,7 @@ var (
 )
 
 type registryEntry struct {
-	Mod      Module
+	Mod      module.Module
 	LazyInit func() error
 }
 
@@ -56,7 +57,7 @@ func NewRegistry(log *log.Logger) *Registry {
 //
 // lazyInit function will be called on first request to get the module from
 // registry.
-func (r *Registry) Register(mod Module, lazyInit func() error) error {
+func (r *Registry) Register(mod module.Module, lazyInit func() error) error {
 	instName := mod.InstanceName()
 	if instName == "" {
 		panic("module with empty instance name cannot be added to the registry")
@@ -114,7 +115,7 @@ func (r *Registry) ensureInitialized(name string, entry *registryEntry) error {
 	return nil
 }
 
-func (r *Registry) Get(name string) (Module, error) {
+func (r *Registry) Get(name string) (module.Module, error) {
 	if name == "" {
 		panic("cannot get module with empty name")
 	}
@@ -135,8 +136,8 @@ func (r *Registry) Get(name string) (Module, error) {
 	return mod.Mod, nil
 }
 
-func (r *Registry) NotInitialized() []Module {
-	notinit := make([]Module, 0, len(r.instances)-len(r.initialized))
+func (r *Registry) NotInitialized() []module.Module {
+	notinit := make([]module.Module, 0, len(r.instances)-len(r.initialized))
 	for name, mod := range r.instances {
 		if _, ok := r.initialized[name]; ok {
 			continue

framework/module/registry.go framework/container/registry.goframework/module/registry.go renamed to framework/container/registry.go

@@ -59,10 +59,7 @@ func (l *Logger) Zap() *zap.Logger {
 }
 
 func (l *Logger) IsDebug() bool {
-	if l.Parent == nil {
-		return l.Debug
-	}
-	return l.Debug || l.Parent.IsDebug()
+	return l.Debug || (l.Parent != nil && l.Parent.IsDebug())
 }
 
 func (l *Logger) Debugf(format string, val ...interface{}) {
@@ -217,19 +214,30 @@ func (l *Logger) DebugWriter() io.Writer {
 	return l2
 }
 
+func (l *Logger) output() Output {
+	if l.Out != nil {
+		return l.Out
+	}
+	if l.Parent != nil {
+		return l.Parent.output()
+	}
+
+	if DefaultLogger.Out == nil {
+		panic("DefaultLogger.Out is not set")
+	}
+	if l.Parent == nil && l != &DefaultLogger {
+		DefaultLogger.Out.Write(time.Now(), true, "logger "+l.Name+" has no parent, this is a bug")
+	}
+	return DefaultLogger.Out
+}
+
 func (l *Logger) log(debug bool, s string) {
 	if l.Name != "" {
 		s = l.Name + ": " + s
 	}
 
-	if l.Out != nil {
-		l.Out.Write(time.Now(), debug, s)
-		return
-	}
-	if DefaultLogger.Out != nil {
-		DefaultLogger.Out.Write(time.Now(), debug, s)
-		return
-	}
+	out := l.output()
+	out.Write(time.Now(), debug, s)
 
 	// Logging is disabled - do nothing.
 }
@@ -240,9 +248,7 @@ func (l *Logger) Sublogger(name string) *Logger {
 	}
 	return &Logger{
 		Parent: l,
-		Out:    l.Out,
 		Name:   name,
-		Debug:  l.Debug,
 	}
 }
 
@@ -253,6 +259,12 @@ func (l *Logger) Sublogger(name string) *Logger {
 // however underlying log.Output may provide necessary serialization.
 var DefaultLogger = Logger{Out: WriterOutput(os.Stderr, false)}
 
+// NopLogger is the logger that discards all messages written to it.
+var NopLogger = Logger{
+	Parent: &DefaultLogger,
+	Out:    NopOutput{},
+}
+
 func Debugf(format string, val ...interface{}) { DefaultLogger.Debugf(format, val...) }
 func Debugln(val ...interface{})               { DefaultLogger.Debugln(val...) }
 func Printf(format string, val ...interface{}) { DefaultLogger.Printf(format, val...) }

framework/log/log.go

@@ -50,25 +50,3 @@ type Module interface {
 	// string if module instance is unnamed.
 	InstanceName() string
 }
-
-// FuncNewModule is function that creates new instance of module with specified name.
-//
-// Module.InstanceName() of the returned module object should return instName.
-// If module is defined inline, instName will be empty.
-//
-// Returned Module may additionally implement LifetimeModule.
-type FuncNewModule func(modName, instName string) (Module, error)
-
-// FuncNewEndpoint is a function that creates new instance of endpoint
-// module.
-//
-// Compared to regular modules, endpoint module instances are:
-// - Not registered in the global registry.
-// - Can't be defined inline.
-// - Don't have an unique name
-// - All config arguments are always passed as an 'addrs' slice and not used as
-// names.
-//
-// As a consequence of having no per-instance name, InstanceName of the module
-// object always returns the same value as Name.
-type FuncNewEndpoint func(modName string, addrs []string) (LifetimeModule, error)

framework/module/module.go

@@ -16,7 +16,7 @@ You should have received a copy of the GNU General Public License
 along with this program.  If not, see <https://www.gnu.org/licenses/>.
 */
 
-package module
+package modules
 
 import (
 	"context"
@@ -25,6 +25,8 @@ import (
 	"github.com/emersion/go-smtp"
 	"github.com/foxcpp/maddy/framework/buffer"
 	"github.com/foxcpp/maddy/framework/config"
+	"github.com/foxcpp/maddy/framework/container"
+	"github.com/foxcpp/maddy/framework/module"
 )
 
 // Dummy is a struct that implements PlainAuth and DeliveryTarget
@@ -58,7 +60,7 @@ func (d *Dummy) Configure(_ []string, _ *config.Map) error {
 	return nil
 }
 
-func (d *Dummy) StartDelivery(ctx context.Context, msgMeta *MsgMetadata, mailFrom string) (Delivery, error) {
+func (d *Dummy) StartDelivery(ctx context.Context, msgMeta *module.MsgMetadata, mailFrom string) (module.Delivery, error) {
 	return dummyDelivery{}, nil
 }
 
@@ -80,8 +82,6 @@ func (dd dummyDelivery) Commit(ctx context.Context) error {
 	return nil
 }
 
-func init() {
-	Register("dummy", func(_, instName string) (Module, error) {
-		return &Dummy{instName: instName}, nil
-	})
+func NewDummy(_ *container.C, _, instName string) (module.Module, error) {
+	return &Dummy{instName: instName}, nil
 }