hive: Fix printing of hook function names

We need to have special handling for printing the start and stop hook
names if the hook is not the "Hook" struct.

To keep things simple just print the type name + method instead of
going via FuncForPC. This makes sure we show reasonable output for
polymorphic structs as well (which would not have file+line or the
type params if FuncForPC was used).

Log output before:
level=info msg="Start hook executed" duration="1.853µs" function="node.newLocalNodeStore.func1 (local_node_store.go:80)" subsys=hive
level=info msg="Start hook executed" duration="5.681µs" function="hive.Hook.Start-fm (<autogenerated>:1)" subsys=hive

Log output after:
level=info msg="Start hook executed" duration=992ns function="node.newLocalNodeStore.func1 (local_node_store.go:80)" subsys=hive
level=info msg="Start hook executed" duration="4.939µs" function="*resource.resource[*k8s.io/api/core/v1.Node].Start" subsys=hive

Signed-off-by: Jussi Maki <jussi@isovalent.com>

pkg/hive/cell/metrics.go

@@ -0,0 +1,28 @@
+// SPDX-License-Identifier: Apache-2.0
+// Copyright Authors of Cilium
+
+package cell
+
+// PLACEHOLDERS. These would be in pkg/metrics.
+type Metric struct {
+	Meta MetricMeta
+	// some prometheus thing here
+}
+type MetricMeta struct {
+	Enabled     bool
+	Description string
+}
+
+type metricsOut struct {
+	Out
+	Metrics []*Metric `group:"metrics,flatten"`
+}
+
+func Metrics[Cfg Flagger](ctor func(Cfg) []*Metric) Cell {
+	// Wrap the constructor to provide the metrics in a
+	// value group.
+	return Provide(func(cfg Cfg) (out metricsOut) {
+		out.Metrics = ctor(cfg)
+		return
+	})
+}

pkg/hive/internal/reflect.go

@@ -9,6 +9,7 @@ import (
 	"reflect"
 	"regexp"
 	"runtime"
+	"strings"
 )
 
 var (
@@ -27,5 +28,9 @@ func FuncNameAndLocation(fn any) string {
 	f := runtime.FuncForPC(reflect.ValueOf(fn).Pointer())
 	file, line := f.FileLine(f.Entry())
 	name := TrimName(f.Name())
-	return fmt.Sprintf("%s (%s:%d)", name, path.Base(file), line)
+	name = strings.TrimSuffix(name, "-fm")
+	if file != "<autogenerated>" {
+		return fmt.Sprintf("%s (%s:%d)", name, path.Base(file), line)
+	}
+	return name
 }

pkg/hive/lifecycle.go

@@ -82,19 +82,15 @@ func (lc *DefaultLifecycle) Start(ctx context.Context) error {
 	defer cancel()
 
 	for _, hook := range lc.hooks {
-		var fn string
-		if hook, ok := hook.(Hook); ok {
-			if hook.OnStart == nil {
-				// Count as started as there might be a stop hook.
-				lc.numStarted++
-				continue
-			}
-			fn = internal.FuncNameAndLocation(hook.OnStart)
-		} else {
-			fn = internal.FuncNameAndLocation(hook.Start)
+		fnName, exists := getHookFuncName(hook, true)
+
+		if !exists {
+			// Count as started as there might be a stop hook.
+			lc.numStarted++
+			continue
 		}
 
-		l := log.WithField("function", fn)
+		l := log.WithField("function", fnName)
 		l.Debug("Executing start hook")
 		t0 := time.Now()
 		if err := hook.Start(ctx); err != nil {
@@ -122,16 +118,11 @@ func (lc *DefaultLifecycle) Stop(ctx context.Context) error {
 		}
 		hook := lc.hooks[lc.numStarted-1]
 
-		var fn string
-		if hook, ok := hook.(Hook); ok {
-			if hook.OnStop == nil {
-				continue
-			}
-			fn = internal.FuncNameAndLocation(hook.OnStop)
-		} else {
-			fn = internal.FuncNameAndLocation(hook.Stop)
+		fnName, exists := getHookFuncName(hook, false)
+		if !exists {
+			continue
 		}
-		l := log.WithField("function", fn)
+		l := log.WithField("function", fnName)
 		l.Debug("Executing stop hook")
 		t0 := time.Now()
 		if err := hook.Stop(ctx); err != nil {
@@ -148,33 +139,56 @@ func (lc *DefaultLifecycle) Stop(ctx context.Context) error {
 func (lc *DefaultLifecycle) PrintHooks() {
 	fmt.Printf("Start hooks:\n\n")
 	for _, hook := range lc.hooks {
-		var fn string
-		if hook, ok := hook.(Hook); ok {
-			if hook.OnStart == nil {
-				continue
-			}
-			fn = internal.FuncNameAndLocation(hook.OnStart)
-		} else {
-			fn = internal.FuncNameAndLocation(hook.Start)
+		fnName, exists := getHookFuncName(hook, true)
+		if !exists {
+			fmt.Printf("%v not defined\n", hook)
+			continue
 		}
-		fmt.Printf("  • %s\n", fn)
+		fmt.Printf("  • %s\n", fnName)
 	}
 
 	fmt.Printf("\nStop hooks:\n\n")
 	for i := len(lc.hooks) - 1; i >= 0; i-- {
 		hook := lc.hooks[i]
-		var fn string
-		if hook, ok := hook.(Hook); ok {
+		fnName, exists := getHookFuncName(hook, false)
+		if !exists {
+			continue
+		}
+		fmt.Printf("  • %s\n", fnName)
+	}
+}
+
+func getHookFuncName(hook HookInterface, start bool) (name string, hasHook bool) {
+	// Ok, we need to get a bit fancy here as runtime.FuncForPC does
+	// not return what we want: we get "hive.Hook.Stop()" when we want
+	// "*foo.Stop(). We do know the concrete type, and we do know
+	// the method name, so we check here whether we're dealing with
+	// "Hook" the struct, or an object implementing HookInterface.
+	//
+	// We could use reflection + FuncForPC to get around this, but it
+	// still wouldn't work for generic types (file would be "<autogenerated>")
+	// and the type params would be missing, so instead we'll just use the
+	// type name + method name.
+	switch hook := hook.(type) {
+	case Hook:
+		if start {
+			if hook.OnStart == nil {
+				return "", false
+			}
+			return internal.FuncNameAndLocation(hook.OnStart), true
+		} else {
 			if hook.OnStop == nil {
-				continue
+				return "", false
 			}
-			fn = internal.FuncNameAndLocation(hook.OnStop)
+			return internal.FuncNameAndLocation(hook.OnStop), true
+		}
+	default:
+		if start {
+			return internal.PrettyType(hook) + ".Start", true
 		} else {
-			fn = internal.FuncNameAndLocation(hook.Stop)
+			return internal.PrettyType(hook) + ".Stop", true
 		}
-		fmt.Printf("  • %s\n", fn)
 	}
-
 }
 
 var _ Lifecycle = &DefaultLifecycle{}