Replace Logger.{Level,SetLevel} with DynamicLevel (#180)

* make Levels be "just" Options; simplify/extensify meta enabled layer
* drop SetLevel/Level Logger API
* add AtomicLevel option for changeable levels

http_handler.go

@@ -26,65 +26,51 @@ import (
 	"net/http"
 )
 
-type httpPayload struct {
-	Level *Level `json:"level"`
-}
-
-type errorResponse struct {
-	Error string `json:"error"`
-}
-
-type levelHandler struct {
-	logger Logger
-}
-
-// NewHTTPHandler returns an HTTP handler that can atomically change the logging
-// level at runtime. Keep in mind that changing a logger's level also affects that
-// logger's ancestors and descendants.
+// ServeHTTP supports changing logging level with an HTTP request.
 //
 // GET requests return a JSON description of the current logging level. PUT
 // requests change the logging level and expect a payload like:
 //   {"level":"info"}
-func NewHTTPHandler(logger Logger) http.Handler {
-	return &levelHandler{logger: logger}
-}
+func (lvl AtomicLevel) ServeHTTP(w http.ResponseWriter, r *http.Request) {
+	type errorResponse struct {
+		Error string `json:"error"`
+	}
+	type payload struct {
+		Level *Level `json:"level"`
+	}
+
+	enc := json.NewEncoder(w)
 
-func (h *levelHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	switch r.Method {
+
 	case "GET":
-		h.getLevel(w, r)
+		current := lvl.Level()
+		enc.Encode(payload{Level: &current})
+
 	case "PUT":
-		h.putLevel(w, r)
-	default:
-		h.error(w, "Only GET and PUT are supported.", http.StatusMethodNotAllowed)
-	}
-}
+		var req payload
 
-func (h *levelHandler) getLevel(w http.ResponseWriter, r *http.Request) {
-	current := h.logger.Level()
-	json.NewEncoder(w).Encode(httpPayload{Level: &current})
-}
+		if errmess := func() string {
+			if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
+				return fmt.Sprintf("Request body must be well-formed JSON: %v", err)
+			}
+			if req.Level == nil {
+				return "Must specify a logging level."
+			}
+			return ""
+		}(); errmess != "" {
+			w.WriteHeader(http.StatusBadRequest)
+			enc.Encode(errorResponse{Error: errmess})
+			return
+		}
 
-func (h *levelHandler) putLevel(w http.ResponseWriter, r *http.Request) {
-	var p httpPayload
-	decoder := json.NewDecoder(r.Body)
-	if err := decoder.Decode(&p); err != nil {
-		h.error(
-			w,
-			fmt.Sprintf("Request body must be well-formed JSON: %v", err),
-			http.StatusBadRequest,
-		)
-		return
-	}
-	if p.Level == nil {
-		h.error(w, "Must specify a logging level.", http.StatusBadRequest)
-		return
-	}
-	h.logger.SetLevel(*p.Level)
-	json.NewEncoder(w).Encode(p)
-}
+		lvl.SetLevel(*req.Level)
+		enc.Encode(req)
 
-func (h *levelHandler) error(w http.ResponseWriter, msg string, status int) {
-	w.WriteHeader(status)
-	json.NewEncoder(w).Encode(errorResponse{Error: msg})
+	default:
+		w.WriteHeader(http.StatusMethodNotAllowed)
+		enc.Encode(errorResponse{
+			Error: "Only GET and PUT are supported.",
+		})
+	}
 }

http_handler_test.go

