From 8b4a2a2df1336422ce959519a06c8f53f4d50db3 Mon Sep 17 00:00:00 2001 From: lvlcn-t <75443136+lvlcn-t@users.noreply.github.com> Date: Thu, 14 May 2026 20:40:49 +0200 Subject: [PATCH 1/4] feat: add polling jitter to target manager intervals Apply configurable jitter to check, registration, and update timer resets to prevent thundering-herd when many sparrow instances poll the same backend simultaneously. - Add helper.ApplyJitter with bounded minimum [d*(1-f), d] - Add Jitter float64 field to General config with [0.0, 1.0] validation - Wire jitter into manager.Reconcile timer resets - Add jitter tests (helper bounds + validation cases) - Document jitter in chart/values.yaml --- chart/values.yaml | 3 + internal/helper/jitter.go | 27 ++++++++ internal/helper/jitter_test.go | 81 +++++++++++++++++++++++ pkg/sparrow/targets/errors.go | 2 + pkg/sparrow/targets/manager.go | 7 +- pkg/sparrow/targets/targetmanager.go | 9 +++ pkg/sparrow/targets/targetmanager_test.go | 48 ++++++++++++++ 7 files changed, 174 insertions(+), 3 deletions(-) create mode 100644 internal/helper/jitter.go create mode 100644 internal/helper/jitter_test.go diff --git a/chart/values.yaml b/chart/values.yaml index f0ad4218..d5b0df38 100644 --- a/chart/values.yaml +++ b/chart/values.yaml @@ -185,6 +185,9 @@ sparrowConfig: # unhealthyThreshold: 600s # registrationInterval: 300s # updateInterval: 900s +# # -- Jitter factor applied to polling intervals [0.0, 1.0]. +# # -- 0.0 means no jitter; 0.2 means intervals vary by up to 20%. +# jitter: 0.2 # gitlab: # token: "" # baseUrl: https://gitlab.com diff --git a/internal/helper/jitter.go b/internal/helper/jitter.go new file mode 100644 index 00000000..fda16fc8 --- /dev/null +++ b/internal/helper/jitter.go @@ -0,0 +1,27 @@ +// SPDX-FileCopyrightText: 2025 Deutsche Telekom IT GmbH +// +// SPDX-License-Identifier: Apache-2.0 + +package helper + +import ( + "math/rand/v2" + "time" +) + +// ApplyJitter applies full jitter with a bounded minimum to a duration. +// The returned duration is in the range [d*(1-factor), d]. +// A factor of 0 returns d unchanged. Factor must be in [0.0, 1.0]. +func ApplyJitter(d time.Duration, factor float64) time.Duration { + if factor <= 0 || d <= 0 { + return d + } + if factor > 1 { + factor = 1 + } + + minDuration := float64(d) * (1 - factor) + jitterRange := float64(d) * factor + + return time.Duration(minDuration + rand.Float64()*jitterRange) //nolint:gosec // jitter does not need crypto rand +} diff --git a/internal/helper/jitter_test.go b/internal/helper/jitter_test.go new file mode 100644 index 00000000..170178d7 --- /dev/null +++ b/internal/helper/jitter_test.go @@ -0,0 +1,81 @@ +// SPDX-FileCopyrightText: 2025 Deutsche Telekom IT GmbH +// +// SPDX-License-Identifier: Apache-2.0 + +package helper + +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" +) + +func TestApplyJitter(t *testing.T) { + tests := []struct { + name string + duration time.Duration + factor float64 + wantMin time.Duration + wantMax time.Duration + wantExact bool + }{ + { + name: "factor 0 returns exact duration", + duration: 10 * time.Second, + factor: 0, + wantExact: true, + }, + { + name: "factor 0.2 bounds", + duration: 10 * time.Second, + factor: 0.2, + wantMin: 8 * time.Second, + wantMax: 10 * time.Second, + }, + { + name: "factor 1.0 full range", + duration: 10 * time.Second, + factor: 1.0, + wantMin: 0, + wantMax: 10 * time.Second, + }, + { + name: "zero duration", + duration: 0, + factor: 0.5, + wantExact: true, + }, + { + name: "negative factor treated as 0", + duration: 10 * time.Second, + factor: -0.5, + wantExact: true, + }, + { + name: "factor above 1 clamped to 1", + duration: 10 * time.Second, + factor: 2.0, + wantMin: 0, + wantMax: 10 * time.Second, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.wantExact { + got := ApplyJitter(tt.duration, tt.factor) + assert.Equal(t, tt.duration, got) + return + } + + // Run many iterations to verify bounds + const iterations = 1000 + for range iterations { + got := ApplyJitter(tt.duration, tt.factor) + assert.GreaterOrEqual(t, got, tt.wantMin, "below minimum") + assert.LessOrEqual(t, got, tt.wantMax, "above maximum") + } + }) + } +} diff --git a/pkg/sparrow/targets/errors.go b/pkg/sparrow/targets/errors.go index 255939ed..cfbb083a 100644 --- a/pkg/sparrow/targets/errors.go +++ b/pkg/sparrow/targets/errors.go @@ -19,4 +19,6 @@ var ( ErrInvalidInteractorType = errors.New("invalid interactor type") // ErrInvalidScheme is returned when the scheme is not http or https ErrInvalidScheme = errors.New("scheme must be 'http' of 'https'") + // ErrInvalidJitter is returned when the jitter factor is out of range + ErrInvalidJitter = errors.New("jitter must be between 0.0 and 1.0") ) diff --git a/pkg/sparrow/targets/manager.go b/pkg/sparrow/targets/manager.go index 7e8562fc..9ab6346f 100644 --- a/pkg/sparrow/targets/manager.go +++ b/pkg/sparrow/targets/manager.go @@ -14,6 +14,7 @@ import ( "github.com/prometheus/client_golang/prometheus" smetrics "github.com/telekom/sparrow/pkg/sparrow/metrics" + "github.com/telekom/sparrow/internal/helper" "github.com/telekom/sparrow/internal/logger" "github.com/telekom/sparrow/pkg/checks" "github.com/telekom/sparrow/pkg/sparrow/targets/remote" @@ -114,19 +115,19 @@ func (t *manager) Reconcile(ctx context.Context) error { if err != nil { log.WarnContext(ctx, "Failed to get global targets", "error", err) } - checkTimer.Reset(t.cfg.CheckInterval) + checkTimer.Reset(helper.ApplyJitter(t.cfg.CheckInterval, t.cfg.Jitter)) case <-registrationTimer.C: err := t.register(ctx) if err != nil { log.WarnContext(ctx, "Failed to register self as global target", "error", err) } - registrationTimer.Reset(t.cfg.RegistrationInterval) + registrationTimer.Reset(helper.ApplyJitter(t.cfg.RegistrationInterval, t.cfg.Jitter)) case <-updateTimer.C: err := t.update(ctx) if err != nil { log.WarnContext(ctx, "Failed to update registration", "error", err) } - updateTimer.Reset(t.cfg.UpdateInterval) + updateTimer.Reset(helper.ApplyJitter(t.cfg.UpdateInterval, t.cfg.Jitter)) } } } diff --git a/pkg/sparrow/targets/targetmanager.go b/pkg/sparrow/targets/targetmanager.go index 146f5f3f..8029cd75 100644 --- a/pkg/sparrow/targets/targetmanager.go +++ b/pkg/sparrow/targets/targetmanager.go @@ -48,6 +48,10 @@ type General struct { // Scheme is the scheme used for the remote target manager // Can either be http or https Scheme string `yaml:"scheme" mapstructure:"scheme"` + // Jitter is the jitter factor applied to polling intervals. + // A value between 0.0 (no jitter) and 1.0 (full jitter). + // The actual interval will be in [interval*(1-jitter), interval]. + Jitter float64 `yaml:"jitter" mapstructure:"jitter"` } // TargetManagerConfig is the configuration for the target manager @@ -85,6 +89,11 @@ func (c *TargetManagerConfig) Validate(ctx context.Context) error { return ErrInvalidScheme } + if c.Jitter < 0 || c.Jitter > 1 { + log.Error("The jitter factor should be between 0.0 and 1.0", "jitter", c.Jitter) + return ErrInvalidJitter + } + switch c.Type { case interactor.Gitlab: return nil diff --git a/pkg/sparrow/targets/targetmanager_test.go b/pkg/sparrow/targets/targetmanager_test.go index 9586a243..3d29d3f0 100644 --- a/pkg/sparrow/targets/targetmanager_test.go +++ b/pkg/sparrow/targets/targetmanager_test.go @@ -190,6 +190,54 @@ func TestTargetManagerConfig_Validate(t *testing.T) { }, wantErr: true, }, + { + name: "valid config - jitter 0.0", + cfg: TargetManagerConfig{ + Type: "gitlab", + General: General{ + Scheme: schemeHTTPS, + CheckInterval: 1 * time.Second, + Jitter: 0.0, + }, + }, + wantErr: false, + }, + { + name: "valid config - jitter 1.0", + cfg: TargetManagerConfig{ + Type: "gitlab", + General: General{ + Scheme: schemeHTTPS, + CheckInterval: 1 * time.Second, + Jitter: 1.0, + }, + }, + wantErr: false, + }, + { + name: "invalid config - jitter negative", + cfg: TargetManagerConfig{ + Type: "gitlab", + General: General{ + Scheme: schemeHTTPS, + CheckInterval: 1 * time.Second, + Jitter: -0.1, + }, + }, + wantErr: true, + }, + { + name: "invalid config - jitter above 1", + cfg: TargetManagerConfig{ + Type: "gitlab", + General: General{ + Scheme: schemeHTTPS, + CheckInterval: 1 * time.Second, + Jitter: 1.5, + }, + }, + wantErr: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From 470ef83d7e12fc330f2116beaa5f1d018b605b1b Mon Sep 17 00:00:00 2001 From: lvlcn-t <75443136+lvlcn-t@users.noreply.github.com> Date: Thu, 14 May 2026 20:40:49 +0200 Subject: [PATCH 2/4] refactor: replace jitter iteration test with fuzz test Replace the hand-rolled 1000-iteration bounds check with a proper Go fuzz test (FuzzApplyJitter) that generates random duration/factor pairs and asserts invariant properties. Add NaN/Inf guards to ApplyJitter to handle degenerate float inputs defensively. - Add math.IsNaN/IsInf guard in ApplyJitter (treat as identity) - Add FuzzApplyJitter with 8 seed corpus entries - Keep deterministic TestApplyJitter_EdgeCases for NaN/Inf/zero --- internal/helper/jitter.go | 3 +- internal/helper/jitter_test.go | 112 ++++++++++++++++++++------------- 2 files changed, 69 insertions(+), 46 deletions(-) diff --git a/internal/helper/jitter.go b/internal/helper/jitter.go index fda16fc8..601c0ddf 100644 --- a/internal/helper/jitter.go +++ b/internal/helper/jitter.go @@ -5,6 +5,7 @@ package helper import ( + "math" "math/rand/v2" "time" ) @@ -13,7 +14,7 @@ import ( // The returned duration is in the range [d*(1-factor), d]. // A factor of 0 returns d unchanged. Factor must be in [0.0, 1.0]. func ApplyJitter(d time.Duration, factor float64) time.Duration { - if factor <= 0 || d <= 0 { + if math.IsNaN(factor) || math.IsInf(factor, 0) || factor <= 0 || d <= 0 { return d } if factor > 1 { diff --git a/internal/helper/jitter_test.go b/internal/helper/jitter_test.go index 170178d7..52f3fb01 100644 --- a/internal/helper/jitter_test.go +++ b/internal/helper/jitter_test.go @@ -5,77 +5,99 @@ package helper import ( + "math" "testing" "time" "github.com/stretchr/testify/assert" ) -func TestApplyJitter(t *testing.T) { +func TestApplyJitter_EdgeCases(t *testing.T) { tests := []struct { - name string - duration time.Duration - factor float64 - wantMin time.Duration - wantMax time.Duration - wantExact bool + name string + duration time.Duration + factor float64 + want time.Duration }{ { - name: "factor 0 returns exact duration", - duration: 10 * time.Second, - factor: 0, - wantExact: true, + name: "factor 0 returns exact duration", + duration: 10 * time.Second, + factor: 0, + want: 10 * time.Second, }, { - name: "factor 0.2 bounds", - duration: 10 * time.Second, - factor: 0.2, - wantMin: 8 * time.Second, - wantMax: 10 * time.Second, + name: "zero duration", + duration: 0, + factor: 0.5, + want: 0, }, { - name: "factor 1.0 full range", + name: "negative factor treated as identity", duration: 10 * time.Second, - factor: 1.0, - wantMin: 0, - wantMax: 10 * time.Second, + factor: -0.5, + want: 10 * time.Second, }, { - name: "zero duration", - duration: 0, - factor: 0.5, - wantExact: true, + name: "NaN factor treated as identity", + duration: 10 * time.Second, + factor: math.NaN(), + want: 10 * time.Second, }, { - name: "negative factor treated as 0", - duration: 10 * time.Second, - factor: -0.5, - wantExact: true, + name: "positive Inf factor treated as identity", + duration: 10 * time.Second, + factor: math.Inf(1), + want: 10 * time.Second, }, { - name: "factor above 1 clamped to 1", + name: "negative Inf factor treated as identity", duration: 10 * time.Second, - factor: 2.0, - wantMin: 0, - wantMax: 10 * time.Second, + factor: math.Inf(-1), + want: 10 * time.Second, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if tt.wantExact { - got := ApplyJitter(tt.duration, tt.factor) - assert.Equal(t, tt.duration, got) - return - } - - // Run many iterations to verify bounds - const iterations = 1000 - for range iterations { - got := ApplyJitter(tt.duration, tt.factor) - assert.GreaterOrEqual(t, got, tt.wantMin, "below minimum") - assert.LessOrEqual(t, got, tt.wantMax, "above maximum") - } + got := ApplyJitter(tt.duration, tt.factor) + assert.Equal(t, tt.want, got) }) } } + +func FuzzApplyJitter(f *testing.F) { + f.Add(int64(10*time.Second), 0.0) + f.Add(int64(10*time.Second), 0.2) + f.Add(int64(10*time.Second), 1.0) + f.Add(int64(0), 0.5) + f.Add(int64(10*time.Second), -0.5) + f.Add(int64(10*time.Second), 2.0) + f.Add(int64(time.Millisecond), 0.99) + f.Add(int64(math.MaxInt64), 0.5) + + f.Fuzz(func(t *testing.T, durationNs int64, factor float64) { + d := time.Duration(durationNs) + got := ApplyJitter(d, factor) + + // Identity: when factor is non-positive, NaN, Inf, or d <= 0 + isIdentity := factor <= 0 || d <= 0 || + math.IsNaN(factor) || math.IsInf(factor, 0) + if isIdentity { + assert.Equal(t, d, got, "expected identity") + return + } + + // Clamp factor for bound assertions + f := min(factor, 1.0) + + // Property: result never exceeds original + assert.LessOrEqual(t, got, d, "above maximum") + + // Property: result respects bounded minimum + minD := time.Duration(float64(d) * (1 - f)) + assert.GreaterOrEqual(t, got, minD, "below minimum") + + // Property: result is non-negative when d > 0 + assert.GreaterOrEqual(t, got, time.Duration(0), "negative result") + }) +} From b275ed34360a7f789fc8110499d23c40132d6636 Mon Sep 17 00:00:00 2001 From: lvlcn-t <75443136+lvlcn-t@users.noreply.github.com> Date: Thu, 14 May 2026 20:40:49 +0200 Subject: [PATCH 3/4] fix: use gosec:disable directive for G404 suppression --- internal/helper/jitter.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/helper/jitter.go b/internal/helper/jitter.go index 601c0ddf..7abb3674 100644 --- a/internal/helper/jitter.go +++ b/internal/helper/jitter.go @@ -24,5 +24,6 @@ func ApplyJitter(d time.Duration, factor float64) time.Duration { minDuration := float64(d) * (1 - factor) jitterRange := float64(d) * factor - return time.Duration(minDuration + rand.Float64()*jitterRange) //nolint:gosec // jitter does not need crypto rand + //gosec:disable G404 -- jitter does not need crypto rand + return time.Duration(minDuration + rand.Float64()*jitterRange) } From d847c292a75dea809e718704d4d26d54eb24341d Mon Sep 17 00:00:00 2001 From: lvlcn-t <75443136+lvlcn-t@users.noreply.github.com> Date: Thu, 14 May 2026 20:40:49 +0200 Subject: [PATCH 4/4] chore: address review comment Signed-off-by: lvlcn-t <75443136+lvlcn-t@users.noreply.github.com> --- internal/helper/jitter.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/internal/helper/jitter.go b/internal/helper/jitter.go index 7abb3674..ce7fac96 100644 --- a/internal/helper/jitter.go +++ b/internal/helper/jitter.go @@ -10,9 +10,10 @@ import ( "time" ) -// ApplyJitter applies full jitter with a bounded minimum to a duration. -// The returned duration is in the range [d*(1-factor), d]. -// A factor of 0 returns d unchanged. Factor must be in [0.0, 1.0]. +// ApplyJitter adds a random jitter to d. factor is a percentage of d +// that will be subtracted from d, so the returned duration is in the +// range [d*(1-factor), d]. A factor of 0 returns d unchanged. +// factor must be in [0.0, 1.0]; values above 1.0 are clamped to 1.0. func ApplyJitter(d time.Duration, factor float64) time.Duration { if math.IsNaN(factor) || math.IsInf(factor, 0) || factor <= 0 || d <= 0 { return d