From f2a27e4b86af36d0b30f8b47c628c97ddf11d303 Mon Sep 17 00:00:00 2001
From: John Cai <jcai@gitlab.com>
Date: Mon, 8 Aug 2022 09:55:13 -0400
Subject: limithandler: Remove RateLimiting feature flag

Now that this feature has been running in production for a few weeks
without issue, we can remove the feature flag.
---
 internal/metadata/featureflag/ff_rate_limiter.go   | 10 ------
 .../middleware/limithandler/middleware_test.go     | 27 ++++++----------
 internal/middleware/limithandler/rate_limiter.go   | 36 ++++++++++------------
 3 files changed, 27 insertions(+), 46 deletions(-)
 delete mode 100644 internal/metadata/featureflag/ff_rate_limiter.go

diff --git a/internal/metadata/featureflag/ff_rate_limiter.go b/internal/metadata/featureflag/ff_rate_limiter.go
deleted file mode 100644
index 7bdbe9afa..000000000
--- a/internal/metadata/featureflag/ff_rate_limiter.go
+++ /dev/null
@@ -1,10 +0,0 @@
-package featureflag
-
-// RateLimit will enable the rate limiter to reject requests beyond a configured
-// rate.
-var RateLimit = NewFeatureFlag(
-	"rate_limit",
-	"v14.10.0",
-	"https://gitlab.com/gitlab-org/gitaly/-/issues/4181",
-	false,
-)
diff --git a/internal/middleware/limithandler/middleware_test.go b/internal/middleware/limithandler/middleware_test.go
index c929d19ea..9a2fb9870 100644
--- a/internal/middleware/limithandler/middleware_test.go
+++ b/internal/middleware/limithandler/middleware_test.go
@@ -16,7 +16,6 @@ import (
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/helper"
-	"gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/middleware/limithandler"
 	pb "gitlab.com/gitlab-org/gitaly/v15/internal/middleware/limithandler/testdata"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper"
@@ -482,10 +481,9 @@ gitaly_requests_dropped_total{grpc_method="Unary",grpc_service="test.limithandle
 
 func TestRateLimitHandler(t *testing.T) {
 	t.Parallel()
-	testhelper.NewFeatureSets(featureflag.RateLimit).Run(t, testRateLimitHandler)
-}
 
-func testRateLimitHandler(t *testing.T, ctx context.Context) {
+	ctx := testhelper.Context(t)
+
 	methodName := "/test.limithandler.Test/Unary"
 	cfg := config.Cfg{
 		RateLimiting: []config.RateLimiting{
@@ -519,21 +517,16 @@ func testRateLimitHandler(t *testing.T, ctx context.Context) {
 		for i := 0; i < 10; i++ {
 			_, err := client.Unary(ctx, &pb.UnaryRequest{})
 
-			if featureflag.RateLimit.IsEnabled(ctx) {
-				s, ok := status.FromError(err)
-				require.True(t, ok)
-				details := s.Details()
-				require.Len(t, details, 1)
-
-				limitErr, ok := details[0].(*gitalypb.LimitError)
-				require.True(t, ok)
+			s, ok := status.FromError(err)
+			require.True(t, ok)
+			details := s.Details()
+			require.Len(t, details, 1)
 
-				assert.Equal(t, limithandler.ErrRateLimit.Error(), limitErr.ErrorMessage)
-				assert.Equal(t, durationpb.New(0), limitErr.RetryAfter)
+			limitErr, ok := details[0].(*gitalypb.LimitError)
+			require.True(t, ok)
 
-			} else {
-				require.NoError(t, err)
-			}
+			assert.Equal(t, limithandler.ErrRateLimit.Error(), limitErr.ErrorMessage)
+			assert.Equal(t, durationpb.New(0), limitErr.RetryAfter)
 		}
 
 		expectedMetrics := `# HELP gitaly_requests_dropped_total Number of requests dropped from the queue
diff --git a/internal/middleware/limithandler/rate_limiter.go b/internal/middleware/limithandler/rate_limiter.go
index 63124b0d7..89f6deeab 100644
--- a/internal/middleware/limithandler/rate_limiter.go
+++ b/internal/middleware/limithandler/rate_limiter.go
@@ -9,7 +9,6 @@ import (
 	"github.com/prometheus/client_golang/prometheus"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/helper"
-	"gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb"
 	"gitlab.com/gitlab-org/labkit/log"
 	"golang.org/x/time/rate"
@@ -43,26 +42,25 @@ func (r *RateLimiter) Limit(ctx context.Context, lockKey string, f LimitedFunc)
 		// For now, we are only emitting this metric to get an idea of the shape
 		// of traffic.
 		r.requestsDroppedMetric.Inc()
-		if featureflag.RateLimit.IsEnabled(ctx) {
-			err := helper.ErrUnavailable(ErrRateLimit)
-
-			detailedErr, errGeneratingDetailedErr := helper.ErrWithDetails(
-				err,
-				&gitalypb.LimitError{
-					ErrorMessage: ErrRateLimit.Error(),
-					RetryAfter:   durationpb.New(0),
-				},
-			)
-			if errGeneratingDetailedErr != nil {
-				log.WithField("rate_limit_error", err).
-					WithError(errGeneratingDetailedErr).
-					Error("failed to generate detailed error")
-
-				return nil, err
-			}
 
-			return nil, detailedErr
+		err := helper.ErrUnavailable(ErrRateLimit)
+
+		detailedErr, errGeneratingDetailedErr := helper.ErrWithDetails(
+			err,
+			&gitalypb.LimitError{
+				ErrorMessage: ErrRateLimit.Error(),
+				RetryAfter:   durationpb.New(0),
+			},
+		)
+		if errGeneratingDetailedErr != nil {
+			log.WithField("rate_limit_error", err).
+				WithError(errGeneratingDetailedErr).
+				Error("failed to generate detailed error")
+
+			return nil, err
 		}
+
+		return nil, detailedErr
 	}
 
 	return f()
-- 
cgit v1.2.3