new internal/lg/ logging package for nsqd nsqlookupd nsqadmin

introduce LogLevel type
NilLogger for disabling logging
consolidate LogLevel tests into internal/lg/
consolidate TestNoLogger

internal/lg/lg.go

@@ -0,0 +1,73 @@
+// short for "log"
+package lg
+
+import (
+	"fmt"
+	"strings"
+)
+
+type LogLevel int
+
+const (
+	DEBUG = LogLevel(1)
+	INFO  = LogLevel(2)
+	WARN  = LogLevel(3)
+	ERROR = LogLevel(4)
+	FATAL = LogLevel(5)
+)
+
+type Logger interface {
+	Output(maxdepth int, s string) error
+}
+
+type NilLogger struct{}
+
+func (l NilLogger) Output(maxdepth int, s string) error {
+	return nil
+}
+
+func (l LogLevel) String() string {
+	switch l {
+	case 1:
+		return "DEBUG"
+	case 2:
+		return "INFO"
+	case 3:
+		return "WARNING"
+	case 4:
+		return "ERROR"
+	case 5:
+		return "FATAL"
+	}
+	panic("invalid LogLevel")
+}
+
+func ParseLogLevel(levelstr string, verbose bool) (LogLevel, error) {
+	lvl := INFO
+
+	switch strings.ToLower(levelstr) {
+	case "debug":
+		lvl = DEBUG
+	case "info":
+		lvl = INFO
+	case "warn":
+		lvl = WARN
+	case "error":
+		lvl = ERROR
+	case "fatal":
+		lvl = FATAL
+	default:
+		return lvl, fmt.Errorf("invalid log-level '%s'", levelstr)
+	}
+	if verbose {
+		lvl = DEBUG
+	}
+	return lvl, nil
+}
+
+func Logf(logger Logger, cfgLevel LogLevel, msgLevel LogLevel, f string, args ...interface{}) {
+	if cfgLevel > msgLevel {
+		return
+	}
+	logger.Output(3, fmt.Sprintf(msgLevel.String()+": "+f, args...))
+}

internal/lg/lg_test.go

@@ -0,0 +1,109 @@
+package lg
+
+import (
+	"log"
+	"os"
+	"testing"
+
+	"github.com/nsqio/nsq/internal/test"
+)
+
+type options struct {
+	LogLevel string `flag:"log-level"`
+	Verbose  bool   `flag:"verbose"` // for backwards compatibility
+	Logger   Logger
+	logLevel LogLevel // private, not really an option
+}
+
+func newOptions() *options {
+	return &options{
+		LogLevel: "info",
+	}
+}
+
+type app struct {
+	opts *options
+}
+
+func (n *app) logf(level LogLevel, f string, args ...interface{}) {
+	Logf(n.opts.Logger, n.opts.logLevel, level, f, args)
+}
+
+func newApp(opts *options) *app {
+	if opts.Logger == nil {
+		opts.Logger = log.New(os.Stderr, "[app] ", log.Ldate|log.Ltime|log.Lmicroseconds)
+	}
+	n := &app{
+		opts: opts,
+	}
+
+	var err error
+	opts.logLevel, err = ParseLogLevel(opts.LogLevel, opts.Verbose)
+	if err != nil {
+		n.logf(FATAL, "%s", err)
+		os.Exit(1)
+	}
+
+	n.logf(INFO, "app 0.1")
+	return n
+}
+
+type mockLogger struct {
+	Count int
+}
+
+func (l *mockLogger) Output(maxdepth int, s string) error {
+	l.Count++
+	return nil
+}
+
+func TestLogging(t *testing.T) {
+	logger := &mockLogger{}
+	opts := newOptions()
+	opts.Logger = logger
+
+	// Test only fatal get through
+	opts.LogLevel = "FaTaL"
+	nsqd := newApp(opts)
+	logger.Count = 0
+	for i := 1; i <= 5; i++ {
+		nsqd.logf(LogLevel(i), "Test")
+	}
+	test.Equal(t, 1, logger.Count)
+
+	// Test only warnings or higher get through
+	opts.LogLevel = "WARN"
+	nsqd = newApp(opts)
+	logger.Count = 0
+	for i := 1; i <= 5; i++ {
+		nsqd.logf(LogLevel(i), "Test")
+	}
+	test.Equal(t, 3, logger.Count)
+
+	// Test everything gets through
+	opts.LogLevel = "debuG"
+	nsqd = newApp(opts)
+	logger.Count = 0
+	for i := 1; i <= 5; i++ {
+		nsqd.logf(LogLevel(i), "Test")
+	}
+	test.Equal(t, 5, logger.Count)
+
+	// Test everything gets through with verbose = true
+	opts.LogLevel = "fatal"
+	opts.Verbose = true
+	nsqd = newApp(opts)
+	logger.Count = 0
+	for i := 1; i <= 5; i++ {
+		nsqd.logf(LogLevel(i), "Test")
+	}
+	test.Equal(t, 5, logger.Count)
+}
+
+func TestNoLogger(t *testing.T) {
+	opts := newOptions()
+	opts.Logger = NilLogger{}
+	app := newApp(opts)
+
+	app.logf(ERROR, "should never be logged")
+}

