caddyhttp: General improvements to access logging (#3301)

* httpcaddyfile: Exclude access logs written to files from default log

Even though any logs can just be ignored, most users don't seem to like
configuring an access log to go to a file only to have it doubly appear
in the default log.

Related to:
- #3294
- https://caddy.community/t/v2-logging-format/7642/4?u=matt
- https://caddy.community/t/caddyfile-questions/7651/3?u=matt

* caddyhttp: General improvements to access log controls (fixes #3310)

* caddyhttp: Move log config nil check higher

* Rename LoggerName -> DefaultLoggerName

2 files changed, 106 insertions, 34 deletions

diff --git a/caddyconfig/httpcaddyfile/httptype.go b/caddyconfig/httpcaddyfile/httptype.go
index a22dd40..eb067bc 100644
--- a/caddyconfig/httpcaddyfile/httptype.go
+++ b/caddyconfig/httpcaddyfile/httptype.go
@@ -38,7 +38,7 @@ type ServerType struct {
 }
 
 // Setup makes a config from the tokens.
-func (st ServerType) Setup(originalServerBlocks []caddyfile.ServerBlock,
+func (st ServerType) Setup(inputServerBlocks []caddyfile.ServerBlock,
 	options map[string]interface{}) (*caddy.Config, []caddyconfig.Warning, error) {
 	var warnings []caddyconfig.Warning
 	gc := counter{new(int)}
@@ -50,15 +50,15 @@ func (st ServerType) Setup(originalServerBlocks []caddyfile.ServerBlock,
 	// chosen to handle a request - we actually will make each
 	// server block's route terminal so that only one will run
 	sbKeys := make(map[string]struct{})
-	serverBlocks := make([]serverBlock, 0, len(originalServerBlocks))
-	for i, sblock := range originalServerBlocks {
+	originalServerBlocks := make([]serverBlock, 0, len(inputServerBlocks))
+	for i, sblock := range inputServerBlocks {
 		for j, k := range sblock.Keys {
 			if _, ok := sbKeys[k]; ok {
 				return nil, warnings, fmt.Errorf("duplicate site address not allowed: '%s' in %v (site block %d, key %d)", k, sblock.Keys, i, j)
 			}
 			sbKeys[k] = struct{}{}
 		}
-		serverBlocks = append(serverBlocks, serverBlock{
+		originalServerBlocks = append(originalServerBlocks, serverBlock{
 			block: sblock,
 			pile:  make(map[string][]ConfigValue),
 		})
@@ -66,12 +66,12 @@ func (st ServerType) Setup(originalServerBlocks []caddyfile.ServerBlock,
 
 	// apply any global options
 	var err error
-	serverBlocks, err = st.evaluateGlobalOptionsBlock(serverBlocks, options)
+	originalServerBlocks, err = st.evaluateGlobalOptionsBlock(originalServerBlocks, options)
 	if err != nil {
 		return nil, warnings, err
 	}
 
-	for _, sb := range serverBlocks {
+	for _, sb := range originalServerBlocks {
 		// replace shorthand placeholders (which are
 		// convenient when writing a Caddyfile) with
 		// their actual placeholder identifiers or
@@ -155,7 +155,7 @@ func (st ServerType) Setup(originalServerBlocks []caddyfile.ServerBlock,
 	}
 
 	// map
-	sbmap, err := st.mapAddressToServerBlocks(serverBlocks, options)
+	sbmap, err := st.mapAddressToServerBlocks(originalServerBlocks, options)
 	if err != nil {
 		return nil, warnings, err
 	}
@@ -193,21 +193,24 @@ func (st ServerType) Setup(originalServerBlocks []caddyfile.ServerBlock,
 	// extract any custom logs, and enforce configured levels
 	var customLogs []namedCustomLog
 	var hasDefaultLog bool
-	for _, sb := range serverBlocks {
-		for _, clVal := range sb.pile["custom_log"] {
-			ncl := clVal.Value.(namedCustomLog)
-			if ncl.name == "" {
-				continue
-			}
-			if ncl.name == "default" {
-				hasDefaultLog = true
-			}
-			if _, ok := options["debug"]; ok && ncl.log.Level == "" {
-				ncl.log.Level = "DEBUG"
+	for _, p := range pairings {
+		for _, sb := range p.serverBlocks {
+			for _, clVal := range sb.pile["custom_log"] {
+				ncl := clVal.Value.(namedCustomLog)
+				if ncl.name == "" {
+					continue
+				}
+				if ncl.name == "default" {
+					hasDefaultLog = true
+				}
+				if _, ok := options["debug"]; ok && ncl.log.Level == "" {
+					ncl.log.Level = "DEBUG"
+				}
+				customLogs = append(customLogs, ncl)
 			}
-			customLogs = append(customLogs, ncl)
 		}
 	}
+
 	if !hasDefaultLog {
 		// if the default log was not customized, ensure we
 		// configure it with any applicable options
@@ -250,6 +253,17 @@ func (st ServerType) Setup(originalServerBlocks []caddyfile.ServerBlock,
 			if ncl.name != "" {
 				cfg.Logging.Logs[ncl.name] = ncl.log
 			}
+			// most users seem to prefer not writing access logs
+			// to the default log when they are directed to a
+			// file or have any other special customization
+			if len(ncl.log.Include) > 0 {
+				defaultLog, ok := cfg.Logging.Logs["default"]
+				if !ok {
+					defaultLog = new(caddy.CustomLog)
+					cfg.Logging.Logs["default"] = defaultLog
+				}
+				defaultLog.Exclude = append(defaultLog.Exclude, ncl.log.Include...)
+			}
 		}
 	}
 
@@ -451,23 +465,46 @@ func (st *ServerType) serversFromPairings(
 			}
 
 			// add log associations
+			// see https://github.com/caddyserver/caddy/issues/3310
+			sblockLogHosts := sblock.hostsFromKeys(true)
 			for _, cval := range sblock.pile["custom_log"] {
 				ncl := cval.Value.(namedCustomLog)
 				if srv.Logs == nil {
-					srv.Logs = &caddyhttp.ServerLogConfig{
-						LoggerNames: make(map[string]string),
-					}
+					srv.Logs = new(caddyhttp.ServerLogConfig)
 				}
 				if sblock.hasHostCatchAllKey() {
-					srv.Logs.LoggerName = ncl.name
+					// all requests for hosts not able to be listed should use
+					// this log because it's a catch-all-hosts server block
+					srv.Logs.DefaultLoggerName = ncl.name
 				} else {
-					for _, h := range sblock.hostsFromKeys(true) {
-						if ncl.name != "" {
+					// map each host to the user's desired logger name
+					for _, h := range sblockLogHosts {
+						// if the custom logger name is non-empty, add it to
+						// the map; otherwise, only map to an empty logger
+						// name if the server block has a catch-all host (in
+						// which case only requests with mapped hostnames will
+						// be access-logged, so it'll be necessary to add them
+						// to the map even if they use default logger)
+						if ncl.name != "" || len(hosts) == 0 {
+							if srv.Logs.LoggerNames == nil {
+								srv.Logs.LoggerNames = make(map[string]string)
+							}
 							srv.Logs.LoggerNames[h] = ncl.name
 						}
 					}
 				}
 			}
+			if srv.Logs != nil && len(sblock.pile["custom_log"]) == 0 {
+				// server has access logs enabled, but this server block does not
+				// enable access logs; therefore, all hosts of this server block
+				// should not be access-logged
+				if len(hosts) == 0 {
+					// if the server block has a catch-all-hosts key, then we should
+					// not log reqs to any host unless it appears in the map
+					srv.Logs.SkipUnmappedHosts = true
+				}
+				srv.Logs.SkipHosts = append(srv.Logs.SkipHosts, sblockLogHosts...)
+			}
 		}
 
 		// a server cannot (natively) serve both HTTP and HTTPS at the
diff --git a/modules/caddyhttp/server.go b/modules/caddyhttp/server.go
index 29a1d2b..fb30f0d 100644
--- a/modules/caddyhttp/server.go
+++ b/modules/caddyhttp/server.go
@@ -101,9 +101,9 @@ type Server struct {
 	// client authentication.
 	StrictSNIHost *bool `json:"strict_sni_host,omitempty"`
 
-	// Customizes how access logs are handled in this server. To
-	// minimally enable access logs, simply set this to a non-null,
-	// empty struct.
+	// Enables access logging and configures how access logs are handled
+	// in this server. To minimally enable access logs, simply set this
+	// to a non-null, empty struct.
 	Logs *ServerLogConfig `json:"logs,omitempty"`
 
 	// Enable experimental HTTP/3 support. Note that HTTP/3 is not a
@@ -157,7 +157,7 @@ func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		loggableReq,
 	)
 
-	if s.accessLogger != nil {
+	if s.shouldLogRequest(r) {
 		wrec := NewResponseRecorder(w, nil, nil)
 		w = wrec
 
@@ -390,17 +390,52 @@ func (*HTTPErrorConfig) WithError(r *http.Request, err error) *http.Request {
 	return r
 }
 
-// ServerLogConfig describes a server's logging configuration.
+// shouldLogRequest returns true if this request should be logged.
+func (s *Server) shouldLogRequest(r *http.Request) bool {
+	if s.accessLogger == nil || s.Logs == nil {
+		// logging is disabled
+		return false
+	}
+	for _, dh := range s.Logs.SkipHosts {
+		// logging for this particular host is disabled
+		if r.Host == dh {
+			return false
+		}
+	}
+	if _, ok := s.Logs.LoggerNames[r.Host]; ok {
+		// this host is mapped to a particular logger name
+		return true
+	}
+	if s.Logs.SkipUnmappedHosts {
+		// this host is not mapped and thus must not be logged
+		return false
+	}
+	return true
+}
+
+// ServerLogConfig describes a server's logging configuration. If
+// enabled without customization, all requests to this server are
+// logged to the default logger; logger destinations may be
+// customized per-request-host.
 type ServerLogConfig struct {
-	// The logger name for all logs emitted by this server unless
-	// the hostname is found in the LoggerNames (logger_names) map.
-	LoggerName string `json:"log_name,omitempty"`
+	// The default logger name for all logs emitted by this server for
+	// hostnames that are not in the LoggerNames (logger_names) map.
+	DefaultLoggerName string `json:"default_logger_name,omitempty"`
 
 	// LoggerNames maps request hostnames to a custom logger name.
 	// For example, a mapping of "example.com" to "example" would
 	// cause access logs from requests with a Host of example.com
 	// to be emitted by a logger named "http.log.access.example".
 	LoggerNames map[string]string `json:"logger_names,omitempty"`
+
+	// By default, all requests to this server will be logged if
+	// access logging is enabled. This field lists the request
+	// hosts for which access logging should be disabled.
+	SkipHosts []string `json:"skip_hosts,omitempty"`
+
+	// If true, requests to any host not appearing in the
+	// LoggerNames (logger_names) map will not be logged.
+	SkipUnmappedHosts bool `json:"skip_unmapped_hosts,omitempty"`
 }
 
 // wrapLogger wraps logger in a logger named according to user preferences for the given host.
@@ -415,7 +450,7 @@ func (slc ServerLogConfig) getLoggerName(host string) string {
 	if loggerName, ok := slc.LoggerNames[host]; ok {
 		return loggerName
 	}
-	return slc.LoggerName
+	return slc.DefaultLoggerName
 }
 
 // errLogValues inspects err and returns the status code