ref(promslog): make log style a type, not settable

We only ever really intend to support these 2 formats, and after discussion it's been decided that this config does not need to be settable. Adopting a custom type for the logging style simplifies a fair bit. Signed-off-by: TJ Hoplock <[email protected]>
prometheus · Aug 28, 2024 · a26557e · a26557e
1 parent a7e4e32
commit a26557e
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 45 deletions.
diff --git a/promslog/slog.go b/promslog/slog.go
@@ -25,15 +25,20 @@ import (
 	"log/slog"
 	"os"
 	"path/filepath"
-	"slices"
 	"strconv"
 	"strings"
 )
 
+type LogStyle string
+
+const (
+	SlogStyle  LogStyle = "slog"
+	GoKitStyle LogStyle = "go-kit"
+)
+
 var (
 	LevelFlagOptions  = []string{"debug", "info", "warn", "error"}
 	FormatFlagOptions = []string{"logfmt", "json"}
-	StyleOptions      = []string{"slog", "go-kit"}
 
 	callerAddFunc             = false
 	defaultWriter             = os.Stderr
@@ -141,35 +146,11 @@ func (f *AllowedFormat) Set(s string) error {
 	return nil
 }
 
-// AllowedStyle is a settable identifier for the log formatting style that a
-// logger can have. Valid options are `slog` (default) or `go-kit`.
-type AllowedStyle struct {
-	s string
-}
-
-func (a *AllowedStyle) String() string {
-	return a.s
-}
-
-// Set updates the value of the allowed logging style. Valid options are `slog`
-// (default), and `go-kit`. Note that while it is possible to call this method
-// and update the allowed logging style after creating a logger, it has no
-// effect; the last call to Set() prior to initializing a new logger is the
-// value that will be used for the lifetime of the logger.
-func (a *AllowedStyle) Set(style string) error {
-	if !slices.Contains(StyleOptions, style) {
-		return fmt.Errorf("unrecognized log style %s", style)
-	}
-
-	a.s = style
-	return nil
-}
-
 // Config is a struct containing configurable settings for the logger
 type Config struct {
 	Level    *AllowedLevel
 	Format   *AllowedFormat
-	Style    *AllowedStyle
+	Style    LogStyle
 	ioWriter io.Writer
 }
 
@@ -190,7 +171,7 @@ func New(config *Config) *slog.Logger {
 		AddSource: true,
 	}
 
-	if config.Style != nil && config.Style.s == "go-kit" {
+	if config.Style == GoKitStyle {
 		logHandlerOpts.ReplaceAttr = goKitStyleReplaceAttrFunc
 	}
 

diff --git a/promslog/slog_test.go b/promslog/slog_test.go
@@ -21,7 +21,6 @@ import (
 	"fmt"
 	"log/slog"
 	"regexp"
-	"slices"
 	"strings"
 	"testing"
 
@@ -102,30 +101,18 @@ func TestDynamicLevels(t *testing.T) {
 	wantedLevelCounts := map[string]int{"info": 1, "debug": 1}
 
 	tests := map[string]struct {
-		logStyle         string
+		logStyle         LogStyle
 		logStyleRegexp   *regexp.Regexp
 		wantedLevelCount map[string]int
 	}{
-		"empty_log_style":  {logStyle: "", logStyleRegexp: slogStyleLogRegexp, wantedLevelCount: wantedLevelCounts},
-		"bad_log_style":    {logStyle: "foo", logStyleRegexp: slogStyleLogRegexp, wantedLevelCount: wantedLevelCounts},
-		"slog_log_style":   {logStyle: "slog", logStyleRegexp: slogStyleLogRegexp, wantedLevelCount: wantedLevelCounts},
-		"go-kit_log_style": {logStyle: "go-kit", logStyleRegexp: goKitStyleLogRegexp, wantedLevelCount: wantedLevelCounts},
+		"slog_log_style":   {logStyle: SlogStyle, logStyleRegexp: slogStyleLogRegexp, wantedLevelCount: wantedLevelCounts},
+		"go-kit_log_style": {logStyle: GoKitStyle, logStyleRegexp: goKitStyleLogRegexp, wantedLevelCount: wantedLevelCounts},
 	}
 
 	for name, tc := range tests {
 		t.Run(name, func(t *testing.T) {
 			buf.Reset() // Ensure buf is reset prior to tests
-			config := &Config{ioWriter: &buf, Style: &AllowedStyle{}}
-			err := config.Style.Set(tc.logStyle)
-			// AllowedStyle.Set() returns an error on invalid style
-			// values and the tests use a mixture of valid and
-			// invalid values. We must expect no error for valid
-			// values, and expect an error for the invalid values.
-			if slices.Contains(StyleOptions, tc.logStyle) {
-				require.NoError(t, err, "log style returned an error setting a valid value", "value", tc.logStyle)
-			} else {
-				require.ErrorContains(t, err, fmt.Sprintf("unrecognized log style %s", tc.logStyle), "expected error setting log style to invalid value", "value", tc.logStyle)
-			}
+			config := &Config{ioWriter: &buf, Style: tc.logStyle}
 			logger := New(config)
 
 			// Test that log level can be adjusted on-the-fly to debug and that a