Remove lumberjack logger (#257)

* Removed lumberjack from coremain

As is mentioned in 251, this fix removed lumberjack from coremain.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>

* 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 <yong.tang.github@outlook.com>

* Update log/README.md as lumberjack has been removed

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>

* Adjust default log output from `ioutil.Discard` to `os.Stdout`

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>

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{
+		log.SetOutput(os.Stdout)
-			Filename:   logfile,
-			MaxSize:    100,
-			MaxAge:     14,
-			MaxBackups: 10,
-		})
 	}
 	log.SetFlags(log.LstdFlags)
 

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
+	Next    middleware.Handler
-	LogFile   string
+	LogFile string
-	Log       *log.Logger
+	Log     *log.Logger
-	LogRoller *roller.LogRoller
+	Debug   bool // if true, errors are written out to client rather than to a log
-	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) {

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 {
+		writer = file
-			file.Close()
-
-			handler.LogRoller.Filename = handler.LogFile
-
-			writer = handler.LogRoller.GetLogWriter()
-		} else {
-			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 {

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)
-			}
-		}
 	}
 }

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
-    }
-}

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 (

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 {
+				writer = file
-					file.Close()
-					rules[i].Roller.Filename = rules[i].OutputFile
-					writer = rules[i].Roller.GetLogWriter()
-				} else {
-					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,
 			})
 		}
 	}

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)
-				}
-			}
 		}
 	}
 

middleware/pkg/roller/roller.go

@@ -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,
-	}
-}