[FEATURE] Evaluation Context behavior is suprising

[jhollanderpax8]

The typescript examples in the OpenFeature docs show this snippet:

// add a value to the context
await OpenFeature.setContext({ myUserData: 'myUserValue' });

// the context is used for all feature flag evaluations automatically.
const boolValue = await client.getBooleanValue('boolFlag', false);

Leading one to believe that a similar approach would work with the Java SDK, however this is not true and it's very surprising behavior if you are used to things like MDC context in Log4j.

One would expect this to work:

OpenFeatureAPI api = OpenFeatureAPI.getInstance();
Map<String, Value> transactionAttrs = new HashMap<>();
transactionAttrs.put("userId", new Value("userId"));
EvaluationContext transactionCtx = new ImmutableContext(transactionAttrs);
api.setTransactionContext(apiCtx);

api.client.getBooleanValue("my-flag", false)

but instead you must do

OpenFeatureAPI api = OpenFeatureAPI.getInstance();
Map<String, Value> transactionAttrs = new HashMap<>();
transactionAttrs.put("userId", new Value("userId"));
EvaluationContext transactionCtx = new ImmutableContext(transactionAttrs);
api.setTransactionContext(apiCtx);

api.client.getBooleanValue("my-flag", false, api.getEvaluationContext)

Which IMO is surprising and inconsistent behavior and makes context propagation less than useful.

[Kavindu-Dodan]

@jhollanderpax8 I think this is related to https://github.com/open-feature/java-sdk/issues/940. Like I said in my response ^1 OpenFeature defines multiple context levels (ex:- API, transaction, client, evaluation). And each of them are merged prior to a flag evaluation.

If you intend to set an API level context, that is valid to all underlying entities derived from OpenFeature API (ex:- clients, hooks ), then you can use example below as a reference,

  OpenFeatureAPI api = OpenFeatureAPI.getInstance();
  api.setEvaluationContext(new ImmutableContext(Map.of("foo", new Value("Bar"))));

  final Client client = api.getClient();
  client.getBooleanDetails("booleanKey", false);

Here the API context foo:bar is available for the booleanKey flag evaluation and you don't have to use api.getEvaluationContext. I hope this helps to clear your doubt.

[jhollanderpax8]

@Kavindu-Dodan thanks for the reply. In my testing this didn't seem to be true. I was uncertain if I should file it as a bug because it wasn't 100% clear from the docs that it should be. I will make sure I can reproduce it in a small test and then open a bug report if I can.

[Kavindu-Dodan]

@jhollanderpax8 sure feel free to. Please also check the the SDK version. 1.8.0 is the latest release.

[toddbaert]

@jhollanderpax8 I think part of the issue here is we are missing some Javadoc. We also are missing a very specific test for setting global context (it's covered, but only as part of other tests).

I've opened a PR that adds missing javadocs, and also adds a test specifically asserting that global context is used in evaluations.

cc @Kavindu-Dodan

[jhollanderpax8]

I was able to confirm that it works as expected and the error was in some Kotlin extension function hackery I was doing. I still consider ImmutableContext to be awkward in light of ThreadLocalTransactionPropagator, but I am likely going to likely using Kotlin extension functions to make the API more ergonomic as the basic abstractions are solid if awkward to work with. Thanks the clarification and documentation updates/test cases.