From 953cfc1de49ffe9748a876c16b4b45c3f518f6d1 Mon Sep 17 00:00:00 2001 From: Yong Tang Date: Fri, 16 Sep 2016 07:50:16 -0700 Subject: [PATCH] Remove lumberjack logger (#257) * Removed lumberjack from coremain As is mentioned in 251, this fix removed lumberjack from coremain. Signed-off-by: Yong Tang * Remove lumberjack from log middleware As mentioned in 251, lumberjack is not suitable for applications like CoreDNS so it is removed from the log middleware. Signed-off-by: Yong Tang * Update log/README.md as lumberjack has been removed Signed-off-by: Yong Tang * Adjust default log output from `ioutil.Discard` to `os.Stdout` Signed-off-by: Yong Tang --- coremain/run.go | 10 +----- middleware/errors/errors.go | 10 +++--- middleware/errors/setup.go | 25 +------------ middleware/errors/setup_test.go | 50 -------------------------- middleware/log/README.md | 31 ----------------- middleware/log/log.go | 2 -- middleware/log/setup.go | 32 +---------------- middleware/log/setup_test.go | 39 --------------------- middleware/pkg/roller/roller.go | 62 --------------------------------- 9 files changed, 7 insertions(+), 254 deletions(-) delete mode 100644 middleware/pkg/roller/roller.go diff --git a/coremain/run.go b/coremain/run.go index e165825d2..94b0076a0 100644 --- a/coremain/run.go +++ b/coremain/run.go @@ -12,7 +12,6 @@ import ( "strings" "github.com/mholt/caddy" - "gopkg.in/natefinch/lumberjack.v2" // Plug in CoreDNS "github.com/miekg/coredns/core" @@ -49,15 +48,8 @@ func Run() { log.SetOutput(os.Stdout) case "stderr": log.SetOutput(os.Stderr) - case "": - log.SetOutput(ioutil.Discard) default: - log.SetOutput(&lumberjack.Logger{ - Filename: logfile, - MaxSize: 100, - MaxAge: 14, - MaxBackups: 10, - }) + log.SetOutput(os.Stdout) } log.SetFlags(log.LstdFlags) diff --git a/middleware/errors/errors.go b/middleware/errors/errors.go index ef178d1a3..6f490ea74 100644 --- a/middleware/errors/errors.go +++ b/middleware/errors/errors.go @@ -9,7 +9,6 @@ import ( "time" "github.com/miekg/coredns/middleware" - "github.com/miekg/coredns/middleware/pkg/roller" "github.com/miekg/coredns/request" "github.com/miekg/dns" @@ -18,11 +17,10 @@ import ( // ErrorHandler handles DNS errors (and errors from other middleware). type ErrorHandler struct { - Next middleware.Handler - LogFile string - Log *log.Logger - LogRoller *roller.LogRoller - Debug bool // if true, errors are written out to client rather than to a log + Next middleware.Handler + LogFile string + Log *log.Logger + Debug bool // if true, errors are written out to client rather than to a log } func (h ErrorHandler) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (int, error) { diff --git a/middleware/errors/setup.go b/middleware/errors/setup.go index e732a1e34..a5fa86921 100644 --- a/middleware/errors/setup.go +++ b/middleware/errors/setup.go @@ -7,7 +7,6 @@ import ( "github.com/miekg/coredns/core/dnsserver" "github.com/miekg/coredns/middleware" - "github.com/miekg/coredns/middleware/pkg/roller" "github.com/hashicorp/go-syslog" "github.com/mholt/caddy" @@ -51,15 +50,7 @@ func setup(c *caddy.Controller) error { if err != nil { return middleware.Error("errors", err) } - if handler.LogRoller != nil { - file.Close() - - handler.LogRoller.Filename = handler.LogFile - - writer = handler.LogRoller.GetLogWriter() - } else { - writer = file - } + writer = file } handler.Log = log.New(writer, "", 0) @@ -91,16 +82,6 @@ func errorsParse(c *caddy.Controller) (ErrorHandler, error) { handler.Debug = true } else { handler.LogFile = where - if c.NextArg() { - if c.Val() == "{" { - c.IncrNest() - logRoller, err := roller.Parse(c) - if err != nil { - return hadBlock, err - } - handler.LogRoller = logRoller - } - } } } } @@ -108,10 +89,6 @@ func errorsParse(c *caddy.Controller) (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/middleware/errors/setup_test.go b/middleware/errors/setup_test.go index c1dbf7267..5b5108d42 100644 --- a/middleware/errors/setup_test.go +++ b/middleware/errors/setup_test.go @@ -4,7 +4,6 @@ import ( "testing" "github.com/mholt/caddy" - "github.com/miekg/coredns/middleware/pkg/roller" ) func TestErrorsParse(t *testing.T) { @@ -27,29 +26,6 @@ func TestErrorsParse(t *testing.T) { LogFile: "", Debug: true, }}, - {`errors { log errors.txt { size 2 age 10 keep 3 } }`, false, ErrorHandler{ - LogFile: "errors.txt", - LogRoller: &roller.LogRoller{ - MaxSize: 2, - MaxAge: 10, - MaxBackups: 3, - LocalTime: true, - }, - }}, - {`errors { log errors.txt { - size 3 - age 11 - keep 5 - } -}`, false, ErrorHandler{ - LogFile: "errors.txt", - LogRoller: &roller.LogRoller{ - MaxSize: 3, - MaxAge: 11, - MaxBackups: 5, - LocalTime: true, - }, - }}, } for i, test := range tests { c := caddy.NewTestController("dns", test.inputErrorsRules) @@ -68,31 +44,5 @@ func TestErrorsParse(t *testing.T) { t.Errorf("Test %d expected Debug to be %v, but got %v", i, test.expectedErrorHandler.Debug, actualErrorsRule.Debug) } - 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 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/middleware/log/README.md b/middleware/log/README.md index 173102007..c4568de1d 100644 --- a/middleware/log/README.md +++ b/middleware/log/README.md @@ -54,26 +54,6 @@ The following place holders are supported: * `{>opcode}`: query OPCODE -## Log Rotation - -If you enable log rotation, log files will be automatically maintained when they get large or old. -You can use rotation by opening a block on your first line, which can be any of the variations -described above: - -~~~ -log ... { - rotate { - size maxsize - age maxage - keep maxkeep - } -} -~~~ - -* `maxsize` is the maximum size of a log file in megabytes (MB) before it gets rotated. Default is 100 MB. -* `maxage` is the maximum age of a rotated log file in days, after which it will be deleted. Default is to never delete old files because of age. -* `maxkeep` is the maximum number of rotated log files to keep. Default is to retain all old log files. - ## Examples Log all requests to a file: @@ -87,14 +67,3 @@ Custom log format: ~~~ log . ../query.log "{proto} Request: {name} {type} {>id}" ~~~ - -With rotation: - -~~~ -log query.log { - rotate { - 100 # Rotate after 100 MB - age 14 # Keep log files for 14 days - keep 10 # Keep at most 10 log files - } -} diff --git a/middleware/log/log.go b/middleware/log/log.go index 9da5e65aa..3c941849b 100644 --- a/middleware/log/log.go +++ b/middleware/log/log.go @@ -10,7 +10,6 @@ import ( "github.com/miekg/coredns/middleware/pkg/dnsrecorder" "github.com/miekg/coredns/middleware/pkg/rcode" "github.com/miekg/coredns/middleware/pkg/replacer" - "github.com/miekg/coredns/middleware/pkg/roller" "github.com/miekg/coredns/request" "github.com/miekg/dns" @@ -61,7 +60,6 @@ type Rule struct { OutputFile string Format string Log *log.Logger - Roller *roller.LogRoller } const ( diff --git a/middleware/log/setup.go b/middleware/log/setup.go index 37e8ebde3..a86aa17f9 100644 --- a/middleware/log/setup.go +++ b/middleware/log/setup.go @@ -7,7 +7,6 @@ import ( "github.com/miekg/coredns/core/dnsserver" "github.com/miekg/coredns/middleware" - "github.com/miekg/coredns/middleware/pkg/roller" "github.com/hashicorp/go-syslog" "github.com/mholt/caddy" @@ -48,13 +47,7 @@ func setup(c *caddy.Controller) error { if err != nil { return middleware.Error("log", err) } - if rules[i].Roller != nil { - file.Close() - rules[i].Roller.Filename = rules[i].OutputFile - writer = rules[i].Roller.GetLogWriter() - } else { - writer = file - } + writer = file } rules[i].Log = log.New(writer, "", 0) @@ -76,33 +69,12 @@ func logParse(c *caddy.Controller) ([]Rule, error) { for c.Next() { args := c.RemainingArgs() - var logRoller *roller.LogRoller - if c.NextBlock() { - if c.Val() == "rotate" { - if c.NextArg() { - if c.Val() == "{" { - var err error - logRoller, err = roller.Parse(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, Rule{ NameScope: ".", OutputFile: DefaultLogFilename, Format: DefaultLogFormat, - Roller: logRoller, }) } else if len(args) == 1 { // Only an output file specified @@ -110,7 +82,6 @@ func logParse(c *caddy.Controller) ([]Rule, error) { NameScope: ".", OutputFile: args[0], Format: DefaultLogFormat, - Roller: logRoller, }) } else { // Name scope, output file, and maybe a format specified @@ -132,7 +103,6 @@ func logParse(c *caddy.Controller) ([]Rule, error) { NameScope: dns.Fqdn(args[0]), OutputFile: args[1], Format: format, - Roller: logRoller, }) } } diff --git a/middleware/log/setup_test.go b/middleware/log/setup_test.go index b38caa52d..243d6aeb6 100644 --- a/middleware/log/setup_test.go +++ b/middleware/log/setup_test.go @@ -3,8 +3,6 @@ package log import ( "testing" - "github.com/miekg/coredns/middleware/pkg/roller" - "github.com/mholt/caddy" ) @@ -64,17 +62,6 @@ func TestLogParse(t *testing.T) { OutputFile: "log.txt", Format: "{when}", }}}, - {`log access.log { rotate { size 2 age 10 keep 3 } }`, false, []Rule{{ - NameScope: ".", - OutputFile: "access.log", - Format: DefaultLogFormat, - Roller: &roller.LogRoller{ - MaxSize: 2, - MaxAge: 10, - MaxBackups: 3, - LocalTime: true, - }, - }}}, } for i, test := range tests { c := caddy.NewTestController("dns", test.inputLogRules) @@ -105,32 +92,6 @@ 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/middleware/pkg/roller/roller.go b/middleware/pkg/roller/roller.go deleted file mode 100644 index c60494736..000000000 --- a/middleware/pkg/roller/roller.go +++ /dev/null @@ -1,62 +0,0 @@ -package roller - -import ( - "io" - "strconv" - - "github.com/mholt/caddy" - "gopkg.in/natefinch/lumberjack.v2" -) - -func Parse(c *caddy.Controller) (*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 &LogRoller{ - MaxSize: size, - MaxAge: age, - MaxBackups: keep, - LocalTime: true, - }, nil -} - -// LogRoller implements a middleware that provides a rolling logger. -type LogRoller struct { - Filename string - MaxSize int - MaxAge int - MaxBackups int - LocalTime bool -} - -// GetLogWriter returns an io.Writer that writes to a rolling logger. -func (l LogRoller) GetLogWriter() io.Writer { - return &lumberjack.Logger{ - Filename: l.Filename, - MaxSize: l.MaxSize, - MaxAge: l.MaxAge, - MaxBackups: l.MaxBackups, - LocalTime: l.LocalTime, - } -}