From 86305257eea682b7ca05383718a1a00beb578eaa Mon Sep 17 00:00:00 2001 From: Sahid Velji Date: Sun, 18 May 2025 22:03:14 -0400 Subject: [PATCH 1/5] refactor: improve and simplify LoggingHook Signed-off-by: Sahid Velji --- openfeature/hooks/logging_hook.go | 61 +++++++++++++------------------ 1 file changed, 26 insertions(+), 35 deletions(-) diff --git a/openfeature/hooks/logging_hook.go b/openfeature/hooks/logging_hook.go index 16109d04..96797401 100644 --- a/openfeature/hooks/logging_hook.go +++ b/openfeature/hooks/logging_hook.go @@ -40,57 +40,48 @@ type MarshaledEvaluationContext struct { Attributes map[string]interface{} } -func (l LoggingHook) buildArgs(hookContext of.HookContext) ([]interface{}, error) { - - args := []interface{}{ - 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) + 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), + ) + 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)) + 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) { } From fde834bd543adc9b3a4d17cc6204ad58e246be04 Mon Sep 17 00:00:00 2001 From: Sahid Velji Date: Sun, 18 May 2025 22:25:14 -0400 Subject: [PATCH 2/5] docs: add go doc comments to logging hook Signed-off-by: Sahid Velji --- openfeature/hooks/logging_hook.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/openfeature/hooks/logging_hook.go b/openfeature/hooks/logging_hook.go index 96797401..fa355294 100644 --- a/openfeature/hooks/logging_hook.go +++ b/openfeature/hooks/logging_hook.go @@ -19,15 +19,19 @@ const ( VALUE_KEY = "value" ) +// 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, From 1791cd816e80a72fef6012a71702242515b7bd97 Mon Sep 17 00:00:00 2001 From: Sahid Velji Date: Sun, 18 May 2025 22:25:42 -0400 Subject: [PATCH 3/5] fix: add missing `stage` key to logging hook Signed-off-by: Sahid Velji --- openfeature/hooks/logging_hook.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/openfeature/hooks/logging_hook.go b/openfeature/hooks/logging_hook.go index fa355294..fb997219 100644 --- a/openfeature/hooks/logging_hook.go +++ b/openfeature/hooks/logging_hook.go @@ -17,6 +17,7 @@ const ( REASON_KEY = "reason" VARIANT_KEY = "variant" VALUE_KEY = "value" + STAGE_KEY = "stage" ) // LoggingHook is a [of.Hook] that logs the flag evaluation lifecycle. @@ -64,6 +65,7 @@ func (h *LoggingHook) buildArgs(hookContext of.HookContext) []slog.Attr { 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 } @@ -76,6 +78,7 @@ func (h *LoggingHook) After(ctx context.Context, hookContext of.HookContext, 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 @@ -83,7 +86,10 @@ func (h *LoggingHook) After(ctx context.Context, hookContext of.HookContext, 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)) + args = append(args, + slog.Any(ERROR_MESSAGE_KEY, err), + slog.String(STAGE_KEY, "error"), + ) h.logger.LogAttrs(ctx, slog.LevelError, "Error stage", args...) } From bf82f39e2eddd4542e1769a414e2bd1c4aeddfaf Mon Sep 17 00:00:00 2001 From: Sahid Velji Date: Mon, 19 May 2025 11:53:16 -0400 Subject: [PATCH 4/5] test: add `stage` key to expected output Signed-off-by: Sahid Velji --- openfeature/hooks/logging_hook_test.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/openfeature/hooks/logging_hook_test.go b/openfeature/hooks/logging_hook_test.go index 832d770d..9431f4d0 100644 --- a/openfeature/hooks/logging_hook_test.go +++ b/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" ) @@ -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", }, } From ffd1f74c728c01ffe6acc156b73d409b4fee6e30 Mon Sep 17 00:00:00 2001 From: Sahid Velji Date: Mon, 19 May 2025 12:15:25 -0400 Subject: [PATCH 5/5] refactor: use short variable declaration Signed-off-by: Sahid Velji --- openfeature/hooks/logging_hook_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openfeature/hooks/logging_hook_test.go b/openfeature/hooks/logging_hook_test.go index 9431f4d0..810e91da 100644 --- a/openfeature/hooks/logging_hook_test.go +++ b/openfeature/hooks/logging_hook_test.go @@ -54,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)