From 1aef807c71b1ea8e70e664765e0010734aee468c Mon Sep 17 00:00:00 2001
From: Mohammed Al Sahaf <msaa1990@gmail.com>
Date: Tue, 28 Mar 2023 00:41:24 +0300
Subject: log: Make sink logs encodable (#5441)
* log: make `sink` encodable
* deduplicate logger fields
* extract common fields into `BaseLog` and embed it into `SinkLog`
* amend godoc on `BaseLog` and `SinkLog`
* minor style change
---------
Co-authored-by: Francis Lavoie <lavofr@gmail.com>
---
caddyconfig/httpcaddyfile/httptype.go | 4 +-
logging.go | 206 ++++++++++++++----------------
modules/caddyhttp/fileserver/command.go | 4 +-
modules/caddyhttp/reverseproxy/command.go | 2 +-
modules/caddyhttp/staticresp.go | 2 +-
5 files changed, 106 insertions(+), 112 deletions(-)
diff --git a/caddyconfig/httpcaddyfile/httptype.go b/caddyconfig/httpcaddyfile/httptype.go
index a066ceb..18f65bb 100644
--- a/caddyconfig/httpcaddyfile/httptype.go
+++ b/caddyconfig/httpcaddyfile/httptype.go
@@ -241,7 +241,9 @@ func (st ServerType) Setup(inputServerBlocks []caddyfile.ServerBlock,
if _, ok := options["debug"]; ok {
customLogs = append(customLogs, namedCustomLog{
name: caddy.DefaultLoggerName,
- log: &caddy.CustomLog{Level: zap.DebugLevel.CapitalString()},
+ log: &caddy.CustomLog{
+ BaseLog: caddy.BaseLog{Level: zap.DebugLevel.CapitalString()},
+ },
})
}
}
diff --git a/logging.go b/logging.go
index c00c831..98b031c 100644
--- a/logging.go
+++ b/logging.go
@@ -62,7 +62,7 @@ type Logging struct {
// in dependencies that are not designed specifically for use
// in Caddy. Because it is global and unstructured, the sink
// lacks most advanced features and customizations.
- Sink *StandardLibLog `json:"sink,omitempty"`
+ Sink *SinkLog `json:"sink,omitempty"`
// Logs are your logs, keyed by an arbitrary name of your
// choosing. The default log can be customized by defining
@@ -259,55 +259,11 @@ func (wdest writerDestructor) Destruct() error {
return wdest.Close()
}
-// StandardLibLog configures the default Go standard library
-// global logger in the log package. This is necessary because
-// module dependencies which are not built specifically for
-// Caddy will use the standard logger. This is also known as
-// the "sink" logger.
-type StandardLibLog struct {
+// BaseLog contains the common logging parameters for logging.
+type BaseLog struct {
// The module that writes out log entries for the sink.
WriterRaw json.RawMessage `json:"writer,omitempty" caddy:"namespace=caddy.logging.writers inline_key=output"`
- writer io.WriteCloser
-}
-
-func (sll *StandardLibLog) provision(ctx Context, logging *Logging) error {
- if sll.WriterRaw != nil {
- mod, err := ctx.LoadModule(sll, "WriterRaw")
- if err != nil {
- return fmt.Errorf("loading sink log writer module: %v", err)
- }
- wo := mod.(WriterOpener)
-
- var isNew bool
- sll.writer, isNew, err = logging.openWriter(wo)
- if err != nil {
- return fmt.Errorf("opening sink log writer %#v: %v", mod, err)
- }
-
- if isNew {
- log.Printf("[INFO] Redirecting sink to: %s", wo)
- log.SetOutput(sll.writer)
- log.Printf("[INFO] Redirected sink to here (%s)", wo)
- }
- }
-
- return nil
-}
-
-// CustomLog represents a custom logger configuration.
-//
-// By default, a log will emit all log entries. Some entries
-// will be skipped if sampling is enabled. Further, the Include
-// and Exclude parameters define which loggers (by name) are
-// allowed or rejected from emitting in this log. If both Include
-// and Exclude are populated, their values must be mutually
-// exclusive, and longer namespaces have priority. If neither
-// are populated, all logs are emitted.
-type CustomLog struct {
- // The writer defines where log entries are emitted.
- WriterRaw json.RawMessage `json:"writer,omitempty" caddy:"namespace=caddy.logging.writers inline_key=output"`
-
// The encoder is how the log entries are formatted or encoded.
EncoderRaw json.RawMessage `json:"encoder,omitempty" caddy:"namespace=caddy.logging.encoders inline_key=format"`
@@ -321,16 +277,6 @@ type CustomLog struct {
// servers.
Sampling *LogSampling `json:"sampling,omitempty"`
- // Include defines the names of loggers to emit in this
- // log. For example, to include only logs emitted by the
- // admin API, you would include "admin.api".
- Include []string `json:"include,omitempty"`
-
- // Exclude defines the names of loggers that should be
- // skipped by this log. For example, to exclude only
- // HTTP access logs, you would exclude "http.log.access".
- Exclude []string `json:"exclude,omitempty"`
-
writerOpener WriterOpener
writer io.WriteCloser
encoder zapcore.Encoder
@@ -338,8 +284,23 @@ type CustomLog struct {
core zapcore.Core
}
-func (cl *CustomLog) provision(ctx Context, logging *Logging) error {
- // Replace placeholder for log level
+func (cl *BaseLog) provisionCommon(ctx Context, logging *Logging) error {
+ if cl.WriterRaw != nil {
+ mod, err := ctx.LoadModule(cl, "WriterRaw")
+ if err != nil {
+ return fmt.Errorf("loading log writer module: %v", err)
+ }
+ cl.writerOpener = mod.(WriterOpener)
+ }
+ if cl.writerOpener == nil {
+ cl.writerOpener = StderrWriter{}
+ }
+ var err error
+ cl.writer, _, err = logging.openWriter(cl.writerOpener)
+ if err != nil {
+ 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 {
@@ -365,52 +326,6 @@ func (cl *CustomLog) provision(ctx Context, logging *Logging) error {
return fmt.Errorf("unrecognized log level: %s", cl.Level)
}
- // If both Include and Exclude lists are populated, then each item must
- // be a superspace or subspace of an item in the other list, because
- // populating both lists means that any given item is either a rule
- // or an exception to another rule. But if the item is not a super-
- // or sub-space of any item in the other list, it is neither a rule
- // nor an exception, and is a contradiction. Ensure, too, that the
- // sets do not intersect, which is also a contradiction.
- if len(cl.Include) > 0 && len(cl.Exclude) > 0 {
- // prevent intersections
- for _, allow := range cl.Include {
- for _, deny := range cl.Exclude {
- if allow == deny {
- return fmt.Errorf("include and exclude must not intersect, but found %s in both lists", allow)
- }
- }
- }
-
- // ensure namespaces are nested
- outer:
- for _, allow := range cl.Include {
- for _, deny := range cl.Exclude {
- if strings.HasPrefix(allow+".", deny+".") ||
- strings.HasPrefix(deny+".", allow+".") {
- continue outer
- }
- }
- return fmt.Errorf("when both include and exclude are populated, each element must be a superspace or subspace of one in the other list; check '%s' in include", allow)
- }
- }
-
- if cl.WriterRaw != nil {
- mod, err := ctx.LoadModule(cl, "WriterRaw")
- if err != nil {
- return fmt.Errorf("loading log writer module: %v", err)
- }
- cl.writerOpener = mod.(WriterOpener)
- }
- if cl.writerOpener == nil {
- cl.writerOpener = StderrWriter{}
- }
-
- cl.writer, _, err = logging.openWriter(cl.writerOpener)
- if err != nil {
- return fmt.Errorf("opening log writer using %#v: %v", cl.writerOpener, err)
- }
-
if cl.EncoderRaw != nil {
mod, err := ctx.LoadModule(cl, "EncoderRaw")
if err != nil {
@@ -428,13 +343,11 @@ func (cl *CustomLog) provision(ctx Context, logging *Logging) error {
}
cl.encoder = newDefaultProductionLogEncoder(colorize)
}
-
cl.buildCore()
-
return nil
}
-func (cl *CustomLog) buildCore() {
+func (cl *BaseLog) buildCore() {
// logs which only discard their output don't need
// to perform encoding or any other processing steps
// at all, so just shorcut to a nop core instead
@@ -463,6 +376,83 @@ func (cl *CustomLog) buildCore() {
cl.core = c
}
+// 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
+// Caddy will use the standard logger. This is also known as
+// the "sink" logger.
+type SinkLog struct {
+ BaseLog
+}
+
+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)))
+ return nil
+}
+
+// CustomLog represents a custom logger configuration.
+//
+// By default, a log will emit all log entries. Some entries
+// will be skipped if sampling is enabled. Further, the Include
+// and Exclude parameters define which loggers (by name) are
+// allowed or rejected from emitting in this log. If both Include
+// and Exclude are populated, their values must be mutually
+// exclusive, and longer namespaces have priority. If neither
+// are populated, all logs are emitted.
+type CustomLog struct {
+ BaseLog
+
+ // Include defines the names of loggers to emit in this
+ // log. For example, to include only logs emitted by the
+ // admin API, you would include "admin.api".
+ Include []string `json:"include,omitempty"`
+
+ // Exclude defines the names of loggers that should be
+ // skipped by this log. For example, to exclude only
+ // HTTP access logs, you would exclude "http.log.access".
+ Exclude []string `json:"exclude,omitempty"`
+}
+
+func (cl *CustomLog) provision(ctx Context, logging *Logging) error {
+ if err := cl.provisionCommon(ctx, logging); err != nil {
+ return err
+ }
+
+ // If both Include and Exclude lists are populated, then each item must
+ // be a superspace or subspace of an item in the other list, because
+ // populating both lists means that any given item is either a rule
+ // or an exception to another rule. But if the item is not a super-
+ // or sub-space of any item in the other list, it is neither a rule
+ // nor an exception, and is a contradiction. Ensure, too, that the
+ // sets do not intersect, which is also a contradiction.
+ if len(cl.Include) > 0 && len(cl.Exclude) > 0 {
+ // prevent intersections
+ for _, allow := range cl.Include {
+ for _, deny := range cl.Exclude {
+ if allow == deny {
+ return fmt.Errorf("include and exclude must not intersect, but found %s in both lists", allow)
+ }
+ }
+ }
+
+ // ensure namespaces are nested
+ outer:
+ for _, allow := range cl.Include {
+ for _, deny := range cl.Exclude {
+ if strings.HasPrefix(allow+".", deny+".") ||
+ strings.HasPrefix(deny+".", allow+".") {
+ continue outer
+ }
+ }
+ return fmt.Errorf("when both include and exclude are populated, each element must be a superspace or subspace of one in the other list; check '%s' in include", allow)
+ }
+ }
+ return nil
+}
+
func (cl *CustomLog) matchesModule(moduleID string) bool {
return cl.loggerAllowed(moduleID, true)
}
diff --git a/modules/caddyhttp/fileserver/command.go b/modules/caddyhttp/fileserver/command.go
index 697ec34..0112a5f 100644
--- a/modules/caddyhttp/fileserver/command.go
+++ b/modules/caddyhttp/fileserver/command.go
@@ -134,7 +134,9 @@ func cmdFileServer(fs caddycmd.Flags) (int, error) {
if debug {
cfg.Logging = &caddy.Logging{
Logs: map[string]*caddy.CustomLog{
- "default": {Level: zap.DebugLevel.CapitalString()},
+ "default": {
+ BaseLog: caddy.BaseLog{Level: zap.DebugLevel.CapitalString()},
+ },
},
}
}
diff --git a/modules/caddyhttp/reverseproxy/command.go b/modules/caddyhttp/reverseproxy/command.go
index 5e8beb1..bd3efcd 100644
--- a/modules/caddyhttp/reverseproxy/command.go
+++ b/modules/caddyhttp/reverseproxy/command.go
@@ -280,7 +280,7 @@ func cmdReverseProxy(fs caddycmd.Flags) (int, error) {
if debug {
cfg.Logging = &caddy.Logging{
Logs: map[string]*caddy.CustomLog{
- "default": {Level: zap.DebugLevel.CapitalString()},
+ "default": {BaseLog: caddy.BaseLog{Level: zap.DebugLevel.CapitalString()}},
},
}
}
diff --git a/modules/caddyhttp/staticresp.go b/modules/caddyhttp/staticresp.go
index 67614c2..0584916 100644
--- a/modules/caddyhttp/staticresp.go
+++ b/modules/caddyhttp/staticresp.go
@@ -407,7 +407,7 @@ func cmdRespond(fl caddycmd.Flags) (int, error) {
if debug {
cfg.Logging = &caddy.Logging{
Logs: map[string]*caddy.CustomLog{
- "default": {Level: zap.DebugLevel.CapitalString()},
+ "default": {BaseLog: caddy.BaseLog{Level: zap.DebugLevel.CapitalString()}},
},
}
}
--
cgit v1.2.3