feat: Add config to allow surpressing notification on launch (flag cache load) (thomaspoignant#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]>

Author: 8ma10s

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"`

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,
 	}

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

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

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)
+		})
+	}
+}

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)