Skip to content

Commit

Permalink
[ASCII-1249] Do not trigger config notification if the value is the s…
Browse files Browse the repository at this point in the history
…ame (#23383)

* feat: do not trigger config notification if the value is the same

* fix: use proper type in test
  • Loading branch information
pgimalac authored Mar 5, 2024
1 parent 8a6ee13 commit 73e2222
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ func TestConfigRefresh(t *testing.T) {
ia := getTestInventoryPayload(t, nil, nil)

assert.False(t, ia.RefreshTriggered())
pkgconfig.Datadog.Set("inventories_max_interval", 10*time.Minute, pkgconfigmodel.SourceAgentRuntime)
pkgconfig.Datadog.Set("inventories_max_interval", 10*60, pkgconfigmodel.SourceAgentRuntime)
assert.True(t, ia.RefreshTriggered())
}

Expand Down
11 changes: 9 additions & 2 deletions pkg/config/model/viper.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"bytes"
"fmt"
"io"
"reflect"
"strconv"
"strings"
"sync"
Expand Down Expand Up @@ -75,8 +76,9 @@ type safeConfig struct {
configEnvVars map[string]struct{}
}

// OnUpdate adds a callback to the list receivers to be called each time a value is change in the configuration
// OnUpdate adds a callback to the list receivers to be called each time a value is changed in the configuration
// by a call to the 'Set' method.
// Callbacks are only called if the value is effectively changed.
func (c *safeConfig) OnUpdate(callback NotificationReceiver) {
c.Lock()
defer c.Unlock()
Expand All @@ -93,9 +95,14 @@ func (c *safeConfig) Set(key string, value interface{}, source Source) {
// modify the config then release the lock to avoid deadlocks
var receivers []NotificationReceiver
c.Lock()
previousValue := c.Viper.Get(key)
c.configSources[source].Set(key, value)
c.mergeViperInstances(key)
receivers = slices.Clone(c.notificationReceivers)
newValue := c.Viper.Get(key)
if !reflect.DeepEqual(previousValue, newValue) {
// if the value has not changed, do not duplicate the slice so that no callback is called
receivers = slices.Clone(c.notificationReceivers)
}
c.Unlock()

// notifying all receiver about the updated setting
Expand Down
14 changes: 14 additions & 0 deletions pkg/config/model/viper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -332,3 +332,17 @@ func TestNotification(t *testing.T) {
assert.Equal(t, []string{"foo", "foo", "foo2"}, updatedKeyCB1)
assert.Equal(t, []string{"foo", "foo2"}, updatedKeyCB2)
}

func TestNotificationNoChange(t *testing.T) {
config := NewConfig("test", "DD", strings.NewReplacer(".", "_"))

updatedKeyCB1 := []string{}

config.OnUpdate(func(key string) { updatedKeyCB1 = append(updatedKeyCB1, key) })

config.Set("foo", "bar", SourceFile)
assert.Equal(t, []string{"foo"}, updatedKeyCB1)

config.Set("foo", "bar", SourceFile)
assert.Equal(t, []string{"foo"}, updatedKeyCB1)
}

0 comments on commit 73e2222

Please sign in to comment.