Repository: caddyserver/caddy (language: go)

Issue:
How to set the default logger programatically?
I'm using Caddy as an embedded component in a service and would like to be able to set at least the default logger to the
zap logger the service has already configured by the time Caddy is started.

The primary reason for that is that setting up the logger in Caddy's config file is missing certain capabilities (primarily related to the encoder). But from management point of view, I want to avoid having to spread/duplicate logger configuration across multiple places (my service & Caddy config).

I guess the easiest place is to "extend" the `Log` function in `logging.go`. Being able to set the `defaultLogger` variable is most likely not going to work as it seems to be "recreated" in `setupNewDefault` based on its `CustomLog` settings - which lack what I need.

Or is there already a way to achieve that without any further code changes?


Discussion / hints:
> The primary reason for that is that setting up the logger in Caddy's config file is missing certain capabilities

What do you mean, exactly? Couldn't we fix that instead? What are your needs?

This is sounding kinda like an [XY Problem](https://xyproblem.info/).


I'd be interested to know what about the encoder we could fix or improve to meet your needs. What are your requirements?
As for the XY Problem, I thought I already stated the problem - I'm using Caddy as an embedded component in a service and already have a configured zap logger by the time I initialise Caddy. Otherwise, I would need to ensure that the logger configuration from the service configuration is kept in sync with the caddy config file. Therefore, I was looking for a solution on how to also use that logger for Caddy.

As to what I'm missing:
* Setting `EncodeTime:TimeEncoder` in EncoderConfig 
  * not sure on how I could define that in JSON as `TimeEncoder` is a function
  * our 'primary' service is using `zapcore.ISO8601TimeEncoder`
* Adding `zap.WithCaller` to `zap.Builder`

I would really like to find a solution to this as I'm now having two different logging formats interspersed in the logs which complicates processing. Not a real show stopper, just an eye sore :)

Otherwise, I'm a very happy user!
You can set `TimeFormat` in the config (aka `time_format` in JSON), which sets up TimeEncoder. See https://github.com/caddyserver/caddy/blob/7e52db8280dcf855e77be6de5bccf7e8a173450c/modules/logging/encoders.go#L223-L260

We can add an option to enable `WithCaller`. 99% of the time it's not useful for users because the line numbers won't mean anything to them, which is why we didn't include it. But yeah, no harm in adding it as an option.

Fix patch:
diff --git a/logging.go b/logging.go
index 98b031c6a2a..58d5b2d3f8a 100644
--- a/logging.go
+++ b/logging.go
@@ -150,12 +150,18 @@ func (logging *Logging) setupNewDefault(ctx Context) error {
 		logging.Logs[DefaultLoggerName] = newDefault.CustomLog
 	}
 
+	// options for the default logger
+	options, err := newDefault.CustomLog.buildOptions()
+	if err != nil {
+		return fmt.Errorf("setting up default log: %v", err)
+	}
+
 	// set up this new log
-	err := newDefault.CustomLog.provision(ctx, logging)
+	err = newDefault.CustomLog.provision(ctx, logging)
 	if err != nil {
 		return fmt.Errorf("setting up default log: %v", err)
 	}
-	newDefault.logger = zap.New(newDefault.CustomLog.core)
+	newDefault.logger = zap.New(newDefault.CustomLog.core, options...)
 
 	// redirect the default caddy logs
 	defaultLoggerMu.Lock()
@@ -201,6 +207,7 @@ func (logging *Logging) closeLogs() error {
 func (logging *Logging) Logger(mod Module) *zap.Logger {
 	modID := string(mod.CaddyModule().ID)
 	var cores []zapcore.Core
+	var options []zap.Option
 
 	if logging != nil {
 		for _, l := range logging.Logs {
@@ -209,6 +216,13 @@ func (logging *Logging) Logger(mod Module) *zap.Logger {
 					cores = append(cores, l.core)
 					continue
 				}
+				if len(options) == 0 {
+					newOptions, err := l.buildOptions()
+					if err != nil {
+						Log().Error("building options for logger", zap.String("module", modID), zap.Error(err))
+					}
+					options = newOptions
+				}
 				cores = append(cores, &filteringCore{Core: l.core, cl: l})
 			}
 		}
@@ -216,7 +230,7 @@ func (logging *Logging) Logger(mod Module) *zap.Logger {
 
 	multiCore := zapcore.NewTee(cores...)
 
-	return zap.New(multiCore).Named(modID)
+	return zap.New(multiCore, options...).Named(modID)
 }
 
 // openWriter opens a writer using opener, and returns true if
@@ -277,6 +291,20 @@ type BaseLog struct {
 	// servers.
 	Sampling *LogSampling `json:"sampling,omitempty"`
 
+	// If true, the log entry will include the caller's
+	// file name and line number. Default off.
+	WithCaller bool `json:"with_caller,omitempty"`
+
+	// If non-zero, and `with_caller` is true, this many
+	// stack frames will be skipped when determining the
+	// caller. Default 0.
+	WithCallerSkip int `json:"with_caller_skip,omitempty"`
+
+	// If not empty, the log entry will include a stack trace
+	// for all logs at the given level or higher. See `level`
+	// for possible values. Default off.
+	WithStacktrace string `json:"with_stacktrace,omitempty"`
+
 	writerOpener WriterOpener
 	writer       io.WriteCloser
 	encoder      zapcore.Encoder
@@ -301,29 +329,10 @@ func (cl *BaseLog) provisionCommon(ctx Context, logging *Logging) error {
 		return fmt.Errorf("opening log writer using %#v: %v", cl.writerOpener, err)
 	}
 
-	repl := NewReplacer()
-	level, err := repl.ReplaceOrErr(cl.Level, true, true)
-	if err != nil {
-		return fmt.Errorf("invalid log level: %v", err)
-	}
-	level = strings.ToLower(level)
-
 	// set up the log level
-	switch level {
-	case "debug":
-		cl.levelEnabler = zapcore.DebugLevel
-	case "", "info":
-		cl.levelEnabler = zapcore.InfoLevel
-	case "warn":
-		cl.levelEnabler = zapcore.WarnLevel
-	case "error":
-		cl.levelEnabler = zapcore.ErrorLevel
-	case "panic":
-		cl.levelEnabler = zapcore.PanicLevel
-	case "fatal":
-		cl.levelEnabler = zapcore.FatalLevel
-	default:
-		return fmt.Errorf("unrecognized log level: %s", cl.Level)
+	cl.levelEnabler, err = parseLevel(cl.Level)
+	if err != nil {
+		return err
 	}
 
 	if cl.EncoderRaw != nil {
@@ -376,6 +385,24 @@ func (cl *BaseLog) buildCore() {
 	cl.core = c
 }
 
+func (cl *BaseLog) buildOptions() ([]zap.Option, error) {
+	var options []zap.Option
+	if cl.WithCaller {
+		options = append(options, zap.AddCaller())
+		if cl.WithCallerSkip != 0 {
+			options = append(options, zap.AddCallerSkip(cl.WithCallerSkip))
+		}
+	}
+	if cl.WithStacktrace != "" {
+		levelEnabler, err := parseLevel(cl.WithStacktrace)
+		if err != nil {
+			return options, fmt.Errorf("setting up default Caddy log: %v", err)
+		}
+		options = append(options, zap.AddStacktrace(levelEnabler))
+	}
+	return options, nil
+}
+
 // SinkLog configures the default Go standard library
 // global logger in the log package. This is necessary because
 // module dependencies which are not built specifically for
@@ -389,7 +416,14 @@ func (sll *SinkLog) provision(ctx Context, logging *Logging) error {
 	if err := sll.provisionCommon(ctx, logging); err != nil {
 		return err
 	}
-	ctx.cleanupFuncs = append(ctx.cleanupFuncs, zap.RedirectStdLog(zap.New(sll.core)))
+
+	options, err := sll.buildOptions()
+	if err != nil {
+		return err
+	}
+
+	logger := zap.New(sll.core, options...)
+	ctx.cleanupFuncs = append(ctx.cleanupFuncs, zap.RedirectStdLog(logger))
 	return nil
 }
 
@@ -678,6 +712,33 @@ func newDefaultProductionLogEncoder(colorize bool) zapcore.Encoder {
 	return zapcore.NewJSONEncoder(encCfg)
 }
 
+func parseLevel(levelInput string) (zapcore.LevelEnabler, error) {
+	repl := NewReplacer()
+	level, err := repl.ReplaceOrErr(levelInput, true, true)
+	if err != nil {
+		return nil, fmt.Errorf("invalid log level: %v", err)
+	}
+	level = strings.ToLower(level)
+
+	// set up the log level
+	switch level {
+	case "debug":
+		return zapcore.DebugLevel, nil
+	case "", "info":
+		return zapcore.InfoLevel, nil
+	case "warn":
+		return zapcore.WarnLevel, nil
+	case "error":
+		return zapcore.ErrorLevel, nil
+	case "panic":
+		return zapcore.PanicLevel, nil
+	case "fatal":
+		return zapcore.FatalLevel, nil
+	default:
+		return nil, fmt.Errorf("unrecognized log level: %s", level)
+	}
+}
+
 // Log returns the current default logger.
 func Log() *zap.Logger {
 	defaultLoggerMu.RLock()
diff --git a/modules/caddyhttp/reverseproxy/streaming.go b/modules/caddyhttp/reverseproxy/streaming.go
index 155a1df0c4a..2a5b5286a17 100644
--- a/modules/caddyhttp/reverseproxy/streaming.go
+++ b/modules/caddyhttp/reverseproxy/streaming.go
@@ -68,7 +68,7 @@ func (h *Handler) handleUpgradeResponse(logger *zap.Logger, rw http.ResponseWrit
 	//nolint:bodyclose
 	conn, brw, hijackErr := http.NewResponseController(rw).Hijack()
 	if errors.Is(hijackErr, http.ErrNotSupported) {
-		h.logger.Sugar().Errorf("can't switch protocols using non-Hijacker ResponseWriter type %T", rw)
+		h.logger.Error("can't switch protocols using non-Hijacker ResponseWriter", zap.String("type", fmt.Sprintf("%T", rw)))
 		return
 	}
 


Test patch: