From 5e0f4083c4e09ca08d4abe3a42ae9744ef84899f Mon Sep 17 00:00:00 2001 From: Tw Date: Mon, 22 Aug 2016 15:40:14 +0800 Subject: [PATCH] log: support multiple log entries under one path scope fix issue #1044 Signed-off-by: Tw --- caddyhttp/log/log.go | 19 ++-- caddyhttp/log/log_test.go | 16 ++-- caddyhttp/log/setup.go | 95 +++++++++++-------- caddyhttp/log/setup_test.go | 183 ++++++++++++++++++++++-------------- 4 files changed, 191 insertions(+), 122 deletions(-) diff --git a/caddyhttp/log/log.go b/caddyhttp/log/log.go index 4d2f8503..b32889b4 100644 --- a/caddyhttp/log/log.go +++ b/caddyhttp/log/log.go @@ -21,7 +21,7 @@ func init() { // Logger is a basic request logging middleware. type Logger struct { Next httpserver.Handler - Rules []Rule + Rules []*Rule ErrorFunc func(http.ResponseWriter, *http.Request, int) // failover error handler } @@ -52,8 +52,10 @@ func (l Logger) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) { status = 0 } - // Write log entry - rule.Log.Println(rep.Replace(rule.Format)) + // Write log entries + for _, e := range rule.Entries { + e.Log.Println(rep.Replace(e.Format)) + } return status, err } @@ -61,9 +63,8 @@ func (l Logger) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) { return l.Next.ServeHTTP(w, r) } -// Rule configures the logging middleware. -type Rule struct { - PathScope string +// Entry represents a log entry under a path scope +type Entry struct { OutputFile string Format string Log *log.Logger @@ -71,6 +72,12 @@ type Rule struct { file *os.File // if logging to a file that needs to be closed } +// Rule configures the logging middleware. +type Rule struct { + PathScope string + Entries []*Entry +} + const ( // DefaultLogFilename is the default log filename. DefaultLogFilename = "access.log" diff --git a/caddyhttp/log/log_test.go b/caddyhttp/log/log_test.go index eeb7cf2e..dce879da 100644 --- a/caddyhttp/log/log_test.go +++ b/caddyhttp/log/log_test.go @@ -26,12 +26,14 @@ func TestLoggedStatus(t *testing.T) { var next erroringMiddleware rule := Rule{ PathScope: "/", - Format: DefaultLogFormat + " {testval}", - Log: log.New(&f, "", 0), + Entries: []*Entry{{ + Format: DefaultLogFormat + " {testval}", + Log: log.New(&f, "", 0), + }}, } logger := Logger{ - Rules: []Rule{rule}, + Rules: []*Rule{&rule}, Next: next, } @@ -65,10 +67,12 @@ func TestLoggedStatus(t *testing.T) { func TestLogRequestBody(t *testing.T) { var got bytes.Buffer logger := Logger{ - Rules: []Rule{{ + Rules: []*Rule{{ PathScope: "/", - Format: "{request_body}", - Log: log.New(&got, "", 0), + Entries: []*Entry{{ + Format: "{request_body}", + Log: log.New(&got, "", 0), + }}, }}, Next: httpserver.HandlerFunc(func(w http.ResponseWriter, r *http.Request) (int, error) { // drain up body diff --git a/caddyhttp/log/setup.go b/caddyhttp/log/setup.go index 87980269..d738812d 100644 --- a/caddyhttp/log/setup.go +++ b/caddyhttp/log/setup.go @@ -20,39 +20,41 @@ func setup(c *caddy.Controller) error { // Open the log files for writing when the server starts c.OnStartup(func() error { - for i := 0; i < len(rules); i++ { - var err error - var writer io.Writer + for _, rule := range rules { + for _, entry := range rule.Entries { + var err error + var writer io.Writer - if rules[i].OutputFile == "stdout" { - writer = os.Stdout - } else if rules[i].OutputFile == "stderr" { - writer = os.Stderr - } else if rules[i].OutputFile == "syslog" { - writer, err = gsyslog.NewLogger(gsyslog.LOG_INFO, "LOCAL0", "caddy") - if err != nil { - return err - } - } else { - err := os.MkdirAll(filepath.Dir(rules[i].OutputFile), 0744) - if err != nil { - return err - } - file, err := os.OpenFile(rules[i].OutputFile, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0644) - if err != nil { - return err - } - if rules[i].Roller != nil { - file.Close() - rules[i].Roller.Filename = rules[i].OutputFile - writer = rules[i].Roller.GetLogWriter() + if entry.OutputFile == "stdout" { + writer = os.Stdout + } else if entry.OutputFile == "stderr" { + writer = os.Stderr + } else if entry.OutputFile == "syslog" { + writer, err = gsyslog.NewLogger(gsyslog.LOG_INFO, "LOCAL0", "caddy") + if err != nil { + return err + } } else { - rules[i].file = file - writer = file + err := os.MkdirAll(filepath.Dir(entry.OutputFile), 0744) + if err != nil { + return err + } + file, err := os.OpenFile(entry.OutputFile, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0644) + if err != nil { + return err + } + if entry.Roller != nil { + file.Close() + entry.Roller.Filename = entry.OutputFile + writer = entry.Roller.GetLogWriter() + } else { + entry.file = file + writer = file + } } - } - rules[i].Log = log.New(writer, "", 0) + entry.Log = log.New(writer, "", 0) + } } return nil @@ -61,8 +63,10 @@ func setup(c *caddy.Controller) error { // When server stops, close any open log files c.OnShutdown(func() error { for _, rule := range rules { - if rule.file != nil { - rule.file.Close() + for _, entry := range rule.Entries { + if entry.file != nil { + entry.file.Close() + } } } return nil @@ -75,8 +79,8 @@ func setup(c *caddy.Controller) error { return nil } -func logParse(c *caddy.Controller) ([]Rule, error) { - var rules []Rule +func logParse(c *caddy.Controller) ([]*Rule, error) { + var rules []*Rule for c.Next() { args := c.RemainingArgs() @@ -103,16 +107,14 @@ func logParse(c *caddy.Controller) ([]Rule, error) { } if len(args) == 0 { // Nothing specified; use defaults - rules = append(rules, Rule{ - PathScope: "/", + rules = appendEntry(rules, "/", &Entry{ OutputFile: DefaultLogFilename, Format: DefaultLogFormat, Roller: logRoller, }) } else if len(args) == 1 { // Only an output file specified - rules = append(rules, Rule{ - PathScope: "/", + rules = appendEntry(rules, "/", &Entry{ OutputFile: args[0], Format: DefaultLogFormat, Roller: logRoller, @@ -133,8 +135,7 @@ func logParse(c *caddy.Controller) ([]Rule, error) { } } - rules = append(rules, Rule{ - PathScope: args[0], + rules = appendEntry(rules, args[0], &Entry{ OutputFile: args[1], Format: format, Roller: logRoller, @@ -144,3 +145,19 @@ func logParse(c *caddy.Controller) ([]Rule, error) { return rules, nil } + +func appendEntry(rules []*Rule, pathScope string, entry *Entry) []*Rule { + for _, rule := range rules { + if rule.PathScope == pathScope { + rule.Entries = append(rule.Entries, entry) + return rules + } + } + + rules = append(rules, &Rule{ + PathScope: pathScope, + Entries: []*Entry{entry}, + }) + + return rules +} diff --git a/caddyhttp/log/setup_test.go b/caddyhttp/log/setup_test.go index 58191804..8d613907 100644 --- a/caddyhttp/log/setup_test.go +++ b/caddyhttp/log/setup_test.go @@ -29,14 +29,15 @@ func TestSetup(t *testing.T) { if myHandler.Rules[0].PathScope != "/" { t.Errorf("Expected / as the default PathScope") } - if myHandler.Rules[0].OutputFile != DefaultLogFilename { + if myHandler.Rules[0].Entries[0].OutputFile != DefaultLogFilename { t.Errorf("Expected %s as the default OutputFile", DefaultLogFilename) } - if myHandler.Rules[0].Format != DefaultLogFormat { + if myHandler.Rules[0].Entries[0].Format != DefaultLogFormat { t.Errorf("Expected %s as the default Log Format", DefaultLogFormat) } - if myHandler.Rules[0].Roller != nil { - t.Errorf("Expected Roller to be nil, got: %v", *myHandler.Rules[0].Roller) + if myHandler.Rules[0].Entries[0].Roller != nil { + t.Errorf("Expected Roller to be nil, got: %v", + *myHandler.Rules[0].Entries[0].Roller) } if !httpserver.SameNext(myHandler.Next, httpserver.EmptyNext) { t.Error("'Next' field of handler was not set properly") @@ -51,65 +52,98 @@ func TestLogParse(t *testing.T) { expectedLogRules []Rule }{ {`log`, false, []Rule{{ - PathScope: "/", - OutputFile: DefaultLogFilename, - Format: DefaultLogFormat, + PathScope: "/", + Entries: []*Entry{{ + OutputFile: DefaultLogFilename, + Format: DefaultLogFormat, + }}, }}}, {`log log.txt`, false, []Rule{{ - PathScope: "/", - OutputFile: "log.txt", - Format: DefaultLogFormat, + PathScope: "/", + Entries: []*Entry{{ + OutputFile: "log.txt", + Format: DefaultLogFormat, + }}, }}}, {`log /api log.txt`, false, []Rule{{ - PathScope: "/api", - OutputFile: "log.txt", - Format: DefaultLogFormat, + PathScope: "/api", + Entries: []*Entry{{ + OutputFile: "log.txt", + Format: DefaultLogFormat, + }}, }}}, {`log /serve stdout`, false, []Rule{{ - PathScope: "/serve", - OutputFile: "stdout", - Format: DefaultLogFormat, + PathScope: "/serve", + Entries: []*Entry{{ + OutputFile: "stdout", + Format: DefaultLogFormat, + }}, }}}, {`log /myapi log.txt {common}`, false, []Rule{{ - PathScope: "/myapi", - OutputFile: "log.txt", - Format: CommonLogFormat, + PathScope: "/myapi", + Entries: []*Entry{{ + OutputFile: "log.txt", + Format: CommonLogFormat, + }}, }}}, {`log /test accesslog.txt {combined}`, false, []Rule{{ - PathScope: "/test", - OutputFile: "accesslog.txt", - Format: CombinedLogFormat, + PathScope: "/test", + Entries: []*Entry{{ + OutputFile: "accesslog.txt", + Format: CombinedLogFormat, + }}, }}}, {`log /api1 log.txt log /api2 accesslog.txt {combined}`, false, []Rule{{ - PathScope: "/api1", - OutputFile: "log.txt", - Format: DefaultLogFormat, + PathScope: "/api1", + Entries: []*Entry{{ + OutputFile: "log.txt", + Format: DefaultLogFormat, + }}, }, { - PathScope: "/api2", - OutputFile: "accesslog.txt", - Format: CombinedLogFormat, + PathScope: "/api2", + Entries: []*Entry{{ + OutputFile: "accesslog.txt", + Format: CombinedLogFormat, + }}, }}}, {`log /api3 stdout {host} log /api4 log.txt {when}`, false, []Rule{{ - PathScope: "/api3", - OutputFile: "stdout", - Format: "{host}", + PathScope: "/api3", + Entries: []*Entry{{ + OutputFile: "stdout", + Format: "{host}", + }}, }, { - PathScope: "/api4", - OutputFile: "log.txt", - Format: "{when}", + PathScope: "/api4", + Entries: []*Entry{{ + OutputFile: "log.txt", + Format: "{when}", + }}, }}}, {`log access.log { rotate { size 2 age 10 keep 3 } }`, false, []Rule{{ - PathScope: "/", - OutputFile: "access.log", - Format: DefaultLogFormat, - Roller: &httpserver.LogRoller{ - MaxSize: 2, - MaxAge: 10, - MaxBackups: 3, - LocalTime: true, - }, + PathScope: "/", + Entries: []*Entry{{ + OutputFile: "access.log", + Format: DefaultLogFormat, + Roller: &httpserver.LogRoller{ + MaxSize: 2, + MaxAge: 10, + MaxBackups: 3, + LocalTime: true, + }, + }}, + }}}, + {`log / stdout {host} + log / log.txt {when}`, false, []Rule{{ + PathScope: "/", + Entries: []*Entry{{ + OutputFile: "stdout", + Format: "{host}", + }, { + OutputFile: "log.txt", + Format: "{when}", + }}, }}}, } for i, test := range tests { @@ -132,39 +166,46 @@ func TestLogParse(t *testing.T) { i, j, test.expectedLogRules[j].PathScope, actualLogRule.PathScope) } - if actualLogRule.OutputFile != test.expectedLogRules[j].OutputFile { - t.Errorf("Test %d expected %dth LogRule OutputFile to be %s , but got %s", - i, j, test.expectedLogRules[j].OutputFile, actualLogRule.OutputFile) + if got, expect := len(actualLogRule.Entries), len(test.expectedLogRules[j].Entries); got != expect { + t.Fatalf("Test %d expected %dth LogRule with %d no of Log entries, but got %d ", + i, j, expect, got) } - if actualLogRule.Format != test.expectedLogRules[j].Format { - t.Errorf("Test %d expected %dth LogRule Format to be %s , but got %s", - i, j, test.expectedLogRules[j].Format, actualLogRule.Format) - } - if actualLogRule.Roller != nil && test.expectedLogRules[j].Roller == nil || actualLogRule.Roller == nil && test.expectedLogRules[j].Roller != nil { - t.Fatalf("Test %d expected %dth LogRule Roller to be %v, but got %v", - i, j, test.expectedLogRules[j].Roller, actualLogRule.Roller) - } - if actualLogRule.Roller != nil && test.expectedLogRules[j].Roller != nil { - if actualLogRule.Roller.Filename != test.expectedLogRules[j].Roller.Filename { - t.Fatalf("Test %d expected %dth LogRule Roller Filename to be %s, but got %s", - i, j, test.expectedLogRules[j].Roller.Filename, actualLogRule.Roller.Filename) + for k, actualEntry := range actualLogRule.Entries { + if actualEntry.OutputFile != test.expectedLogRules[j].Entries[k].OutputFile { + t.Errorf("Test %d expected %dth LogRule OutputFile to be %s , but got %s", + i, j, test.expectedLogRules[j].Entries[k].OutputFile, actualEntry.OutputFile) } - if actualLogRule.Roller.MaxAge != test.expectedLogRules[j].Roller.MaxAge { - t.Fatalf("Test %d expected %dth LogRule Roller MaxAge to be %d, but got %d", - i, j, test.expectedLogRules[j].Roller.MaxAge, actualLogRule.Roller.MaxAge) + + if actualEntry.Format != test.expectedLogRules[j].Entries[k].Format { + t.Errorf("Test %d expected %dth LogRule Format to be %s , but got %s", + i, j, test.expectedLogRules[j].Entries[k].Format, actualEntry.Format) } - if actualLogRule.Roller.MaxBackups != test.expectedLogRules[j].Roller.MaxBackups { - t.Fatalf("Test %d expected %dth LogRule Roller MaxBackups to be %d, but got %d", - i, j, test.expectedLogRules[j].Roller.MaxBackups, actualLogRule.Roller.MaxBackups) + if actualEntry.Roller != nil && test.expectedLogRules[j].Entries[k].Roller == nil || actualEntry.Roller == nil && test.expectedLogRules[j].Entries[k].Roller != nil { + t.Fatalf("Test %d expected %dth LogRule Roller to be %v, but got %v", + i, j, test.expectedLogRules[j].Entries[k].Roller, actualEntry.Roller) } - if actualLogRule.Roller.MaxSize != test.expectedLogRules[j].Roller.MaxSize { - t.Fatalf("Test %d expected %dth LogRule Roller MaxSize to be %d, but got %d", - i, j, test.expectedLogRules[j].Roller.MaxSize, actualLogRule.Roller.MaxSize) - } - if actualLogRule.Roller.LocalTime != test.expectedLogRules[j].Roller.LocalTime { - t.Fatalf("Test %d expected %dth LogRule Roller LocalTime to be %t, but got %t", - i, j, test.expectedLogRules[j].Roller.LocalTime, actualLogRule.Roller.LocalTime) + if actualEntry.Roller != nil && test.expectedLogRules[j].Entries[k].Roller != nil { + if actualEntry.Roller.Filename != test.expectedLogRules[j].Entries[k].Roller.Filename { + t.Fatalf("Test %d expected %dth LogRule Roller Filename to be %s, but got %s", + i, j, test.expectedLogRules[j].Entries[k].Roller.Filename, actualEntry.Roller.Filename) + } + if actualEntry.Roller.MaxAge != test.expectedLogRules[j].Entries[k].Roller.MaxAge { + t.Fatalf("Test %d expected %dth LogRule Roller MaxAge to be %d, but got %d", + i, j, test.expectedLogRules[j].Entries[k].Roller.MaxAge, actualEntry.Roller.MaxAge) + } + if actualEntry.Roller.MaxBackups != test.expectedLogRules[j].Entries[k].Roller.MaxBackups { + t.Fatalf("Test %d expected %dth LogRule Roller MaxBackups to be %d, but got %d", + i, j, test.expectedLogRules[j].Entries[k].Roller.MaxBackups, actualEntry.Roller.MaxBackups) + } + if actualEntry.Roller.MaxSize != test.expectedLogRules[j].Entries[k].Roller.MaxSize { + t.Fatalf("Test %d expected %dth LogRule Roller MaxSize to be %d, but got %d", + i, j, test.expectedLogRules[j].Entries[k].Roller.MaxSize, actualEntry.Roller.MaxSize) + } + if actualEntry.Roller.LocalTime != test.expectedLogRules[j].Entries[k].Roller.LocalTime { + t.Fatalf("Test %d expected %dth LogRule Roller LocalTime to be %t, but got %t", + i, j, test.expectedLogRules[j].Entries[k].Roller.LocalTime, actualEntry.Roller.LocalTime) + } } } }