Allow evaluation context to be managed per provider in the web SDK

[BuddhaBing]

Observed behavior

The lib allows for setting named providers and getting the client associated with that provider by name also, but removes (from js-sdk and server-sdk) the ability to set context on the client level (client.setContext(...)), rather than global level (OpenFeature.setContext(...)). This means that when I instantiate two different providers/clients and then set some context which is specific to the first one, followed by setting some different context that is specific to the second one, the second setContext overwrites the context set for the first provider/client.

Expected Behavior

I have two proposals:

1. The setContext function should be added to the Client object, similar to how this is handled in the other JS SDKs.
2. The OpenFeature.setContext function could take a name property, similar to the setProvider function, so that the context is only associated with the provider and client of the same name.

Steps to reproduce

See "Observed behavior" above

[beeme1mr]

Hey @BuddhaBing, the client exposed by the web-sdk doesn't support setContext in order to follow the static context paradigm. The idea is that a context change may lead to an async operation and we wanted to make that clear by only allowing people to update context in a single location. That does mean that context is shared across all the registered providers. Does that cause an issue within one of your providers?

I think we should avoid adding setContext on the client but we may be able to overload setContext on the API. If we end up going this route, users would have to manage context for each provider individually or add support for basic merging.

FYI, @toddbaert @lukas-reining

[BuddhaBing]

@beeme1mr thanks for the quick response! Yeah I had thought the same thing and I can understand the reasons for this change, and that's why I tend to lean towards my second proposal, which would allow for context to be namespaced, similar to how the provider and client can already be namespaced. This would avoid any crossover between providers and prevent triggering an async operation on another provider. But to answer your question: yes, this is causing an issue with one of my providers because they need different context and changing it for one changes it for both. This means that when I set context on one provider, it triggers an async operation on the other provider and replaces the cache with values that are undesirable.

[beeme1mr]

Got it, thanks. I'm going to make this a feature request instead of a bug because it's working as designed. However, I completely understand why this is an issue for you.

My gut feeling is that your namespace proposal may work well but I'd like to think through this end to end.

Here's some pseudocode of what it may look like:

import { OpenFeature } from "@openfeature/web-sdk";

const localStorageProvider = "localstorage";
const memoryProvider = "memory";

OpenFeature.setProvider(new MyDefaultProvider());
OpenFeature.setProvider(localStorageProvider, new MyLocalStorageProvider());
OpenFeature.setProvider(memoryProvider, new MyMemoryProvider());

// Global context that affects all providers
await OpenFeature.setContext({ browser: "chrome" });

// Merges with global context for the local storage provider
// Context - { browser: "chrome", email: "mike@test.com" }
await OpenFeature.setContext(localStorageProvider, { email: "mike@test.com" });

// Merges with global context for the memory provider
// Context - { browser: "edge" }
await OpenFeature.setContext(localStorageProvider, { browser: "edge" });

This may work but it introduces some challenges. For example, you wouldn't be able to define context that only affects the default provider. We would also need to define what happens (if anything) if the namespace is invalid.

We also had some feedback that we should allow users to set context during the provider registration process. This would help prevent a provider from being initialized with an empty context accidently. This should be kept in mind as we think about supporting this use case.

[beeme1mr]

Okay, I thought about this a bit more. I think we could avoid the limitations I mentioned above by maintaining a global context map within the OpenFeature web sdk and not merging context.

import { OpenFeature } from "@openfeature/web-sdk";

const localStorageProvider = "localstorage";
const memoryProvider = "memory";

OpenFeature.setProvider(new MyDefaultProvider(), { browser: "chrome" });
OpenFeature.setProvider(localStorageProvider, new MyLocalStorageProvider(), { email: "mike@test.com" });
OpenFeature.setProvider(memoryProvider, new MyMemoryProvider());

// The global context map managed by OpenFeature
// const globalContextMap = {
//   default: { browser: "chrome" },
//   namespacedProviders: {
//     [localStorageProvider]: { email: "mike@test.com" }
//   }
// }

// Merges with global context for the local storage provider
// Context - { browser: "chrome", email: "mike@test.com" }
await OpenFeature.setContext(localStorageProvider, { email: "todd@test.com" });

// Updated global context map managed by OpenFeature
// const globalContextMap = {
//   default: { browser: "chrome" },
//   namespacedProviders: {
//     [localStorageProvider]: { email: "todd@test.com" }
//   }
// }

That would allow you to either share context across all providers or manage namespace provider context individually. If you don't explicitly set context on a namespace provider, it will use the default context. This would only apply to the web SDK and would likely require a small update to the OpenFeature spec.

@BuddhaBing @toddbaert @lukas-reining What do you think?

[toddbaert]

I think we need some solution for context-scoping in the static-context paradigm (aka client-side SDKs). I like this solution because it seems like it will be easy for users to understand, and it's consistent with the way we scope providers (with names) already. It also seems like it will be easy to implement and backwards compatible. I would support a change like this in the spec, which would apply to static-context implementations.

[lukas-reining]

I also like the solution and that it is not breaking and get that this is something that people need.

[BuddhaBing]

@beeme1mr looks good to me!

[beeme1mr]

I started to implement this today. The basics are in place but I still need to write a bunch of tests. We also need to define the expected behavior in a few situations.

• What should happen if you try and set context using a name that doesn't map to a registered provider? We could throw, add it to the named context map anyway, or log an error.
• Should we include a deleteContext(name: string) method? This wasn't necessary before but now you may need a way to clear context for a specific provider so that it can use from the default context again.

[BuddhaBing]

@beeme1mr

What should happen if you try and set context using a name that doesn't map to a registered provider? We could throw, add it to the named context map anyway, or log an error.

I think adding it to the named context map anyway makes most sense to me.

Should we include a deleteContext(name: string) method? This wasn't necessary before but now you may need a way to clear context for a specific provider so that it can use from the default context again.

That's a good point and I think that makes sense 👍

[toddbaert]

@BuddhaBing take a look here.

[BuddhaBing]

@toddbaert LGTM ✅