Repository: open-feature/go-sdk
Pull request #283 (2024-08-30 17:55:02)

feat: add TransactionContext
## This PR

- Adds backwards compatible, basic implementation of 'TransactionContext' 

The functions that are important are
```go
WithTranscationContext(ctx, EvaluationContext)
TransactionContext(ctx)
```

### Related Issues

https://github.com/open-feature/go-sdk/issues/256

### Notes
For next steps, 
- I see the API suggested in https://github.com/open-feature/go-sdk/issues/256 is to remove the local evalCtx which would make the API much cleaner in my opinion, but I am not sure what is the process for breaking changes
- I am also not sure what the right behavior of merging should be from the API's perspective if they run `WithTransactionContext` multiple times. My guess is we should expect the user to handle this on their end, but would be interested in opinions on if WithTransactionContext should merge existing TransactionContexts

### How to test
I updated the associated tests for merging order / evaluation. Please let me know if any other tests will be helpful!


codecov[bot]: ## [Codecov](https://app.codecov.io/gh/open-feature/go-sdk/pull/283?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 82.04%. Comparing base [(`80c0235`)](https://app.codecov.io/gh/open-feature/go-sdk/commit/80c0235d26250a739781729855cb0264e19a4409?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-feature) to head [(`4d08b89`)](https://app.codecov.io/gh/open-feature/go-sdk/commit/4d08b89f6ea2cb2202ec5d7c57e28b6722c33dad?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     #283      +/-   ##
==========================================
+ Coverage   81.92%   82.04%   +0.12%     
==========================================
  Files          10       10              
  Lines         979      986       +7     
==========================================
+ Hits          802      809       +7     
  Misses        156      156              
  Partials       21       21              
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/open-feature/go-sdk/pull/283?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).

