hive: take Logger as part of Start/Stop or Run

Instead of having to provide a logger already at hive creation time,
pass the logger to Start/Stop/Run, which is when the underlying cells
actually need it.

Also revert the changes to the Apply interface by passing the logger
when the invokes are actually run instead of during apply.

Finally, add hivetest.Logger to create a logger from a testing.TB, so
that the logs can be attributed to the correct test when running tests
in parallel. Unfortunately, t.Helper() doesn't correctly work in
combination with slog - we log the real source location as an attribute
as a workaround.

Signed-off-by: David Bimmler <[email protected]>

cell/cell.go

@@ -4,9 +4,6 @@
 package cell
 
 import (
-	"log/slog"
-	"time"
-
 	"go.uber.org/dig"
 )
 
@@ -24,7 +21,7 @@ type Cell interface {
 	Info(container) Info
 
 	// Apply the cell to the dependency graph container.
-	Apply(*slog.Logger, container, time.Duration) error
+	Apply(container) error
 }
 
 // In when embedded into a struct used as constructor parameter makes the exported

cell/config.go

@@ -5,10 +5,8 @@ package cell
 
 import (
 	"fmt"
-	"log/slog"
 	"reflect"
 	"strings"
-	"time"
 
 	"github.com/cilium/hive/internal"
 	"github.com/mitchellh/mapstructure"
@@ -146,7 +144,7 @@ func decoderConfig(target any, extraHooks DecodeHooks) *mapstructure.DecoderConf
 	}
 }
 
