refactor: simplify and improve logging hook (#361)

Signed-off-by: Sahid Velji <[email protected]>

Author: sahidvelji

openfeature/hooks/logging_hook.go

@@ -17,17 +17,22 @@ const (
 	REASON_KEY             = "reason"
 	VARIANT_KEY            = "variant"
 	VALUE_KEY              = "value"
+	STAGE_KEY              = "stage"
 )
 
+// LoggingHook is a [of.Hook] that logs the flag evaluation lifecycle.
 type LoggingHook struct {
 	includeEvaluationContext bool
 	logger                   *slog.Logger
 }
 
+// NewLoggingHook returns a new [LoggingHook] with the default logger.
+// To provide a custom logger, use [NewCustomLoggingHook].
 func NewLoggingHook(includeEvaluationContext bool) (*LoggingHook, error) {
 	return NewCustomLoggingHook(includeEvaluationContext, slog.Default())
 }
 
+// NewCustomLoggingHook returns a new [LoggingHook] with the provided logger.
 func NewCustomLoggingHook(includeEvaluationContext bool, logger *slog.Logger) (*LoggingHook, error) {
 	return &LoggingHook{
 		logger:                   logger,
@@ -40,57 +45,53 @@ type MarshaledEvaluationContext struct {
 	Attributes   map[string]any
 }
 
-func (l LoggingHook) buildArgs(hookContext of.HookContext) ([]any, error) {
-
-	args := []any{
-		DOMAIN_KEY, hookContext.ClientMetadata().Domain(),
-		PROVIDER_NAME_KEY, hookContext.ProviderMetadata().Name,
-		FLAG_KEY_KEY, hookContext.FlagKey(),
-		DEFAULT_VALUE_KEY, hookContext.DefaultValue(),
+func (h *LoggingHook) buildArgs(hookContext of.HookContext) []slog.Attr {
+	args := []slog.Attr{
+		slog.String(DOMAIN_KEY, hookContext.ClientMetadata().Domain()),
+		slog.String(PROVIDER_NAME_KEY, hookContext.ProviderMetadata().Name),
+		slog.String(FLAG_KEY_KEY, hookContext.FlagKey()),
+		slog.Any(DEFAULT_VALUE_KEY, hookContext.DefaultValue()),
 	}
-	if l.includeEvaluationContext {
+	if h.includeEvaluationContext {
 		marshaledEvaluationContext := MarshaledEvaluationContext{
 			TargetingKey: hookContext.EvaluationContext().TargetingKey(),
 			Attributes:   hookContext.EvaluationContext().Attributes(),
 		}
-		args = append(args, EVALUATION_CONTEXT_KEY, marshaledEvaluationContext)
+		args = append(args, slog.Any(EVALUATION_CONTEXT_KEY, marshaledEvaluationContext))
 	}
 
-	return args, nil
+	return args
 }
 
-func (h *LoggingHook) Before(ctx context.Context, hookContext of.HookContext,
-	hint of.HookHints) (*of.EvaluationContext, error) {
-	var args, err = h.buildArgs(hookContext)
-	if err != nil {
-		return nil, err
-	}
-	h.logger.Debug("Before stage", args...)
+func (h *LoggingHook) Before(ctx context.Context, hookContext of.HookContext, hookHints of.HookHints) (*of.EvaluationContext, error) {
+	args := h.buildArgs(hookContext)
+	args = append(args, slog.String(STAGE_KEY, "before"))
+	h.logger.LogAttrs(ctx, slog.LevelDebug, "Before stage", args...)
 	return nil, nil
 }
 
 func (h *LoggingHook) After(ctx context.Context, hookContext of.HookContext,
-	flagEvaluationDetails of.InterfaceEvaluationDetails, hookHints of.HookHints) error {
-	var args, err = h.buildArgs(hookContext)
-	if err != nil {
-		return err
-	}
-	args = append(args, REASON_KEY, flagEvaluationDetails.Reason)
-	args = append(args, VARIANT_KEY, flagEvaluationDetails.Variant)
-	args = append(args, VALUE_KEY, flagEvaluationDetails.Value)
-	h.logger.Debug("After stage", args...)
+	flagEvaluationDetails of.InterfaceEvaluationDetails, hookHints of.HookHints,
+) error {
+	args := h.buildArgs(hookContext)
+	args = append(args,
+		slog.String(REASON_KEY, string(flagEvaluationDetails.Reason)),
+		slog.String(VARIANT_KEY, flagEvaluationDetails.Variant),
+		slog.Any(VALUE_KEY, flagEvaluationDetails.Value),
+		slog.String(STAGE_KEY, "after"),
+	)
+	h.logger.LogAttrs(ctx, slog.LevelDebug, "After stage", args...)
 	return nil
 }
 
-func (h *LoggingHook) Error(ctx context.Context, hookContext of.HookContext, err error, hint of.HookHints) {
-	args, buildArgsErr := h.buildArgs(hookContext)
-	if buildArgsErr != nil {
-		slog.Error("Error building args", "error", buildArgsErr)
-	}
-	args = append(args, ERROR_MESSAGE_KEY, err)
-	h.logger.Error("Error stage", args...)
+func (h *LoggingHook) Error(ctx context.Context, hookContext of.HookContext, err error, hookHints of.HookHints) {
+	args := h.buildArgs(hookContext)
+	args = append(args,
+		slog.Any(ERROR_MESSAGE_KEY, err),
+		slog.String(STAGE_KEY, "error"),
+	)
+	h.logger.LogAttrs(ctx, slog.LevelError, "Error stage", args...)
 }
 
-func (h *LoggingHook) Finally(ctx context.Context, hCtx of.HookContext, flagEvaluationDetails of.InterfaceEvaluationDetails, hint of.HookHints) {
-
+func (h *LoggingHook) Finally(ctx context.Context, hookContext of.HookContext, flagEvaluationDetails of.InterfaceEvaluationDetails, hookHints of.HookHints) {
 }

openfeature/hooks/logging_hook_test.go

@@ -4,11 +4,10 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
+	"log/slog"
 	"os"
 	"testing"
 
-	"log/slog"
-
 	"github.com/open-feature/go-sdk/openfeature"
 	"github.com/open-feature/go-sdk/openfeature/memprovider"
 )
@@ -55,7 +54,7 @@ func TestLoggingHookHandlesNilLoggerGracefully(t *testing.T) {
 }
 
 func TestLoggingHookLogsMessagesAsExpected(t *testing.T) {
-	var buf *bytes.Buffer = new(bytes.Buffer)
+	buf := new(bytes.Buffer)
 	handler := slog.NewJSONHandler(buf, &slog.HandlerOptions{Level: slog.LevelDebug})
 	logger := slog.New(handler)
 
@@ -128,15 +127,17 @@ func testLoggingHookLogsMessagesAsExpected(hook LoggingHook, logger *slog.Logger
 
 		ms := prepareOutput(buf, t)
 
-		var expected = map[string]map[string]any{
+		expected := map[string]map[string]any{
 			"Before stage": {
 				"provider_name": "InMemoryProvider",
 				"domain":        "test-app",
+				"stage":         "before",
 			},
 			"After stage": {
 				"provider_name": "InMemoryProvider",
 				"domain":        "test-app",
 				"flag_key":      "boolFlag",
+				"stage":         "after",
 			},
 		}
 
@@ -164,15 +165,17 @@ func testLoggingHookLogsMessagesAsExpected(hook LoggingHook, logger *slog.Logger
 
 		ms := prepareOutput(buf, t)
 
-		var expected = map[string]map[string]any{
+		expected := map[string]map[string]any{
 			"Before stage": {
 				"provider_name": "InMemoryProvider",
 				"domain":        "test-app",
+				"stage":         "before",
 			},
 			"Error stage": {
 				"provider_name": "InMemoryProvider",
 				"domain":        "test-app",
 				"flag_key":      "non-existing",
+				"stage":         "error",
 			},
 		}