Simplify open feature setup (#107632)

* Simplify open feature setup

* Fix linter issues

* Apply review feedback

* Fix integration tests setup
This commit is contained in:
Tania
2025-07-14 16:22:04 +02:00
committed by GitHub
parent d1f785c8bf
commit e079cb3738
11 changed files with 79 additions and 101 deletions
+13 -2
View File
@@ -4,6 +4,7 @@ import (
"bytes"
"context"
"encoding/json"
"fmt"
"io"
"net/http"
"net/url"
@@ -59,10 +60,20 @@ func NewAPIBuilder(providerType string, url *url.URL, insecure bool, caFile stri
}
}
func RegisterAPIService(apiregistration builder.APIRegistrar, cfg *setting.Cfg, staticEvaluator featuremgmt.StaticFlagEvaluator) *APIBuilder {
func RegisterAPIService(apiregistration builder.APIRegistrar, cfg *setting.Cfg) (*APIBuilder, error) {
var staticEvaluator featuremgmt.StaticFlagEvaluator // No static evaluator needed for non-static provider
var err error
if cfg.OpenFeature.ProviderType == setting.StaticProviderType {
staticEvaluator, err = featuremgmt.CreateStaticEvaluator(cfg)
if err != nil {
return nil, fmt.Errorf("failed to create static evaluator: %w", err)
}
}
b := NewAPIBuilder(cfg.OpenFeature.ProviderType, cfg.OpenFeature.URL, true, "", staticEvaluator)
apiregistration.RegisterAPI(b)
return b
return b, nil
}
func (b *APIBuilder) GetAuthorizer() authorizer.Authorizer {
+6
View File
@@ -11,6 +11,7 @@ import (
"strconv"
"sync"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"golang.org/x/sync/errgroup"
"github.com/prometheus/client_golang/prometheus"
@@ -131,6 +132,11 @@ func (s *Server) Init() error {
return err
}
// Initialize the OpenFeature feature flag system
if err := featuremgmt.InitOpenFeatureWithCfg(s.cfg); err != nil {
return err
}
return s.provisioningService.RunInitProvisioners(s.context)
}
-2
View File
@@ -326,8 +326,6 @@ var wireBasicSet = wire.NewSet(
expr.ProvideService,
featuremgmt.ProvideManagerService,
featuremgmt.ProvideToggles,
featuremgmt.ProvideOpenFeatureService,
featuremgmt.ProvideStaticEvaluator,
dashboardservice.ProvideDashboardServiceImpl,
wire.Bind(new(dashboards.PermissionsRegistrationService), new(*dashboardservice.DashboardServiceImpl)),
dashboardservice.ProvideDashboardService,
+5 -15
View File
File diff suppressed because one or more lines are too long
-1
View File
@@ -156,7 +156,6 @@ var wireExtsBaseCLISet = wire.NewSet(
metrics.WireSet,
featuremgmt.ProvideManagerService,
featuremgmt.ProvideToggles,
featuremgmt.ProvideOpenFeatureService,
hooks.ProvideService,
setting.ProvideProvider, wire.Bind(new(setting.Provider), new(*setting.OSSImpl)),
licensing.ProvideService, wire.Bind(new(licensing.Licensing), new(*licensing.OSSLicensingService)),
+13 -31
View File
@@ -4,46 +4,37 @@ import (
"fmt"
"net/url"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/setting"
"github.com/open-feature/go-sdk/openfeature"
)
type OpenFeatureService struct {
log log.Logger
provider openfeature.FeatureProvider
Client openfeature.IClient
}
// ProvideOpenFeatureService is used for wiring dependencies in single tenant grafana
func ProvideOpenFeatureService(cfg *setting.Cfg) (*OpenFeatureService, error) {
func InitOpenFeatureWithCfg(cfg *setting.Cfg) error {
confFlags, err := setting.ReadFeatureTogglesFromInitFile(cfg.Raw.Section("feature_toggles"))
if err != nil {
return nil, fmt.Errorf("failed to read feature toggles from config: %w", err)
return fmt.Errorf("failed to read feature flags from config: %w", err)
}
err = initOpenFeature(cfg.OpenFeature.ProviderType, cfg.OpenFeature.URL, confFlags)
if err != nil {
return fmt.Errorf("failed to initialize OpenFeature: %w", err)
}
openfeature.SetEvaluationContext(openfeature.NewEvaluationContext(cfg.OpenFeature.TargetingKey, cfg.OpenFeature.ContextAttrs))
return newOpenFeatureService(cfg.OpenFeature.ProviderType, cfg.OpenFeature.URL, confFlags)
return nil
}
// TODO: might need to be public, so other MT services could set up open feature client
func newOpenFeatureService(pType string, u *url.URL, staticFlags map[string]bool) (*OpenFeatureService, error) {
p, err := createProvider(pType, u, staticFlags)
func initOpenFeature(providerType string, u *url.URL, staticFlags map[string]bool) error {
p, err := createProvider(providerType, u, staticFlags)
if err != nil {
return nil, fmt.Errorf("failed to create feature provider: type %s, %w", pType, err)
return fmt.Errorf("failed to create feature provider: type %s, %w", providerType, err)
}
if err := openfeature.SetProviderAndWait(p); err != nil {
return nil, fmt.Errorf("failed to set global feature provider: %s, %w", pType, err)
return fmt.Errorf("failed to set global feature provider: %s, %w", providerType, err)
}
client := openfeature.NewClient("grafana-openfeature-client")
return &OpenFeatureService{
log: log.New("openfeatureservice"),
provider: p,
Client: client,
}, nil
return nil
}
func createProvider(providerType string, u *url.URL, staticFlags map[string]bool) (openfeature.FeatureProvider, error) {
@@ -57,12 +48,3 @@ func createProvider(providerType string, u *url.URL, staticFlags map[string]bool
return newGOFFProvider(u.String())
}
func createClient(provider openfeature.FeatureProvider) (openfeature.IClient, error) {
if err := openfeature.SetProviderAndWait(provider); err != nil {
return nil, fmt.Errorf("failed to set global feature provider: %w", err)
}
client := openfeature.NewClient("grafana-openfeature-client")
return client, nil
}
+4 -7
View File
@@ -11,7 +11,7 @@ import (
"github.com/stretchr/testify/require"
)
func TestProvideOpenFeatureManager(t *testing.T) {
func TestCreateProvider(t *testing.T) {
u, err := url.Parse("http://localhost:1031")
require.NoError(t, err)
@@ -44,17 +44,14 @@ func TestProvideOpenFeatureManager(t *testing.T) {
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
cfg := setting.NewCfg()
cfg.OpenFeature = tc.cfg
p, err := ProvideOpenFeatureService(cfg)
provider, err := createProvider(tc.cfg.ProviderType, tc.cfg.URL, nil)
require.NoError(t, err)
if tc.expectedProvider == setting.GOFFProviderType {
_, ok := p.provider.(*gofeatureflag.Provider)
_, ok := provider.(*gofeatureflag.Provider)
assert.True(t, ok, "expected provider to be of type goff.Provider")
} else {
_, ok := p.provider.(*inMemoryBulkProvider)
_, ok := provider.(*inMemoryBulkProvider)
assert.True(t, ok, "expected provider to be of type memprovider.InMemoryProvider")
}
})
+13 -25
View File
@@ -3,7 +3,6 @@ package featuremgmt
import (
"context"
"fmt"
"net/url"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/setting"
@@ -17,43 +16,32 @@ type StaticFlagEvaluator interface {
EvalAllFlags(ctx context.Context) (OFREPBulkResponse, error)
}
// ProvideStaticEvaluator creates a static evaluator from configuration
// This can be used in wire dependency injection
func ProvideStaticEvaluator(cfg *setting.Cfg) (StaticFlagEvaluator, error) {
if cfg.OpenFeature.ProviderType == setting.GOFFProviderType {
l := log.New("static-evaluator")
l.Debug("cannot create static evaluator if configured provider is goff")
return &staticEvaluator{}, nil
// CreateStaticEvaluator is a dependency for ofrep APIBuilder
func CreateStaticEvaluator(cfg *setting.Cfg) (StaticFlagEvaluator, error) {
if cfg.OpenFeature.ProviderType != setting.StaticProviderType {
return nil, fmt.Errorf("provider is not a static provider, type %s", setting.StaticProviderType)
}
confFlags, err := setting.ReadFeatureTogglesFromInitFile(cfg.Raw.Section("feature_toggles"))
staticFlags, err := setting.ReadFeatureTogglesFromInitFile(cfg.Raw.Section("feature_toggles"))
if err != nil {
return nil, fmt.Errorf("failed to read feature toggles from config: %w", err)
return nil, fmt.Errorf("failed to read feature flags from config: %w", err)
}
return createStaticEvaluator(cfg.OpenFeature.ProviderType, cfg.OpenFeature.URL, confFlags)
}
// createStaticEvaluator evaluator that allows evaluating static flags from config.ini
func createStaticEvaluator(providerType string, u *url.URL, staticFlags map[string]bool) (StaticFlagEvaluator, error) {
provider, err := createProvider(providerType, u, staticFlags)
staticProvider, err := newStaticProvider(staticFlags)
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to create static provider: %w", err)
}
staticProvider, ok := provider.(*inMemoryBulkProvider)
p, ok := staticProvider.(*inMemoryBulkProvider)
if !ok {
return nil, fmt.Errorf("provider is not a static provider")
return nil, fmt.Errorf("static provider is not of type inMemoryBulkProvider")
}
client, err := createClient(provider)
if err != nil {
return nil, err
}
c := openfeature.GetApiInstance().GetClient()
return &staticEvaluator{
provider: staticProvider,
client: client,
provider: p,
client: c,
log: log.New("static-evaluator"),
}, nil
}
@@ -20,9 +20,9 @@ func Test_StaticProvider(t *testing.T) {
stFeatValue := stFeat.Expression == "true"
t.Run("empty config loads standard flags", func(t *testing.T) {
p := setup(t, []byte(``))
setup(t, []byte(``))
// Check for one of the standard flags
feat, err := p.Client.BooleanValueDetails(ctx, stFeatName, !stFeatValue, evalCtx)
feat, err := openfeature.GetApiInstance().GetClient().BooleanValueDetails(ctx, stFeatName, !stFeatValue, evalCtx)
assert.NoError(t, err)
assert.True(t, stFeatValue == feat.Value)
})
@@ -32,40 +32,46 @@ func Test_StaticProvider(t *testing.T) {
[feature_toggles]
featureOne = true
`)
p := setup(t, conf)
feat, err := p.Client.BooleanValueDetails(ctx, "featureOne", false, evalCtx)
setup(t, conf)
feat, err := openfeature.GetApiInstance().GetClient().BooleanValueDetails(ctx, "featureOne", false, evalCtx)
assert.NoError(t, err)
assert.True(t, feat.Value)
})
t.Run("missing feature should return default evaluation value and an error", func(t *testing.T) {
p := setup(t, []byte(``))
missingFeature, err := p.Client.BooleanValueDetails(ctx, "missingFeature", true, evalCtx)
setup(t, []byte(``))
missingFeature, err := openfeature.GetApiInstance().GetClient().BooleanValueDetails(ctx, "missingFeature", true, evalCtx)
assert.Error(t, err)
assert.True(t, missingFeature.Value)
assert.Equal(t, openfeature.ErrorCode("FLAG_NOT_FOUND"), missingFeature.ErrorCode)
})
}
func setup(t *testing.T, conf []byte) *OpenFeatureService {
func setup(t *testing.T, conf []byte) {
t.Helper()
cfg, err := setting.NewCfgFromBytes(conf)
require.NoError(t, err)
p, err := ProvideOpenFeatureService(cfg)
err = InitOpenFeatureWithCfg(cfg)
require.NoError(t, err)
return p
}
func Test_CompareStaticProviderWithFeatureManager(t *testing.T) {
cfg := setting.NewCfg()
sec, err := cfg.Raw.NewSection("feature_toggles")
conf := []byte(`
[feature_toggles]
ABCD = true
`)
cfg, err := setting.NewCfgFromBytes(conf)
require.NoError(t, err)
_, err = sec.NewKey("ABCD", "true")
// InitOpenFeatureWithCfg needed to initialize OpenFeature with the static provider configuration,
// so that StaticFlagEvaluator can use an open feature client.
// In real scenarios, this would be done during server startup.
err = InitOpenFeatureWithCfg(cfg)
require.NoError(t, err)
// Use StaticFlagEvaluator instead of OpenFeatureService for static evaluation
staticEvaluator, err := ProvideStaticEvaluator(cfg)
staticEvaluator, err := CreateStaticEvaluator(cfg)
require.NoError(t, err)
ctx := openfeature.WithTransactionContext(context.Background(), openfeature.NewEvaluationContext("grafana", nil))
+2 -4
View File
@@ -37,7 +37,6 @@ type ServiceImpl struct {
pluginSettings pluginsettings.Service
starService star.Service
features featuremgmt.FeatureToggles
openFeature *featuremgmt.OpenFeatureService
dashboardService dashboards.DashboardService
accesscontrolService ac.Service
kvStore kvstore.KVStore
@@ -60,7 +59,7 @@ type NavigationAppConfig struct {
func ProvideService(cfg *setting.Cfg, accessControl ac.AccessControl, pluginStore pluginstore.Store, pluginSettings pluginsettings.Service, starService star.Service,
features featuremgmt.FeatureToggles, dashboardService dashboards.DashboardService, accesscontrolService ac.Service, kvStore kvstore.KVStore, apiKeyService apikey.Service,
license licensing.Licensing, authnService authn.Service, openFeature *featuremgmt.OpenFeatureService) navtree.Service {
license licensing.Licensing, authnService authn.Service) navtree.Service {
service := &ServiceImpl{
cfg: cfg,
log: log.New("navtree service"),
@@ -70,7 +69,6 @@ func ProvideService(cfg *setting.Cfg, accessControl ac.AccessControl, pluginStor
pluginSettings: pluginSettings,
starService: starService,
features: features,
openFeature: openFeature,
dashboardService: dashboardService,
accesscontrolService: accesscontrolService,
kvStore: kvStore,
@@ -189,7 +187,7 @@ func (s *ServiceImpl) GetNavTree(c *contextmodel.ReqContext, prefs *pref.Prefere
treeRoot.RemoveSectionByID(navtree.NavIDCfg)
}
enabled := s.openFeature.Client.Boolean(ctx, featuremgmt.FlagPinNavItems, true, openfeature.TransactionContext(ctx))
enabled := openfeature.GetApiInstance().GetClient().Boolean(ctx, featuremgmt.FlagPinNavItems, true, openfeature.TransactionContext(ctx))
if enabled && c.IsSignedIn {
treeRoot.AddSection(&navtree.NavLink{
Text: "Bookmarks",
+3
View File
@@ -12,6 +12,7 @@ import (
"testing"
"time"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/prometheus/client_golang/prometheus"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
@@ -82,6 +83,8 @@ func StartGrafanaEnv(t *testing.T, grafDir, cfgPath string) (string, *server.Tes
runstore = true
}
err = featuremgmt.InitOpenFeatureWithCfg(cfg)
require.NoError(t, err)
env, err := server.InitializeForTest(t, t, cfg, serverOpts, apiServerOpts)
require.NoError(t, err)