open-feature / open-feature/go-sdk
Deprecate MergeTransactionContext function
- Dominant language
- Go
- Stars
- 249
- Forks
- 61
- Avg merge
- 2d 10h
- Merged PRs (30d)
- 20
Description
Related to #454
[MergeTransactionContext](https://github.com/open-feature/go-sdk/blob/v1.17.0/openfeature/evaluation_context.go#L75) does a couple of things:
- Call `TransactionContext` to extract an EvaluationContext from the given context.Context
- Merge the given EvaluationContext with the one extracted from the step above
- Return a new context.Context that contains the merged EvaluationContext from the step above.
I propose that we deprecate this function. I would argue that it's doing too much and naming it more appropriately is not possible without creating a really long name.
Before we attempt to fill the gap, I'd like to establish the usefulness of this function, given that it's not defined in the spec (https://openfeature.dev/specification/sections/evaluation-context). Here I am specifically referring to the ability of merging evaluation contexts, whether directly via a function that takes multiple evaluation contexts, or indirectly by merging the "transaction context" in the "transaction context propagator" with a different evaluation context. We need to determine whether similar behaviour exists in the other languages. If so, we should consider updating the spec if we want this functionality implemented consistently across all languages.
Either way, there are two ways to implement this functionality, if we decide that we need this:
1. Export (and rename) the existing `mergeContexts` function to allow users to merge evaluation contexts.
2. Merge evaluation contexts indirectly as described in the paragraph above (essentially what MergeTransactionContext does), but rename the function. We'll need to come up with a good name in this case.
https://github.com/open-feature/go-sdk/blob/4b3d6d953076f3056d78c512b31c9fc2ec29ab89/openfeature/client.go#L836
I personally prefer the first option, which is a more more modular approach. I think that the existing MergeTransactionContext function does too many things.
Contributor guide
Assessment
This issue has not been assessed yet.