nsqadmin/http.go

@@ -19,6 +19,7 @@ import (
 	"github.com/julienschmidt/httprouter"
 	"github.com/nsqio/nsq/internal/clusterinfo"
 	"github.com/nsqio/nsq/internal/http_api"
+	"github.com/nsqio/nsq/internal/lg"
 	"github.com/nsqio/nsq/internal/protocol"
 	"github.com/nsqio/nsq/internal/version"
 )
@@ -734,14 +735,12 @@ func (s *httpServer) doConfig(w http.ResponseWriter, req *http.Request, ps httpr
 			}
 		case "log_level":
 			logLevelStr := string(body)
-			logLevelInt := s.ctx.nsqadmin.logLevelFromString(logLevelStr)
-			if logLevelInt == -1 {
+			logLevel, err := lg.ParseLogLevel(logLevelStr, opts.Verbose)
+			if err != nil {
 				return nil, http_api.Err{400, "INVALID_VALUE"}
 			}
-
-			// save the log level
 			opts.LogLevel = logLevelStr
-			opts.logLevel = logLevelInt
+			opts.logLevel = logLevel
 		default:
 			return nil, http_api.Err{400, "INVALID_OPTION"}
 		}

nsqadmin/logger.go

@@ -1,57 +1,20 @@
 package nsqadmin
 
 import (
-	"fmt"
-	"strings"
+	"github.com/nsqio/nsq/internal/lg"
 )
 
-type Logger interface {
-	Output(maxdepth int, s string) error
-}
+type Logger lg.Logger
 
 const (
-	LOG_DEBUG = 1
-	LOG_INFO  = 2
-	LOG_WARN  = 3
-	LOG_ERROR = 4
-	LOG_FATAL = 5
+	LOG_DEBUG = lg.DEBUG
+	LOG_INFO  = lg.INFO
+	LOG_WARN  = lg.WARN
+	LOG_ERROR = lg.ERROR
+	LOG_FATAL = lg.FATAL
 )
 
-func (n *NSQAdmin) logLevelFromString(level string) int {
-	// check log-level is valid and translate to int
-	switch strings.ToLower(level) {
-	case "debug":
-		return LOG_DEBUG
-	case "info":
-		return LOG_INFO
-	case "warn":
-		return LOG_WARN
-	case "error":
-		return LOG_ERROR
-	case "fatal":
-		return LOG_FATAL
-	default:
-		return -1
-	}
-}
-
-func (n *NSQAdmin) logf(level int, f string, args ...interface{}) {
-	levelString := "INFO"
-	switch level {
-	case LOG_DEBUG:
-		levelString = "DEBUG"
-	case LOG_INFO:
-		levelString = "INFO"
-	case LOG_WARN:
-		levelString = "WARNING"
-	case LOG_ERROR:
-		levelString = "ERROR"
-	case LOG_FATAL:
-		levelString = "FATAL"
-	}
-
+func (n *NSQAdmin) logf(level lg.LogLevel, f string, args ...interface{}) {
 	opts := n.getOpts()
-	if level >= opts.logLevel || opts.Verbose {
-		n.getOpts().Logger.Output(2, fmt.Sprintf(levelString+": "+f, args...))
-	}
+	lg.Logf(opts.Logger, opts.logLevel, level, f, args...)
 }

nsqadmin/nsqadmin.go

@@ -15,6 +15,7 @@ import (
 	"sync/atomic"
 
 	"github.com/nsqio/nsq/internal/http_api"
+	"github.com/nsqio/nsq/internal/lg"
 	"github.com/nsqio/nsq/internal/util"
 	"github.com/nsqio/nsq/internal/version"
 )
@@ -39,10 +40,10 @@ func New(opts *Options) *NSQAdmin {
 	}
 	n.swapOpts(opts)
 
-	// check log-level is valid and translate to int
-	opts.logLevel = n.logLevelFromString(opts.LogLevel)
-	if opts.logLevel == -1 {
-		n.logf(LOG_FATAL, "log level '%s' should be one of: debug, info, warn, error, or fatal", opts.LogLevel)
+	var err error
+	opts.logLevel, err = lg.ParseLogLevel(opts.LogLevel, opts.Verbose)
+	if err != nil {
+		n.logf(LOG_FATAL, "%s", err)
 		os.Exit(1)
 	}