Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Client provider guidelines #14

Open
wants to merge 24 commits into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
3702d36
feat: Client provider spec
thomaspoignant Apr 30, 2024
34f336a
Update provider/specs/client.md
thomaspoignant May 2, 2024
dbdd5f4
Update provider/specs/client.md
thomaspoignant May 2, 2024
14eaec1
Update provider/specs/client.md
thomaspoignant May 2, 2024
667bb96
Update provider/specs/client.md
thomaspoignant May 2, 2024
e4fa8a3
Update provider/specs/client.md
thomaspoignant May 2, 2024
680d546
Update provider/specs/client.md
thomaspoignant May 2, 2024
4a8a859
Update provider/specs/client.md
thomaspoignant May 2, 2024
175abb3
Update provider/specs/client.md
thomaspoignant May 2, 2024
25d6fbe
Update provider/specs/client.md
thomaspoignant May 2, 2024
f01598f
Update provider/specs/client.md
thomaspoignant May 2, 2024
7f735b2
Update provider/specs/client.md
thomaspoignant May 2, 2024
af3beac
Update provider/specs/client.md
thomaspoignant May 2, 2024
14ac393
Update provider/specs/client.md
thomaspoignant May 2, 2024
4cecfc5
Update provider/specs/client.md
thomaspoignant May 2, 2024
87d4238
update with review comments
thomaspoignant May 2, 2024
0038810
Move to guideline folder
thomaspoignant May 2, 2024
3008500
Replace specification
thomaspoignant May 2, 2024
f3ec588
add OpenAPI spec validator based on redocly cli (#15)
Kavindu-Dodan May 13, 2024
2522810
chore(deps): update actions/checkout action to v4 (#16)
renovate[bot] May 14, 2024
ad36231
Update guideline/static-context-provider.md
thomaspoignant May 17, 2024
4a3f70a
Update guideline/static-context-provider.md
thomaspoignant May 17, 2024
961f3c7
Update guideline/static-context-provider.md
thomaspoignant May 17, 2024
69b6f5b
Merge branch 'main' into client-spec
thomaspoignant May 18, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 95 additions & 0 deletions guideline/static-context-provider.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
# Creating an OFREP client provider

OpenFeature Remote Evaluation Protocol (OFREP) is an API specification for feature flagging that allows the use of generic providers to connect to any feature flag management systems that supports the protocol.

In this document, we will specify how to write an OFREP provider using the [static-context-paradigm](https://openfeature.dev/specification/glossary/#static-context-paradigm) that is used on client side applications typically operate in the context of a single user.
We will keep the document language agnostic.

**Pre-requisite:**
- Understanding of [general provider concepts](https://openfeature.dev/docs/reference/concepts/provider/)
- Understanding of the [OFREP](../../README.md)
- Understanding of the [OFREP OpenAPI specification](../../service/openapi.yaml)


## Constructor
An implementation of an OFREP client provider should allow in the creation of the provider to take as options:
- `baseURL`: The base URL of the [flag management system](https://openfeature.dev/specification/glossary#flag-management-system). This should be the base of the URL pointing before the `/ofrep` namespace of the API.
- `headers`: The headers to use when calling the OFREP endpoints *(ex:`Authorization`, Custom headers, etc ...)*.
- `pollInteral`: The polling interval defining when to call again the flag management system.

In the constructor, the provider should check if the `baseURL` is a valid URL and return an error if the URL is invalid.

## Initialize the provider
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit torn here wether my thinking is going to deep or the use case is too fringe but bare with me:

Some applications have strict rules or goals of how fast they need to be able to show their initial screen. These apps may not have time (or do not want) to wait for network requests to finish before considering a feature flagging SDK to be initialised.

Obviously there are many ways one can solve this, but we have written mobile providers with disk persistence where we then can pass a FetchingStrategy to the constructor where the app owner can decide wether or not they want the initialize() function to block until fresh data is available or simply load any previous data from disk (and then refresh from the backend in the background).

Firebase Remote Config explains the concepts here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea of various initialization strategies and the link to Firebase Remote Config is a great resource.

Perhaps one way to implement that would be to put this section under a "Fetch on load" strategy. We could start with a single strategy (and perhaps make it the default) but that would give us the ability to add additional strategies to support specific use cases.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting point here. I see the value of having multiple fetching strategies but I am not sure how to frame that into this document.

I see 2 options:

  1. We try to define the different possible ways of doing the initialisation for the OFREP client providers, and we add them to the document.
  2. We remove that block since there is some chance that it could be different to initialize something depending on the platform (web/mobile).

Do you have any preference?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm thinking that it might be most reasonable to ignore my comment about this and go ahead with a blocking network call as part of the initialize function in order to not block this document.

If we believe that the topic warrants additional discussion we can open an issue to discuss it there and amend the spec or add an appendix. wdyt?

An implementation of an OFREP client provider should start with an initialization of the provider.

The `initialize()` function should follow those steps:
1. Make a GET request to the `/ofrep/v1/configuration` endpoint to retrieve the configurations return by the flag management system and store them in memory to be available for all the function of the provider. *(See [Annexe 1](#annexe-1) for the description of the endpoint response)*
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this step be optional? The provider should be able to work without having to call the configuration API.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is also my general feeling, that this should be an opt-in feature as @beeme1mr says.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was more thinking toward something that can be enabled by default and opt-out on demand.
See #14 (comment)

2. Make a POST request to the `/ofrep/v1/evaluate/flags` endpoint with the evaluation context in the body.
- If the endpoint returns an error, the `initialize()` function must error and exit.
- If the request is successful, we should store in a local cache all of the flags evaluation results returned by the API in a local cache. We should also store the `ETag` header in the provider to be able to send it back later.
Comment on lines +28 to +29
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be better to leave the we out and talk in third person about the provider.

3. If polling is enabled, the polling loop should start now *(See [polling section](#polling))*.

## Evaluation
The evaluation should not perform any remote API calls.

When calling an evaluation function the provider should check if the associated type is supported, by checking the key `capabilities.flagEvaluation.unsupportedTypes` from the configuration endpoint.
- If the type is unsupported, we should exit early and directly return an error.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we know which error we want to throw here? I think it would be nice to specify here.
Maybe we can even have a new Error type, the case that a system does not support a type happens pretty often. (independently from OFREP)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is true, should we add that to the OF spec?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mh yes I think we should add it to the OF spec.
I could open a PR next days, what do you think @thomaspoignant ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be awesome

- If the type is supported, we should retrieve the flag from the local cache of the flags evaluation.
- If you are not able to find the flag, you should return an FlagNotFound error.
- If the remote evaluation for this flag has return an error, you should map the error in provider and return the associated error.
- If the value retrieve from the cache has a different type than the one expected you should return a TypeMismatch error.
- If the cached evaluation is in success you should return the evaluation response.

```mermaid
flowchart TD
A[evaluation\nfunction] --> B{Is function type in\nunsupportedTypes?}
B --> |YES| C(return an error)
B --> |NO| D{Is flag key stored\nin local cache?}
D --> |NO| E(return a FlagNotFound error)
D --> |YES| F{Is cached evaluation\nresponse in error?}
F --> |YES| G(Map the error as a\nprovider error and return)
F --> |NO| H{Is the flag value the\nsame type as the\nevaluation function?}
H --> |YES| I(return the evaluation response)
H --> |NO| J(return a TypeMismatch error)
```
Comment on lines +43 to +54
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love it! :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is excellent!


## Polling
The polling system will make a POST request periodically to the `/ofrep/v1/evaluate/flags` endpoint to check if there is a change in the flags evaluation to be able to store it.

If an `ETag` of a former evaluation is available we should always add the header `If-None-Match` with the `ETag` value.
- If the cache is still up-to-date we will receive a `304` telling us that the nothing has changed on the flag management system side.
- If the cache is outdated we will receive a `200` with the new values of all the flags. In that situation we should:
1. Replace the actual local cache of flags evaluations.
2. Store the new `ETag` value for the future call.


## Annexe 1
**`/ofrep/v1/configuration` description:**
The endpoint will return a list of configurations for the provider:
- `name`: Name of the flag management system, it should be used in the `metadata.name` name (ex: `OFREP Web Provider ${metadata.name}`).
- `capabilities`: List of capabilities of the flag management system and their associated configuration. *Check the [capabilities section](#capabilities) for the details of each capability.*

### Capabilities
In this section we will describe each capability and describe their default value and usage.

#### Cache Invalidation
The capability `cacheInvalidation` describes how the mechanism of cache invalidation works.

##### Polling
`polling`: define how the provider should do the polling

- `enabled`: if `true` the provider should poll the `/ofrep/v1/evaluate/flags` regularly to check if any flag evaluation has changed in the flag management system.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OFREP web provider by default is doing polling.
You have a default value set if no pollingInterval is provided during the initialization.

https://github.com/open-feature/js-sdk-contrib/blob/c00f998227f55ca5a64b01d039ab6ec2aff18f7c/libs/providers/ofrep-web/src/lib/ofrep-web-provider.ts#L78

- `minPollingInterval`: define the minimum polling interval acceptable by the flag management system. If for any reason the `pollInteral` provided in the constructor is lower than this `minPollingInterval` we should default on this value.

If the key `polling` is not available, the provider should use those default values:

| key | default value |
| -------------------- | ------------- |
| `enabled` | `true` |
| `minPollingInterval` | 60000 |


#### Flag Evaluation
`flagEvaluation`: define how to manage flag evaluation in the provider.
- `unsupportedTypes`: Some flag management systems do not support all types. This array should contain all the types not supported by the flag management system. Acceptable values are `int`, `float`, `string`, `boolean`, and `object`.
Default value: `[]`