Skip to content

feat: add OpenFeature Remote Evaluation Protocol (OFREP) Provider #319

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

Closed
wants to merge 457 commits into from

Conversation

weyert
Copy link
Contributor

@weyert weyert commented Mar 29, 2025

This PR

The provider allows users to evaluate feature flags through the OFREP API, making it adaptable to various feature flag management systems that support the OFREP specification.

Features:

  • Full implementation of the OpenFeature REST Evaluation Protocol specification
  • Support for all standard flag types (boolean, string, integer, double, object)
  • Configuration options for HTTP client, behaviour, and authorisation

Related Issues

Closes #413

Follow-up Tasks

  • Add DI support
  • Add Events Support
  • Investigate the lifecycle of the HttpClient
  • Add E2E tests
  • Check Hooks affecting this provider only, works
  • Check Evaluation Context affecting this provider only, works

How to test

Manually tested

chrfwow and others added 6 commits January 27, 2025 09:07
…e#305 (open-feature#309)

Signed-off-by: christian.lutnik <christian.lutnik@dynatrace.com>
Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org>
open-feature#288)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Signed-off-by: Michael Richardson <michael.richardson@octopus.com>
…-feature#318)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@weyert weyert added the contribfest A good issue for Contribfest KubeCon EU '24 label Mar 29, 2025
@weyert weyert self-assigned this Mar 29, 2025
@weyert weyert requested review from a team as code owners March 29, 2025 20:53
@weyert weyert force-pushed the add-ofrep-provider branch from 582555b to 81fa968 Compare March 29, 2025 22:39
renovate bot and others added 19 commits April 7, 2025 14:45
…feature#231)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Signed-off-by: André Silva <2493377+askpt@users.noreply.github.com>
…n-feature#321)

Signed-off-by: André Silva <2493377+askpt@users.noreply.github.com>
Co-authored-by: Michael Beemer <beeme1mr@users.noreply.github.com>
…pen-feature#325)

Signed-off-by: André Silva <2493377+askpt@users.noreply.github.com>
…ature#274)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…e#289)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Michael Beemer <beeme1mr@users.noreply.github.com>
Co-authored-by: André Silva <2493377+askpt@users.noreply.github.com>
…v0.5.21 (open-feature#291)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

[Fact]
public async Task EvaluateFlag_WithNestedEvaluationContext_ShouldSerializeCorrectly()
{
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar to the other evaluation context tests, it would be good to check that the context is serialized as we expect it


// Verify the unicode characters were properly URL encoded
var request = this._mockHandler.Requests[0];
Assert.Contains("ofrep/v1/evaluate/flags/", request.RequestUri?.ToString());
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't verify that the uri contains the unicode character in the flag key


[Fact]
public async Task EvaluateFlag_WithSlowResponse_ShouldReturnSuccessfully()
{
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure how this test diffs from the other normal happy path tests

renovate bot and others added 2 commits June 10, 2025 12:53
@weyert weyert requested a review from kylejuliandev June 19, 2025 12:15
@toddbaert toddbaert self-requested a review June 19, 2025 12:19
weyert added 5 commits June 19, 2025 14:48
…ge`-attributes

Signed-off-by: Weyert de Boer <weyert@innerfuse.biz>
Signed-off-by: Weyert de Boer <weyert@innerfuse.biz>
Signed-off-by: Weyert de Boer <weyert@innerfuse.biz>
Signed-off-by: Weyert de Boer <weyert@innerfuse.biz>
Signed-off-by: Weyert de Boer <weyert@innerfuse.biz>
@weyert weyert force-pushed the add-ofrep-provider branch from 390fb9f to f3d3ba1 Compare June 19, 2025 13:50
weyert added 3 commits June 19, 2025 14:52
Signed-off-by: Weyert de Boer <weyert@innerfuse.biz>
- Fix test expectation for special characters (!@#) to match Uri.EscapeDataString behavior
- Update test verification to check multiple URI properties for encoded content
- Ensure proper URL encoding preservation in HttpRequestMessage construction

Signed-off-by: Weyert de Boer <weyert@innerfuse.biz>
Signed-off-by: Weyert de Boer <weyert@innerfuse.biz>
@weyert weyert force-pushed the add-ofrep-provider branch from e1badcd to ed5cf97 Compare June 19, 2025 15:12
Comment on lines 140 to 147
#if NET8_0_OR_GREATER
ArgumentNullException.ThrowIfNull(flagKey);
#else
if (flagKey == null)
{
throw new ArgumentNullException(nameof(flagKey));
}
#endif
Copy link
Member

Choose a reason for hiding this comment

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

If this is just to make use of a language feature, I would say vote to remove the compiler directive and just use the common version. I think adding compiler directives like this takes away from readability so I recommend only doing it when we really have to.

If we "really have to" here for some reason I don't understand, that's OK.

Comment on lines 53 to 66
#if NET8_0_OR_GREATER
ArgumentNullException.ThrowIfNull(configuration);
ArgumentNullException.ThrowIfNull(handler);
#else
if (configuration == null)
{
throw new ArgumentNullException(nameof(configuration));
}

if (handler == null)
{
throw new ArgumentNullException(nameof(handler));
}
#endif
Copy link
Member

Choose a reason for hiding this comment

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

Same as https://github.com/open-feature/dotnet-sdk-contrib/pull/319/files#r2157403557, I would apply this all over if you aren't opposed.

Signed-off-by: Weyert de Boer <weyert@innerfuse.biz>
@toddbaert toddbaert self-requested a review June 19, 2025 17:06
Copy link
Member

@toddbaert toddbaert left a comment

Choose a reason for hiding this comment

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

I don't have as much OFREP expertise as some others, but from an overall provider perspective this looks good to me, at least for a first pass.

.tool-versions Outdated
Copy link
Member

Choose a reason for hiding this comment

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

Could you please remove this file from the repo? If you want, you can add this file to the .gitignore.

weyert added 2 commits June 19, 2025 18:32
Signed-off-by: Weyert de Boer <weyert@innerfuse.biz>
Signed-off-by: Weyert de Boer <weyert@innerfuse.biz>
@thomaspoignant
Copy link
Member

@weyert Why is this PR closed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribfest A good issue for Contribfest KubeCon EU '24
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create OFREP provider