feat: Complete Implementation of Multi-Provider

[jblacker]

This PR

• Adds Event Handling to Multiprovider
• Adds Hooks support to Multiprovider
• Adds basic logging functionality to Multiprovider (separate from logging hooks)

Related Issues

Fixes #599

Notes

This should complete the implementation of the multiprovider per the specification. The hooks support implementation within HookIsolator is largely based on the code in the main OpenFeature SDK for Go along with some inspiration from the JS implementation of their multiprovider.

The strategies could probably use a refactoring to clean them up at this point, but not a blocker.

Follow-up Tasks

• e2e tests should be added
• Additional logging is a nice to have
• Additional documentation

How to test

This can be tested by building two providers, adding them to a multiprovider and testing it provides the expected results based on the selected strategy.

[aepfli]

thank you for this great contribution, multiprovider is a feature a lot of people are using, I am still wondering, as this is spec'd within our spec repo, should we maybe move the multiprovider to the go-sdk, and maybe start writing some gherkin tests to unifiy this behavior

[toddbaert]

@jblacker Thanks! I will formally review this tomorrow. I do suspect that you could reduce some code by adding this directly to the SDK as @aepfli suggested (the rest of the community also supports this).

Pending review, I am happy to merge this here and then deprecate/move it later, but I'd also support a refactored addition directly to the SDK; that's up to you.

[jblacker]

These are great points. I'm thinking that assuming this looks good to go we merge it in here just in case anyone else has a need for it while I work on a cleaned up refactored version for directly in the SDK.

[erka]

Suggested change

	conf.logger = nil
	conf.logger = slog.New(slog.DiscardHandler)

it's probably a better option and ConditionalLogger could be removed. wdyt?

[jblacker]

That's a great idea. I'm not super familiar with slog usage in libraries so I wasn't sure if it was acceptable to "override" the handler being used in the main application. I'm now realizing that this would only apply to the logging calls within the multiprovider so it's not overriding anything. 🙂
I'll accept this change and clean it all up

[jblacker]

Actually, I'm almost done with the ported implementation as a package in the go-sdk with a more streamlined implementation of the strategies. I can still make that change here, but let me know if it's worthwhile.

[beeme1mr]

Hey @jblacker, it's up to you but we'll likely deprecate this package once it's available in the SDK.

[erka]

Just as FYI. The latest version (v1.14.1) of go-sdk doesn't have this func anymore. If the latest version of sdk is used it could simplify this PR

[beeme1mr]

State management moved to the SDK. Good catch @erka.

[jblacker]

Unfortunately issue #671 prevents us from using a version of the SDK above 13.1 in any module.

[erka]

Some observations.

I tested this provider with go-sdk@v0.14.1, and it doesn't seem to work correctly. Specifically:

• With SetNamedProvider, it fails with provider not yet initialized.
• With SetNamedProviderAndWait, it hangs indefinitely.

The providerA and providerB work fine if they are set in SetNamedProvider. MultiProvider works with v0.13.1. There seems to be something off with how this provider interacts with go-sdk@v0.14.1.

This is my code

package main

import (
	"context"
	"fmt"

	flipt "github.com/open-feature/go-sdk-contrib/providers/flipt/pkg/provider"
	gofeatureflag "github.com/open-feature/go-sdk-contrib/providers/go-feature-flag/pkg"
	mp "github.com/open-feature/go-sdk-contrib/providers/multi-provider/pkg"

	"github.com/open-feature/go-sdk/openfeature"
)

func main() {
	ctx := context.Background()
	providerA, _ := gofeatureflag.NewProviderWithContext(ctx, gofeatureflag.ProviderOptions{
		Endpoint: "http://localhost:1031",
	},
	)
	providerB := flipt.NewProvider()

	providermap := make(mp.ProviderMap)
	providermap["goff"] = providerA
	providermap["flipt"] = providerB
	provider, err := mp.NewMultiProvider(providermap, mp.StrategyFirstMatch)
	if err != nil {
		panic(err)
	}
	openfeature.SetNamedProvider("multi", provider)
	client := openfeature.NewClient("multi")
	res, err := client.BooleanValueDetails(ctx, "this-demo", false, openfeature.NewEvaluationContext("some", nil))
	if err != nil {
		panic(err)
	}
	fmt.Println(res)
}

[jblacker]

@erka Interesting findings, I'll do some digging into finding out what's going on here. I'm going to see if this issue occurs with the implementation that I have a draft PR open here on the main SDK: open-feature/go-sdk#354

[erka]

Should this PR be closed in this case? Having multiple similar PRs can make it hard to focus and determine where to continue the discussion. wdyt @jblacker?

[jblacker]

I'll close this out. My draft PR is almost done; just finishing up testing. Hoping to have it ready for review by the end of the week.