Skip to content

Commit

Permalink
feat: Add config to allow surpressing notification on launch (flag ca…
Browse files Browse the repository at this point in the history
…che load) (#2534)

* Change the existing `UpdateCache` method to `UpdateCacheAndNotify`

* Add config to control whether to notify on application init

* Add a method to the cacheManager interface to update cache without notification

* Don't send notification on cache initialization if DisableNotificationOnInit is set to true

* Update docs to refer to the newly available configuration

* Fix golangci-lint errors

* Make the wait longer to ensure that async call has completed

This also aligns the interval with rest of the test code

* Add mutext to prevent race condition

* use wasNotifyCalled() instead of directly calling notifyCalled

* DisableNotificationOnInit -> DisableNotifierOnInit

* Reduce number of interface methods by exposing the internal parameter of `CacheManager`

* Introduce an additional parameter to `retrieveFlagsAndUpdateCache` to indicate initialization

* Add comment to `retrieveFlags` to indicate its functionality

* Use mocks from testutils/mock instead of creating mocks within the test files

* Remove unnecessary references to  / declarations of UpdateCacheAndNotify

* Fix README description to be more accurate about DisableNotifierOnInit

* Reduce length of the line to comply with linter

---------

Co-authored-by: Thomas Poignant <[email protected]>
  • Loading branch information
8ma10s and thomaspoignant authored Oct 24, 2024
1 parent a1955f0 commit 1351a3f
Show file tree
Hide file tree
Showing 12 changed files with 288 additions and 20 deletions.
8 changes: 8 additions & 0 deletions cmd/relayproxy/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down
1 change: 1 addition & 0 deletions cmd/relayproxy/service/gofeatureflag.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ func NewGoFeatureFlagClient(
DataExporter: exp,
StartWithRetrieverError: proxyConf.StartWithRetrieverError,
EnablePollingJitter: proxyConf.EnablePollingJitter,
DisableNotifierOnInit: proxyConf.DisableNotifierOnInit,
EvaluationContextEnrichment: proxyConf.EvaluationContextEnrichment,
PersistentFlagConfigurationFile: proxyConf.PersistentFlagConfigurationFile,
}
Expand Down
8 changes: 8 additions & 0 deletions config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
32 changes: 24 additions & 8 deletions feature_flag.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 != "":
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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))
}
Expand All @@ -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.
Expand Down Expand Up @@ -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
}
Expand All @@ -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
Expand All @@ -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
Expand Down
51 changes: 51 additions & 0 deletions feature_flag_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
})
}
}
10 changes: 6 additions & 4 deletions internal/cache/cache_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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()
Expand 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)
Expand Down
Loading

0 comments on commit 1351a3f

Please sign in to comment.