fix(ratelimit): bound memory and CPU per request

The sustained gate stored one entry for every attempt, denied ones too, and summed all entries on each request. A flood grew memory with rate times interval and made each request slower. Both gates now keep only the newest limit plus one attempt times. A Lean proof shows this decides the same as the full count. One check dropped from 29,502 ns and 11 allocations to 43 ns and none. The go-ratecounter dependency is gone.

go.mod

@@ -20,7 +20,6 @@ require (
 	github.com/graphql-go/graphql v0.8.1
 	github.com/jackc/pgx/v5 v5.12.0
 	github.com/lukaszraczylo/ask v0.0.0-20240916204100-6e9ef53a62d9
-	github.com/lukaszraczylo/go-ratecounter v0.1.13
 	github.com/lukaszraczylo/go-simple-graphql v1.2.89
 	github.com/lukaszraczylo/oss-telemetry v0.2.5
 	github.com/redis/go-redis/v9 v9.23.0

go.sum

@@ -72,8 +72,6 @@ github.com/klauspost/cpuid/v2 v2.4.0 h1:S6Hrbc7+ywsr0r+RLapfGBHfyefhCTwEh3A0tV91
 github.com/klauspost/cpuid/v2 v2.4.0/go.mod h1:19jmZ9mjzoF//ddRSUsv0zfBTJWh3QJh9FNxZTMrGxU=
 github.com/lukaszraczylo/ask v0.0.0-20240916204100-6e9ef53a62d9 h1:pL8B9mjv6RPUfKYYGm/uJ7QL6Ndf+z+OEl0qJE6KmEc=
 github.com/lukaszraczylo/ask v0.0.0-20240916204100-6e9ef53a62d9/go.mod h1:M+UVdyqZs++xtEPrascaVmZdOMhCnxjZ2SgH+xHpR0c=
-github.com/lukaszraczylo/go-ratecounter v0.1.13 h1:p1r53oyMb9ZnMbqub6IF4CWhURFiz7k967I0GbUyQMA=
-github.com/lukaszraczylo/go-ratecounter v0.1.13/go.mod h1:TqXEOCtFJStk1i0tkipprv1kiDHGon1MVUisjSTBSKM=
 github.com/lukaszraczylo/go-simple-graphql v1.2.89 h1:Xbu1Ny+a0lT2Sr2SaSC8mcHmGQDwGD4TJKk4DDd+PwA=
 github.com/lukaszraczylo/go-simple-graphql v1.2.89/go.mod h1:PxQYblQDZISmYYj8sNfazAWxAOh1rhAtU208y+uPV8s=
 github.com/lukaszraczylo/oss-telemetry v0.2.5 h1:W6Qgwp7IY4yBRO361MmnEAmM59motdQpbAFAx8v6fJM=

ratelimit.go

@@ -8,17 +8,17 @@ import (
 	"time"
 
 	"github.com/goccy/go-json"
-	goratecounter "github.com/lukaszraczylo/go-ratecounter"
 	libpack_logger "github.com/lukaszraczylo/graphql-monitoring-proxy/logging"
 )
 
 // RateLimitConfig holds the rate limit configuration for a role
 type RateLimitConfig struct {
-	RateCounterTicker *goratecounter.RateCounter
+	// sustained enforces Req over the full Interval.
+	sustained *attemptWindow
 	// burstCounter enforces Burst over a short trailing window, independent
-	// of RateCounterTicker's sustained-rate window. Nil when Burst <= 0
+	// of the sustained window. Nil when Burst <= 0
 	// (burst disabled). See burstWindow and checkRateLimit for semantics.
-	burstCounter *burstWindow
+	burstCounter *attemptWindow
 	Endpoints    []string      `json:"endpoints,omitempty"`
 	Interval     time.Duration `json:"interval"`
 	Req          int           `json:"req"`
@@ -107,51 +107,6 @@ func burstWindowDuration(interval time.Duration) time.Duration {
 	return w
 }
 
-// burstWindow is a minimal, clock-injectable, trailing-window request
-// counter used to enforce RateLimitConfig.Burst independently of the
-// sustained-rate RateCounterTicker.
-//
-// [email protected] cannot provide this second, shorter window
-// cleanly: the window length lives on *RateCounter itself, not per named
-// Counter (helpers.go: GetRate -> c.parent.interval.Seconds()), so two
-// different window lengths need two separate RateCounter instances. Each
-// instance drives its own background goroutine off wall-clock time.Now with
-// no seam to inject a fake clock (helpers.go: start(), cleanUpOldValues(),
-// addValue() all call time.Now() directly). That would force any test of
-// window rollover to sleep on real time. burstWindow is implemented here
-// instead, with a swappable clock, so tests can advance time deterministically.
-type burstWindow struct {
-	mu     sync.Mutex
-	now    func() time.Time
-	window time.Duration
-	hits   []time.Time
-}
-
-// newBurstWindow returns a burstWindow that tracks hits over the trailing
-// duration window, using the real wall clock.
-func newBurstWindow(window time.Duration) *burstWindow {
-	return &burstWindow{now: time.Now, window: window}
-}
-
-// recordAndCount records a hit at the current time, drops hits older than
-// the trailing window, and returns the resulting hit count within the
-// window. Safe for concurrent use.
-func (b *burstWindow) recordAndCount() int {
-	b.mu.Lock()
-	defer b.mu.Unlock()
-	now := b.now()
-	cutoff := now.Add(-b.window)
-	live := b.hits[:0]
-	for _, t := range b.hits {
-		if t.After(cutoff) {
-			live = append(live, t)
-		}
-	}
-	live = append(live, now)
-	b.hits = live
-	return len(b.hits)
-}
-
 // Variable to hold the current load config function - allows for testing
 var loadConfigFunc = loadConfigFromPath
 
@@ -237,14 +192,12 @@ func loadConfigFromPath(path string) error {
 
 	newRateLimits := make(map[string]RateLimitConfig, len(config.RateLimit))
 	for key, value := range config.RateLimit {
-		value.RateCounterTicker = goratecounter.NewRateCounter().WithConfig(goratecounter.RateCounterConfig{
-			Interval: value.Interval,
-		})
+		value.sustained = newAttemptWindow(value.Interval, value.Req)
 
 		// Burst disabled (<=0) leaves burstCounter nil, so checkRateLimit
 		// performs the Req-only check exactly as before burst existed.
 		if value.Burst > 0 {
-			value.burstCounter = newBurstWindow(burstWindowDuration(value.Interval))
+			value.burstCounter = newAttemptWindow(burstWindowDuration(value.Interval), value.Burst)
 		}
 
 		if cfg.LogLevel == "DEBUG" {
@@ -279,7 +232,7 @@ func rateLimitedRequest(userID, userRole string) bool {
 	// Try to get config from atomic value first for better performance
 	if configInterface := rateLimitConfigAtomic.Load(); configInterface != nil {
 		if config, ok := configInterface.(map[string]RateLimitConfig); ok {
-			if roleConfig, exists := config[userRole]; exists && roleConfig.RateCounterTicker != nil {
+			if roleConfig, exists := config[userRole]; exists && roleConfig.sustained != nil {
 				return checkRateLimit(userID, userRole, roleConfig, "")
 			}
 		}
@@ -290,7 +243,7 @@ func rateLimitedRequest(userID, userRole string) bool {
 	roleConfig, ok := rateLimits[userRole]
 	rateLimitMu.RUnlock()
 
-	if !ok || roleConfig.RateCounterTicker == nil {
+	if !ok || roleConfig.sustained == nil {
 		cfg.Logger.Warning(&libpack_logger.LogMessage{
 			Message: "Rate limit role not found or ticker not initialized - defaulting to deny",
 			Pairs:   map[string]any{"user_role": userRole},
@@ -315,45 +268,37 @@ func rateLimitedRequest(userID, userRole string) bool {
 // sustained count. Burst >= Req therefore never rejects anything the sustained
 // gate admits; Burst only takes effect when it is below Req.
 func checkRateLimit(userID, userRole string, roleConfig RateLimitConfig, endpoint string) bool {
-	roleConfig.RateCounterTicker.Incr(1)
-	tickerRate := roleConfig.RateCounterTicker.Get()
+	debug := cfg.Logger.IsLevelEnabled(libpack_logger.LEVEL_DEBUG)
 
-	logDetails := map[string]any{
-		"user_role":   userRole,
-		"user_id":     userID,
-		"rate":        tickerRate,
-		"config_rate": roleConfig.Req,
-		"interval":    roleConfig.Interval,
-		"endpoint":    endpoint,
-	}
-
-	cfg.Logger.Debug(&libpack_logger.LogMessage{
-		Message: "Rate limit ticker",
-		Pairs:   map[string]any{"log_details": logDetails},
-	})
-
-	if tickerRate > int64(roleConfig.Req) {
-		cfg.Logger.Debug(&libpack_logger.LogMessage{
-			Message: "Rate limit exceeded",
-			Pairs:   map[string]any{"log_details": logDetails},
-		})
+	if roleConfig.sustained.recordAndExceeds() {
+		if debug {
+			cfg.Logger.Debug(&libpack_logger.LogMessage{
+				Message: "Rate limit exceeded",
+				Pairs: map[string]any{
+					"user_role":   userRole,
+					"user_id":     userID,
+					"config_rate": roleConfig.Req,
+					"interval":    roleConfig.Interval,
+					"endpoint":    endpoint,
+				},
+			})
+		}
 		return false
 	}
 
 	if roleConfig.burstCounter != nil {
-		burstCount := roleConfig.burstCounter.recordAndCount()
-		burstLogDetails := map[string]any{
-			"user_role":    userRole,
-			"user_id":      userID,
-			"burst_count":  burstCount,
-			"config_burst": roleConfig.Burst,
-			"endpoint":     endpoint,
-		}
-		if burstCount > roleConfig.Burst {
-			cfg.Logger.Debug(&libpack_logger.LogMessage{
-				Message: "Burst limit exceeded",
-				Pairs:   map[string]any{"log_details": burstLogDetails},
-			})
+		if roleConfig.burstCounter.recordAndExceeds() {
+			if debug {
+				cfg.Logger.Debug(&libpack_logger.LogMessage{
+					Message: "Burst limit exceeded",
+					Pairs: map[string]any{
+						"user_role":    userRole,
+						"user_id":      userID,
+						"config_burst": roleConfig.Burst,
+						"endpoint":     endpoint,
+					},
+				})
+			}
 			return false
 		}
 	}

ratelimit_test.go

@@ -10,7 +10,6 @@ import (
 	"time"
 
 	"github.com/goccy/go-json"
-	goratecounter "github.com/lukaszraczylo/go-ratecounter"
 	libpack_logger "github.com/lukaszraczylo/graphql-monitoring-proxy/logging"
 )
 
@@ -65,8 +64,8 @@ func (suite *Tests) Test_loadRatelimitConfig() {
 		suite.Contains(rateLimits, "user")
 		suite.Equal(100, rateLimits["admin"].Req)
 		suite.Equal(10, rateLimits["user"].Req)
-		suite.NotNil(rateLimits["admin"].RateCounterTicker)
-		suite.NotNil(rateLimits["user"].RateCounterTicker)
+		suite.NotNil(rateLimits["admin"].sustained)
+		suite.NotNil(rateLimits["user"].sustained)
 	})
 
 	// Test loading config from non-existent path
@@ -154,23 +153,17 @@ func (suite *Tests) Test_rateLimitedRequest() {
 	rateLimits = make(map[string]RateLimitConfig)
 
 	// Admin role with high limit
-	adminCounter := goratecounter.NewRateCounter().WithConfig(goratecounter.RateCounterConfig{
-		Interval: 1 * time.Second,
-	})
 	rateLimits["admin"] = RateLimitConfig{
-		RateCounterTicker: adminCounter,
-		Interval:          1 * time.Second,
-		Req:               100,
+		sustained: newAttemptWindow(1*time.Second, 100),
+		Interval:  1 * time.Second,
+		Req:       100,
 	}
 
 	// User role with low limit
-	userCounter := goratecounter.NewRateCounter().WithConfig(goratecounter.RateCounterConfig{
-		Interval: 1 * time.Second,
-	})
 	rateLimits["user"] = RateLimitConfig{
-		RateCounterTicker: userCounter,
-		Interval:          1 * time.Second,
-		Req:               2, // Set very low for testing
+		sustained: newAttemptWindow(1*time.Second, 2),
+		Interval:  1 * time.Second,
+		Req:       2, // Set very low for testing
 	}
 	rateLimitMu.Unlock()
 
@@ -327,19 +320,17 @@ func (f *fakeClock) Advance(d time.Duration) {
 }
 
 // newBurstTestRoleConfig mirrors the RateLimitConfig construction performed
-// by loadConfigFromPath: a real RateCounterTicker for the sustained check,
+// by loadConfigFromPath: a real sustained window for the Req check,
 // and (only when burst > 0) a burstCounter sized by burstWindowDuration.
 func newBurstTestRoleConfig(interval time.Duration, req, burst int) RateLimitConfig {
 	rc := RateLimitConfig{
-		RateCounterTicker: goratecounter.NewRateCounter().WithConfig(goratecounter.RateCounterConfig{
-			Interval: interval,
-		}),
-		Interval: interval,
-		Req:      req,
-		Burst:    burst,
+		sustained: newAttemptWindow(interval, req),
+		Interval:  interval,
+		Req:       req,
+		Burst:     burst,
 	}
 	if burst > 0 {
-		rc.burstCounter = newBurstWindow(burstWindowDuration(interval))
+		rc.burstCounter = newAttemptWindow(burstWindowDuration(interval), burst)
 	}
 	return rc
 }

ratelimit_window.go

@@ -0,0 +1,49 @@
+package main
+
+import (
+	"sync"
+	"time"
+)
+
+// attemptWindow decides "more than limit attempts in the trailing window" while
+// storing only the newest limit+1 attempt times. The attempt number limit+1
+// counted back from now lies inside the window exactly when the full count
+// exceeds limit (see theorems/Theorems/SlidingWindow.lean), so memory and time
+// per attempt stay constant however hard a client floods.
+type attemptWindow struct {
+	now    func() time.Time
+	base   time.Time // monotonic origin for stored offsets
+	ring   []int64   // offsets from base, grows to capacity then wraps
+	mu     sync.Mutex
+	window time.Duration
+	head   int // index of the oldest stored attempt once the ring is full
+	size   int // limit+1: attempts kept; the ring grows lazily up to this
+}
+
+func newAttemptWindow(window time.Duration, limit int) *attemptWindow {
+	return &attemptWindow{
+		now:    time.Now,
+		base:   time.Now(),
+		window: window,
+		size:   max(limit, 0) + 1,
+	}
+}
+
+// recordAndExceeds records an attempt now and reports whether the attempts in
+// the trailing window, this one included, outnumber the limit.
+func (w *attemptWindow) recordAndExceeds() bool {
+	w.mu.Lock()
+	defer w.mu.Unlock()
+
+	t := int64(w.now().Sub(w.base))
+	if len(w.ring) < w.size {
+		w.ring = append(w.ring, t)
+	} else {
+		w.ring[w.head] = t
+		w.head = (w.head + 1) % len(w.ring)
+	}
+	if len(w.ring) < w.size {
+		return false
+	}
+	return w.ring[w.head] > t-int64(w.window)
+}

ratelimit_window_test.go

@@ -0,0 +1,64 @@
+package main
+
+import (
+	"math/rand"
+	"testing"
+	"time"
+)
+
+// naiveExceeds is the exact reference: count every attempt in the trailing window.
+func naiveExceeds(attempts []time.Time, now time.Time, window time.Duration, limit int) bool {
+	cutoff := now.Add(-window)
+	n := 0
+	for _, t := range attempts {
+		if t.After(cutoff) {
+			n++
+		}
+	}
+	return n > limit
+}
+
+// Pins theorems/Theorems/SlidingWindow.lean: storing the newest limit+1 attempts decides like the full count.
+func TestAttemptWindow_MatchesExactCount(t *testing.T) {
+	for _, limit := range []int{0, 1, 3, 10} {
+		rng := rand.New(rand.NewSource(int64(limit) + 1))
+		window := 10 * time.Second
+		fc := newFakeClock(time.Unix(1_000_000, 0))
+		w := newAttemptWindow(window, limit)
+		w.now = fc.Now
+		w.base = fc.Now()
+
+		var attempts []time.Time
+		for i := 0; i < 5000; i++ {
+			fc.Advance(time.Duration(rng.Intn(3000)) * time.Millisecond)
+			attempts = append(attempts, fc.Now())
+			want := naiveExceeds(attempts, fc.Now(), window, limit)
+			if got := w.recordAndExceeds(); got != want {
+				t.Fatalf("limit=%d step=%d: got %v, want %v", limit, i, got, want)
+			}
+		}
+	}
+}
+
+func TestAttemptWindow_StorageIsBounded(t *testing.T) {
+	const limit = 10
+	fc := newFakeClock(time.Unix(1_000_000, 0))
+	w := newAttemptWindow(time.Hour, limit)
+	w.now = fc.Now
+	w.base = fc.Now()
+	for i := 0; i < 100_000; i++ {
+		w.recordAndExceeds()
+		fc.Advance(time.Millisecond)
+	}
+	if got := len(w.ring); got > limit+1 {
+		t.Fatalf("stored %d timestamps, want <= %d", got, limit+1)
+	}
+}
+
+func BenchmarkAttemptWindow(b *testing.B) {
+	w := newAttemptWindow(time.Hour, 1000)
+	b.ReportAllocs()
+	for i := 0; i < b.N; i++ {
+		w.recordAndExceeds()
+	}
+}

websocket_test.go

@@ -11,7 +11,6 @@ import (
 
 	fiber "github.com/gofiber/fiber/v3"
 	gorillaws "github.com/gorilla/websocket"
-	goratecounter "github.com/lukaszraczylo/go-ratecounter"
 	libpack_logger "github.com/lukaszraczylo/graphql-monitoring-proxy/logging"
 	libpack_monitoring "github.com/lukaszraczylo/graphql-monitoring-proxy/monitoring"
 	"github.com/stretchr/testify/assert"
@@ -652,11 +651,9 @@ func TestHandleWebSocketOrDefault_RateLimitedUserRejected(t *testing.T) {
 	rateLimitMu.Lock()
 	rateLimits = map[string]RateLimitConfig{
 		defaultValue: {
-			RateCounterTicker: goratecounter.NewRateCounter().WithConfig(goratecounter.RateCounterConfig{
-				Interval: 1 * time.Second,
-			}),
-			Interval: 1 * time.Second,
-			Req:      0,
+			sustained: newAttemptWindow(1*time.Second, 0),
+			Interval:  1 * time.Second,
+			Req:       0,
 		},
 	}
 	rateLimitConfigAtomic.Store(rateLimits)