public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

addressing review comments

Browse Source
This commit is contained in:
grambbledook
2026-01-12 16:30:41 +01:00
parent 3d2a176847
commit 9358bbe040
8 changed files with 139 additions and 127 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/services/featuremgmt/models.go
+1
View File
@@ -129,6 +129,7 @@ func (s *FeatureFlagStage) UnmarshalJSON(b []byte) error {
type FeatureFlagType int
const (
// Boolean -- Type of a flag
Boolean FeatureFlagType = iota
Integer
Float
pkg/services/featuremgmt/openfeature.go
+3 -2
View File
@@ -8,6 +8,7 @@ import (
clientauthmiddleware "github.com/grafana/grafana/pkg/clientauth/middleware"
"github.com/grafana/grafana/pkg/setting"
"github.com/open-feature/go-sdk/openfeature/memprovider"
sdkhttpclient "github.com/grafana/grafana-plugin-sdk-go/backend/httpclient"
"github.com/open-feature/go-sdk/openfeature"
@@ -26,7 +27,7 @@ type OpenFeatureConfig struct {
// HTTPClient is a pre-configured HTTP client (optional, used by features-service + OFREP providers)
HTTPClient *http.Client
// StaticFlags are the feature flags to use with static provider
StaticFlags map[string]setting.FeatureToggle
StaticFlags map[string]memprovider.InMemoryFlag
// TargetingKey is used for evaluation context
TargetingKey string
// ContextAttrs are additional attributes for evaluation context
@@ -100,7 +101,7 @@ func InitOpenFeatureWithCfg(cfg *setting.Cfg) error {
func createProvider(
providerType string,
u *url.URL,
staticFlags map[string]setting.FeatureToggle,
staticFlags map[string]memprovider.InMemoryFlag,
httpClient *http.Client,
) (openfeature.FeatureProvider, error) {
if providerType == setting.FeaturesServiceProviderType || providerType == setting.OFREPProviderType {
pkg/services/featuremgmt/service.go
+1 -1
View File
@@ -47,7 +47,7 @@ func ProvideManagerService(cfg *setting.Cfg) (*FeatureManager, error) {
}
mgmt.warnings[key] = "unknown flag in config"
}
mgmt.startup[key] = val.Value == true
mgmt.startup[key] = setting.IsEnabled(val)
}
// update the values
pkg/services/featuremgmt/static_provider.go
+10 -56
View File
@@ -1,9 +1,8 @@
package featuremgmt
import (
"encoding/json"
"fmt"
"strconv"
"reflect"
"github.com/grafana/grafana/pkg/setting"
"github.com/open-feature/go-sdk/openfeature"
@@ -33,15 +32,12 @@ func (p *inMemoryBulkProvider) ListFlags() ([]string, error) {
return keys, nil
}
func newStaticProvider(confFlags map[string]setting.FeatureToggle, standardFlags []FeatureFlag) (openfeature.FeatureProvider, error) {
func newStaticProvider(confFlags map[string]memprovider.InMemoryFlag, standardFlags []FeatureFlag) (openfeature.FeatureProvider, error) {
flags := make(map[string]memprovider.InMemoryFlag, len(standardFlags))
index := make(map[string]FeatureFlag, len(standardFlags))
// Add standard flags
for _, flag := range standardFlags {
inMemFlag, err := createTypedFlag(flag)
if err != nil {
return nil, err
}
inMemFlag := setting.ParseFlag(flag.Name, flag.Expression)
flags[flag.Name] = inMemFlag
index[flag.Name] = flag
@@ -49,60 +45,18 @@ func newStaticProvider(confFlags map[string]setting.FeatureToggle, standardFlags
// Add flags from config.ini file
for name, flag := range confFlags {
standard, exists := index[flag.Name]
standard, exists := flags[flag.Key]
// Fail fast if a flag is declared with a mismatched type
if exists && standard.Type.String() != string(flag.Type) {
return nil, fmt.Errorf("type mismatch for flag '%s' detected", flag.Name)
standardValue, _ := setting.GetDefaultValue(standard)
flagValue, _ := setting.GetDefaultValue(flag)
if exists && reflect.TypeOf(standardValue) != reflect.TypeOf(flagValue) {
return nil, fmt.Errorf("type mismatch for flag '%s' detected", flag.Key)
}
flags[name] = createInMemoryFlag(flag)
flags[name] = flag
}
return newInMemoryBulkProvider(flags), nil
}
func createInMemoryFlag(flag setting.FeatureToggle) memprovider.InMemoryFlag {
variant := "default"
return memprovider.InMemoryFlag{
Key: flag.Name,
DefaultVariant: variant,
Variants: map[string]any{
variant: flag.Value,
},
}
}
func createTypedFlag(flag FeatureFlag) (memprovider.InMemoryFlag, error) {
defaultVariant := "default"
var value any
var err error
switch flag.Type {
case Boolean:
value = flag.Expression == "true"
case Integer:
value, err = strconv.Atoi(flag.Expression)
case Float:
value, err = strconv.ParseFloat(flag.Expression, 64)
case String:
value = flag.Expression
case Structure:
err = json.Unmarshal([]byte(flag.Expression), &value)
default:
return memprovider.InMemoryFlag{}, fmt.Errorf("unsupported flag type %s", flag.Type)
}
if err != nil {
return memprovider.InMemoryFlag{}, err
}
return memprovider.InMemoryFlag{
Key: flag.Name,
DefaultVariant: defaultVariant,
Variants: map[string]any{
defaultVariant: value,
},
}, nil
}
pkg/services/featuremgmt/static_provider_test.go
+21 -13
View File
@@ -5,6 +5,7 @@ import (
"testing"
"github.com/grafana/grafana/pkg/setting"
"github.com/open-feature/go-sdk/openfeature/memprovider"
"github.com/open-feature/go-sdk/openfeature"
"github.com/stretchr/testify/assert"
@@ -95,16 +96,17 @@ ABCD = true
}
func Test_StaticProvider_FailfastOnMismatchedType(t *testing.T) {
staticFlags := map[string]setting.FeatureToggle{"oldBooleanFlag": {
Type: setting.Boolean,
Name: "oldBooleanFlag",
Value: true,
staticFlags := map[string]memprovider.InMemoryFlag{"oldBooleanFlag": {
Key: "oldBooleanFlag",
DefaultVariant: setting.DefaultVariantName,
Variants: map[string]any{
setting.DefaultVariantName: true,
},
}}
flag := FeatureFlag{
Name: "oldBooleanFlag",
Expression: "1.0",
Type: Float,
}
_, err := newStaticProvider(staticFlags, []FeatureFlag{flag})
assert.EqualError(t, err, "type mismatch for flag 'oldBooleanFlag' detected")
@@ -201,7 +203,7 @@ func Test_StaticProvider_ConfigOverride(t *testing.T) {
name: "int",
typ: Integer,
originalValue: "0",
configValue: int64(1),
configValue: 1,
},
{
name: "float",
@@ -251,20 +253,26 @@ func makeFlags(tt struct {
typ FeatureFlagType
originalValue string
configValue any
}) (map[string]setting.FeatureToggle, []FeatureFlag) {
}) (map[string]memprovider.InMemoryFlag, []FeatureFlag) {
orig := FeatureFlag{
Name: tt.name,
Expression: tt.originalValue,
Type: tt.typ,
}
config := map[string]setting.FeatureToggle{
tt.name: {
Name: tt.name,
Type: setting.FeatureFlagType(tt.typ.String()),
Value: tt.configValue,
},
config := map[string]memprovider.InMemoryFlag{
tt.name: makeInMemoryFlag(tt.name, tt.configValue),
}
return config, []FeatureFlag{orig}
}
func makeInMemoryFlag(name string, value any) memprovider.InMemoryFlag {
return memprovider.InMemoryFlag{
Key: name,
DefaultVariant: setting.DefaultVariantName,
Variants: map[string]any{
setting.DefaultVariantName: value,
},
}
}
pkg/services/updatemanager/plugins_test.go
+4 -2
View File
@@ -9,7 +9,9 @@ import (
"sync"
"testing"
"cuelang.org/go/pkg/strconv"
"github.com/open-feature/go-sdk/openfeature"
"github.com/open-feature/go-sdk/openfeature/memprovider"
"github.com/stretchr/testify/require"
"github.com/grafana/grafana/pkg/infra/log"
@@ -378,8 +380,8 @@ func setupOpenFeatureProvider(t *testing.T, flagValue bool) {
err := featuremgmt.InitOpenFeature(featuremgmt.OpenFeatureConfig{
ProviderType: setting.StaticProviderType,
StaticFlags: map[string]setting.FeatureToggle{
featuremgmt.FlagPluginsAutoUpdate: {Value: flagValue, Type: setting.Boolean},
StaticFlags: map[string]memprovider.InMemoryFlag{
featuremgmt.FlagPluginsAutoUpdate: setting.ParseFlag(featuremgmt.FlagPluginsAutoUpdate, strconv.FormatBool(flagValue)),
},
})
require.NoError(t, err)
pkg/setting/setting_feature_toggles.go
+61 -25
View File
@@ -2,9 +2,10 @@ package setting
import (
"encoding/json"
"fmt"
"errors"
"strconv"
"github.com/open-feature/go-sdk/openfeature/memprovider"
"gopkg.in/ini.v1"
"github.com/grafana/grafana/pkg/util"
@@ -20,6 +21,8 @@ const (
String FeatureFlagType = "string"
)
const DefaultVariantName = ""
type FeatureToggle struct {
Type FeatureFlagType `json:"type"`
Name string `json:"name"`
@@ -33,6 +36,7 @@ func (cfg *Cfg) readFeatureToggles(iniFile *ini.File) error {
if err != nil {
return err
}
// TODO IsFeatureToggleEnabled has been deprecated for 2 years now, we should remove this function completely
// nolint:staticcheck
cfg.IsFeatureToggleEnabled = func(key string) bool {
@@ -41,22 +45,19 @@ func (cfg *Cfg) readFeatureToggles(iniFile *ini.File) error {
return false
}
return toggle.Type == Boolean && toggle.Value == true
return IsEnabled(toggle)
}
return nil
}
func ReadFeatureTogglesFromInitFile(featureTogglesSection *ini.Section) (map[string]FeatureToggle, error) {
featureToggles := make(map[string]FeatureToggle, 10)
func ReadFeatureTogglesFromInitFile(featureTogglesSection *ini.Section) (map[string]memprovider.InMemoryFlag, error) {
featureToggles := make(map[string]memprovider.InMemoryFlag, 10)
// parse the comma separated list in `enable`.
featuresTogglesStr := valueAsString(featureTogglesSection, "enable", "")
for _, feature := range util.SplitString(featuresTogglesStr) {
featureToggles[feature] = FeatureToggle{
Type: Boolean,
Name: feature,
Value: true,
}
featureToggles[feature] = createFlag(feature, true)
}
// read all other settings under [feature_toggles]. If a toggle is
@@ -66,36 +67,71 @@ func ReadFeatureTogglesFromInitFile(featureTogglesSection *ini.Section) (map[str
continue
}
b, err := ParseFlag(v.Name(), v.Value())
if err != nil {
return featureToggles, err
}
flag, exists := featureToggles[v.Name()]
if exists && flag.Type != b.Type {
return nil, fmt.Errorf("type mismatch during flag declaration '%s': %s, %s", v.Name(), flag.Type, b.Type)
}
b := ParseFlag(v.Name(), v.Value())
featureToggles[v.Name()] = b
}
return featureToggles, nil
}
func ParseFlag(name, value string) (FeatureToggle, error) {
func ParseFlag(name, value string) memprovider.InMemoryFlag {
var structure map[string]any
if integer, err := strconv.Atoi(value); err == nil {
return FeatureToggle{Type: Integer, Name: name, Value: integer}, nil
return createFlag(name, integer)
}
if float, err := strconv.ParseFloat(value, 64); err == nil {
return FeatureToggle{Type: Float, Name: name, Value: float}, nil
return createFlag(name, float)
}
if err := json.Unmarshal([]byte(value), &structure); err == nil {
return FeatureToggle{Type: Structure, Name: name, Value: structure}, nil
return createFlag(name, structure)
}
if boolean, err := strconv.ParseBool(value); err == nil {
return FeatureToggle{Type: Boolean, Name: name, Value: boolean}, nil
return createFlag(name, boolean)
}
return FeatureToggle{Type: String, Name: name, Value: value}, nil
return createFlag(name, value)
}
func SerializeFlag(flag memprovider.InMemoryFlag) string {
value, _ := flag.Variants[DefaultVariantName]
switch castedValue := value.(type) {
case bool:
return strconv.FormatBool(castedValue)
case int64:
return strconv.FormatInt(castedValue, 10)
case float64:
return strconv.FormatFloat(castedValue, 'f', -1, 64)
case string:
return castedValue
default:
val, _ := json.Marshal(value)
return string(val)
}
}
func createFlag(name string, value any) memprovider.InMemoryFlag {
return memprovider.InMemoryFlag{
Key: name,
DefaultVariant: DefaultVariantName,
Variants: map[string]any{
DefaultVariantName: value,
},
}
}
func GetDefaultValue(flag memprovider.InMemoryFlag) (any, error) {
if value, ok := flag.Variants[flag.DefaultVariant]; !ok {
return nil, errors.New("no default variant found")
} else {
return value, nil
}
}
func IsEnabled(flag memprovider.InMemoryFlag) bool {
if value, ok := flag.Variants[flag.DefaultVariant]; !ok {
return false
} else {
return value == true
}
}
pkg/setting/setting_feature_toggles_test.go
+38 -28
View File
@@ -1,9 +1,9 @@
package setting
import (
"errors"
"testing"
"github.com/open-feature/go-sdk/openfeature/memprovider"
"github.com/stretchr/testify/require"
"gopkg.in/ini.v1"
)
@@ -13,16 +13,16 @@ func TestFeatureToggles(t *testing.T) {
name string
conf map[string]string
err error
expectedToggles map[string]FeatureToggle
expectedToggles map[string]memprovider.InMemoryFlag
}{
{
name: "can parse feature toggles passed in the `enable` array",
conf: map[string]string{
"enable": "feature1,feature2",
},
expectedToggles: map[string]FeatureToggle{
"feature1": {Name: "feature1", Type: Boolean, Value: true},
"feature2": {Name: "feature2", Type: Boolean, Value: true},
expectedToggles: map[string]memprovider.InMemoryFlag{
"feature1": makeInMemoryFlag("feature1", true),
"feature2": makeInMemoryFlag("feature2", true),
},
},
{
@@ -31,10 +31,10 @@ func TestFeatureToggles(t *testing.T) {
"enable": "feature1,feature2",
"feature3": "true",
},
expectedToggles: map[string]FeatureToggle{
"feature1": {Name: "feature1", Type: Boolean, Value: true},
"feature2": {Name: "feature2", Type: Boolean, Value: true},
"feature3": {Name: "feature3", Type: Boolean, Value: true},
expectedToggles: map[string]memprovider.InMemoryFlag{
"feature1": makeInMemoryFlag("feature1", true),
"feature2": makeInMemoryFlag("feature2", true),
"feature3": makeInMemoryFlag("feature3", true),
},
},
{
@@ -43,20 +43,20 @@ func TestFeatureToggles(t *testing.T) {
"enable": "feature1,feature2",
"feature2": "false",
},
expectedToggles: map[string]FeatureToggle{
"feature1": {Name: "feature1", Type: Boolean, Value: true},
"feature2": {Name: "feature2", Type: Boolean, Value: false},
expectedToggles: map[string]memprovider.InMemoryFlag{
"feature1": makeInMemoryFlag("feature1", true),
"feature2": makeInMemoryFlag("feature2", false),
},
},
{
name: "conflict in type declaration is be detected",
conf: map[string]string{
"enable": "feature1,feature2",
"feature2": "invalid",
},
expectedToggles: map[string]FeatureToggle{},
err: errors.New("type mismatch during flag declaration 'feature2': boolean, string"),
},
//{
// name: "conflict in type declaration is be detected",
// conf: map[string]string{
// "enable": "feature1,feature2",
// "feature2": "invalid",
// },
// expectedToggles: map[string]memprovider.InMemoryFlag{},
// err: errors.New("type mismatch during flag declaration 'feature2': boolean, string"),
//},
{
name: "type of the feature flag is handled correctly",
conf: map[string]string{
@@ -64,13 +64,13 @@ func TestFeatureToggles(t *testing.T) {
"feature3": `{"foo":"bar"}`, "feature4": "bar",
"feature5": "t", "feature6": "T",
},
expectedToggles: map[string]FeatureToggle{
"feature1": {Name: "feature1", Type: Integer, Value: 1},
"feature2": {Name: "feature2", Type: Float, Value: 1.0},
"feature3": {Name: "feature3", Type: Structure, Value: map[string]any{"foo": "bar"}},
"feature4": {Name: "feature4", Type: String, Value: "bar"},
"feature5": {Name: "feature5", Type: Boolean, Value: true},
"feature6": {Name: "feature6", Type: Boolean, Value: true},
expectedToggles: map[string]memprovider.InMemoryFlag{
"feature1": makeInMemoryFlag("feature1", 1),
"feature2": makeInMemoryFlag("feature2", 1.0),
"feature3": makeInMemoryFlag("feature3", map[string]any{"foo": "bar"}),
"feature4": makeInMemoryFlag("feature4", "bar"),
"feature5": makeInMemoryFlag("feature5", true),
"feature6": makeInMemoryFlag("feature6", true),
},
},
}
@@ -97,3 +97,13 @@ func TestFeatureToggles(t *testing.T) {
}
}
}
func makeInMemoryFlag(name string, value any) memprovider.InMemoryFlag {
return memprovider.InMemoryFlag{
Key: name,
DefaultVariant: DefaultVariantName,
Variants: map[string]any{
DefaultVariantName: value,
},
}
}