From bb5a322ce27a95965c081c490db42568c7555d7e Mon Sep 17 00:00:00 2001 From: Maxime Date: Sat, 8 Aug 2015 16:13:10 +0200 Subject: [PATCH 1/2] Added lumberjack library for log rolling --- config/setup/errors.go | 8 +++++++- config/setup/log.go | 8 +++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/config/setup/errors.go b/config/setup/errors.go index 58f22dfa..9221ee92 100644 --- a/config/setup/errors.go +++ b/config/setup/errors.go @@ -11,6 +11,7 @@ import ( "github.com/hashicorp/go-syslog" "github.com/mholt/caddy/middleware" "github.com/mholt/caddy/middleware/errors" + "gopkg.in/natefinch/lumberjack.v2" ) // Errors configures a new gzip middleware instance. @@ -35,10 +36,15 @@ func Errors(c *Controller) (middleware.Middleware, error) { return err } } else if handler.LogFile != "" { - file, err = os.OpenFile(handler.LogFile, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0644) + _, err = os.OpenFile(handler.LogFile, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0644) if err != nil { return err } + file = &lumberjack.Logger{ + Filename: handler.LogFile, + MaxSize: 20, + MaxBackups: 10, + } } handler.Log = log.New(file, "", 0) diff --git a/config/setup/log.go b/config/setup/log.go index 0c15c3d0..6bf9f312 100644 --- a/config/setup/log.go +++ b/config/setup/log.go @@ -9,6 +9,7 @@ import ( "github.com/mholt/caddy/middleware" caddylog "github.com/mholt/caddy/middleware/log" "github.com/mholt/caddy/server" + "gopkg.in/natefinch/lumberjack.v2" ) // Log sets up the logging middleware. @@ -34,10 +35,15 @@ func Log(c *Controller) (middleware.Middleware, error) { return err } } else { - file, err = os.OpenFile(rules[i].OutputFile, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0644) + _, err = os.OpenFile(rules[i].OutputFile, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0644) if err != nil { return err } + file = &lumberjack.Logger{ + Filename: rules[i].OutputFile, + MaxSize: 20, + MaxBackups: 10, + } } rules[i].Log = log.New(file, "", 0) From 008160998abd17e4d56fd1f4bb26581e3744c020 Mon Sep 17 00:00:00 2001 From: Maxime Date: Wed, 2 Sep 2015 15:13:31 +0200 Subject: [PATCH 2/2] Added LogRoller parser and entity. The errors and logs can now have log rolling if provided by the user. The current customisable parameter of it are: The maximal size of the file before rolling. The maximal age/time of the file before rolling. The number of backups to keep. --- config/parse/dispenser.go | 5 ++ config/setup/errors.go | 40 ++++++++--- config/setup/errors_test.go | 137 ++++++++++++++++++++++++++++++++++++ config/setup/log.go | 47 ++++++++++--- config/setup/log_test.go | 42 ++++++++++- config/setup/roller.go | 39 ++++++++++ middleware/errors/errors.go | 1 + middleware/log/log.go | 1 + middleware/roller.go | 25 +++++++ 9 files changed, 314 insertions(+), 23 deletions(-) create mode 100644 config/setup/errors_test.go create mode 100644 config/setup/roller.go create mode 100644 middleware/roller.go diff --git a/config/parse/dispenser.go b/config/parse/dispenser.go index e8a0c847..a7457f56 100644 --- a/config/parse/dispenser.go +++ b/config/parse/dispenser.go @@ -119,6 +119,11 @@ func (d *Dispenser) NextBlock() bool { return true } +func (d *Dispenser) IncrNest() { + d.nesting++ + return +} + // Val gets the text of the current token. If there is no token // loaded, it returns empty string. func (d *Dispenser) Val() string { diff --git a/config/setup/errors.go b/config/setup/errors.go index 9221ee92..ae42f381 100644 --- a/config/setup/errors.go +++ b/config/setup/errors.go @@ -11,7 +11,6 @@ import ( "github.com/hashicorp/go-syslog" "github.com/mholt/caddy/middleware" "github.com/mholt/caddy/middleware/errors" - "gopkg.in/natefinch/lumberjack.v2" ) // Errors configures a new gzip middleware instance. @@ -24,30 +23,35 @@ func Errors(c *Controller) (middleware.Middleware, error) { // Open the log file for writing when the server starts c.Startup = append(c.Startup, func() error { var err error - var file io.Writer + var writer io.Writer if handler.LogFile == "stdout" { - file = os.Stdout + writer = os.Stdout } else if handler.LogFile == "stderr" { - file = os.Stderr + writer = os.Stderr } else if handler.LogFile == "syslog" { - file, err = gsyslog.NewLogger(gsyslog.LOG_ERR, "LOCAL0", "caddy") + writer, err = gsyslog.NewLogger(gsyslog.LOG_ERR, "LOCAL0", "caddy") if err != nil { return err } } else if handler.LogFile != "" { - _, err = os.OpenFile(handler.LogFile, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0644) + var file *os.File + file, err = os.OpenFile(handler.LogFile, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0644) if err != nil { return err } - file = &lumberjack.Logger{ - Filename: handler.LogFile, - MaxSize: 20, - MaxBackups: 10, + if handler.LogRoller != nil { + file.Close() + + handler.LogRoller.Filename = handler.LogFile + + writer = handler.LogRoller.GetLogWriter() + } else { + writer = file } } - handler.Log = log.New(file, "", 0) + handler.Log = log.New(writer, "", 0) return nil }) @@ -77,6 +81,16 @@ func errorsParse(c *Controller) (*errors.ErrorHandler, error) { if what == "log" { handler.LogFile = where + if c.NextArg() { + if c.Val() == "{" { + c.IncrNest() + logRoller, err := parseRoller(c) + if err != nil { + return hadBlock, err + } + handler.LogRoller = logRoller + } + } } else { // Error page; ensure it exists where = path.Join(c.Root, where) @@ -97,6 +111,10 @@ func errorsParse(c *Controller) (*errors.ErrorHandler, error) { } for c.Next() { + // weird hack to avoid having the handler values overwritten. + if c.Val() == "}" { + continue + } // Configuration may be in a block hadBlock, err := optionalBlock() if err != nil { diff --git a/config/setup/errors_test.go b/config/setup/errors_test.go new file mode 100644 index 00000000..5ece288d --- /dev/null +++ b/config/setup/errors_test.go @@ -0,0 +1,137 @@ +package setup + +import ( + "testing" + + "github.com/mholt/caddy/middleware" + "github.com/mholt/caddy/middleware/errors" +) + +func TestErrors(t *testing.T) { + + c := NewTestController(`errors`) + + mid, err := Errors(c) + + if err != nil { + t.Errorf("Expected no errors, got: %v", err) + } + + if mid == nil { + t.Fatal("Expected middleware, was nil instead") + } + + handler := mid(EmptyNext) + myHandler, ok := handler.(*errors.ErrorHandler) + + if !ok { + t.Fatalf("Expected handler to be type ErrorHandler, got: %#v", handler) + } + + if myHandler.LogFile != errors.DefaultLogFilename { + t.Errorf("Expected %s as the default LogFile", errors.DefaultLogFilename) + } + if myHandler.LogRoller != nil { + t.Errorf("Expected LogRoller to be nil, got: %v", *myHandler.LogRoller) + } + if !SameNext(myHandler.Next, EmptyNext) { + t.Error("'Next' field of handler was not set properly") + } +} + +func TestErrorsParse(t *testing.T) { + tests := []struct { + inputErrorsRules string + shouldErr bool + expectedErrorHandler errors.ErrorHandler + }{ + {`errors`, false, errors.ErrorHandler{ + LogFile: errors.DefaultLogFilename, + }}, + {`errors errors.txt`, false, errors.ErrorHandler{ + LogFile: "errors.txt", + }}, + {`errors { log errors.txt + 404 404.html + 500 500.html +}`, false, errors.ErrorHandler{ + LogFile: "errors.txt", + ErrorPages: map[int]string{ + 404: "404.html", + 500: "500.html", + }, + }}, + {`errors { log errors.txt { size 2 age 10 keep 3 } }`, false, errors.ErrorHandler{ + LogFile: "errors.txt", + LogRoller: &middleware.LogRoller{ + MaxSize: 2, + MaxAge: 10, + MaxBackups: 3, + }, + }}, + {`errors { log errors.txt { + size 3 + age 11 + keep 5 + } + 404 404.html + 503 503.html +}`, false, errors.ErrorHandler{ + LogFile: "errors.txt", + ErrorPages: map[int]string{ + 404: "404.html", + 503: "503.html", + }, + LogRoller: &middleware.LogRoller{ + MaxSize: 3, + MaxAge: 11, + MaxBackups: 5, + }, + }}, + } + for i, test := range tests { + c := NewTestController(test.inputErrorsRules) + actualErrorsRule, err := errorsParse(c) + + if err == nil && test.shouldErr { + t.Errorf("Test %d didn't error, but it should have", i) + } else if err != nil && !test.shouldErr { + t.Errorf("Test %d errored, but it shouldn't have; got '%v'", i, err) + } + if actualErrorsRule.LogFile != test.expectedErrorHandler.LogFile { + t.Errorf("Test %d expected LogFile to be %s , but got %s", + i, test.expectedErrorHandler.LogFile, actualErrorsRule.LogFile) + } + if actualErrorsRule.LogRoller != nil && test.expectedErrorHandler.LogRoller == nil || actualErrorsRule.LogRoller == nil && test.expectedErrorHandler.LogRoller != nil { + t.Fatalf("Test %d expected LogRoller to be %v, but got %v", + i, test.expectedErrorHandler.LogRoller, actualErrorsRule.LogRoller) + } + if len(actualErrorsRule.ErrorPages) != len(test.expectedErrorHandler.ErrorPages) { + t.Fatalf("Test %d expected %d no of Error pages, but got %d ", + i, len(test.expectedErrorHandler.ErrorPages), len(actualErrorsRule.ErrorPages)) + } + if actualErrorsRule.LogRoller != nil && test.expectedErrorHandler.LogRoller != nil { + if actualErrorsRule.LogRoller.Filename != test.expectedErrorHandler.LogRoller.Filename { + t.Fatalf("Test %d expected LogRoller Filename to be %s, but got %s", + i, test.expectedErrorHandler.LogRoller.Filename, actualErrorsRule.LogRoller.Filename) + } + if actualErrorsRule.LogRoller.MaxAge != test.expectedErrorHandler.LogRoller.MaxAge { + t.Fatalf("Test %d expected LogRoller MaxAge to be %d, but got %d", + i, test.expectedErrorHandler.LogRoller.MaxAge, actualErrorsRule.LogRoller.MaxAge) + } + if actualErrorsRule.LogRoller.MaxBackups != test.expectedErrorHandler.LogRoller.MaxBackups { + t.Fatalf("Test %d expected LogRoller MaxBackups to be %d, but got %d", + i, test.expectedErrorHandler.LogRoller.MaxBackups, actualErrorsRule.LogRoller.MaxBackups) + } + if actualErrorsRule.LogRoller.MaxSize != test.expectedErrorHandler.LogRoller.MaxSize { + t.Fatalf("Test %d expected LogRoller MaxSize to be %d, but got %d", + i, test.expectedErrorHandler.LogRoller.MaxSize, actualErrorsRule.LogRoller.MaxSize) + } + if actualErrorsRule.LogRoller.LocalTime != test.expectedErrorHandler.LogRoller.LocalTime { + t.Fatalf("Test %d expected LogRoller LocalTime to be %t, but got %t", + i, test.expectedErrorHandler.LogRoller.LocalTime, actualErrorsRule.LogRoller.LocalTime) + } + } + } + +} diff --git a/config/setup/log.go b/config/setup/log.go index 6bf9f312..8bb4788a 100644 --- a/config/setup/log.go +++ b/config/setup/log.go @@ -9,7 +9,6 @@ import ( "github.com/mholt/caddy/middleware" caddylog "github.com/mholt/caddy/middleware/log" "github.com/mholt/caddy/server" - "gopkg.in/natefinch/lumberjack.v2" ) // Log sets up the logging middleware. @@ -23,30 +22,33 @@ func Log(c *Controller) (middleware.Middleware, error) { c.Startup = append(c.Startup, func() error { for i := 0; i < len(rules); i++ { var err error - var file io.Writer + var writer io.Writer if rules[i].OutputFile == "stdout" { - file = os.Stdout + writer = os.Stdout } else if rules[i].OutputFile == "stderr" { - file = os.Stderr + writer = os.Stderr } else if rules[i].OutputFile == "syslog" { - file, err = gsyslog.NewLogger(gsyslog.LOG_INFO, "LOCAL0", "caddy") + writer, err = gsyslog.NewLogger(gsyslog.LOG_INFO, "LOCAL0", "caddy") if err != nil { return err } } else { - _, err = os.OpenFile(rules[i].OutputFile, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0644) + var file *os.File + file, err = os.OpenFile(rules[i].OutputFile, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0644) if err != nil { return err } - file = &lumberjack.Logger{ - Filename: rules[i].OutputFile, - MaxSize: 20, - MaxBackups: 10, + if rules[i].Roller != nil { + file.Close() + rules[i].Roller.Filename = rules[i].OutputFile + writer = rules[i].Roller.GetLogWriter() + } else { + writer = file } } - rules[i].Log = log.New(file, "", 0) + rules[i].Log = log.New(writer, "", 0) } return nil @@ -63,12 +65,33 @@ func logParse(c *Controller) ([]caddylog.Rule, error) { for c.Next() { args := c.RemainingArgs() + var logRoller *middleware.LogRoller + if c.NextBlock() { + if c.Val() == "rotate" { + if c.NextArg() { + if c.Val() == "{" { + var err error + logRoller, err = parseRoller(c) + if err != nil { + return nil, err + } + // This part doesn't allow having something after the rotate block + if c.Next() { + if c.Val() != "}" { + return nil, c.ArgErr() + } + } + } + } + } + } if len(args) == 0 { // Nothing specified; use defaults rules = append(rules, caddylog.Rule{ PathScope: "/", OutputFile: caddylog.DefaultLogFilename, Format: caddylog.DefaultLogFormat, + Roller: logRoller, }) } else if len(args) == 1 { // Only an output file specified @@ -76,6 +99,7 @@ func logParse(c *Controller) ([]caddylog.Rule, error) { PathScope: "/", OutputFile: args[0], Format: caddylog.DefaultLogFormat, + Roller: logRoller, }) } else { // Path scope, output file, and maybe a format specified @@ -97,6 +121,7 @@ func logParse(c *Controller) ([]caddylog.Rule, error) { PathScope: args[0], OutputFile: args[1], Format: format, + Roller: logRoller, }) } } diff --git a/config/setup/log_test.go b/config/setup/log_test.go index 6f8e1cce..a9693a69 100644 --- a/config/setup/log_test.go +++ b/config/setup/log_test.go @@ -3,6 +3,7 @@ package setup import ( "testing" + "github.com/mholt/caddy/middleware" caddylog "github.com/mholt/caddy/middleware/log" ) @@ -36,6 +37,9 @@ func TestLog(t *testing.T) { if myHandler.Rules[0].Format != caddylog.DefaultLogFormat { t.Errorf("Expected %s as the default Log Format", caddylog.DefaultLogFormat) } + if myHandler.Rules[0].Roller != nil { + t.Errorf("Expected Roller to be nil, got: %v", *myHandler.Rules[0].Roller) + } if !SameNext(myHandler.Next, EmptyNext) { t.Error("'Next' field of handler was not set properly") } @@ -78,7 +82,7 @@ func TestLogParse(t *testing.T) { OutputFile: "accesslog.txt", Format: caddylog.CombinedLogFormat, }}}, - {`log /api1 log.txt + {`log /api1 log.txt log /api2 accesslog.txt {combined}`, false, []caddylog.Rule{{ PathScope: "/api1", OutputFile: "log.txt", @@ -98,6 +102,16 @@ func TestLogParse(t *testing.T) { OutputFile: "log.txt", Format: "{when}", }}}, + {`log access.log { rotate { size 2 age 10 keep 3 } }`, false, []caddylog.Rule{{ + PathScope: "/", + OutputFile: "access.log", + Format: caddylog.DefaultLogFormat, + Roller: &middleware.LogRoller{ + MaxSize: 2, + MaxAge: 10, + MaxBackups: 3, + }, + }}}, } for i, test := range tests { c := NewTestController(test.inputLogRules) @@ -128,6 +142,32 @@ func TestLogParse(t *testing.T) { 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) + } + 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 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 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) + } + } } } diff --git a/config/setup/roller.go b/config/setup/roller.go new file mode 100644 index 00000000..3a5b8a8d --- /dev/null +++ b/config/setup/roller.go @@ -0,0 +1,39 @@ +package setup + +import ( + "strconv" + + "github.com/mholt/caddy/middleware" +) + +func parseRoller(c *Controller) (*middleware.LogRoller, error) { + var size, age, keep int + // This is kind of a hack to support nested blocks: + // As we are already in a block: either log or errors, + // c.nesting > 0 but, as soon as c meets a }, it thinks + // the block is over and return false for c.NextBlock. + for c.NextBlock() { + what := c.Val() + if !c.NextArg() { + return nil, c.ArgErr() + } + value := c.Val() + var err error + switch what { + case "size": + size, err = strconv.Atoi(value) + case "age": + age, err = strconv.Atoi(value) + case "keep": + keep, err = strconv.Atoi(value) + } + if err != nil { + return nil, err + } + } + return &middleware.LogRoller{ + MaxSize: size, + MaxAge: age, + MaxBackups: keep, + }, nil +} diff --git a/middleware/errors/errors.go b/middleware/errors/errors.go index 8f1e04b4..44451ab1 100644 --- a/middleware/errors/errors.go +++ b/middleware/errors/errors.go @@ -20,6 +20,7 @@ type ErrorHandler struct { ErrorPages map[int]string // map of status code to filename LogFile string Log *log.Logger + LogRoller *middleware.LogRoller } func (h ErrorHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) { diff --git a/middleware/log/log.go b/middleware/log/log.go index 45a3e9a2..f2fbb421 100644 --- a/middleware/log/log.go +++ b/middleware/log/log.go @@ -47,6 +47,7 @@ type Rule struct { OutputFile string Format string Log *log.Logger + Roller *middleware.LogRoller } const ( diff --git a/middleware/roller.go b/middleware/roller.go new file mode 100644 index 00000000..0f8a2b2e --- /dev/null +++ b/middleware/roller.go @@ -0,0 +1,25 @@ +package middleware + +import ( + "io" + + "gopkg.in/natefinch/lumberjack.v2" +) + +type LogRoller struct { + Filename string + MaxSize int + MaxAge int + MaxBackups int + LocalTime bool +} + +func (l LogRoller) GetLogWriter() io.Writer { + return &lumberjack.Logger{ + Filename: l.Filename, + MaxSize: l.MaxSize, + MaxAge: l.MaxAge, + MaxBackups: l.MaxBackups, + LocalTime: l.LocalTime, + } +}