From 0d44e3ecbaa0b16894e936068785e7fe32f41b48 Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Tue, 5 Mar 2024 19:03:59 -0500 Subject: [PATCH] logging: Implement `log_append` handler (#6066) * logging: Implement `extra_log` handler * Rename to `log_append` * Rename `skip_log` to `log_skip` --------- Co-authored-by: Matt Holt --- caddyconfig/httpcaddyfile/builtins.go | 17 +++- caddyconfig/httpcaddyfile/directives.go | 3 +- .../caddyfile_adapt/log_add.caddyfiletest | 71 ++++++++++++++ .../log_except_catchall_blocks.caddyfiletest | 8 +- modules/caddyhttp/logging.go | 2 +- modules/caddyhttp/logging/caddyfile.go | 53 +++++++++++ modules/caddyhttp/logging/logadd.go | 94 +++++++++++++++++++ modules/caddyhttp/server.go | 2 +- modules/caddyhttp/standard/imports.go | 1 + 9 files changed, 239 insertions(+), 12 deletions(-) create mode 100644 caddytest/integration/caddyfile_adapt/log_add.caddyfiletest create mode 100644 modules/caddyhttp/logging/caddyfile.go create mode 100644 modules/caddyhttp/logging/logadd.go diff --git a/caddyconfig/httpcaddyfile/builtins.go b/caddyconfig/httpcaddyfile/builtins.go index 5040924d..505885d2 100644 --- a/caddyconfig/httpcaddyfile/builtins.go +++ b/caddyconfig/httpcaddyfile/builtins.go @@ -49,7 +49,8 @@ func init() { RegisterDirective("handle_errors", parseHandleErrors) RegisterHandlerDirective("invoke", parseInvoke) RegisterDirective("log", parseLog) - RegisterHandlerDirective("skip_log", parseSkipLog) + RegisterHandlerDirective("skip_log", parseLogSkip) + RegisterHandlerDirective("log_skip", parseLogSkip) } // parseBind parses the bind directive. Syntax: @@ -1038,13 +1039,19 @@ func parseLogHelper(h Helper, globalLogNames map[string]struct{}) ([]ConfigValue return configValues, nil } -// parseSkipLog parses the skip_log directive. Syntax: +// parseLogSkip parses the log_skip directive. Syntax: // -// skip_log [] -func parseSkipLog(h Helper) (caddyhttp.MiddlewareHandler, error) { +// log_skip [] +func parseLogSkip(h Helper) (caddyhttp.MiddlewareHandler, error) { h.Next() // consume directive name + + // "skip_log" is deprecated, replaced by "log_skip" + if h.Val() == "skip_log" { + caddy.Log().Named("config.adapter.caddyfile").Warn("the 'skip_log' directive is deprecated, please use 'log_skip' instead!") + } + if h.NextArg() { return nil, h.ArgErr() } - return caddyhttp.VarsMiddleware{"skip_log": true}, nil + return caddyhttp.VarsMiddleware{"log_skip": true}, nil } diff --git a/caddyconfig/httpcaddyfile/directives.go b/caddyconfig/httpcaddyfile/directives.go index 9a549a18..13026fa4 100644 --- a/caddyconfig/httpcaddyfile/directives.go +++ b/caddyconfig/httpcaddyfile/directives.go @@ -43,7 +43,8 @@ var directiveOrder = []string{ "vars", "fs", "root", - "skip_log", + "log_append", + "log_skip", "header", "copy_response_headers", // only in reverse_proxy's handle_response diff --git a/caddytest/integration/caddyfile_adapt/log_add.caddyfiletest b/caddytest/integration/caddyfile_adapt/log_add.caddyfiletest new file mode 100644 index 00000000..4f91e464 --- /dev/null +++ b/caddytest/integration/caddyfile_adapt/log_add.caddyfiletest @@ -0,0 +1,71 @@ +:80 { + log + + vars foo foo + + log_append const bar + log_append vars foo + log_append placeholder {path} + + log_append /only-for-this-path secret value +} +---------- +{ + "apps": { + "http": { + "servers": { + "srv0": { + "listen": [ + ":80" + ], + "routes": [ + { + "handle": [ + { + "foo": "foo", + "handler": "vars" + } + ] + }, + { + "match": [ + { + "path": [ + "/only-for-this-path" + ] + } + ], + "handle": [ + { + "handler": "log_append", + "key": "secret", + "value": "value" + } + ] + }, + { + "handle": [ + { + "handler": "log_append", + "key": "const", + "value": "bar" + }, + { + "handler": "log_append", + "key": "vars", + "value": "foo" + }, + { + "handler": "log_append", + "key": "placeholder", + "value": "{http.request.uri.path}" + } + ] + } + ], + "logs": {} + } + } + } + } +} diff --git a/caddytest/integration/caddyfile_adapt/log_except_catchall_blocks.caddyfiletest b/caddytest/integration/caddyfile_adapt/log_except_catchall_blocks.caddyfiletest index 6fbc6c7c..e1362f8f 100644 --- a/caddytest/integration/caddyfile_adapt/log_except_catchall_blocks.caddyfiletest +++ b/caddytest/integration/caddyfile_adapt/log_except_catchall_blocks.caddyfiletest @@ -1,7 +1,7 @@ http://localhost:2020 { log - skip_log /first-hidden* - skip_log /second-hidden* + log_skip /first-hidden* + log_skip /second-hidden* respond 200 } @@ -34,7 +34,7 @@ http://localhost:2020 { "handle": [ { "handler": "vars", - "skip_log": true + "log_skip": true } ], "match": [ @@ -49,7 +49,7 @@ http://localhost:2020 { "handle": [ { "handler": "vars", - "skip_log": true + "log_skip": true } ], "match": [ diff --git a/modules/caddyhttp/logging.go b/modules/caddyhttp/logging.go index 728befd2..81b2830f 100644 --- a/modules/caddyhttp/logging.go +++ b/modules/caddyhttp/logging.go @@ -166,7 +166,7 @@ func (e *ExtraLogFields) Set(field zap.Field) { const ( // Variable name used to indicate that this request // should be omitted from the access logs - SkipLogVar string = "skip_log" + LogSkipVar string = "log_skip" // For adding additional fields to the access logs ExtraLogFieldsCtxKey caddy.CtxKey = "extra_log_fields" diff --git a/modules/caddyhttp/logging/caddyfile.go b/modules/caddyhttp/logging/caddyfile.go new file mode 100644 index 00000000..010b4891 --- /dev/null +++ b/modules/caddyhttp/logging/caddyfile.go @@ -0,0 +1,53 @@ +// Copyright 2015 Matthew Holt and The Caddy Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package logging + +import ( + "github.com/caddyserver/caddy/v2/caddyconfig/caddyfile" + "github.com/caddyserver/caddy/v2/caddyconfig/httpcaddyfile" + "github.com/caddyserver/caddy/v2/modules/caddyhttp" +) + +func init() { + httpcaddyfile.RegisterHandlerDirective("log_append", parseCaddyfile) +} + +// parseCaddyfile sets up the log_append handler from Caddyfile tokens. Syntax: +// +// log_append [] +func parseCaddyfile(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, error) { + handler := new(LogAppend) + err := handler.UnmarshalCaddyfile(h.Dispenser) + return handler, err +} + +// UnmarshalCaddyfile implements caddyfile.Unmarshaler. +func (h *LogAppend) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { + d.Next() // consume directive name + if !d.NextArg() { + return d.ArgErr() + } + h.Key = d.Val() + if !d.NextArg() { + return d.ArgErr() + } + h.Value = d.Val() + return nil +} + +// Interface guards +var ( + _ caddyfile.Unmarshaler = (*LogAppend)(nil) +) diff --git a/modules/caddyhttp/logging/logadd.go b/modules/caddyhttp/logging/logadd.go new file mode 100644 index 00000000..3b554367 --- /dev/null +++ b/modules/caddyhttp/logging/logadd.go @@ -0,0 +1,94 @@ +// Copyright 2015 Matthew Holt and The Caddy Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package logging + +import ( + "net/http" + "strings" + + "go.uber.org/zap" + + "github.com/caddyserver/caddy/v2" + "github.com/caddyserver/caddy/v2/modules/caddyhttp" +) + +func init() { + caddy.RegisterModule(LogAppend{}) +} + +// LogAppend implements a middleware that takes a key and value, where +// the key is the name of a log field and the value is a placeholder, +// or variable key, or constant value to use for that field. +type LogAppend struct { + // Key is the name of the log field. + Key string `json:"key,omitempty"` + + // Value is the value to use for the log field. + // If it is a placeholder (with surrounding `{}`), + // it will be evaluated when the log is written. + // If the value is a key that exists in the `vars` + // map, the value of that key will be used. Otherwise + // the value will be used as-is as a constant string. + Value string `json:"value,omitempty"` +} + +// CaddyModule returns the Caddy module information. +func (LogAppend) CaddyModule() caddy.ModuleInfo { + return caddy.ModuleInfo{ + ID: "http.handlers.log_append", + New: func() caddy.Module { return new(LogAppend) }, + } +} + +func (h LogAppend) ServeHTTP(w http.ResponseWriter, r *http.Request, next caddyhttp.Handler) error { + // Run the next handler in the chain first. + // If an error occurs, we still want to add + // any extra log fields that we can, so we + // hold onto the error and return it later. + handlerErr := next.ServeHTTP(w, r) + + // On the way back up the chain, add the extra log field + ctx := r.Context() + + vars := ctx.Value(caddyhttp.VarsCtxKey).(map[string]any) + repl := ctx.Value(caddy.ReplacerCtxKey).(*caddy.Replacer) + extra := ctx.Value(caddyhttp.ExtraLogFieldsCtxKey).(*caddyhttp.ExtraLogFields) + + var varValue any + if strings.HasPrefix(h.Value, "{") && + strings.HasSuffix(h.Value, "}") && + strings.Count(h.Value, "{") == 1 { + // the value looks like a placeholder, so get its value + varValue, _ = repl.Get(strings.Trim(h.Value, "{}")) + } else if val, ok := vars[h.Value]; ok { + // the value is a key in the vars map + varValue = val + } else { + // the value is a constant string + varValue = h.Value + } + + // Add the field to the extra log fields. + // We use zap.Any because it will reflect + // to the correct type for us. + extra.Add(zap.Any(h.Key, varValue)) + + return handlerErr +} + +// Interface guards +var ( + _ caddyhttp.MiddlewareHandler = (*LogAppend)(nil) +) diff --git a/modules/caddyhttp/server.go b/modules/caddyhttp/server.go index 9ea04acb..04ae003a 100644 --- a/modules/caddyhttp/server.go +++ b/modules/caddyhttp/server.go @@ -715,7 +715,7 @@ func (s *Server) logRequest( repl *caddy.Replacer, bodyReader *lengthReader, shouldLogCredentials bool, ) { // this request may be flagged as omitted from the logs - if skipLog, ok := GetVar(r.Context(), SkipLogVar).(bool); ok && skipLog { + if skip, ok := GetVar(r.Context(), LogSkipVar).(bool); ok && skip { return } diff --git a/modules/caddyhttp/standard/imports.go b/modules/caddyhttp/standard/imports.go index d7bb2800..236e7be1 100644 --- a/modules/caddyhttp/standard/imports.go +++ b/modules/caddyhttp/standard/imports.go @@ -10,6 +10,7 @@ import ( _ "github.com/caddyserver/caddy/v2/modules/caddyhttp/encode/zstd" _ "github.com/caddyserver/caddy/v2/modules/caddyhttp/fileserver" _ "github.com/caddyserver/caddy/v2/modules/caddyhttp/headers" + _ "github.com/caddyserver/caddy/v2/modules/caddyhttp/logging" _ "github.com/caddyserver/caddy/v2/modules/caddyhttp/map" _ "github.com/caddyserver/caddy/v2/modules/caddyhttp/proxyprotocol" _ "github.com/caddyserver/caddy/v2/modules/caddyhttp/push"