Repository: open-feature/go-sdk
Pull request #361 (2025-05-19 02:34:31)

refactor: simplify and improve logging hook
<!-- Please use this template for your pull request. -->
<!-- Please use the sections that you need and delete other sections -->

## This PR
<!-- add the description of the PR here -->

- add go doc comments
- add a `stage` key to the logs as per [the spec](https://openfeature.dev/specification/appendix-a#logging-hook)
- ensure LoggingHook method receivers are all named `h` (the `buildArgs` method had a receiver named `l`)
- use consistent parameter names for the Before, After, Error, and Finally methods
- simplify the `buildArgs` method by removing the error return parameter
- use [slog.Attr](https://pkg.go.dev/log/slog#Attr) instead of `interface{}` types to log attributes
- the above allows the use of [Logger.LogAttrs](https://pkg.go.dev/log/slog#Logger.LogAttrs), which is [the most efficient form of log output](https://pkg.go.dev/log/slog#Logger.LogAttrs:~:text=For%20the%20most%20efficient,ctx%2C%20%22hello%22%2C%20%22count%22%2C%203)
- as a consequence of the above, the logging performed by the logging hook is now context aware (the `ctx` parameter is passed to `Logger.LogAttrs`)

### Related Issues
<!-- add here the GitHub issue that this PR resolves if applicable -->


### Notes
<!-- any additional notes for this PR -->

### Follow-up Tasks
<!-- anything that is related to this PR but not done here should be noted under this section -->
<!-- if there is a need for a new issue, please link it here -->

### How to test
<!-- if applicable, add testing instructions under this section -->



codecov[bot]: ## [Codecov](https://app.codecov.io/gh/open-feature/go-sdk/pull/361?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-feature) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 85.83%. Comparing base [(`9ff5e20`)](https://app.codecov.io/gh/open-feature/go-sdk/commit/9ff5e204c726f3a60c8c04c89f8841dc16a6ac53?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-feature) to head [(`4810adf`)](https://app.codecov.io/gh/open-feature/go-sdk/commit/4810adf6ab9246958be963c19300635fe27ceaa5?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-feature).

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             main     #361      +/-   ##
==========================================
- Coverage   88.13%   85.83%   -2.31%     
==========================================
  Files          14       20       +6     
  Lines        1408     1454      +46     
==========================================
+ Hits         1241     1248       +7     
- Misses        143      185      +42     
+ Partials       24       21       -3     
```

| [Flag](https://app.codecov.io/gh/open-feature/go-sdk/pull/361/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-feature) | Coverage Δ | |
|---|---|---|
| [e2e](https://app.codecov.io/gh/open-feature/go-sdk/pull/361/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-feature) | `85.83% <100.00%> (-2.31%)` | :arrow_down: |
| [unit](https://app.codecov.io/gh/open-feature/go-sdk/pull/361/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-feature) | `85.83% <100.00%> (-2.31%)` | :arrow_down: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-feature#carryforward-flags-in-the-pull-request-comment) to find out more.

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/open-feature/go-sdk/pull/361?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-feature).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-feature).

<details><summary> :rocket: New features to boost your workflow: </summary>

- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
</details>
sahidvelji: Good call on tests. I added the `stage` key as expected output in the tests.
beeme1mr: Hey @sahidvelji, could you please also add a short section to the readme that mentions the hook? Here's an example from [Java](https://github.com/open-feature/java-sdk#logging-hook). It would be nice to include a short code example too (something currently missing from the Java example).
sahidvelji: > Hey @sahidvelji, could you please also add a short section to the readme that mentions the hook? Here's an example from [Java](https://github.com/open-feature/java-sdk#logging-hook). It would be nice to include a short code example too (something currently missing from the Java example).

@beeme1mr Yes that makes sense, but I'd like to do that in a separate PR. I have created an issue. Feel free to assign me to it.

Patch:
diff --git a/openfeature/hooks/logging_hook.go b/openfeature/hooks/logging_hook.go
index 915b256..680e18a 100644
--- a/openfeature/hooks/logging_hook.go
+++ b/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) {
 }


Test patch:
diff --git a/openfeature/hooks/logging_hook_test.go b/openfeature/hooks/logging_hook_test.go
index ef3306e..ba32cfc 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"
 )
@@ -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",
 			},
 		}