diff --git a/providers/aws/recommendations/ratelimiter.go b/providers/aws/recommendations/ratelimiter.go index 9f90c40dc..d24ab4c06 100644 --- a/providers/aws/recommendations/ratelimiter.go +++ b/providers/aws/recommendations/ratelimiter.go @@ -4,7 +4,7 @@ import ( "context" "errors" "math" - "math/rand" + "math/rand/v2" "strings" "time" ) @@ -75,8 +75,7 @@ func (r *RateLimiter) Wait(ctx context.Context) error { // Add jitter (up to 20% of delay). Math/rand is intentional: jitter only // needs uniform distribution, not cryptographic randomness. - jitter := time.Duration(float64(delay) * 0.2 * rand.Float64()) // #nosec G404 - delay += jitter + delay += computeJitter(delay, rand.Float64()) // #nosec G404 select { case <-time.After(delay): @@ -86,6 +85,13 @@ func (r *RateLimiter) Wait(ctx context.Context) error { } } +// computeJitter returns the jitter component to add to a backoff delay. +// r is a uniform random value in [0, 1); jitter is up to 20% of delay. +// Extracted as a pure function so it can be unit-tested without sleeping. +func computeJitter(delay time.Duration, r float64) time.Duration { + return time.Duration(float64(delay) * 0.2 * r) +} + // ShouldRetry checks if we should retry based on error type and retry count. // Only throttling and transient errors are retried. func (r *RateLimiter) ShouldRetry(err error) bool { diff --git a/providers/aws/recommendations/ratelimiter_test.go b/providers/aws/recommendations/ratelimiter_test.go index 7353ffa1c..f8be208f7 100644 --- a/providers/aws/recommendations/ratelimiter_test.go +++ b/providers/aws/recommendations/ratelimiter_test.go @@ -287,3 +287,35 @@ func TestRateLimiter_ExhaustsRetries(t *testing.T) { assert.Equal(t, 2, limiter.GetRetryCount()) assert.Error(t, lastErr) } + +// TestComputeJitter verifies that computeJitter produces values in [0, 0.2*delay] +// and that the output varies (i.e. is not stuck at zero). The test is deterministic: +// it calls the pure function directly and never sleeps, so OS scheduling cannot cause +// spurious failures (replaces the former wall-clock-based TestWait_JitterRange). +func TestComputeJitter(t *testing.T) { + const iterations = 10_000 + delay := 100 * time.Millisecond + maxJitter := time.Duration(float64(delay) * 0.2) // 20 ms + + var observedMax time.Duration + for i := 0; i < iterations; i++ { + // Use evenly-spaced r values to cover the full [0,1) range deterministically. + r := float64(i) / float64(iterations) + j := computeJitter(delay, r) + + require.GreaterOrEqualf(t, j, time.Duration(0), + "jitter must be non-negative for r=%f", r) + require.LessOrEqualf(t, j, maxJitter, + "jitter %s exceeds 20%% of delay %s for r=%f", j, delay, r) + + if j > observedMax { + observedMax = j + } + } + + // The spread check: with r spanning [0,1) the maximum jitter should reach + // close to maxJitter (within 1 step of iterations). We use 95% as the floor. + minSpread := time.Duration(float64(maxJitter) * 0.95) + assert.GreaterOrEqual(t, observedMax, minSpread, + "jitter spread appears stuck: max observed %s, expected >= %s", observedMax, minSpread) +}