beeme1mr: Hey @kevinschoonover, could you please update the readme to include a section about this? Please follow the template found [here](https://github.com/open-feature/.github/tree/main/templates/READMEs#transaction-context-propagation). Thanks!
beeme1mr: > I am also not sure what the right behavior of merging should be from the API's perspective if they run WithTransactionContext multiple times. My guess is we should expect the user to handle this on their end, but would be interested in opinions on if WithTransactionContext should merge existing TransactionContexts

Yes, I agree. Currently, all implementations override existing transaction context, but adding another method that merges would be useful.
kevinschoonover: > Hey @kevinschoonover, could you please update the readme to include a section about this? Please follow the template found [here](https://github.com/open-feature/.github/tree/main/templates/READMEs#transaction-context-propagation). Thanks!

Done!

> > I am also not sure what the right behavior of merging should be from the API's perspective if they run WithTransactionContext multiple times. My guess is we should expect the user to handle this on their end, but would be interested in opinions on if WithTransactionContext should merge existing TransactionContexts
> 
> Yes, I agree. Currently, all implementations override existing transaction context, but adding another method that merges would be useful.

Added a `MergeTransactionContext` function which should solve this case. Let me know if there is anything else I can update!

Patch:
diff --git a/README.md b/README.md
index 8b1aabc..ab6d3ae 100644
--- a/README.md
+++ b/README.md
@@ -98,6 +98,7 @@ See [here](https://pkg.go.dev/github.com/open-feature/go-sdk/pkg/openfeature) fo
 | ✅      | [Domains](#domains)             | Logically bind clients with providers.|
 | ✅      | [Eventing](#eventing)           | React to state changes in the provider or flag management system.                                                                 |
 | ✅      | [Shutdown](#shutdown)           | Gracefully clean up a provider during application shutdown.                                                                       |
+| ✅      | [Transaction Context Propagation](#transaction-context-propagation) | Set a specific [evaluation context](https://openfeature.dev/docs/reference/concepts/evaluation-context) for a transaction (e.g. an HTTP request or a thread) |
 | ✅      | [Extending](#extending)         | Extend OpenFeature with custom providers and hooks.                                                                               |
 
 <sub>Implemented: ✅ | In-progress: ⚠️ | Not implemented yet: ❌</sub>
@@ -250,6 +251,29 @@ import "github.com/open-feature/go-sdk/openfeature"
 openfeature.Shutdown()
 ```
 
+
+### Transaction Context Propagation
+
+Transaction context is a container for transaction-specific evaluation context (e.g. user id, user agent, IP).
+Transaction context can be set where specific data is available (e.g. an auth service or request handler), and by using the transaction context propagator, it will automatically be applied to all flag evaluations within a transaction (e.g. a request or thread).
+
+```go
+import "github.com/open-feature/go-sdk/openfeature"
+
+// set the TransactionContext
+ctx := openfeature.WithTransactionContext(context.Background(), openfeature.EvaluationContext{})
+
+// get the TransactionContext from a context
+ec := openfeature.TransactionContext(ctx)
+
+// merge an EvaluationContext with the existing TransactionContext, preferring
+// the context that is passed to MergeTransactionContext
+tCtx := openfeature.MergeTransactionContext(ctx, openfeature.EvaluationContext{})
+
+// use TransactionContext in a flag evaluation
+client.BooleanValue(tCtx, ....)
+```
+
 ## Extending
 
 ### Develop a provider
diff --git a/openfeature/client.go b/openfeature/client.go
index 337d565..f66e192 100644
--- a/openfeature/client.go
+++ b/openfeature/client.go
@@ -677,7 +677,7 @@ func (c *Client) evaluate(
 	// ensure that the same provider & hooks are used across this transaction to avoid unexpected behaviour
 	provider, globalHooks, globalCtx := c.api.ForEvaluation(c.metadata.name)
 
-	evalCtx = mergeContexts(evalCtx, c.evaluationContext, globalCtx)                                                           // API (global) -> client -> invocation
+	evalCtx = mergeContexts(evalCtx, c.evaluationContext, TransactionContext(ctx), globalCtx)                                  // API (global) -> transaction -> client -> invocation
 	apiClientInvocationProviderHooks := append(append(append(globalHooks, c.hooks...), options.hooks...), provider.Hooks()...) // API, Client, Invocation, Provider
 	providerInvocationClientApiHooks := append(append(append(provider.Hooks(), options.hooks...), c.hooks...), globalHooks...) // Provider, Invocation, Client, API
 
diff --git a/openfeature/evaluation_context.go b/openfeature/evaluation_context.go
index 1913555..9155096 100644
--- a/openfeature/evaluation_context.go
+++ b/openfeature/evaluation_context.go
@@ -1,5 +1,11 @@
 package openfeature
 
+import (
+	"context"
+
+	"github.com/open-feature/go-sdk/openfeature/internal"
+)
+
 // EvaluationContext provides ambient information for the purposes of flag evaluation
 // The use of the constructor, NewEvaluationContext, is enforced to set EvaluationContext's fields in order
 // to enforce immutability.
@@ -53,3 +59,36 @@ func NewEvaluationContext(targetingKey string, attributes map[string]interface{}
 func NewTargetlessEvaluationContext(attributes map[string]interface{}) EvaluationContext {
 	return NewEvaluationContext("", attributes)
 }
+
+// NewTransactionContext constructs a TransactionContext
+//
+// ctx - the context to embed the EvaluationContext in
+// ec - the EvaluationContext to embed into the context
+func WithTransactionContext(ctx context.Context, ec EvaluationContext) context.Context {
+	return context.WithValue(ctx, internal.TransactionContext, ec)
+}
+
+// MergeTransactionContext merges the provided EvaluationContext with the current TransactionContext (if it exists)
+//
+// ctx - the context to pull existing TransactionContext from
+// ec - the EvaluationContext to merge with the existing TransactionContext
+func MergeTransactionContext(ctx context.Context, ec EvaluationContext) context.Context {
+	oldTc := TransactionContext(ctx)
+	mergedTc := mergeContexts(ec, oldTc)
+	return WithTransactionContext(ctx, mergedTc)
+}
+
+// TransactionContext extracts a EvaluationContext from the current
+// golang.org/x/net/context. if no EvaluationContext exist, it will construct
+// an empty EvaluationContext
+//
+// ctx - the context to pull EvaluationContext from
+func TransactionContext(ctx context.Context) EvaluationContext {
+	ec, ok := ctx.Value(internal.TransactionContext).(EvaluationContext)
+
+	if !ok {
+		return EvaluationContext{}
+	}
+
+	return ec
+}
diff --git a/openfeature/internal/context_key.go b/openfeature/internal/context_key.go
new file mode 100644
index 0000000..a46e932
--- /dev/null
+++ b/openfeature/internal/context_key.go
@@ -0,0 +1,10 @@
+package internal
+
+// ContextKey is just an empty struct. It exists so TransactionContext can be
+// an immutable public variable with a unique type. It's immutable
+// because nobody else can create a ContextKey, being unexported.
+type ContextKey struct{}
+
+// TransactionContext is the context key to use with golang.org/x/net/context's
+// WithValue function to associate an EvaluationContext value with a context.
+var TransactionContext ContextKey


Test patch:
diff --git a/openfeature/evaluation_context_test.go b/openfeature/evaluation_context_test.go
index 550c3bf..1e1c87b 100644
--- a/openfeature/evaluation_context_test.go
+++ b/openfeature/evaluation_context_test.go
@@ -6,6 +6,7 @@ import (
 	"testing"
 
 	"github.com/golang/mock/gomock"
+	"github.com/open-feature/go-sdk/openfeature/internal"
 )
 
 // The `evaluation context` structure MUST define an optional `targeting key` field of type string,
@@ -83,7 +84,7 @@ func TestRequirement_3_2_1(t *testing.T) {
 	})
 }
 
-// Evaluation context MUST be merged in the order: API (global) - client - invocation,
+// Evaluation context MUST be merged in the order: API (global) - transaction - client - invocation,
 // with duplicate values being overwritten.
 func TestRequirement_3_2_2(t *testing.T) {
 	defer t.Cleanup(initSingleton)
@@ -93,12 +94,22 @@ func TestRequirement_3_2_2(t *testing.T) {
 		targetingKey: "API",
 		attributes: map[string]interface{}{
 			"invocationEvalCtx": true,
-			"foo":               2,
-			"user":              2,
+			"foo":               3,
+			"user":              3,
 		},
 	}
 	SetEvaluationContext(apiEvalCtx)
 
+	transactionEvalCtx := EvaluationContext{
+		targetingKey: "Transcation",
+		attributes: map[string]interface{}{
+			"transactionEvalCtx": true,
+			"foo":                2,
+			"user":               2,
+		},
+	}
+	transactionCtx := WithTransactionContext(context.Background(), transactionEvalCtx)
+
 	mockProvider := NewMockFeatureProvider(ctrl)
 	mockProvider.EXPECT().Metadata().AnyTimes()
 
@@ -130,21 +141,21 @@ func TestRequirement_3_2_2(t *testing.T) {
 	expectedMergedEvalCtx := EvaluationContext{
 		targetingKey: "Client",
 		attributes: map[string]interface{}{
-			"apiEvalCtx":        true,
-			"invocationEvalCtx": true,
-			"clientEvalCtx":     true,
-			"foo":               "bar",
-			"user":              1,
+			"apiEvalCtx":         true,
+			"transactionEvalCtx": true,
+			"invocationEvalCtx":  true,
+			"clientEvalCtx":      true,
+			"foo":                "bar",
+			"user":               1,
 		},
 	}
 	flatCtx := flattenContext(expectedMergedEvalCtx)
 	mockProvider.EXPECT().StringEvaluation(gomock.Any(), gomock.Any(), gomock.Any(), flatCtx)
 
-	_, err = client.StringValue(context.Background(), "foo", "bar", invocationEvalCtx)
+	_, err = client.StringValue(transactionCtx, "foo", "bar", invocationEvalCtx)
 	if err != nil {
 		t.Error(err)
 	}
-
 }
 
 func TestEvaluationContext_AttributesNotPassedByReference(t *testing.T) {
@@ -160,6 +171,18 @@ func TestEvaluationContext_AttributesNotPassedByReference(t *testing.T) {
 	}
 }
 
+func TestRequirement_3_3_1(t *testing.T) {
+	t.Run("The API MUST have a method for setting the evaluation context of the transaction context propagator for the current transaction.", func(t *testing.T) {
+		ctx := context.Background()
+		ctx = WithTransactionContext(ctx, EvaluationContext{})
+		val, ok := ctx.Value(internal.TransactionContext).(EvaluationContext)
+
+		if !ok {
+			t.Fatalf("failed to find transcation context set from WithTransactionContext: %v", val)
+		}
+	})
+}
+
 func TestEvaluationContext_AttributesFuncNotPassedByReference(t *testing.T) {
 	evalCtx := NewEvaluationContext("foo", map[string]interface{}{
 		"foo": "bar",
@@ -186,3 +209,44 @@ func TestNewTargetlessEvaluationContext(t *testing.T) {
 		t.Errorf("we expect no difference in the attributes")
 	}
 }
+
+func TestMergeTransactionContext(t *testing.T) {
+	oldEvalCtx := NewEvaluationContext("old", map[string]interface{}{
+		"old":       true,
+		"overwrite": "old",
+	})
+	newEvalCtx := NewEvaluationContext("new", map[string]interface{}{
+		"new":       true,
+		"overwrite": "new",
+	})
+
+	ctx := WithTransactionContext(context.Background(), oldEvalCtx)
+	ctx = MergeTransactionContext(ctx, newEvalCtx)
+
+	expectedMergedEvalCtx := EvaluationContext{
+		targetingKey: "new",
+		attributes: map[string]interface{}{
+			"old":       true,
+			"new":       true,
+			"overwrite": "new",
+		},
+	}
+
+	finalTransactionContext := TransactionContext(ctx)
+
+	if finalTransactionContext.targetingKey != expectedMergedEvalCtx.targetingKey {
+		t.Errorf(
+			"targetingKey is not expected value, finalTransactionContext.targetingKey: %s, newEvalCtx.targetingKey: %s",
+			finalTransactionContext.TargetingKey(),
+			expectedMergedEvalCtx.TargetingKey(),
+		)
+	}
+
+	if !reflect.DeepEqual(finalTransactionContext.Attributes(), expectedMergedEvalCtx.Attributes()) {
+		t.Errorf(
+			"attributes are not expected value, finalTransactionContext.Attributes(): %v, expectedMergedEvalCtx.Attributes(): %v",
+			finalTransactionContext.Attributes(),
+			expectedMergedEvalCtx.Attributes(),
+		)
+	}
+}