Use HandlerFn type instead of func signature

Improves readability

Author: Russ Egan

v2/config_from_env.go

@@ -66,6 +66,8 @@ func MustConfigFromEnv(envvars ...string) {
 func UnmarshalEnv(o *HandlerOptions, envvars ...string) error {
 	for _, v := range envvars {
 		if configString := os.Getenv(v); configString != "" {
+			// todo: need to add a branch here to handle when the environment variable is
+			// set to a raw levels string, and isn't JSON
 			err := json.Unmarshal([]byte(configString), o)
 			if err != nil {
 				return fmt.Errorf("parsing configuration from environment variable %v: %w", v, err)
@@ -85,15 +87,21 @@ func resetBuiltInHandlerFns() {
 	textHandlerFn := func(_ string, w io.Writer, opts *slog.HandlerOptions) slog.Handler {
 		return slog.NewTextHandler(w, opts)
 	}
-	handlerFns.Store(TextHandler, textHandlerFn)
+	registerHandlerFn(TextHandler, textHandlerFn)
 	// for v1 compatibility, "console" is an alias for "text"
-	handlerFns.Store(ConsoleHandler, textHandlerFn)
-	handlerFns.Store(JSONHandler, func(_ string, w io.Writer, opts *slog.HandlerOptions) slog.Handler {
+	registerHandlerFn(ConsoleHandler, textHandlerFn)
+	registerHandlerFn(JSONHandler, func(_ string, w io.Writer, opts *slog.HandlerOptions) slog.Handler {
 		return slog.NewJSONHandler(w, opts)
 	})
-	handlerFns.Store(TermHandler, termHandlerFn(false))
-	handlerFns.Store(TermColorHandler, termHandlerFn(true))
-	handlerFns.Store(NoopHandler, func(_ string, _ io.Writer, _ *slog.HandlerOptions) slog.Handler {
+	registerHandlerFn(TermHandler, func(_ string, w io.Writer, opts *slog.HandlerOptions) slog.Handler {
+		termOpts := termHandlerOptions(opts)
+		termOpts.NoColor = true
+		return console.NewHandler(w, termOpts)
+	})
+	registerHandlerFn(TermColorHandler, func(_ string, w io.Writer, opts *slog.HandlerOptions) slog.Handler {
+		return console.NewHandler(w, termHandlerOptions(opts))
+	})
+	registerHandlerFn(NoopHandler, func(_ string, _ io.Writer, _ *slog.HandlerOptions) slog.Handler {
 		return noop
 	})
 }
@@ -104,17 +112,23 @@ func initHandlerFns() {
 	})
 }
 
-func LookupHandlerFn(name string) func(string, io.Writer, *slog.HandlerOptions) slog.Handler {
+func LookupHandlerFn(name string) HandlerFn {
 	initHandlerFns()
 	v, ok := handlerFns.Load(name)
 	if !ok {
 		return nil
 	}
-	return v.(func(string, io.Writer, *slog.HandlerOptions) slog.Handler) //nolint:forcetypeassert // if it's not a HandlerFn, we should panic
+	// fn := v.(func(string, io.Writer, *slog.HandlerOptions) slog.Handler) //nolint:forcetypeassert // if it's not a HandlerFn, we should panic
+	fn := v.(HandlerFn) //nolint:forcetypeassert // if it's not a HandlerFn, we should panic
+	return fn
 }
 
-func RegisterHandlerFn(name string, fn func(string, io.Writer, *slog.HandlerOptions) slog.Handler) {
+func RegisterHandlerFn(name string, fn HandlerFn) {
 	initHandlerFns()
+	registerHandlerFn(name, fn)
+}
+
+func registerHandlerFn(name string, fn HandlerFn) {
 	if fn == nil {
 		panic(fmt.Sprintf("constructor for sink %q is nil", name))
 	}
@@ -124,19 +138,26 @@ func RegisterHandlerFn(name string, fn func(string, io.Writer, *slog.HandlerOpti
 	handlerFns.Store(name, fn)
 }
 
-func termHandlerFn(color bool) func(string, io.Writer, *slog.HandlerOptions) slog.Handler {
-	return func(_ string, w io.Writer, opts *slog.HandlerOptions) slog.Handler {
-		if opts == nil {
-			opts = &slog.HandlerOptions{}
-		}
-		return console.NewHandler(w, &console.HandlerOptions{
-			NoColor:            !color,
-			AddSource:          opts.AddSource,
-			Theme:              console.NewDefaultTheme(),
-			ReplaceAttr:        opts.ReplaceAttr,
-			TimeFormat:         "15:04:05.000",
-			HeaderFormat:       "%t %[" + LoggerKey + "]8h %l | %m",
-			TruncateSourcePath: 2,
-		})
+func termHandlerOptions(opts *slog.HandlerOptions) *console.HandlerOptions {
+	// todo: it would be nice if consumers could tweak this, either programatically
+	// or via configuration, but that would mean exposing the dependency on console-slog,
+	// which is currently opaque.  For now, I want to keep this opaque.  That means, if
+	// a consumer was a slightly different configuration of console-slog, they will
+	// have to construct it from scratch themselves.
+	if opts == nil {
+		opts = &slog.HandlerOptions{}
+	}
+	theme := console.NewDefaultTheme()
+	theme.Name = "flume"
+	theme.Source = console.ToANSICode(console.BrightBlack, console.Italic)
+	theme.AttrKey = console.ToANSICode(console.Green, console.Faint)
+	return &console.HandlerOptions{
+		AddSource:          opts.AddSource,
+		ReplaceAttr:        opts.ReplaceAttr,
+		Level:              opts.Level,
+		Theme:              theme,
+		TimeFormat:         "15:04:05.000",
+		HeaderFormat:       "%t %[" + LoggerKey + "]8h |%l| %m %a %(source){→ %s%}",
+		TruncateSourcePath: 2,
 	}
 }

v2/flumetest/flumetest.go

@@ -25,15 +25,12 @@ import (
 	"encoding/json"
 	"flag"
 	"fmt"
-	"io"
-	"log/slog"
 	"os"
 	"strconv"
 	"sync"
 	"sync/atomic"
 
 	"github.com/ThalesGroup/flume/v2"
-	"github.com/ansel1/console-slog"
 )
 
 var Disabled bool
@@ -90,20 +87,9 @@ func MustSetDefaults() {
 }
 
 func TestDefaults() *flume.HandlerOptions {
-	return &flume.HandlerOptions{
-		Level: slog.LevelDebug,
-		HandlerFn: func(_ string, w io.Writer, opts *slog.HandlerOptions) slog.Handler {
-			return console.NewHandler(w, &console.HandlerOptions{
-				TimeFormat:         "15:04:05.000",
-				Level:              opts.Level,
-				AddSource:          opts.AddSource,
-				NoColor:            true,
-				TruncateSourcePath: 2,
-				HeaderFormat:       "%t %[" + flume.LoggerKey + "]12h %l | %m",
-			})
-		},
-		AddSource: true,
-	}
+	dd := flume.DevDefaults()
+	dd.Level = flume.LevelAll
+	return dd
 }
 
 // Start captures all logs written during the test.  If the test succeeds, the

v2/flumetest/flumetest_test.go

@@ -12,7 +12,12 @@ import (
 )
 
 func init() {
-	MustSetDefaults()
+	opts := &flume.HandlerOptions{
+		HandlerFn: flume.LookupHandlerFn(flume.TermHandler),
+		AddSource: true,
+		Level:     flume.LevelAll,
+	}
+	flume.Default().SetHandlerOptions(opts)
 }
 
 type mockT struct {

v2/handler_options.go

@@ -18,6 +18,8 @@ var (
 	ErrInvalidLevelType   = errors.New("levels must be a string or int value")
 )
 
+type HandlerFn func(name string, w io.Writer, opts *slog.HandlerOptions) slog.Handler
+
 type HandlerOptions struct {
 	// default level for all loggers, defaults to slog.LevelInfo
 	Level slog.Leveler
@@ -27,14 +29,20 @@ type HandlerOptions struct {
 	AddSource bool
 	// replace attributes
 	ReplaceAttrs []func(groups []string, a slog.Attr) slog.Attr
-	// constructor for sinks, defaults to slog.NewTextHandler
-	HandlerFn func(name string, w io.Writer, opts *slog.HandlerOptions) slog.Handler
+	// If set, will be called to construct handler instances.
+	// `name` is the name of the logger being created.  For the default
+	// logger, this will be empty.
+	// `w` and `opts` will naver be nil.
+	//
+	// `name` may be used to return different handlers or unique middleware
+	// for particular loggers.
+	HandlerFn HandlerFn
 	// middleware applied to all sinks
 	Middleware []Middleware
 }
 
-func DevDefaults() HandlerOptions {
-	return HandlerOptions{
+func DevDefaults() *HandlerOptions {
+	return &HandlerOptions{
 		HandlerFn: LookupHandlerFn(TermColorHandler),
 		AddSource: true,
 	}
@@ -70,13 +78,15 @@ func (o *HandlerOptions) UnmarshalJSON(bytes []byte) error {
 		return fmt.Errorf("invalid json config: %w", err)
 	}
 
-	var opts HandlerOptions
-	if o != nil {
-		opts = *o
-	}
+	var opts *HandlerOptions
 
-	if s.Development {
+	switch {
+	case s.Development:
 		opts = DevDefaults()
+	case o == nil:
+		opts = &HandlerOptions{}
+	default:
+		opts = o.Clone()
 	}
 
 	if s.Level != nil {
@@ -111,6 +121,14 @@ func (o *HandlerOptions) UnmarshalJSON(bytes []byte) error {
 		return fmt.Errorf("%w: %v", ErrInvalidLevelsValue, s.Levels)
 	}
 
+	// for backward compatibility with flumev1, if there is a level named "*"
+	// in the levels map, treat it like setting the default level.  Again,
+	// to match v1 behavior, this overrides the default level option.
+	if lvl, ok := opts.Levels["*"]; ok {
+		opts.Level = lvl
+		delete(opts.Levels, "*")
+	}
+
 	// for backward compat with v1, allow "addCaller" as
 	// an alias for "addSource"
 	if s.AddSource == nil {
@@ -131,7 +149,7 @@ func (o *HandlerOptions) UnmarshalJSON(bytes []byte) error {
 		opts.HandlerFn = LookupHandlerFn(s.Handler)
 	}
 
-	*o = opts
+	*o = *opts
 
 	return nil
 }