-
Notifications
You must be signed in to change notification settings - Fork 4
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
base: main
Are you sure you want to change the base?
Changes from all commits
3702d36
34f336a
dbdd5f4
14eaec1
667bb96
e4fa8a3
680d546
4a8a859
175abb3
25d6fbe
f01598f
7f735b2
af3beac
14ac393
4cecfc5
87d4238
0038810
3008500
f3ec588
2522810
ad36231
4a3f70a
961f3c7
69b6f5b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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 | ||
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)* | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it would be better to leave the |
||
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. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, this is true, should we add that to the OF spec? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mh yes I think we should add it to the OF spec. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Love it! :) There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This isn't the current behavior in the OFREP web provider. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OFREP web provider by default is doing polling. |
||
- `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: `[]` |
There was a problem hiding this comment.
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 theinitialize()
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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:
Do you have any preference?
There was a problem hiding this comment.
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?