Handle spiky load pattern by limiting based on static rate (#862)

Author: lahsivjar

processor/ratelimitprocessor/config.go

@@ -83,7 +83,7 @@ type DynamicRateLimiting struct {
 	// WindowConfigurator is the component ID of the extension to dynamically
 	// determine the window multiplier. The extension is expected to implement
 	// the `WindowConfigurator` interface. The window configurator is used in
-	// the hot path so it should respond fast.If the configurator returns a
+	// the hot path so it should respond fast. If the configurator returns a
 	// negative multiplier then the default multiplier will be used.
 	WindowConfigurator component.ID `mapstructure:"window_configurator"`
 }

processor/ratelimitprocessor/gubernator.go

@@ -411,17 +411,24 @@ func (r *gubernatorRateLimiter) getDynamicLimit(ctx context.Context,
 	// Only record the incoming hits when the current rate is within the allowed
 	// range, otherwise, do not record the hits and return the calculated rate.
 	// MaxAllowed sets a ceiling on the rate with the window duration. If the
-	// previous period had hits and the window multiplier is suggesting lowering
-	// the ingestion rate then the MaxAllowed will be allowed to go below the
-	// static rate (to as low as `1`). As soon as the window multiplier suggests
-	// increasing the ingestion rate, the MaxAllowed will jump to a minimum of
-	// static rate.
+	// the window multiplier is suggesting lowering the ingestion rate then the
+	// MaxAllowed will be allowed to go below the static rate (to as low as `1`).
+	// As soon as the window multiplier suggests increasing the ingestion rate,
+	// the MaxAllowed will jump to a minimum of static rate.
 	//
 	// NOTE(marclop) We may want to add a follow-up static ceiling to avoid
 	// unbounded growth.
 	var maxAllowed float64
-	if previous > 0 && windowMultiplier <= 1 {
-		maxAllowed = max(1, previous*windowMultiplier)
+	if windowMultiplier <= 1 {
+		// multiplier indicates scale down. If we have hits in the previous
+		// multiplier then scale that down, otherwise scale the static rate
+		// as per the multiplier instead of returning the default static rate.
+		// This should protect against deployments with spiky load patterns.
+		if previous > 0 {
+			maxAllowed = max(1, previous*windowMultiplier)
+		} else {
+			maxAllowed = max(1, staticRate*windowMultiplier)
+		}
 	} else {
 		maxAllowed = max(staticRate, previous*windowMultiplier)
 	}

processor/ratelimitprocessor/gubernator_test.go

@@ -850,13 +850,15 @@ func TestGubernatorRateLimiter_WindowConfigurator(t *testing.T) {
 			actual, expectedLimit, expectedLimit-actual, gubernator.Status_UNDER_LIMIT,
 		)
 
-		// wait for a few periods to assert rate that the rate goes to be equal to the
-		// static rate as data is NOT sent in the first waiting window causing the
-		// previous rate to go to zero in the next window which we will assert.
+		// wait for a few periods to assert that the rate goes to be calculated
+		// based on static rate and the multiplier as data is NOT sent in the previous
+		// waiting window causing the previous rate to go to zero. As the window
+		// multiplier is still suggesting lowering the rate, we apply the multiplier
+		// to the static limit.
 		waitUntilNextPeriod(windowDuration)
 		waitUntilNextPeriod(windowDuration)
-		expectedLimit = int64(staticRatePerSec)
-		actual = int64(100) * int64(windowDuration) / int64(time.Second) // send 100 hits
+		expectedLimit = int64(float64(staticRatePerSec) * 0.01)
+		actual = int64(5) * int64(windowDuration) / int64(time.Second)
 		require.NoError(t, rateLimiter.RateLimit(ctx, int(actual)))
 		assertRequestRateLimitEvent(
 			t, uniqueKey,