diff --git a/cmd/relayproxy/config/config.go b/cmd/relayproxy/config/config.go index a11cc7a0c90..404e77416fe 100644 --- a/cmd/relayproxy/config/config.go +++ b/cmd/relayproxy/config/config.go @@ -175,6 +175,14 @@ type Config struct { // Default: false EnablePollingJitter bool `mapstructure:"enablePollingJitter" koanf:"enablepollingjitter"` + // DisableNotifierOnInit (optional) set to true if you do not want to call any notifier + // when the flags are loaded. + // This is useful if you do not want a Slack/Webhook notification saying that + // the flags have been added every time you start the application. + // Default is set to false for backward compatibility. + // Default: false + DisableNotifierOnInit bool `mapstructure:"DisableNotifierOnInit" koanf:"DisableNotifierOnInit"` + // FileFormat (optional) is the format of the file to retrieve (available YAML, TOML and JSON) // Default: YAML FileFormat string `mapstructure:"fileFormat" koanf:"fileformat"` diff --git a/cmd/relayproxy/service/gofeatureflag.go b/cmd/relayproxy/service/gofeatureflag.go index ca0d4dd6cac..f8bd96d8218 100644 --- a/cmd/relayproxy/service/gofeatureflag.go +++ b/cmd/relayproxy/service/gofeatureflag.go @@ -92,6 +92,7 @@ func NewGoFeatureFlagClient( DataExporter: exp, StartWithRetrieverError: proxyConf.StartWithRetrieverError, EnablePollingJitter: proxyConf.EnablePollingJitter, + DisableNotifierOnInit: proxyConf.DisableNotifierOnInit, EvaluationContextEnrichment: proxyConf.EvaluationContextEnrichment, PersistentFlagConfigurationFile: proxyConf.PersistentFlagConfigurationFile, } diff --git a/config.go b/config.go index 145f077a885..159daf8617c 100644 --- a/config.go +++ b/config.go @@ -28,6 +28,14 @@ type Config struct { // Default: false EnablePollingJitter bool + // DisableNotifierOnInit (optional) set to true if you do not want to call any notifier + // when the flags are loaded. + // This is useful if you do not want a Slack/Webhook notification saying that + // the flags have been added every time you start the application. + // Default is set to false for backward compatibility. + // Default: false + DisableNotifierOnInit bool + // Deprecated: Use LeveledLogger instead // Logger (optional) logger use by the library // Default: No log diff --git a/feature_flag.go b/feature_flag.go index cd91c97be37..a2087a7e3bf 100644 --- a/feature_flag.go +++ b/feature_flag.go @@ -102,7 +102,7 @@ func New(config Config) (*GoFeatureFlag, error) { return nil, fmt.Errorf("impossible to initialize the retrievers, please check your configuration: %v", err) } - err = retrieveFlagsAndUpdateCache(goFF.config, goFF.cache, goFF.retrieverManager) + err = retrieveFlagsAndUpdateCache(goFF.config, goFF.cache, goFF.retrieverManager, true) if err != nil { switch { case config.PersistentFlagConfigurationFile != "": @@ -154,7 +154,7 @@ func retrievePersistentLocalDisk(ctx context.Context, config Config, goFF *GoFea return err } defer func() { _ = fallBackRetrieverManager.Shutdown(ctx) }() - err = retrieveFlagsAndUpdateCache(goFF.config, goFF.cache, fallBackRetrieverManager) + err = retrieveFlagsAndUpdateCache(goFF.config, goFF.cache, fallBackRetrieverManager, true) if err != nil { return err } @@ -192,7 +192,7 @@ func (g *GoFeatureFlag) startFlagUpdaterDaemon() { select { case <-g.bgUpdater.ticker.C: if !g.IsOffline() { - err := retrieveFlagsAndUpdateCache(g.config, g.cache, g.retrieverManager) + err := retrieveFlagsAndUpdateCache(g.config, g.cache, g.retrieverManager, false) if err != nil { g.config.internalLogger.Error("Error while updating the cache.", slog.Any("error", err)) } @@ -203,8 +203,13 @@ func (g *GoFeatureFlag) startFlagUpdaterDaemon() { } } -// retrieveFlagsAndUpdateCache is called every X seconds to refresh the cache flag. -func retrieveFlagsAndUpdateCache(config Config, cache cache.Manager, retrieverManager *retriever.Manager) error { +// retreiveFlags is a function that will retrieve the flags from the retrievers, +// merge them and convert them to the flag struct. +func retreiveFlags( + config Config, + cache cache.Manager, + retrieverManager *retriever.Manager, +) (map[string]dto.DTO, error) { retrievers := retrieverManager.GetRetrievers() // Results is the type that will receive the results when calling // all the retrievers. @@ -250,7 +255,7 @@ func retrieveFlagsAndUpdateCache(config Config, cache cache.Manager, retrieverMa retrieversResults := make([]map[string]dto.DTO, len(retrievers)) for v := range resultsChan { if v.Error != nil { - return v.Error + return nil, v.Error } retrieversResults[v.Index] = v.Value } @@ -262,8 +267,19 @@ func retrieveFlagsAndUpdateCache(config Config, cache cache.Manager, retrieverMa newFlags[flagName] = value } } + return newFlags, nil +} + +// retrieveFlagsAndUpdateCache is a function that retrieves the flags from the retrievers, +// and update the cache with the new flags. +func retrieveFlagsAndUpdateCache(config Config, cache cache.Manager, + retrieverManager *retriever.Manager, isInit bool) error { + newFlags, err := retreiveFlags(config, cache, retrieverManager) + if err != nil { + return err + } - err := cache.UpdateCache(newFlags, config.internalLogger) + err = cache.UpdateCache(newFlags, config.internalLogger, isInit && !config.DisableNotifierOnInit) if err != nil { log.Printf("error: impossible to update the cache of the flags: %v", err) return err @@ -286,7 +302,7 @@ func (g *GoFeatureFlag) ForceRefresh() bool { if g.IsOffline() { return false } - err := retrieveFlagsAndUpdateCache(g.config, g.cache, g.retrieverManager) + err := retrieveFlagsAndUpdateCache(g.config, g.cache, g.retrieverManager, false) if err != nil { g.config.internalLogger.Error("Error while force updating the cache.", slog.Any("error", err)) return false diff --git a/feature_flag_test.go b/feature_flag_test.go index 3c85d19b148..ede71d55589 100644 --- a/feature_flag_test.go +++ b/feature_flag_test.go @@ -14,6 +14,7 @@ import ( "github.com/thomaspoignant/go-feature-flag/ffcontext" "github.com/thomaspoignant/go-feature-flag/internal/flag" "github.com/thomaspoignant/go-feature-flag/model" + "github.com/thomaspoignant/go-feature-flag/notifier" "github.com/thomaspoignant/go-feature-flag/retriever" "github.com/thomaspoignant/go-feature-flag/retriever/fileretriever" "github.com/thomaspoignant/go-feature-flag/retriever/s3retriever" @@ -706,3 +707,53 @@ func Test_UseCustomBucketingKey(t *testing.T) { assert.Equal(t, want, got) } } + +func Test_DisableNotifierOnInit(t *testing.T) { + tests := []struct { + name string + config *ffclient.Config + disableNotification bool + expectedNotifyCalled bool + }{ + { + name: "DisableNotifierOnInit is true", + config: &ffclient.Config{ + PollingInterval: 60 * time.Second, + Retriever: &fileretriever.Retriever{Path: "testdata/flag-config.yaml"}, + DisableNotifierOnInit: true, + }, + expectedNotifyCalled: false, + }, + { + name: "DisableNotifierOnInit is false", + config: &ffclient.Config{ + PollingInterval: 60 * time.Second, + Retriever: &fileretriever.Retriever{Path: "testdata/flag-config.yaml"}, + DisableNotifierOnInit: false, + }, + expectedNotifyCalled: true, + }, + { + name: "DisableNotifierOnInit is not set", + config: &ffclient.Config{ + PollingInterval: 60 * time.Second, + Retriever: &fileretriever.Retriever{Path: "testdata/flag-config.yaml"}, + }, + expectedNotifyCalled: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockNotifier := &mock.Notifier{} + tt.config.Notifiers = []notifier.Notifier{mockNotifier} + + gffClient, err := ffclient.New(*tt.config) + assert.NoError(t, err) + defer gffClient.Close() + + time.Sleep(2 * time.Second) // wait for the goroutine to call Notify() + assert.Equal(t, tt.expectedNotifyCalled, mockNotifier.GetNotifyCalls() > 0) + }) + } +} diff --git a/internal/cache/cache_manager.go b/internal/cache/cache_manager.go index 8229563caf1..836b5e5ac5e 100644 --- a/internal/cache/cache_manager.go +++ b/internal/cache/cache_manager.go @@ -19,7 +19,7 @@ import ( type Manager interface { ConvertToFlagStruct(loadedFlags []byte, fileFormat string) (map[string]dto.DTO, error) - UpdateCache(newFlags map[string]dto.DTO, log *fflog.FFLogger) error + UpdateCache(newFlags map[string]dto.DTO, log *fflog.FFLogger, notifyChanges bool) error Close() GetFlag(key string) (flag.Flag, error) AllFlags() (map[string]flag.Flag, error) @@ -60,7 +60,7 @@ func (c *cacheManagerImpl) ConvertToFlagStruct(loadedFlags []byte, fileFormat st return newFlags, err } -func (c *cacheManagerImpl) UpdateCache(newFlags map[string]dto.DTO, log *fflog.FFLogger) error { +func (c *cacheManagerImpl) UpdateCache(newFlags map[string]dto.DTO, log *fflog.FFLogger, notifyChanges bool) error { newCache := NewInMemoryCache(c.logger) newCache.Init(newFlags) newCacheFlags := newCache.All() @@ -75,8 +75,10 @@ func (c *cacheManagerImpl) UpdateCache(newFlags map[string]dto.DTO, log *fflog.F c.latestUpdate = time.Now() c.mutex.Unlock() - // notify the changes - c.notificationService.Notify(oldCacheFlags, newCacheFlags, log) + if notifyChanges { + // notify the changes + c.notificationService.Notify(oldCacheFlags, newCacheFlags, log) + } // persist the cache on disk if c.persistentFlagConfigurationFile != "" { c.PersistCache(oldCacheFlags, newCacheFlags) diff --git a/internal/cache/cache_manager_test.go b/internal/cache/cache_manager_test.go index e807628221c..4c3c29ccb71 100644 --- a/internal/cache/cache_manager_test.go +++ b/internal/cache/cache_manager_test.go @@ -11,6 +11,7 @@ import ( "github.com/thomaspoignant/go-feature-flag/internal/flag" "github.com/thomaspoignant/go-feature-flag/model/dto" "github.com/thomaspoignant/go-feature-flag/notifier" + "github.com/thomaspoignant/go-feature-flag/testutils/mock" "github.com/thomaspoignant/go-feature-flag/testutils/testconvert" "github.com/thomaspoignant/go-feature-flag/utils/fflog" "gopkg.in/yaml.v3" @@ -254,7 +255,7 @@ variation = "false_var" assert.Error(t, err) return } - err = fCache.UpdateCache(newFlags, nil) + err = fCache.UpdateCache(newFlags, nil, true) if tt.wantErr { assert.Error(t, err, "UpdateCache() error = %v, wantErr %v", err, tt.wantErr) return @@ -417,7 +418,7 @@ test-flag2: assert.Error(t, err) return } - err = fCache.UpdateCache(newFlags, &fflog.FFLogger{LeveledLogger: slog.Default()}) + err = fCache.UpdateCache(newFlags, &fflog.FFLogger{LeveledLogger: slog.Default()}, true) assert.NoError(t, err) allFlags, err := fCache.AllFlags() @@ -455,7 +456,7 @@ func Test_cacheManagerImpl_GetLatestUpdateDate(t *testing.T) { fCache := cache.New(cache.NewNotificationService([]notifier.Notifier{}), "", nil) timeBefore := fCache.GetLatestUpdateDate() newFlags, _ := fCache.ConvertToFlagStruct(loadedFlags, "yaml") - _ = fCache.UpdateCache(newFlags, &fflog.FFLogger{LeveledLogger: slog.Default()}) + _ = fCache.UpdateCache(newFlags, &fflog.FFLogger{LeveledLogger: slog.Default()}, true) timeAfter := fCache.GetLatestUpdateDate() assert.True(t, timeBefore.Before(timeAfter)) @@ -485,7 +486,7 @@ func Test_persistCacheAndRestartCacheWithIt(t *testing.T) { assert.NoError(t, err) fCache := cache.New(cache.NewNotificationService([]notifier.Notifier{}), file.Name(), nil) - err = fCache.UpdateCache(loadedFlagsMap, &fflog.FFLogger{LeveledLogger: slog.Default()}) + err = fCache.UpdateCache(loadedFlagsMap, &fflog.FFLogger{LeveledLogger: slog.Default()}, true) assert.NoError(t, err) allFlags1, err := fCache.AllFlags() assert.NoError(t, err) @@ -499,7 +500,7 @@ func Test_persistCacheAndRestartCacheWithIt(t *testing.T) { loadedFlagsMap2 := map[string]dto.DTO{} err = yaml.Unmarshal(content, &loadedFlagsMap) assert.NoError(t, err) - err = fCache2.UpdateCache(loadedFlagsMap2, &fflog.FFLogger{LeveledLogger: slog.Default()}) + err = fCache2.UpdateCache(loadedFlagsMap2, &fflog.FFLogger{LeveledLogger: slog.Default()}, true) assert.NoError(t, err) allFlags2, err := fCache.AllFlags() assert.NoError(t, err) @@ -507,3 +508,134 @@ func Test_persistCacheAndRestartCacheWithIt(t *testing.T) { // Compare the 2 caches assert.Equal(t, allFlags1, allFlags2) } + +func TestCacheManager_UpdateCache(t *testing.T) { + tests := []struct { + name string + initialFlags map[string]dto.DTO + updatedFlags map[string]dto.DTO + }{ + { + name: "Update existing flags", + initialFlags: map[string]dto.DTO{ + "flag1": { + DTOv1: dto.DTOv1{ + Variations: &map[string]*interface{}{}, + DefaultRule: &flag.Rule{ + VariationResult: testconvert.String("true"), + }, + }, + }, + }, + updatedFlags: map[string]dto.DTO{ + "flag1": { + DTOv1: dto.DTOv1{ + Variations: &map[string]*interface{}{ + "true": testconvert.Interface(true), + }, + DefaultRule: &flag.Rule{ + VariationResult: testconvert.String("true"), + }, + }, + }, + "flag2": { + DTOv1: dto.DTOv1{ + Variations: &map[string]*interface{}{ + "false": testconvert.Interface(false), + }, + DefaultRule: &flag.Rule{ + VariationResult: testconvert.String("false"), + }, + }, + }, + }, + }, + { + name: "Empty initial flags", + initialFlags: map[string]dto.DTO{}, + updatedFlags: map[string]dto.DTO{ + "flag1": { + DTOv1: dto.DTOv1{ + Variations: &map[string]*interface{}{ + "true": testconvert.Interface(true), + }, + DefaultRule: &flag.Rule{ + VariationResult: testconvert.String("true"), + }, + }, + }, + }, + }, + { + name: "Remove a flag", + initialFlags: map[string]dto.DTO{ + "flag1": { + DTOv1: dto.DTOv1{ + Variations: &map[string]*interface{}{ + "true": testconvert.Interface(true), + }, + DefaultRule: &flag.Rule{ + VariationResult: testconvert.String("true"), + }, + }, + }, + "flag2": { + DTOv1: dto.DTOv1{ + Variations: &map[string]*interface{}{ + "false": testconvert.Interface(false), + }, + DefaultRule: &flag.Rule{ + VariationResult: testconvert.String("false"), + }, + }, + }, + }, + updatedFlags: map[string]dto.DTO{ + "flag1": { + DTOv1: dto.DTOv1{ + Variations: &map[string]*interface{}{ + "true": testconvert.Interface(true), + }, + DefaultRule: &flag.Rule{ + VariationResult: testconvert.String("true"), + }, + }, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Test UpdateCache without notification + mockNotificationService := &mock.NotificationService{} + cm := cache.New(mockNotificationService, "", &fflog.FFLogger{LeveledLogger: slog.Default()}) + + err := cm.UpdateCache(tt.initialFlags, nil, false) + assert.NoError(t, err) + + err = cm.UpdateCache(tt.updatedFlags, nil, false) + assert.NoError(t, err) + assert.Equal(t, 0, mockNotificationService.GetNotifyCalls(), "Notify should not be called for UpdateCache") + + flags, err := cm.AllFlags() + assert.NoError(t, err) + assert.Len(t, flags, len(tt.updatedFlags), "Cache should be updated with correct number of flags") + + // Test UpdateCacache with notification + mockNotificationService = &mock.NotificationService{} + cm = cache.New(mockNotificationService, "", &fflog.FFLogger{LeveledLogger: slog.Default()}) + + err = cm.UpdateCache(tt.initialFlags, nil, false) + assert.NoError(t, err) + + err = cm.UpdateCache(tt.updatedFlags, nil, true) + assert.NoError(t, err) + assert.Equal(t, 1, mockNotificationService.GetNotifyCalls(), "Notify should be called once for UpdateCache with notification") + + flags, err = cm.AllFlags() + assert.NoError(t, err) + assert.Len(t, flags, len(tt.updatedFlags), "Cache should be updated with correct number of flags") + }) + } +} diff --git a/testutils/mock/notification_service_mock.go b/testutils/mock/notification_service_mock.go new file mode 100644 index 00000000000..2d027e1a0e4 --- /dev/null +++ b/testutils/mock/notification_service_mock.go @@ -0,0 +1,36 @@ +package mock + +import ( + "sync" + + "github.com/thomaspoignant/go-feature-flag/internal/flag" + "github.com/thomaspoignant/go-feature-flag/utils/fflog" +) + +type NotificationService struct { + NotifyCalls int + CloseCalled bool + mu sync.Mutex +} + +func (n *NotificationService) Notify(oldCache map[string]flag.Flag, newCache map[string]flag.Flag, log *fflog.FFLogger) { + n.mu.Lock() + defer n.mu.Unlock() + n.NotifyCalls++ +} + +func (n *NotificationService) Close() { + n.mu.Lock() + defer n.mu.Unlock() + n.CloseCalled = true +} + +func (n *NotificationService) GetNotifyCalls() int { + n.mu.Lock() + defer n.mu.Unlock() + return n.NotifyCalls +} + +func (n *NotificationService) WasCloseCalled() bool { + return n.CloseCalled +} diff --git a/testutils/mock/notifier_mock.go b/testutils/mock/notifier_mock.go index 14d102ce708..52100740de0 100644 --- a/testutils/mock/notifier_mock.go +++ b/testutils/mock/notifier_mock.go @@ -1,14 +1,25 @@ package mock import ( + "sync" + "github.com/thomaspoignant/go-feature-flag/notifier" ) type Notifier struct { - NumberCalls int + NotifyCalls int + mu sync.Mutex } func (n *Notifier) Notify(cache notifier.DiffCache) error { - n.NumberCalls++ + n.mu.Lock() + defer n.mu.Unlock() + n.NotifyCalls++ return nil } + +func (n *Notifier) GetNotifyCalls() int { + n.mu.Lock() + defer n.mu.Unlock() + return n.NotifyCalls +} diff --git a/variation_test.go b/variation_test.go index a95e6c0c97e..1bc8cd856a6 100644 --- a/variation_test.go +++ b/variation_test.go @@ -46,9 +46,10 @@ func (c *cacheMock) GetLatestUpdateDate() time.Time { func (c *cacheMock) ConvertToFlagStruct(_ []byte, _ string) (map[string]dto.DTO, error) { return nil, nil } -func (c *cacheMock) UpdateCache(_ map[string]dto.DTO, _ *fflog.FFLogger) error { +func (c *cacheMock) UpdateCache(_ map[string]dto.DTO, _ *fflog.FFLogger, _ bool) error { return nil } + func (c *cacheMock) Close() {} func (c *cacheMock) GetFlag(_ string) (flag.Flag, error) { return c.flag, c.err diff --git a/website/docs/go_module/configuration.md b/website/docs/go_module/configuration.md index 579c23c7cd5..918df560e6b 100644 --- a/website/docs/go_module/configuration.md +++ b/website/docs/go_module/configuration.md @@ -24,6 +24,7 @@ During the initialization you must give a [`ffclient.Config{}`](https://pkg.go.d | `PollingInterval` | (optional) Duration to wait before refreshing the flags.
The minimum polling interval is 1 second.
Default: **60 * time.Second** | | `EnablePollingJitter` | (optional) Set to true if you want to avoid having true periodicity when retrieving your flags. It is useful to avoid having spike on your flag configuration storage in case your application is starting multiple instance at the same time.
We ensure a deviation that is maximum ±10% of your polling interval.
Default: **false** | | `StartWithRetrieverError` | *(optional)* If **true**, the SDK will start even if we did not get any flags from the retriever. It will serve only default values until the retriever returns the flags.
The init method will not return any error if the flag file is unreachable.
Default: **false** | +| `DisableNotifierOnInit` | *(optional)* If **true**, the SDK will not call any notifier when the flags are loaded during initialization. This is useful if you do not want a Slack/Webhook notification saying that the flags have been added every time you start the application.
Default: **false** | | `Offline` | *(optional)* If **true**, the SDK will not try to retrieve the flag file and will not export any data. No notifications will be sent either.
Default: **false** | | `EvaluationContextEnrichment` |

*(optional)* It is a free `map[string]interface{}` field that will be merged with the evaluation context sent during the evaluations. It is useful to add common attributes to all the evaluation, such as a server version, environment, ...

All those fields will be included in the custom attributes of the evaluation context.

_If in the evaluation context you have a field with the same name, it will be overridden by the `evaluationContextEnrichment`._

_If you have a key `env` in your `EvaluationContextEnrichment` and you also have the `Environment` set in your configuration, the `env` key from `EvaluationContextEnrichment` will be ignored._

Default: **nil** | | `PersistentFlagConfigurationFile` | *(optional)* If set GO Feature Flag will store the flags configuration in this file to be able to serve the flags even if none of the retrievers is available during starting time.
By default, the flag configuration is not persisted and stays on the retriever system. By setting a file here, you ensure that GO Feature Flag will always start with a configuration but which can be out-dated.

_(example: `/tmp/goff_persist_conf.yaml`)_ | diff --git a/website/docs/relay_proxy/configure_relay_proxy.md b/website/docs/relay_proxy/configure_relay_proxy.md index b4fd789aa51..fa8e73a303c 100644 --- a/website/docs/relay_proxy/configure_relay_proxy.md +++ b/website/docs/relay_proxy/configure_relay_proxy.md @@ -38,6 +38,7 @@ ex: `AUTHORIZEDKEYS_EVALUATION=my-first-key,my-second-key`)_. | `listen` | int | `1031` | This is the port used by the relay proxy when starting the HTTP server. | | `pollingInterval` | int | `60000` | This is the time interval **in millisecond** when the relay proxy is reloading the configuration file.
The minimum time accepted is 1000 millisecond. | | `enablePollingJitter` | boolean | `false` | Set to true if you want to avoid having true periodicity when retrieving your flags. It is useful to avoid having spike on your flag configuration storage in case your application is starting multiple instance at the same time.
We ensure a deviation that is maximum ±10% of your polling interval.
Default: false | +| `DisableNotifierOnInit` | boolean | `false` | If **true**, the relay proxy will not call any notifier when the flags are loaded during initialization. This is useful if you do not want a Slack/Webhook notification saying that the flags have been added every time you start the proxy.
Default: **false** | | `hideBanner` | boolean | `false` | Should we display the beautiful **go-feature-flag** banner when starting the relay proxy | | `enableSwagger` | boolean | `false` | Enables Swagger for testing the APIs directly. If you are enabling Swagger you will have to provide the `host` configuration and the Swagger UI will be available at `http://:/swagger/`. | | `host` | string | `localhost` | This is the DNS you will use to access the relay proxy. This field is used by Swagger to query the API at the right place. |