From 1500fa577269c8d30bfbb3eaed5440560ddad8a1 Mon Sep 17 00:00:00 2001 From: Tania <10127682+undef1nd@users.noreply.github.com> Date: Fri, 13 Jun 2025 17:30:53 +0200 Subject: [PATCH] Extend OpenFeature service (#106707) --- pkg/services/featuremgmt/openfeature.go | 103 ++++++++++++------ pkg/services/featuremgmt/openfeature_test.go | 89 ++++----------- pkg/services/featuremgmt/static_provider.go | 25 ++++- .../featuremgmt/static_provider_test.go | 34 ++++++ pkg/setting/setting.go | 8 ++ pkg/setting/setting_openfeature.go | 51 +++++++++ pkg/setting/setting_openfeature_test.go | 57 ++++++++++ 7 files changed, 265 insertions(+), 102 deletions(-) create mode 100644 pkg/setting/setting_openfeature.go create mode 100644 pkg/setting/setting_openfeature_test.go diff --git a/pkg/services/featuremgmt/openfeature.go b/pkg/services/featuremgmt/openfeature.go index 123fb1287b5..26caec9e210 100644 --- a/pkg/services/featuremgmt/openfeature.go +++ b/pkg/services/featuremgmt/openfeature.go @@ -1,71 +1,112 @@ package featuremgmt import ( + "context" "fmt" + "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/setting" + "github.com/open-feature/go-sdk/openfeature" ) -const ( - staticProviderType = "static" - goffProviderType = "goff" - - configSectionName = "feature_toggles.openfeature" - contextSectionName = "feature_toggles.openfeature.context" -) - type OpenFeatureService struct { + cfg *setting.Cfg + log log.Logger provider openfeature.FeatureProvider Client openfeature.IClient } func ProvideOpenFeatureService(cfg *setting.Cfg) (*OpenFeatureService, error) { - conf := cfg.Raw.Section(configSectionName) - provType := conf.Key("provider").MustString(staticProviderType) - url := conf.Key("url").MustString("") - key := conf.Key("targetingKey").MustString(cfg.AppURL) - var provider openfeature.FeatureProvider var err error - if provType == goffProviderType { - provider, err = newGOFFProvider(url) + if cfg.OpenFeature.ProviderType == setting.GOFFProviderType { + if cfg.OpenFeature.URL == nil { + return nil, fmt.Errorf("feature provider url is required for GOFFProviderType") + } + + provider, err = newGOFFProvider(cfg.OpenFeature.URL.String()) } else { provider, err = newStaticProvider(cfg) } if err != nil { - return nil, fmt.Errorf("failed to create %s feature provider: %w", provType, err) + return nil, fmt.Errorf("failed to create %s feature provider: %w", cfg.OpenFeature.ProviderType, err) } if err := openfeature.SetProviderAndWait(provider); err != nil { - return nil, fmt.Errorf("failed to set global %s feature provider: %w", provType, err) + return nil, fmt.Errorf("failed to set global %s feature provider: %w", cfg.OpenFeature.ProviderType, err) } - attrs := ctxAttrs(cfg) - openfeature.SetEvaluationContext(openfeature.NewEvaluationContext(key, attrs)) + openfeature.SetEvaluationContext(openfeature.NewEvaluationContext(cfg.OpenFeature.TargetingKey, cfg.OpenFeature.ContextAttrs)) client := openfeature.NewClient("grafana-openfeature-client") - return &OpenFeatureService{ + cfg: cfg, + log: log.New("openfeatureservice"), provider: provider, Client: client, }, nil } -// ctxAttrs uses config.ini [feature_toggles.openfeature.context] section to build the eval context attributes -func ctxAttrs(cfg *setting.Cfg) map[string]any { - ctxConf := cfg.Raw.Section(contextSectionName) - - attrs := map[string]any{} - for _, key := range ctxConf.KeyStrings() { - attrs[key] = ctxConf.Key(key).String() +func (s *OpenFeatureService) EvalFlagWithStaticProvider(ctx context.Context, flagKey string) (openfeature.BooleanEvaluationDetails, error) { + _, ok := s.provider.(*inMemoryBulkProvider) + if !ok { + return openfeature.BooleanEvaluationDetails{}, fmt.Errorf("not a static provider, request must be sent to open feature service") } - // Some default attributes - if _, ok := attrs["grafana_version"]; !ok { - attrs["grafana_version"] = setting.BuildVersion + result, err := s.Client.BooleanValueDetails(ctx, flagKey, false, openfeature.TransactionContext(ctx)) + if err != nil { + return openfeature.BooleanEvaluationDetails{}, fmt.Errorf("failed to evaluate flag %s: %w", flagKey, err) } - return attrs + return result, nil +} + +func (s *OpenFeatureService) EvalAllFlagsWithStaticProvider(ctx context.Context) (OFREPBulkResponse, error) { + p, ok := s.provider.(*inMemoryBulkProvider) + if !ok { + return OFREPBulkResponse{}, fmt.Errorf("not a static provider, request must be sent to open feature service") + } + + flags, err := p.ListFlags() + if err != nil { + return OFREPBulkResponse{}, fmt.Errorf("static provider failed to list all flags: %w", err) + } + + allFlags := make([]OFREPFlag, 0, len(flags)) + for _, flagKey := range flags { + result, err := s.Client.BooleanValueDetails(ctx, flagKey, false, openfeature.TransactionContext(ctx)) + if err != nil { + s.log.Error("failed to evaluate flag during bulk evaluation", "flagKey", flagKey, "error", err) + continue + } + + allFlags = append(allFlags, OFREPFlag{ + Key: flagKey, + Value: result.Value, + Reason: "static provider evaluation result", + Variant: result.Variant, + ErrorCode: string(result.ErrorCode), + ErrorDetails: result.ErrorMessage, + }) + } + + return OFREPBulkResponse{Flags: allFlags}, nil +} + +// Bulk evaluation response +type OFREPBulkResponse struct { + Flags []OFREPFlag `json:"flags"` + Metadata map[string]any `json:"metadata,omitempty"` +} + +type OFREPFlag struct { + Key string `json:"key"` + Value bool `json:"value"` + Reason string `json:"reason"` + Variant string `json:"variant,omitempty"` + Metadata map[string]any `json:"metadata,omitempty"` + ErrorCode string `json:"errorCode,omitempty"` + ErrorDetails string `json:"errorDetails,omitempty"` } diff --git a/pkg/services/featuremgmt/openfeature_test.go b/pkg/services/featuremgmt/openfeature_test.go index 919c3592e6e..7176fc1e6a1 100644 --- a/pkg/services/featuremgmt/openfeature_test.go +++ b/pkg/services/featuremgmt/openfeature_test.go @@ -1,113 +1,62 @@ package featuremgmt import ( + "net/url" "testing" "github.com/grafana/grafana/pkg/setting" gofeatureflag "github.com/open-feature/go-sdk-contrib/providers/go-feature-flag/pkg" - "github.com/open-feature/go-sdk/openfeature/memprovider" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) func TestProvideOpenFeatureManager(t *testing.T) { + u, err := url.Parse("http://localhost:1031") + require.NoError(t, err) + testCases := []struct { name string - cfg string + cfg setting.OpenFeatureSettings expectedProvider string }{ { name: "static provider", - expectedProvider: staticProviderType, + expectedProvider: setting.StaticProviderType, }, { name: "goff provider", - cfg: ` -[feature_toggles.openfeature] -provider = goff -url = http://localhost:1031 -targetingKey = grafana -`, - expectedProvider: goffProviderType, + cfg: setting.OpenFeatureSettings{ + ProviderType: setting.GOFFProviderType, + URL: u, + TargetingKey: "grafana", + }, + expectedProvider: setting.GOFFProviderType, }, { name: "invalid provider", - cfg: ` -[feature_toggles.openfeature] -provider = some_provider -`, - expectedProvider: staticProviderType, + cfg: setting.OpenFeatureSettings{ + ProviderType: "some_provider", + }, + expectedProvider: setting.StaticProviderType, }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { cfg := setting.NewCfg() - if tc.cfg != "" { - err := cfg.Raw.Append([]byte(tc.cfg)) - require.NoError(t, err) - } + cfg.OpenFeature = tc.cfg p, err := ProvideOpenFeatureService(cfg) require.NoError(t, err) - if tc.expectedProvider == goffProviderType { + if tc.expectedProvider == setting.GOFFProviderType { _, ok := p.provider.(*gofeatureflag.Provider) assert.True(t, ok, "expected provider to be of type goff.Provider") } else { - _, ok := p.provider.(memprovider.InMemoryProvider) + _, ok := p.provider.(*inMemoryBulkProvider) assert.True(t, ok, "expected provider to be of type memprovider.InMemoryProvider") } }) } } - -func Test_CtxAttrs(t *testing.T) { - testCases := []struct { - name string - conf string - expected map[string]any - }{ - { - name: "empty config - only default attributes should be present", - expected: map[string]any{ - "grafana_version": "", - }, - }, - { - name: "config with some attributes", - conf: ` -[feature_toggles.openfeature.context] -foo = bar -baz = qux -quux = corge`, - expected: map[string]any{ - "foo": "bar", - "baz": "qux", - "quux": "corge", - "grafana_version": "", - }, - }, - { - name: "config with an attribute that overrides a default one", - conf: ` -[feature_toggles.openfeature.context] -grafana_version = 10.0.0 -foo = bar`, - expected: map[string]any{ - "grafana_version": "10.0.0", - "foo": "bar", - }, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - cfg, err := setting.NewCfgFromBytes([]byte(tc.conf)) - require.NoError(t, err) - - assert.Equal(t, tc.expected, ctxAttrs(cfg)) - }) - } -} diff --git a/pkg/services/featuremgmt/static_provider.go b/pkg/services/featuremgmt/static_provider.go index 03ae93a8086..a69e7c28b4e 100644 --- a/pkg/services/featuremgmt/static_provider.go +++ b/pkg/services/featuremgmt/static_provider.go @@ -8,6 +8,29 @@ import ( "github.com/open-feature/go-sdk/openfeature/memprovider" ) +// inMemoryBulkProvider is a wrapper around memprovider.InMemoryProvider that +// also allows for bulk evaluation of flags, necessary to proxy OFREP requests. +type inMemoryBulkProvider struct { + memprovider.InMemoryProvider + flags map[string]memprovider.InMemoryFlag +} + +func newInMemoryBulkProvider(flags map[string]memprovider.InMemoryFlag) *inMemoryBulkProvider { + return &inMemoryBulkProvider{ + InMemoryProvider: memprovider.NewInMemoryProvider(flags), + flags: flags, + } +} + +// ListFlags returns a list of all flags registered with the provider. +func (p *inMemoryBulkProvider) ListFlags() ([]string, error) { + keys := make([]string, 0, len(p.flags)) + for key := range p.flags { + keys = append(keys, key) + } + return keys, nil +} + func newStaticProvider(cfg *setting.Cfg) (openfeature.FeatureProvider, error) { confFlags, err := setting.ReadFeatureTogglesFromInitFile(cfg.Raw.Section("feature_toggles")) if err != nil { @@ -29,7 +52,7 @@ func newStaticProvider(cfg *setting.Cfg) (openfeature.FeatureProvider, error) { } } - return memprovider.NewInMemoryProvider(flags), nil + return newInMemoryBulkProvider(flags), nil } func createInMemoryFlag(name string, enabled bool) memprovider.InMemoryFlag { diff --git a/pkg/services/featuremgmt/static_provider_test.go b/pkg/services/featuremgmt/static_provider_test.go index 97ce98fa656..86d0eaa318c 100644 --- a/pkg/services/featuremgmt/static_provider_test.go +++ b/pkg/services/featuremgmt/static_provider_test.go @@ -56,3 +56,37 @@ func provider(t *testing.T, conf []byte) *OpenFeatureService { require.NoError(t, err) return p } + +func Test_CompareStaticProviderWithFeatureManager(t *testing.T) { + cfg := setting.NewCfg() + sec, err := cfg.Raw.NewSection("feature_toggles") + require.NoError(t, err) + _, err = sec.NewKey("ABCD", "true") + require.NoError(t, err) + + p, err := ProvideOpenFeatureService(cfg) + require.NoError(t, err) + + _, ok := p.provider.(*inMemoryBulkProvider) + if !ok { + t.Fatalf("expected inMemoryBulkProvider, got %T", p.provider) + } + + ctx := openfeature.WithTransactionContext(context.Background(), openfeature.NewEvaluationContext("grafana", nil)) + allFlags, err := p.EvalAllFlagsWithStaticProvider(ctx) + require.NoError(t, err) + + openFeatureEnabledFlags := map[string]bool{} + for _, flag := range allFlags.Flags { + if flag.Value { + openFeatureEnabledFlags[flag.Key] = true + } + } + + mgr, err := ProvideManagerService(cfg) + require.NoError(t, err) + + // compare enabled feature flags match between OpenFeature static provider and Feature Manager + enabledFeatureManager := mgr.GetEnabled(ctx) + assert.Equal(t, openFeatureEnabledFlags, enabledFeatureManager) +} diff --git a/pkg/setting/setting.go b/pkg/setting/setting.go index fcb680a0f4f..6e4baaefe1b 100644 --- a/pkg/setting/setting.go +++ b/pkg/setting/setting.go @@ -476,6 +476,9 @@ type Cfg struct { // Query history QueryHistoryEnabled bool + // Open feature settings + OpenFeature OpenFeatureSettings + Storage StorageSettings Search SearchSettings @@ -1319,6 +1322,11 @@ func (cfg *Cfg) parseINIFile(iniFile *ini.File) error { return err } + if err := cfg.readOpenFeatureSettings(); err != nil { + cfg.Logger.Error("Failed to read open feature settings", "error", err) + return err + } + cfg.readDataSourcesSettings() cfg.readDataSourceSecuritySettings() cfg.readK8sDashboardCleanupSettings() diff --git a/pkg/setting/setting_openfeature.go b/pkg/setting/setting_openfeature.go new file mode 100644 index 00000000000..13131f89111 --- /dev/null +++ b/pkg/setting/setting_openfeature.go @@ -0,0 +1,51 @@ +package setting + +import ( + "fmt" + "net/url" +) + +const ( + StaticProviderType = "static" + GOFFProviderType = "goff" +) + +type OpenFeatureSettings struct { + ProviderType string + URL *url.URL + TargetingKey string + ContextAttrs map[string]any +} + +func (cfg *Cfg) readOpenFeatureSettings() error { + cfg.OpenFeature = OpenFeatureSettings{} + + config := cfg.Raw.Section("feature_toggles.openfeature") + cfg.OpenFeature.ProviderType = config.Key("provider").MustString(StaticProviderType) + cfg.OpenFeature.TargetingKey = config.Key("targetingKey").MustString(cfg.AppURL) + + strURL := config.Key("url").MustString("") + + if strURL != "" && cfg.OpenFeature.ProviderType == GOFFProviderType { + u, err := url.Parse(strURL) + if err != nil { + return fmt.Errorf("invalid feature provider url: %w", err) + } + cfg.OpenFeature.URL = u + } + + // build the eval context attributes using [feature_toggles.openfeature.context] section + ctxConf := cfg.Raw.Section("feature_toggles.openfeature.context") + attrs := map[string]any{} + for _, key := range ctxConf.KeyStrings() { + attrs[key] = ctxConf.Key(key).String() + } + + // Some default attributes + if _, ok := attrs["grafana_version"]; !ok { + attrs["grafana_version"] = BuildVersion + } + + cfg.OpenFeature.ContextAttrs = attrs + return nil +} diff --git a/pkg/setting/setting_openfeature_test.go b/pkg/setting/setting_openfeature_test.go new file mode 100644 index 00000000000..f03db2f079a --- /dev/null +++ b/pkg/setting/setting_openfeature_test.go @@ -0,0 +1,57 @@ +package setting + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func Test_CtxAttrs(t *testing.T) { + testCases := []struct { + name string + conf string + expected map[string]any + }{ + { + name: "empty config - only default attributes should be present", + expected: map[string]any{ + "grafana_version": "", + }, + }, + { + name: "config with some attributes", + conf: ` +[feature_toggles.openfeature.context] +foo = bar +baz = qux +quux = corge`, + expected: map[string]any{ + "foo": "bar", + "baz": "qux", + "quux": "corge", + "grafana_version": "", + }, + }, + { + name: "config with an attribute that overrides a default one", + conf: ` +[feature_toggles.openfeature.context] +grafana_version = 10.0.0 +foo = bar`, + expected: map[string]any{ + "grafana_version": "10.0.0", + "foo": "bar", + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + cfg, err := NewCfgFromBytes([]byte(tc.conf)) + require.NoError(t, err) + + assert.Equal(t, tc.expected, cfg.OpenFeature.ContextAttrs) + }) + } +}