@@ -37,9 +37,10 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func newHandler() (Logger, http.Handler) {
-	logger, _ := spy.New()
-	return logger, NewHTTPHandler(logger)
+func newHandler() (AtomicLevel, Logger) {
+	lvl := DynamicLevel()
+	logger, _ := spy.New(lvl)
+	return lvl, logger
 }
 
 func assertCodeOK(t testing.TB, code int) {
@@ -86,45 +87,45 @@ func makeRequest(t testing.TB, method string, handler http.Handler, reader io.Re
 }
 
 func TestHTTPHandlerGetLevel(t *testing.T) {
-	logger, handler := newHandler()
-	code, body := makeRequest(t, "GET", handler, nil)
+	lvl, _ := newHandler()
+	code, body := makeRequest(t, "GET", lvl, nil)
 	assertCodeOK(t, code)
-	assertResponse(t, logger.Level(), body)
+	assertResponse(t, lvl.Level(), body)
 }
 
 func TestHTTPHandlerPutLevel(t *testing.T) {
-	logger, handler := newHandler()
+	lvl, _ := newHandler()
 
-	code, body := makeRequest(t, "PUT", handler, strings.NewReader(`{"level":"warn"}`))
+	code, body := makeRequest(t, "PUT", lvl, strings.NewReader(`{"level":"warn"}`))
 
 	assertCodeOK(t, code)
-	assertResponse(t, logger.Level(), body)
+	assertResponse(t, lvl.Level(), body)
 }
 
 func TestHTTPHandlerPutUnrecognizedLevel(t *testing.T) {
-	_, handler := newHandler()
-	code, body := makeRequest(t, "PUT", handler, strings.NewReader(`{"level":"unrecognized-level"}`))
+	lvl, _ := newHandler()
+	code, body := makeRequest(t, "PUT", lvl, strings.NewReader(`{"level":"unrecognized-level"}`))
 	assertCodeBadRequest(t, code)
 	assertJSONError(t, body)
 }
 
 func TestHTTPHandlerNotJSON(t *testing.T) {
-	_, handler := newHandler()
-	code, body := makeRequest(t, "PUT", handler, strings.NewReader(`{`))
+	lvl, _ := newHandler()
+	code, body := makeRequest(t, "PUT", lvl, strings.NewReader(`{`))
 	assertCodeBadRequest(t, code)
 	assertJSONError(t, body)
 }
 
 func TestHTTPHandlerNoLevelSpecified(t *testing.T) {
-	_, handler := newHandler()
-	code, body := makeRequest(t, "PUT", handler, strings.NewReader(`{}`))
+	lvl, _ := newHandler()
+	code, body := makeRequest(t, "PUT", lvl, strings.NewReader(`{}`))
 	assertCodeBadRequest(t, code)
 	assertJSONError(t, body)
 }
 
 func TestHTTPHandlerMethodNotAllowed(t *testing.T) {
-	_, handler := newHandler()
-	code, body := makeRequest(t, "POST", handler, strings.NewReader(`{`))
+	lvl, _ := newHandler()
+	code, body := makeRequest(t, "POST", lvl, strings.NewReader(`{`))
 	assertCodeMethodNotAllowed(t, code)
 	assertJSONError(t, body)
 }

level.go

@@ -23,6 +23,8 @@ package zap
 import (
 	"errors"
 	"fmt"
+
+	"github.com/uber-go/atomic"
 )
 
 var errMarshalNilLevel = errors.New("can't marshal a nil *Level to text")
@@ -33,6 +35,20 @@ var errMarshalNilLevel = errors.New("can't marshal a nil *Level to text")
 // New to override the default logging priority.
 type Level int32
 
+// LevelEnabler decides whether a given logging level is enabled when logging a
+// message.
+//
+// Enablers are intended to be used to implement deterministic filters;
+// concerns like sampling are better implemented as a Logger implementation.
+//
+// Each concrete Level value implements a static LevelEnabler which returns
+// true for itself and all higher logging levels. For example WarnLevel.Enabled()
+// will return true for WarnLevel, ErrorLevel, PanicLevel, and FatalLevel, but
+// return false for InfoLevel and DebugLevel.
+type LevelEnabler interface {
+	Enabled(Level) bool
+}
+
 const (
 	// DebugLevel logs are typically voluminous, and are usually disabled in
 	// production.
@@ -105,3 +121,41 @@ func (l *Level) UnmarshalText(text []byte) error {
 	}
 	return nil
 }
+
+// Enabled returns true if the given level is at or above this level.
+func (l Level) Enabled(lvl Level) bool {
+	return lvl >= l
+}
+
+// DynamicLevel creates an atomically changeable dynamic logging level.  The
+// returned level can be passed as a logger option just like a concrete level.
+//
+// The value's SetLevel() method may be called later to change the enabled
+// logging level of all loggers that were passed the value (either explicitly,
+// or by creating sub-loggers with Logger.With).
+func DynamicLevel() AtomicLevel {
+	return AtomicLevel{
+		l: atomic.NewInt32(int32(InfoLevel)),
+	}
+}
+
+// AtomicLevel wraps an atomically change-able Level value. It must be created
+// by the DynamicLevel() function to allocate the internal atomic pointer.
+type AtomicLevel struct {
+	l *atomic.Int32
+}
+
+// Enabled loads the level value, and calls its Enabled method.
+func (lvl AtomicLevel) Enabled(l Level) bool {
+	return lvl.Level().Enabled(l)
+}
+
+// Level returns the minimum enabled log level.
+func (lvl AtomicLevel) Level() Level {
+	return Level(lvl.l.Load())
+}
+
+// SetLevel alters the logging level.
+func (lvl AtomicLevel) SetLevel(l Level) {
+	lvl.l.Store(int32(l))
+}

logger.go

@@ -30,13 +30,6 @@ var _exit = os.Exit
 // A Logger enables leveled, structured logging. All methods are safe for
 // concurrent use.
 type Logger interface {
-	// Check the minimum enabled log level.
-	Level() Level
-	// Change the level of this logger, as well as all its ancestors and
-	// descendants. This makes it easy to change the log level at runtime
-	// without restarting your application.
-	SetLevel(Level)
-
 	// Create a child logger, and optionally add some context to that logger.
 	With(...Field) Logger
 

logger_test.go

@@ -74,44 +74,42 @@ func withJSONLogger(t testing.TB, opts []Option, f func(Logger, *testBuffer)) {
 	assert.Empty(t, errSink.String(), "Expected error sink to be empty.")
 }
 
-func TestJSONLoggerSetLevel(t *testing.T) {
-	withJSONLogger(t, nil, func(logger Logger, _ *testBuffer) {
-		assert.Equal(t, DebugLevel, logger.Level(), "Unexpected initial level.")
-		logger.SetLevel(ErrorLevel)
-		assert.Equal(t, ErrorLevel, logger.Level(), "Unexpected level after SetLevel.")
-	})
+func TestDynamicLevel(t *testing.T) {
+	lvl := DynamicLevel()
+	assert.Equal(t, InfoLevel, lvl.Level(), "Unexpected initial level.")
+	lvl.SetLevel(ErrorLevel)
+	assert.Equal(t, ErrorLevel, lvl.Level(), "Unexpected level after SetLevel.")
 }
 
-func TestJSONLoggerConcurrentLevelMutation(t *testing.T) {
+func TestDynamicLevel_concurrentMutation(t *testing.T) {
+	lvl := DynamicLevel()
 	// Trigger races for non-atomic level mutations.
-	logger := New(newJSONEncoder())
-
 	proceed := make(chan struct{})
 	wg := &sync.WaitGroup{}
 	runConcurrently(10, 100, wg, func() {
 		<-proceed
-		logger.Level()
+		lvl.Level()
 	})
 	runConcurrently(10, 100, wg, func() {
 		<-proceed
-		logger.SetLevel(WarnLevel)
+		lvl.SetLevel(WarnLevel)
 	})
 	close(proceed)
 	wg.Wait()
 }
 
-func TestJSONLoggerRuntimeLevelChange(t *testing.T) {
-	// Test that changing a logger's level also changes the level of all
-	// ancestors and descendants.
-	withJSONLogger(t, opts(InfoLevel), func(grandparent Logger, buf *testBuffer) {
+func TestJSONLogger_DynamicLevel(t *testing.T) {
+	// Test that the DynamicLevel applys to all ancestors and descendants.
+	dl := DynamicLevel()
+	withJSONLogger(t, opts(dl), func(grandparent Logger, buf *testBuffer) {
 		parent := grandparent.With(Int("generation", 2))
 		child := parent.With(Int("generation", 3))
 		all := []Logger{grandparent, parent, child}
 
-		assert.Equal(t, InfoLevel, parent.Level(), "expected initial InfoLevel")
+		assert.Equal(t, InfoLevel, dl.Level(), "expected initial InfoLevel")
 
 		for round, lvl := range []Level{InfoLevel, DebugLevel, WarnLevel} {
-			parent.SetLevel(lvl)
+			dl.SetLevel(lvl)
 			for loggerI, log := range all {
 				log.Debug("@debug", Int("round", round), Int("logger", loggerI))
 			}
@@ -353,8 +351,7 @@ func TestJSONLoggerDFatal(t *testing.T) {
 }
 
 func TestJSONLoggerNoOpsDisabledLevels(t *testing.T) {
-	withJSONLogger(t, nil, func(logger Logger, buf *testBuffer) {
-		logger.SetLevel(WarnLevel)
+	withJSONLogger(t, opts(WarnLevel), func(logger Logger, buf *testBuffer) {
 		logger.Info("silence!")
 		assert.Equal(t, []string{}, buf.Lines(), "Expected logging at a disabled level to produce no output.")
 	})