Skip to content
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

logging: Add zap.Option support #5944

Merged
merged 1 commit into from
Dec 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
113 changes: 87 additions & 26 deletions logging.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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 {
Expand All @@ -209,14 +216,21 @@ 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
Comment on lines +220 to +224
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a non-fatal error log because this call chain doesn't support bubbling up an error. Probably good enough, it's very rare (only if user uses an invalid level arg for with_stacktrace).

}
cores = append(cores, &filteringCore{Core: l.core, cl: l})
}
}
}

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
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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
}

Expand Down Expand Up @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion modules/caddyhttp/reverseproxy/streaming.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a minor cleanup. I was looking for any .Sugar() cause I wanted to check if we needed to set with_caller_skip to anything higher than 0 by default, and I think it's only relevant for sugared loggers. This was the only case except for metrics.go which I don't care about.

return
}

Expand Down