-func (c *config[Cfg]) Apply(log *slog.Logger, cont container, logThreshold time.Duration) error {
+func (c *config[Cfg]) Apply(cont container) error {
 	// Register the flags to the global set of all flags.
 	err := cont.Invoke(
 		func(allFlags *pflag.FlagSet) {

cell/decorator.go

@@ -5,8 +5,6 @@ package cell
 
 import (
 	"fmt"
-	"log/slog"
-	"time"
 
 	"github.com/cilium/hive/internal"
 )
@@ -40,14 +38,14 @@ type decorator struct {
 	cells     []Cell
 }
 
-func (d *decorator) Apply(log *slog.Logger, c container, logThreshold time.Duration) error {
+func (d *decorator) Apply(c container) error {
 	scope := c.Scope(fmt.Sprintf("(decorate %s)", internal.PrettyType(d.decorator)))
 	if err := scope.Decorate(d.decorator); err != nil {
 		return err
 	}
 
 	for _, cell := range d.cells {
-		if err := cell.Apply(log, scope, logThreshold); err != nil {
+		if err := cell.Apply(scope); err != nil {
 			return err
 		}
 	}

cell/group.go

@@ -3,11 +3,6 @@
 
 package cell
 
-import (
-	"log/slog"
-	"time"
-)
-
 type group []Cell
 
 // Group a set of cells. Unlike Module(), Group() does not create a new
@@ -16,9 +11,9 @@ func Group(cells ...Cell) Cell {
 	return group(cells)
 }
 
-func (g group) Apply(log *slog.Logger, c container, logThreshold time.Duration) error {
+func (g group) Apply(c container) error {
 	for _, cell := range g {
-		if err := cell.Apply(log, c, logThreshold); err != nil {
+		if err := cell.Apply(c); err != nil {
 			return err
 		}
 	}

cell/invoke.go

@@ -29,7 +29,7 @@ type namedFunc struct {
 }
 
 type InvokerList interface {
-	AppendInvoke(func() error)
+	AppendInvoke(func(*slog.Logger, time.Duration) error)
 }
 
 func (inv *invoker) invoke(log *slog.Logger, cont container, logThreshold time.Duration) error {
@@ -62,9 +62,9 @@ func (inv *invoker) invoke(log *slog.Logger, cont container, logThreshold time.D
 	return nil
 }
 
-func (inv *invoker) Apply(log *slog.Logger, c container, logThreshold time.Duration) error {
+func (inv *invoker) Apply(c container) error {
 	// Remember the scope in which we need to invoke.
-	invoker := func() error { return inv.invoke(log, c, logThreshold) }
+	invoker := func(log *slog.Logger, logThreshold time.Duration) error { return inv.invoke(log, c, logThreshold) }
 
 	// Append the invoker to the list of invoke functions. These are invoked
 	// prior to start to build up the objects. They are not invoked directly

cell/module.go

@@ -9,7 +9,6 @@ import (
 	"regexp"
 	"slices"
 	"strings"
-	"time"
 
 	"go.uber.org/dig"
 )
@@ -163,7 +162,7 @@ func (m *module) modulePrivateProviders(scope *dig.Scope) error {
 	return scope.Invoke(provide)
 }
 
-func (m *module) Apply(log *slog.Logger, c container, logThreshold time.Duration) error {
+func (m *module) Apply(c container) error {
 	scope := c.Scope(m.id)
 
 	// Provide ModuleID and FullModuleID in the module's scope.
@@ -191,7 +190,7 @@ func (m *module) Apply(log *slog.Logger, c container, logThreshold time.Duration
 	}
 
 	for _, cell := range m.cells {
-		if err := cell.Apply(log, scope, logThreshold); err != nil {
+		if err := cell.Apply(scope); err != nil {
 			return err
 		}
 	}

cell/provide.go

@@ -5,11 +5,9 @@ package cell
 
 import (
 	"fmt"
-	"log/slog"
 	"sort"
 	"strings"
 	"sync"
-	"time"
 
 	"go.uber.org/dig"
 
@@ -24,7 +22,7 @@ type provider struct {
 	export  bool
 }
 
-func (p *provider) Apply(log *slog.Logger, c container, logThreshold time.Duration) error {
+func (p *provider) Apply(c container) error {
 	// Since the same Provide cell may be used multiple times
 	// in different hives we use a mutex to protect it and we
 	// fill the provide info only the first time.

example/main.go

@@ -4,6 +4,8 @@
 package main
 
 import (
+	"log/slog"
+
 	"github.com/spf13/cobra"
 
 	"github.com/cilium/hive"
@@ -53,7 +55,7 @@ var (
 			// and then constructs all objects, followed by executing the start
 			// hooks in dependency order. It will then block waiting for signals
 			// after which it will run the stop hooks in reverse order.
-			if err := Hive.Run(); err != nil {
+			if err := Hive.Run(slog.Default()); err != nil {
 				// Run() can fail if:
 				// - There are missing types in the object graph
 				// - Executing the lifecycle start or stop hooks fails

hive.go

@@ -23,8 +23,6 @@ import (
 )
 
 type Options struct {
-	Logger *slog.Logger
-
 	// EnvPrefix is the prefix to use for environment variables, e.g.
 	// with prefix "CILIUM" the flag "foo" can be set with environment
 	// variable "CILIUM_FOO".
@@ -74,7 +72,6 @@ type Options struct {
 
 func DefaultOptions() Options {
 	return Options{
-		Logger:           nil, // Will use slog.Default()
 		EnvPrefix:        "",
 		ModuleDecorators: nil,
 		StartTimeout:     defaultStartTimeout,
@@ -100,7 +97,6 @@ const (
 //
 // See pkg/hive/example for a runnable example application.
 type Hive struct {
-	log             *slog.Logger
 	opts            Options
 	container       *dig.Container
 	cells           []cell.Cell
@@ -109,7 +105,7 @@ type Hive struct {
 	viper           *viper.Viper
 	lifecycle       cell.Lifecycle
 	populated       bool
-	invokes         []func() error
+	invokes         []func(*slog.Logger, time.Duration) error
 	configOverrides []any
 }
 
@@ -127,11 +123,7 @@ func New(cells ...cell.Cell) *Hive {
 }
 
 func NewWithOptions(opts Options, cells ...cell.Cell) *Hive {
-	if opts.Logger == nil {
-		opts.Logger = slog.Default()
-	}
 	h := &Hive{
-		log:       opts.Logger,
 		opts:      opts,
 		container: dig.New(),
 		cells:     cells,
@@ -152,7 +144,7 @@ func NewWithOptions(opts Options, cells ...cell.Cell) *Hive {
 	// and adds all config flags. Invokes are delayed until Start() is
 	// called.
 	for _, cell := range cells {
-		if err := cell.Apply(opts.Logger, h.container, opts.LogThreshold); err != nil {
+		if err := cell.Apply(h.container); err != nil {
 			panic(fmt.Sprintf("Failed to apply cell: %s", err))
 		}
 	}
@@ -195,8 +187,6 @@ type defaults struct {
 
 	Flags                  *pflag.FlagSet
 	Lifecycle              cell.Lifecycle
-	Logger                 *slog.Logger
-	RootLogger             cell.RootLogger
 	Shutdowner             Shutdowner
 	InvokerList            cell.InvokerList
 	EmptyFullModuleID      cell.FullModuleID
@@ -211,8 +201,6 @@ func (h *Hive) provideDefaults() error {
 		return defaults{
 			Flags:                  h.flags,
 			Lifecycle:              h.lifecycle,
-			Logger:                 h.opts.Logger,
-			RootLogger:             cell.RootLogger(h.opts.Logger),
 			Shutdowner:             h,
 			InvokerList:            h,
 			EmptyFullModuleID:      nil,
@@ -233,36 +221,36 @@ func AddConfigOverride[Cfg cell.Flagger](h *Hive, override func(*Cfg)) {
 
 // Run populates the cell configurations and runs the hive cells.
 // Interrupt signal or call to Shutdowner.Shutdown() will cause the hive to stop.
-func (h *Hive) Run() error {
+func (h *Hive) Run(log *slog.Logger) error {
 	startCtx, cancel := context.WithTimeout(context.Background(), h.opts.StartTimeout)
 	defer cancel()
 
 	var errs error
-	if err := h.Start(startCtx); err != nil {
+	if err := h.Start(log, startCtx); err != nil {
 		errs = errors.Join(errs, fmt.Errorf("failed to start: %w", err))
 	}
 
 	// If start was successful, wait for Shutdown() or interrupt.
 	if errs == nil {
-		errs = errors.Join(errs, h.waitForSignalOrShutdown())
+		errs = errors.Join(errs, h.waitForSignalOrShutdown(log))
 	}
 
 	stopCtx, cancel := context.WithTimeout(context.Background(), h.opts.StopTimeout)
 	defer cancel()
 
-	if err := h.Stop(stopCtx); err != nil {
+	if err := h.Stop(log, stopCtx); err != nil {
 		errs = errors.Join(errs, fmt.Errorf("failed to stop: %w", err))
 	}
 	return errs
 }
 
-func (h *Hive) waitForSignalOrShutdown() error {
+func (h *Hive) waitForSignalOrShutdown(log *slog.Logger) error {
 	signals := make(chan os.Signal, 1)
 	defer signal.Stop(signals)
 	signal.Notify(signals, os.Interrupt, syscall.SIGTERM)
 	select {
 	case sig := <-signals:
-		h.log.Info("Signal received", "signal", sig)
+		log.Info("Signal received", "signal", sig)
 		return nil
 	case err := <-h.shutdown:
 		return err
@@ -271,7 +259,7 @@ func (h *Hive) waitForSignalOrShutdown() error {
 
 // Populate instantiates the hive. Use for testing that the hive can
 // be instantiated.
-func (h *Hive) Populate() error {
+func (h *Hive) Populate(log *slog.Logger) error {
 	if h.populated {
 		return nil
 	}
@@ -285,6 +273,16 @@ func (h *Hive) Populate() error {
 	if err != nil {
 		return err
 	}
+	// Provide the user-provide logging infrastructure. This happens here so
+	// that the hive can be created prior to having to lock down the logging
+	// configuration.
+	err = h.container.Provide(
+		func() (*slog.Logger, cell.RootLogger) {
+			return log, cell.RootLogger(log)
+		})
+	if err != nil {
+		return err
+	}
 
 	// Provide config overriders if any
 	for _, o := range h.configOverrides {
@@ -315,45 +313,45 @@ func (h *Hive) Populate() error {
 
 	// Execute the invoke functions to construct the objects.
 	for _, invoke := range h.invokes {
-		if err := invoke(); err != nil {
+		if err := invoke(log, h.opts.LogThreshold); err != nil {
 			return err
 		}
 	}
 	return nil
 }
 
-func (h *Hive) AppendInvoke(invoke func() error) {
+func (h *Hive) AppendInvoke(invoke func(*slog.Logger, time.Duration) error) {
 	h.invokes = append(h.invokes, invoke)
 }
 
 // Start starts the hive. The context allows cancelling the start.
 // If context is cancelled and the start hooks do not respect the cancellation
 // then after 5 more seconds the process will be terminated forcefully.
-func (h *Hive) Start(ctx context.Context) error {
-	if err := h.Populate(); err != nil {
+func (h *Hive) Start(log *slog.Logger, ctx context.Context) error {
+	if err := h.Populate(log); err != nil {
 		return err
 	}
 
 	defer close(h.fatalOnTimeout(ctx))
 
-	h.log.Info("Starting")
+	log.Info("Starting")
 	start := time.Now()
-	err := h.lifecycle.Start(h.log, ctx)
+	err := h.lifecycle.Start(log, ctx)
 	if err == nil {
-		h.log.Info("Started", "duration", time.Since(start))
+		log.Info("Started", "duration", time.Since(start))
 	} else {
-		h.log.Error("Start failed", "error", err, "duration", time.Since(start))
+		log.Error("Start failed", "error", err, "duration", time.Since(start))
 	}
 	return err
 }
 
 // Stop stops the hive. The context allows cancelling the stop.
 // If context is cancelled and the stop hooks do not respect the cancellation
 // then after 5 more seconds the process will be terminated forcefully.
-func (h *Hive) Stop(ctx context.Context) error {
+func (h *Hive) Stop(log *slog.Logger, ctx context.Context) error {
 	defer close(h.fatalOnTimeout(ctx))
-	h.log.Info("Stopping")
-	return h.lifecycle.Stop(h.log, ctx)
+	log.Info("Stopping")
+	return h.lifecycle.Stop(log, ctx)
 }
 
 func (h *Hive) fatalOnTimeout(ctx context.Context) chan struct{} {
@@ -394,7 +392,7 @@ func (h *Hive) Shutdown(opts ...ShutdownOption) {
 }
 
 func (h *Hive) PrintObjects() {
-	if err := h.Populate(); err != nil {
+	if err := h.Populate(slog.Default()); err != nil {
 		panic(fmt.Sprintf("Failed to populate object graph: %s", err))
 	}
 
@@ -408,7 +406,7 @@ func (h *Hive) PrintObjects() {
 }
 
 func (h *Hive) PrintDotGraph() {
-	if err := h.Populate(); err != nil {
+	if err := h.Populate(slog.Default()); err != nil {
 		panic(fmt.Sprintf("Failed to populate object graph: %s", err))
 	}