feat: Implement multiprovider

[jblacker]

This PR

Implements the Multiprovider specified in "Appendix A: Included Utilities" of the OpenFeature Spec. This implementation is an improvement upon what I initially implemented in the go-sdk-contrib repository here & here.

This includes implementations for all three strategies described in the spec along with some common functions. Unlike my original implementation this includes full support for hooks & event handling from the included handlers.

Related Issues

open-feature/go-sdk-contrib#599

Notes

I reimplemented this here from the go-sdk-contrib repo on suggestion of other members. This includes the refactoring improvements that I was mentioning in discussions on my open PR over there.

This PR is currently in draft because it is built on top of the exported mocks in order to provide a full unit test suite.

Follow-up Tasks

• e2e / Gherkin tests should be written
• Minor clean up & improvements

How to test

• Create an application and pass a multiprovider to the OpenFeature SDK that includes 2 or more providers. Verify that the results are as expected per the selected strategy.

[erka]

Hey @jblacker.

Is there any road block to move this forward? Do you need any help?

[jblacker]

I've actually been on vacation. I'm going to pick back up on this tomorrow.

[codecov]

Codecov Report

❌ Patch coverage is 71.96368% with 247 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.65%. Comparing base (40fe27e) to head (500bcb1).

Files with missing lines	Patch %	Lines
openfeature/multi/multiprovider.go	63.20%	119 Missing and 5 partials ⚠️
openfeature/multi/isolation.go	61.16%	82 Missing and 5 partials ⚠️
openfeature/multi/comparison_strategy.go	86.24%	21 Missing and 5 partials ⚠️
openfeature/multi/strategies.go	88.37%	8 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #354      +/-   ##
==========================================
- Coverage   85.57%   80.65%   -4.92%     
==========================================
  Files          20       27       +7     
  Lines        1435     2316     +881     
==========================================
+ Hits         1228     1868     +640     
- Misses        186      411     +225     
- Partials       21       37      +16     

Flag	Coverage Δ
e2e	80.65% <71.96%> (-4.92%)	⬇️
unit	80.65% <71.96%> (-4.92%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[sahidvelji]

If we take into account how little free time Jordan has and how perfect this PR needs to be, it will never get merged.... It’s been open since May — we’ll be throwing it a six-month birthday party soon. This is insane 😂

Please merge it so other members can do the cleanup or close it....

You're right that it has taken quite long. To be fair, it is a very large PR and I don't get enough free time to review this in one pass. And even if I approved the PR, I would still need a second approval before being able to merge it anyways.

But I'll try to focus on the exported API only in my review so that we can get this merged and work on other, internal aspects afterwards.

[beeme1mr]

Yeah, this is a big one. Let's focus the reviews on the API surface and consider marking it as experimental for now. That's what we did in the .NET SDK.

[jblacker]

I handled all of the additional comments. IMHO most of the latest comments are nits rather than true blockers, but I've handled them nonetheless.

I like the idea of considering this experimental until it has some more usage under its belt by someone other than me.

[sahidvelji]

@jblacker Ok let's add a package doc comment stating that the package is experimental and that breaking changes are to be expected. Once added, I think we're good to merge.

[jblacker]

I want to call out that while I was writing additional documentation* I found a major bug in how the custom strategy is implemented. I realized that there's no way to provide the providers in this case since all of the other strategies have "constructors" to close over the provider slice. I added a new type StrategyConstructor to allow implementers to also close over the providers and added tests.

* put together a full readme with a big warning that the multi package is experimental in addition to the package comment

[sahidvelji]

Thanks for all the work here @jblacker!

[erka]

@jblacker Could you please look into the DCO issue?

[jblacker]

@erka The DCO issue is because I merged in your PR to my branch too quickly because you had two unsigned commits. @toddbaert said to not worry about it because he can squash them at merge time to eliminate that error here.

[beeme1mr]

Yeah, I can bypass the check. If we're ready, I can merge tomorrow.

[sahidvelji]

@beeme1mr I think we're good to merge, especially since we decided to mark the package experimental.