feat: Specify caching for OFREP in server providers

[thomaspoignant]

This PR

Why this feature?

With the current implementation of OFREP on the server side the provider has to do a remote call to an API for each evaluation which can add latency to the application.
This caching mechanism helps to reduce the overhead of calling the API each time.

The cacheable field is here for the flag management system to notify the provider that it is ok to cache this flag evaluation. We don't want to cache all evaluations because some of them contain time-based changes and in those cases, we accept calling multiple times the API because the evaluation result can change and the cache will not be reliable.
By setting the cacheable field we let the decision to the flag management system.

Sequence diagram

This sequence diagram explains how it can work from a provider's perspective.

sequenceDiagram
    participant c as Server Application
    participant p as OFREP Provider
    participant f as Flag Management<br/>System
    c ->> p: client.getBooleanValue('v2_enabled', false);
    p -->> p: retrieve combination of flag + context<br/>from the cache
    alt not available in the cache
        p ->> f: call /ofrep/v1/evaluate/flags/v2_enabled<br/>with evaluation context
        f -->> p: 200 + evaluation result
        p -->> p: If cacheable = true, store in a cache<br/>Key: combination of flag + context<br/>Value: evaluation result<br/>(Data will be evicted from the cache after the TTL<br/>OR<br/>If we get notify from a configuration change)
        p -->> c: evaluation result
    else available in the cache
        p -->> c: evaluation result (with CACHED reason)
    end

Loading

[thomaspoignant]

Default TTL can be 0 and we will relay only on the polling to evict data from the cache.

[Kavindu-Dodan]

Agree, by default the behaviour should be no-cache so every evaluation is done remotely.

[lukas-reining]

I really like the idea and think we should implement server cache control.

I am wondering if we could not just use the Cache-Control header with the max-age directive here and leave the TTL out of the response body.
This feels more idiomatic to me and could be nice in cases where other HTTP tooling like proxies is in the loop.

[beeme1mr]

How will the cache be busted when the configuration changes without SSE, WebSockets, or polling?

[thomaspoignant]

How will the cache be busted when the configuration changes without SSE, WebSockets, or polling?

@beeme1mr I am not sure I understand what you mean here.
Are you talking about the cases where the flag configuration changes based on time or automated actions?

If yes for me those are the usecases when we should set cacheable to false, and the provider will continue to always do remote calls for this flag because we will never have any cache value for this specific flag.

[thomaspoignant]

I really like the idea and think we should implement server cache control.

I am wondering if we could not just use the Cache-Control header with the max-age directive here and leave the TTL out of the response body. This feels more idiomatic to me and could be nice in cases where other HTTP tooling like proxies is in the loop.

@lukas-reining I like the idea but I have one doubt.
You are talking about proxies here that may serve the cache because of the Cache-Control headers but it can result in serving a wrong value because caching typically does not take into account the request body.

We will do POST requests to the same URL with different evaluation contexts in the body and have a risk that the proxy in the middle is serving an API response not linked to the context.

sequenceDiagram
    participant p as provider
    participant px as proxy
    participant f as flag management system

    Note over p,f: Evaluation for user 1
    p ->> px: POST /ofrep/v1/evaluate/flags/my_flag<br/>(targetingKey: user1)
    px ->> f: POST /ofrep/v1/evaluate/flags/my_flag<br/>(targetingKey: user1)
    f -->>px: response (user1)
    px -->> p: response (user1)

    Note over p,f: Evaluation for user 2
    p ->> px: POST /ofrep/v1/evaluate/flags/my_flag<br/>(targetingKey: user2)
    px-->>px: retrieve the response from the cache
    px -->> p: response (user1)
    Note over p,px: ⚠️ Most of the proxies are not using<br/>the body in their caching policies.

Loading

[lukas-reining]

I really like the idea and think we should implement server cache control.
I am wondering if we could not just use the Cache-Control header with the max-age directive here and leave the TTL out of the response body. This feels more idiomatic to me and could be nice in cases where other HTTP tooling like proxies is in the loop.

@lukas-reining I like the idea but I have one doubt. You are talking about proxies here that may serve the cache because of the Cache-Control headers but it can result in serving a wrong value because caching typically does not take into account the request body.

We will do POST requests to the same URL with different evaluation contexts in the body and have a risk that the proxy in the middle is serving an API response not linked to the context.

sequenceDiagram
    participant p as provider
    participant px as proxy
    participant f as flag management system

    Note over p,f: Evaluation for user 1
    p ->> px: POST /ofrep/v1/evaluate/flags/my_flag<br/>(targetingKey: user1)
    px ->> f: POST /ofrep/v1/evaluate/flags/my_flag<br/>(targetingKey: user1)
    f -->>px: response (user1)
    px -->> p: response (user1)

    Note over p,f: Evaluation for user 2
    p ->> px: POST /ofrep/v1/evaluate/flags/my_flag<br/>(targetingKey: user2)
    px-->>px: retrieve the response from the cache
    px -->> p: response (user1)
    Note over p,px: ⚠️ Most of the proxies are not using<br/>the body in their caching policies.

Loading

Mh good point, I was missing the targeting key.
Yes you are right @thomaspoignant, this will not work :)

[Kavindu-Dodan]

I have this doubt , otherwise this seems a good opt-in feature.

[Kavindu-Dodan]

How will the cache be busted when the configuration changes without SSE, WebSockets, or polling?

@beeme1mr I am not sure I understand what you mean here. Are you talking about the cases where the flag configuration changes based on time or automated actions?

If yes for me those are the usecases when we should set cacheable to false, and the provider will continue to always do remote calls for this flag because we will never have any cache value for this specific flag.

I think what @beeme1mr meant is upstream (flag mgmt system) changes and how to invalidate OFREP provider cache when a change happen.

I think the cacheable should set an appropriate ttl so systems do not get overloaded with evaluation calls. And it should be small enough so that upstream changes are visible to end systems (ex:- UI). The exact value (10seconds vs 2 seconds) should be decided by the vendor.

[Kavindu-Dodan]

@thomaspoignant I think this we should get some more approvals and get this merged. Besides, the only change is that we decided the configuration endpoint to be an extension. So we will need to define a server provider guideline, where we could define the default behavior of caching (ex:- enable caching through OFREP provider constructor options)

[thepabloaguilar]

Hi guys, I have a question. What's essentially difference between featureCacheInvalidation and featureCaching?
Are we not talking about the same thing but with two different mechanisms?

I understand that featureCacheInvalidation is about the provider polling the system each minPollingIntervalMs time and featureCaching is about invalidating the cache locally and then fetching the new state (if any). The main difference would be the last one returning an error if for some reason the request fails to get the new state as the local state is not valid anymore.

This is not so clear at first glance and maybe we could improve the wording/explanation of both. Also I can't see both being enabled at the same time, it'd be great to have a featureCache object and a strategy field with oneOf option