fix: jitter backoff strategy reset (#9342) (#9459)

* fix: jitter backoff strategy reset

* add PR to changelog fragment

(cherry picked from commit 23236ef)

Co-authored-by: Ruben Ruiz de Gauna <[email protected]>

Author: mergify[bot]

changelog/fragments/1755076749-fix-jitter-backoff-strategy-reset.yaml

@@ -0,0 +1,32 @@
+# Kind can be one of:
+# - breaking-change: a change to previously-documented behavior
+# - deprecation: functionality that is being removed in a later release
+# - bug-fix: fixes a problem in a previous version
+# - enhancement: extends functionality but does not break or fix existing behavior
+# - feature: new functionality
+# - known-issue: problems that we are aware of in a given version
+# - security: impacts on the security of a product or a user’s deployment.
+# - upgrade: important information for someone upgrading from a prior version
+# - other: does not fit into any of the other categories
+kind: bug-fix
+
+# Change summary; a 80ish characters long description of the change.
+summary: Fixed jitter backoff strategy reset
+
+# Long description; in case the summary is not enough to describe the change
+# this field accommodate a description without length limits.
+# NOTE: This field will be rendered only for breaking-change and known-issue kinds at the moment.
+#description:
+
+# Affected component; usually one of "elastic-agent", "fleet-server", "filebeat", "metricbeat", "auditbeat", "all", etc.
+component: elastic-agent
+
+# PR URL; optional; the PR number that added the changeset.
+# If not present is automatically filled by the tooling finding the PR where this changelog fragment has been added.
+# NOTE: the tooling supports backports, so it's able to fill the original PR number instead of the backport PR number.
+# Please provide it if you are adding a fragment for a different PR.
+pr: https://github.com/elastic/elastic-agent/pull/9342
+
+# Issue URL; optional; the GitHub issue related to this changeset (either closes or is part of).
+# If not present is automatically filled by the tooling with the issue linked to the PR number.
+issue: https://github.com/elastic/elastic-agent/issues/8864

internal/pkg/core/backoff/backoff_test.go

@@ -32,7 +32,7 @@ func TestCloseChannel(t *testing.T) {
 			c := make(chan struct{})
 			b := f(c)
 			close(c)
-			assert.False(t, b.Wait(), "should return false because the channel shuld get closed faster than the next wait duration")
+			assert.False(t, b.Wait(), "should return false because the channel should get closed faster than the next wait duration")
 		})
 	}
 }
@@ -96,3 +96,46 @@ func TestNextWait(t *testing.T) {
 		})
 	}
 }
+
+func TestReset(t *testing.T) {
+	init := time.Millisecond
+	max := 5 * time.Second
+
+	tests := map[string]factory{
+		"ExpBackoff": func(done <-chan struct{}) Backoff {
+			return NewExpBackoff(done, init, max)
+		},
+		"EqualJitterBackoff": func(done <-chan struct{}) Backoff {
+			// Use a custom randFn to ensure the jitter value is always the same for testing
+			// purposes. Constructor uses rand.N which is not deterministic.
+			bo := &EqualJitterBackoff{
+				done: done,
+				init: init,
+				max:  max,
+				// force the random function to return the max value for deterministic behavior
+				randFn: func(t time.Duration) time.Duration { return t },
+			}
+			bo.Reset()
+			return bo
+		},
+	}
+
+	for name, f := range tests {
+		t.Run(name, func(t *testing.T) {
+			c := make(chan struct{})
+			b := f(c)
+
+			startWait := b.NextWait()
+
+			b.Wait()
+			nextWait := b.NextWait()
+			t.Logf("startWait: %s nextWait: %s", startWait, nextWait)
+			assert.Greater(t, nextWait, startWait, "nextWait value did not increase after Wait")
+
+			b.Reset()
+			restartWait := b.NextWait()
+			t.Logf("startWait: %s restartWait: %s", startWait, restartWait)
+			assert.Equal(t, startWait, restartWait, "wait value did not reset to initial value after reset")
+		})
+	}
+}

internal/pkg/core/backoff/equal_jitter.go

@@ -9,6 +9,10 @@ import (
 	"time"
 )
 
+// randFn defines a signature for functions that return a random duration.
+// It's used to allow injecting a deterministic random function for testing purposes.
+type randFn func(n time.Duration) time.Duration
+
 // EqualJitterBackoff implements an equal jitter strategy, meaning the wait time will consist of two parts,
 // the first will be exponential and the other half will be random and will provide the jitter
 // necessary to distribute the wait on remote endpoint.
@@ -21,23 +25,29 @@ type EqualJitterBackoff struct {
 	nextRand time.Duration
 
 	last time.Time
+
+	// Expose randFn to ensure the jitter is always the same for testing
+	// purposes. Constructor uses rand.N which is not deterministic.
+	randFn randFn
 }
 
 // NewEqualJitterBackoff returns a new EqualJitter object.
 func NewEqualJitterBackoff(done <-chan struct{}, init, max time.Duration) Backoff {
-	return &EqualJitterBackoff{
-		duration: init * 2, // Allow to sleep at least the init period on the first wait.
-		done:     done,
-		init:     init,
-		max:      max,
-		nextRand: rand.N(init),
+	bo := &EqualJitterBackoff{
+		done:   done,
+		init:   init,
+		max:    max,
+		randFn: rand.N[time.Duration],
 	}
+	bo.Reset()
+	return bo
 }
 
 // Reset resets the duration of the backoff.
 func (b *EqualJitterBackoff) Reset() {
 	// Allow to sleep at least the init period on the first wait.
 	b.duration = b.init * 2
+	b.nextRand = b.randFn(b.init)
 }
 
 func (b *EqualJitterBackoff) NextWait() time.Duration {
@@ -51,7 +61,7 @@ func (b *EqualJitterBackoff) Wait() bool {
 	backoff := b.NextWait()
 
 	// increase duration for next wait.
-	b.nextRand = rand.N(b.duration)
+	b.nextRand = b.randFn(b.duration)
 	b.duration *= 2
 	if b.duration > b.max {
 		b.duration = b.max