From 1896b420d818e3ba702b9a55aad9a694502e4007 Mon Sep 17 00:00:00 2001 From: Toby Allen Date: Tue, 27 Mar 2018 00:17:43 +0100 Subject: [PATCH] log: 'except' subdirective to skip logging certain requests (#2028) * proof of concept * Initial implementation with debug code * Tidy up debug code * remove unneeded import * removed extra line * Move ShouldLog function to rule entry Logger type * add tests for ShouldLog * Added tests for log exceptions * Fix logic * fix govet fail for test * Updates requested for code clarity * Update requested for style * log: Minor style tweaks to logic of log exceptions --- caddyhttp/httpserver/logger.go | 12 +++++ caddyhttp/log/log.go | 5 +++ caddyhttp/log/log_test.go | 82 ++++++++++++++++++++++++++++++++++ caddyhttp/log/setup.go | 9 +++- 4 files changed, 107 insertions(+), 1 deletion(-) diff --git a/caddyhttp/httpserver/logger.go b/caddyhttp/httpserver/logger.go index f8ec8b276..1fc262419 100644 --- a/caddyhttp/httpserver/logger.go +++ b/caddyhttp/httpserver/logger.go @@ -44,6 +44,7 @@ type Logger struct { V4ipMask net.IPMask V6ipMask net.IPMask IPMaskExists bool + Exceptions []string } // NewTestLogger creates logger suitable for testing purposes @@ -84,6 +85,17 @@ func (l Logger) MaskIP(ip string) string { } +// ShouldLog returns true if the path is not exempted from +// being logged (i.e. it is not found in l.Exceptions). +func (l Logger) ShouldLog(path string) bool { + for _, exc := range l.Exceptions { + if Path(path).Matches(exc) { + return false + } + } + return true +} + // Attach binds logger Start and Close functions to // controller's OnStartup and OnShutdown hooks. func (l *Logger) Attach(controller *caddy.Controller) { diff --git a/caddyhttp/log/log.go b/caddyhttp/log/log.go index 251334fa9..251f091ca 100644 --- a/caddyhttp/log/log.go +++ b/caddyhttp/log/log.go @@ -67,6 +67,10 @@ func (l Logger) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) { // Write log entries for _, e := range rule.Entries { + // Check if there is an exception to prevent log being written + if !e.Log.ShouldLog(r.URL.Path) { + continue + } // Mask IP Address if e.Log.IPMaskExists { @@ -78,6 +82,7 @@ func (l Logger) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) { } } e.Log.Println(rep.Replace(e.Format)) + } return status, err diff --git a/caddyhttp/log/log_test.go b/caddyhttp/log/log_test.go index b804dcc3b..0b1329be8 100644 --- a/caddyhttp/log/log_test.go +++ b/caddyhttp/log/log_test.go @@ -177,3 +177,85 @@ func TestMultiEntries(t *testing.T) { t.Errorf("Expected %q, but got %q", expect, got) } } + +func TestLogExcept(t *testing.T) { + tests := []struct { + LogRules []Rule + logPath string + shouldLog bool + }{ + {[]Rule{{ + PathScope: "/", + Entries: []*Entry{{ + Log: &httpserver.Logger{ + + Exceptions: []string{"/soup"}, + }, + Format: DefaultLogFormat, + }}, + }}, `/soup`, false}, + {[]Rule{{ + PathScope: "/", + Entries: []*Entry{{ + Log: &httpserver.Logger{ + + Exceptions: []string{"/tart"}, + }, + Format: DefaultLogFormat, + }}, + }}, `/soup`, true}, + {[]Rule{{ + PathScope: "/", + Entries: []*Entry{{ + Log: &httpserver.Logger{ + + Exceptions: []string{"/soup"}, + }, + Format: DefaultLogFormat, + }}, + }}, `/tomatosoup`, true}, + {[]Rule{{ + PathScope: "/", + Entries: []*Entry{{ + Log: &httpserver.Logger{ + + Exceptions: []string{"/pie/"}, + }, + Format: DefaultLogFormat, + }}, + // Check exception with a trailing slash does not match without + }}, `/pie`, true}, + {[]Rule{{ + PathScope: "/", + Entries: []*Entry{{ + Log: &httpserver.Logger{ + + Exceptions: []string{"/pie.php"}, + }, + Format: DefaultLogFormat, + }}, + }}, `/pie`, true}, + {[]Rule{{ + PathScope: "/", + Entries: []*Entry{{ + Log: &httpserver.Logger{ + + Exceptions: []string{"/pie"}, + }, + Format: DefaultLogFormat, + }}, + // Check that a word without trailing slash will match a filename + }}, `/pie.php`, false}, + } + for i, test := range tests { + for _, LogRule := range test.LogRules { + for _, e := range LogRule.Entries { + shouldLog := e.Log.ShouldLog(test.logPath) + if shouldLog != test.shouldLog { + t.Fatalf("Test %d expected shouldLog=%t but got shouldLog=%t,", i, test.shouldLog, shouldLog) + } + } + } + + } +} diff --git a/caddyhttp/log/setup.go b/caddyhttp/log/setup.go index 64f2f77ca..0c889a9f7 100644 --- a/caddyhttp/log/setup.go +++ b/caddyhttp/log/setup.go @@ -44,7 +44,7 @@ func setup(c *caddy.Controller) error { func logParse(c *caddy.Controller) ([]*Rule, error) { var rules []*Rule - + var logExceptions []string for c.Next() { args := c.RemainingArgs() @@ -91,6 +91,12 @@ func logParse(c *caddy.Controller) ([]*Rule, error) { } + } else if what == "except" { + + for i := 0; i < len(where); i++ { + logExceptions = append(logExceptions, where[i]) + } + } else if httpserver.IsLogRollerSubdirective(what) { if err := httpserver.ParseRoller(logRoller, what, where...); err != nil { @@ -133,6 +139,7 @@ func logParse(c *caddy.Controller) ([]*Rule, error) { V4ipMask: ip4Mask, V6ipMask: ip6Mask, IPMaskExists: ipMaskExists, + Exceptions: logExceptions, }, Format: format, })