Revise the loglevel API to be more golang idiomatic (#165)

- LogLevel has a String method to convert the numeric level into its string version
- There is only one name used to identify the default log level, and it is declared as a const `DefaultName`
- Removed the "Get" name for functions that lookup a value.
- A `DefaultLevel` function returns the current log level and handles locking.
- Function to convert string to LogLevel is named `Parse`

Author: gammazero

levels.go

@@ -6,6 +6,14 @@ import "go.uber.org/zap/zapcore"
 // enum.
 type LogLevel zapcore.Level
 
+// DefaultName is the subsystem name that identifies the default log level.
+const DefaultName = ""
+
+// String returns the name of a LogLevel.
+func (lvl LogLevel) String() string {
+	return zapcore.Level(lvl).String()
+}
+
 var (
 	LevelDebug  = LogLevel(zapcore.DebugLevel)
 	LevelInfo   = LogLevel(zapcore.InfoLevel)
@@ -16,51 +24,43 @@ var (
 	LevelFatal  = LogLevel(zapcore.FatalLevel)
 )
 
-// LevelFromString parses a string-based level and returns the corresponding
-// LogLevel.
-//
-// Supported strings are: DEBUG, INFO, WARN, ERROR, DPANIC, PANIC, FATAL, and
-// their lower-case forms.
-//
-// The returned LogLevel must be discarded if error is not nil.
-func LevelFromString(level string) (LogLevel, error) {
-	lvl := zapcore.InfoLevel // zero value
-	err := lvl.Set(level)
+// Parse parses a string-based level and returns the corresponding LogLevel. An
+// error is returned of the string is not the name of a supported LogLevel.
+func Parse(name string) (LogLevel, error) {
+	var lvl zapcore.Level
+	err := lvl.Set(name)
 	return LogLevel(lvl), err
 }
 
-// LevelName returns the name of a LogLevel.
-func LevelName(level LogLevel) string {
-	return zapcore.Level(level).String()
+// DefaultLevel returns the current default LogLevel.
+func DefaultLevel() LogLevel {
+	loggerMutex.RLock()
+	lvl := defaultLevel
+	loggerMutex.RUnlock()
+	return lvl
 }
 
-// GetLogLevel returns the current log level for a given subsystem as a string.
-// Passing name="*" or name="" returns the defaultLevel.
-func GetLogLevel(name string) (string, error) {
-	if name == "*" || name == "" {
-		loggerMutex.RLock()
-		defLvl := defaultLevel
-		loggerMutex.RUnlock()
-		return LevelName(defLvl), nil
+// SubsystemLevelName returns the current log level name for a given subsystem.
+// An empty name, "", returns the default LogLevel name.
+func SubsystemLevelName(subsys string) (string, error) {
+	if subsys == DefaultName {
+		return DefaultLevel().String(), nil
 	}
-	if lvl, ok := levels[name]; ok {
-		return lvl.Level().String(), nil
+	lvl, ok := levels[subsys]
+	if !ok {
+		return "", ErrNoSuchLogger
 	}
-	return "", ErrNoSuchLogger
+	return lvl.Level().String(), nil
 }
 
-// GetAllLogLevels returns a map of all current log levels for all subsystems as strings.
-// The map includes a special "*" key that represents the defaultLevel.
-func GetAllLogLevels() map[string]string {
+// SubsystemLevelNames returns a map of all facility names to their current log
+// levels as strings. The map includes the default log level identified by the
+// defaultName string as the map key.
+func SubsystemLevelNames() map[string]string {
 	result := make(map[string]string, len(levels)+1)
 
-	// Add the default level with "*" key
-	loggerMutex.RLock()
-	defLvl := defaultLevel
-	loggerMutex.RUnlock()
-	result["*"] = LevelName(defLvl)
+	result[DefaultName] = DefaultLevel().String()
 
-	// Add all subsystem levels
 	for name, level := range levels {
 		result[name] = level.Level().String()
 	}

log_level_test.go

@@ -69,7 +69,7 @@ func clearLoggerState() {
 	clear(levels)
 }
 
-func TestGetDefaultLevel(t *testing.T) {
+func TestDefaultLevel(t *testing.T) {
 	originalConfig := GetConfig()
 	defer SetupLogging(originalConfig)
 
@@ -81,47 +81,39 @@ func TestGetDefaultLevel(t *testing.T) {
 	for _, expected := range testCases {
 		SetupLogging(Config{Level: expected, Stderr: true})
 
-		// empty string arg
-		lvl, err := GetLogLevel("")
-		if err != nil {
-			t.Errorf("GetLogLevel() returned error: %v", err)
-		} else if lvl != LevelName(expected) {
-			t.Errorf("GetLogLevel() = %v, want %v", lvl, LevelName(expected))
-		}
-
-		// explicit "*"
-		lvl, err = GetLogLevel("*")
-		if err != nil {
-			t.Errorf(`GetLogLevel("*") returned error: %v`, err)
-		} else if lvl != LevelName(expected) {
-			t.Errorf(`GetLogLevel("*") = %v, want %v`, lvl, LevelName(expected))
+		// check default
+		if DefaultLevel() != expected {
+			t.Fatalf("wrong default level, want %v, got %v", DefaultLevel(), expected)
 		}
 
-		// empty string
-		lvl, err = GetLogLevel("")
+		// empty string subsystem
+		lvl, err := SubsystemLevelName("")
 		if err != nil {
-			t.Errorf(`GetLogLevel("") returned error: %v`, err)
-		} else if lvl != LevelName(expected) {
-			t.Errorf(`GetLogLevel("") = %v, want %v`, lvl, LevelName(expected))
+			t.Errorf("SubsystemLevelName returned error: %v", err)
+		} else if lvl != expected.String() {
+			t.Errorf("SubsystemLevelName returned %v, want %v", lvl, expected)
 		}
 	}
 }
 
-func TestGetAllLogLevels(t *testing.T) {
+func TestGetSubsystemLevelNames(t *testing.T) {
 	originalConfig := GetConfig()
 	defer SetupLogging(originalConfig)
 
 	// Clear any state from previous tests first
 	clearLoggerState()
 
 	SetupLogging(Config{Level: LevelWarn, Stderr: true})
-	base := GetAllLogLevels()
+	base := SubsystemLevelNames()
 
 	if len(base) != 1 {
-		t.Errorf("baseline GetAllLogLevels() length = %d; want 1", len(base))
+		t.Errorf("baseline SubsystemLevelNames() length = %d; want 1", len(base))
+	}
+	if DefaultLevel() != LevelWarn {
+		t.Fatal("wrong default level")
 	}
-	if base["*"] != LevelName(LevelWarn) {
-		t.Errorf("baseline GetAllLogLevels()[\"*\"] = %v; want %v", base["*"], LevelName(LevelWarn))
+	if base[DefaultName] != LevelWarn.String() {
+		t.Errorf("baseline SubsystemLevelNames()[\"\"] = %v; want %v", base["*"], LevelWarn.String())
 	}
 
 	expected := map[string]LogLevel{
@@ -135,19 +127,19 @@ func TestGetAllLogLevels(t *testing.T) {
 		Stderr:          true,
 	})
 
-	all := GetAllLogLevels()
+	all := SubsystemLevelNames()
 
-	if all["*"] != LevelName(LevelError) {
-		t.Errorf(`GetAllLogLevels()["*"] = %v; want %v`, all["*"], LevelName(LevelError))
+	if all[""] != DefaultLevel().String() {
+		t.Errorf(`SubsystemLevelNames()[""] = %v; want %v`, all[""], DefaultLevel().String())
 	}
 	for name, want := range expected {
 		got, ok := all[name]
 		if !ok {
-			t.Errorf("missing key %q in GetAllLogLevels()", name)
+			t.Errorf("missing key %q in SubsystemLevelNames()", name)
 			continue
 		}
-		if got != LevelName(want) {
-			t.Errorf(`GetAllLogLevels()["%s"] = %v; want %v`, name, got, LevelName(want))
+		if got != want.String() {
+			t.Errorf(`SubsystemLevelNames()["%s"] = %v; want %v`, name, got, want.String())
 		}
 	}
 
@@ -157,36 +149,36 @@ func TestGetAllLogLevels(t *testing.T) {
 		t.Fatalf("SetLogLevel(dynamic) failed: %v", err)
 	}
 
-	all = GetAllLogLevels()
+	all = SubsystemLevelNames()
 	if lvl, ok := all["dynamic"]; !ok {
 		t.Error(`missing "dynamic" key after creation`)
-	} else if lvl != LevelName(LevelFatal) {
-		t.Errorf(`GetAllLogLevels()["dynamic"] = %v; want %v`, lvl, LevelName(LevelFatal))
+	} else if lvl != LevelFatal.String() {
+		t.Errorf(`SubsystemLevelNames()["dynamic"] = %v; want %v`, lvl, LevelFatal.String())
 	}
 
 	// ensure immutability
-	snapshot := GetAllLogLevels()
-	snapshot["*"] = LevelName(LevelDebug)
-	snapshot["newkey"] = LevelName(LevelInfo)
+	snapshot := SubsystemLevelNames()
+	snapshot[DefaultName] = DefaultLevel().String()
+	snapshot["newkey"] = LevelInfo.String()
 
 	// ensure original state unchanged
-	fresh := GetAllLogLevels()
-	if fresh["*"] != LevelName(LevelError) {
-		t.Errorf(`immutable check failed: fresh["*"] = %v; want %v`, fresh["*"], LevelName(LevelError))
+	fresh := SubsystemLevelNames()
+	if fresh[DefaultName] != LevelError.String() {
+		t.Errorf(`immutable check failed: fresh[DefaultName] = %v; want %v`, fresh[DefaultName], LevelError.String())
 	}
 	if _, exists := fresh["newkey"]; exists {
 		t.Error(`immutable check failed: "newkey" should not leak into real map`)
 	}
 }
 
-func TestLevelName(t *testing.T) {
+func TestLogLevelString(t *testing.T) {
 	testLevels := []LogLevel{LevelDebug, LevelInfo, LevelWarn, LevelError}
 	expectNames := []string{"debug", "info", "warn", "error"}
 
 	for i := range testLevels {
-		name := LevelName(testLevels[i])
-		if name != expectNames[i] {
-			t.Errorf("unexpected name for level: expected %s, got %s", expectNames[i], name)
+		if testLevels[i].String() != expectNames[i] {
+			t.Errorf("unexpected name for level: expected %s, got %s", expectNames[i], testLevels[i].String())
 		}
 	}
+
 }

setup.go

@@ -198,7 +198,7 @@ func setAllLoggers(lvl LogLevel) {
 // SetLogLevel changes the log level of a specific subsystem
 // name=="*" changes all subsystems
 func SetLogLevel(name, level string) error {
-	lvl, err := LevelFromString(level)
+	lvl, err := Parse(level)
 	if err != nil {
 		return err
 	}
@@ -226,7 +226,7 @@ func SetLogLevel(name, level string) error {
 // SetLogLevelRegex sets all loggers to level `l` that match expression `e`.
 // An error is returned if `e` fails to compile.
 func SetLogLevelRegex(e, l string) error {
-	lvl, err := LevelFromString(l)
+	lvl, err := Parse(l)
 	if err != nil {
 		return err
 	}
@@ -321,7 +321,7 @@ func configFromEnv() Config {
 	if lvl != "" {
 		for _, kvs := range strings.Split(lvl, ",") {
 			kv := strings.SplitN(kvs, "=", 2)
-			lvl, err := LevelFromString(kv[len(kv)-1])
+			lvl, err := Parse(kv[len(kv)-1])
 			if err != nil {
 				fmt.Fprintf(os.Stderr, "error setting log level %q: %s\n", kvs, err)
 				continue