[Bug]: OFREP Bulk Evaluation

[lukas-reining]

Bug Description

Hey, I tried to use the OFREP client provider [1] with flipt. The implementation of OFREP in flipt looks great, but there is one thing that does not fit to how we intended the bulk evaluation endpoint to be used.

When I try to use the OFREP client provider I get the following error: {"errorCode":"INVALID_CONTEXT","errorDetails":"flags were not provided in context"} and I can find the line in the code that returns this. [2]

This behavior does not fit to the intended use of the endpoint as we described in the following ADR: [3] As this describes, for the client we expect all the flags to be loaded for synchronous evaluation.

Do you have a reason for adding the flags context key as mandatory? If not can we make it optional or remove it?

[1] https://github.com/open-feature/js-sdk-contrib/tree/main/libs/providers/ofrep-web [2] https://github.com/open-feature/protocol/blob/main/service/adrs/0002-two-evaluation-endpoints.md#context [3] https://github.com/open-feature/protocol/blob/main/service/adrs/0002-two-evaluation-endpoints.md#2-two-evaluation-endpoints

Version Info

v1.48.1

Search

• [X] I searched for other open and closed issues before opening this

Steps to Reproduce

Try this:

curl --request POST \
  --url https://try.flipt.io/ofrep/v1/evaluate/flags \
  --header 'Content-Type: application/json' \
  --header 'Accept: application/json' \
  --header 'X-Flipt-Namespace: default' \
  --data '{
  "context": {
    "targetingKey": "targetingKey1"
  }
}'

or try to use the OFREP client provider with flipt: https://github.com/open-feature/js-sdk-contrib/tree/main/libs/providers/ofrep-web#configurations-and-usage

Expected Behavior

No response

Additional Context

No response

[GeorgeMac]

Thanks for reporting this @lukas-reining Bare with me while I report this phishing nonsense 😓

[GeorgeMac]

Just for posterity I reported and then deleted a couple of phising comments. They contained some funky looking links, which I can only assume contained malicious content.

Update: GitHub acknowledged they were malicious and I think the accounts have been removed.

[erka]

Thank you for your report @lukas-reining

I'd really like to discuss this https://github.com/open-feature/protocol/blob/main/service/adrs/0004-no-flag-filter-in-bulk-evaluation-request.md as well. How do you interpret the phrase For systems that rely on a list of flags? The definition seems a little unclear on how those systems should handle requests that don’t include flags.

[lukas-reining]

I think you found a problem in the ADR there @erka. In my opinion, this is just a proposal for all those who want to optionally have the option to filter for specific flags and I think we should fix that wording in the ADR. To be able to support the static context paradigm, in our case the OFREP Web SDK, a service needs to support loading all the flags I think. Or the provider implementation has to get an option for specifying that, but I would prefer the first option. What do you think @erka? cc @beemer @toddbaert @Kavindu-Dodan @thomaspoignant

[erka]

@lukas-reining Your question is quite challenging. I understand the concept, and it's a great solution for a small number of flags. However, in practice, a flag provider and an enterprise customer might deal with dozens or even hundreds of flags at once. Moreover, not all of those flags will be web or UI-related or client-related. It's similar to the REST vs GraphQL debate.

This is why I would prefer to see the specification with clearly defined expectations for bulk evaluation.

[lukas-reining]

Mh I get that this is quite challenging and know the cases of enterprises with hundreds of flags @erka. But this is what really many of the provider SDKs are doing and you are also sending the whole ruleset for a namespace to the client, so what differentiates this is that the evaluation is taking place in the client instead of the server. The amount of transferred data is more in that approach, while the server processing is less [1][2].

I am completely with you that our spec should be more precise on the reason for and expected behavior of the bulk evaluation endpoint. But please also keep in mind that OFREP is really experimental right now and we are really happy to get feedback from the industry here.

So what do you think could we make it possible to have the bulk evaluation without a filter, or do you see any point missing about the spec to make it more compatible with how you are doing the bulk/client evaluation? One thing that comes to my mind, is that you could expect the namespace as optional context property and return the flags of default if no namespace was given. But maybe you have a better idea.

[1] https://github.com/flipt-io/flipt-client-sdks/blob/4821cb227c6c8b10419b96674d44ad1d6668a647/flipt-client-browser/src/index.ts#L41 [2] https://github.com/flipt-io/flipt/blob/4fd23b255acc6710f596ce9ddd4eefb61a85e29c/internal/server/evaluation/data/server.go#L122

[erka]

@lukas-reining I’m afraid I’m not in a position to discuss the internal decisions of any flag providers.

Back to the OFREP bulk evaluation, it seems like you’re aiming for a design based on a static context paradigm. One challenge I encountered is that this approach isn’t well documented in the OpenAPI specification and the ofrep-web provider. As alien I personally had some difficulty with ofrep-web. Even after following the examples in the repository, I only received the default value, and my flag evaluation context didn’t seem to impact the flag evaluation. It was only after digging into the source and setting OpenFeature.setContext({...}); ...; await OpenFeature.setProviderAndWait(...); that I was able to achieve the expected result. Additionally, I was wondering about the best approach for a scenario where a service needs to evaluate 5 flags for each of 1,000 users during a batch or cron job execution using the OFREP API. What your guidance could be for this case?

As you said that OFREP is experimental the Flipt implementation is the same. We are open to making adjustments as we gain a better understanding of the design and goals.

[erka]

@GeorgeMac @markphelps @lukas-reining I have created the issue. Please add the details which I may have missed