Skip to content

Added rate limit to health check API #150

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Added rate limit to health check API #150

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion health-check/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,16 @@ module health
go 1.20

require (
github.com/Real-Dev-Squad/feature-flag-backend v0.0.0-20250121203708-a49b2080c46c
github.com/aws/aws-lambda-go v1.40.0
github.com/stretchr/testify v1.7.2
github.com/stretchr/testify v1.8.2
)

require (
github.com/aws/aws-sdk-go v1.44.248 // indirect
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/jmespath/go-jmespath v0.4.0 // indirect
github.com/joho/godotenv v1.5.1 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
)
51 changes: 49 additions & 2 deletions health-check/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,61 @@
github.com/Real-Dev-Squad/feature-flag-backend v0.0.0-20250121203708-a49b2080c46c h1:7R8Ia4jNuf3H6jzU+hPdzX5avyeiiri3oG02hOPq9Xo=
github.com/Real-Dev-Squad/feature-flag-backend v0.0.0-20250121203708-a49b2080c46c/go.mod h1:5PjUN7WNRsmIsAFRipvthv2FR5EmDXPUKdpst0ejrDo=
github.com/aws/aws-lambda-go v1.40.0 h1:6dKcDpXsTpapfCFF6Debng6CiV/Z3sNHekM6bwhI2J0=
github.com/aws/aws-lambda-go v1.40.0/go.mod h1:jwFe2KmMsHmffA1X2R09hH6lFzJQxzI8qK17ewzbQMM=
github.com/aws/aws-sdk-go v1.44.248 h1:GvkxpgsxqNc03LmhXiaxKpzbyxndnex7V+OThLx4g5M=
github.com/aws/aws-sdk-go v1.44.248/go.mod h1:aVsgQcEevwlmQ7qHE9I3h+dtQgpqhFB+i8Phjh7fkwI=
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/jmespath/go-jmespath v0.4.0 h1:BEgLn5cpjn8UN1mAw4NjwDrS35OdebyEtFe+9YPoQUg=
github.com/jmespath/go-jmespath v0.4.0/go.mod h1:T8mJZnbsbmF+m6zOOFylbeCJqk5+pHWvzYPziyZiYoo=
github.com/jmespath/go-jmespath/internal/testify v1.5.1 h1:shLQSRRSCCPj3f2gpwzGwWFoC7ycTf1rcQZHOlsJ6N8=
github.com/jmespath/go-jmespath/internal/testify v1.5.1/go.mod h1:L3OGu8Wl2/fWfCI6z80xFu9LTZmf1ZRjMHUOPmWr69U=
github.com/joho/godotenv v1.5.1 h1:7eLL/+HRGLY0ldzfGMeQkb7vMd0as4CfYvUVzLqw0N0=
github.com/joho/godotenv v1.5.1/go.mod h1:f4LDr5Voq0i2e/R5DDNOoa2zzDfwtkZa6DnEwAbqwq4=
github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/testify v1.7.2 h1:4jaiDzPyXQvSd7D0EjG45355tLlV3VOECpq10pLC+8s=
github.com/stretchr/testify v1.7.2/go.mod h1:R6va5+xMeoiuVRoj+gSkQ7d3FALtqAAGI1FQKckRals=
github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw=
github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo=
github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
github.com/stretchr/testify v1.8.2 h1:+h33VjcLVPDHtOdpUCuF+7gSuG3yGIftsP1YvFihtJ8=
github.com/stretchr/testify v1.8.2/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4=
github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY=
golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc=
golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4=
golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg=
golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c=
golang.org/x/net v0.1.0 h1:hZ/3BUoy5aId7sCpA/Tc5lt8DkFgdVS2onTpJsZ/fl0=
golang.org/x/net v0.1.0/go.mod h1:Cx3nUiGt4eDBEyega/BKRp+/AlGL8hYe7U9odMt2Cco=
golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8=
golang.org/x/term v0.1.0/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8=
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ=
golang.org/x/text v0.4.0 h1:BrVqGRd7+k1DiOgtnFvAkoQEWQvBc25ouMJM6429SFg=
golang.org/x/text v0.4.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8=
golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc=
golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/yaml.v2 v2.2.8 h1:obN1ZagJSUGI0Ek/LBmuj4SNLPfIny3KsKFopxRdj10=
gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
7 changes: 7 additions & 0 deletions health-check/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,16 @@ package main
import (
"github.com/aws/aws-lambda-go/events"
"github.com/aws/aws-lambda-go/lambda"
"github.com/Real-Dev-Squad/feature-flag-backend/database"
"github.com/Real-Dev-Squad/feature-flag-backend/utils"

)

var checkRequestAllowed = utils.CheckRequestAllowed
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Consider function initialization in handler to improve testability

The global variable checkRequestAllowed makes testing more difficult. Consider initializing this within the handler function or using dependency injection to improve testability.

-var checkRequestAllowed = utils.CheckRequestAllowed
 func handler(request events.APIGatewayProxyRequest)(events.APIGatewayProxyResponse, error){
 	db := database.CreateDynamoDB()
-
-	checkRequestAllowed(db, utils.ConcurrencyDisablingLambda)
+	utils.CheckRequestAllowed(db, utils.ConcurrencyDisablingLambda)

Committable suggestion skipped: line range outside the PR's diff.

func handler(request events.APIGatewayProxyRequest)(events.APIGatewayProxyResponse, error){
db := database.CreateDynamoDB()

checkRequestAllowed(db, utils.ConcurrencyDisablingLambda)
Comment on lines +13 to +15
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Missing documentation for rate limiting behavior

The implementation adds significant rate limiting behavior to the health check endpoint, but there's no documentation on how this works, what limits are applied, or how to configure them. Consider adding comments or documentation.

 func handler(request events.APIGatewayProxyRequest)(events.APIGatewayProxyResponse, error){
+	// Create DynamoDB client for rate limiting operations
 	db := database.CreateDynamoDB()
 
+	// Apply rate limiting to health check endpoint
+	// This uses a counter in DynamoDB to track and limit requests
+	// When the limit is reached, concurrency is disabled for other lambdas
 	checkRequestAllowed(db, utils.ConcurrencyDisablingLambda)

🛠️ Refactor suggestion

Add error handling for database operations and rate limiting

The calls to CreateDynamoDB() and checkRequestAllowed() don't include any error handling, which could lead to unexpected behavior if these operations fail. Based on the provided snippets, it appears that CheckRequestAllowed handles errors internally but doesn't return them to the caller.

 func handler(request events.APIGatewayProxyRequest)(events.APIGatewayProxyResponse, error){
-	db := database.CreateDynamoDB()
+	// Wrap the database creation in a defer-recover block to handle panics
+	var db *dynamodb.DynamoDB
+	func() {
+		defer func() {
+			if r := recover(); r != nil {
+				log.Printf("Recovered from database creation: %v", r)
+			}
+		}()
+		db = database.CreateDynamoDB()
+	}()
+	
+	if db == nil {
+		return events.APIGatewayProxyResponse{
+			Body:       "Internal server error - database client initialization failed",
+			StatusCode: 500,
+		}, nil
+	}

 	checkRequestAllowed(db, utils.ConcurrencyDisablingLambda)

💡 Verification agent

🧩 Analysis chain

Consider performance impact of DynamoDB operations on health check endpoint

Adding DynamoDB operations to a health check endpoint may increase latency and reduce reliability. Health checks are typically lightweight and shouldn't depend on external services. Consider if this is the appropriate place for rate limiting, or if it should be implemented at an API Gateway or proxy layer instead.

🏁 Script executed:

#!/bin/bash
# Check if there are any tests that validate the performance of the health check
find . -name "*_test.go" | xargs grep -l "health" | xargs grep -l "performance\|latency\|benchmark"

Length of output: 99

🏁 Script executed:

#!/bin/bash
# Search for Go benchmark tests related to the health‐check endpoint
# This looks for any Benchmark functions that include "health" (case-insensitive)
rg -n 'func Benchmark.*health' -i -t go || true

Length of output: 48

🏁 Script executed:

#!/bin/bash
# Locate invocation of checkRequestAllowed with context
rg -n 'checkRequestAllowed' -C5 -t go || true

# Locate the definition of checkRequestAllowed
rg -n 'func checkRequestAllowed' -C5 -t go || true

# Locate the CreateDynamoDB factory method
rg -n 'func CreateDynamoDB' -C5 -t go || true

Length of output: 1422

Avoid external calls in health‐check handler

The health‐check lambda in health-check/main.go currently initializes a DynamoDB client and invokes the rate‐limiting check on every probe, which can increase latency and introduce a dependency on an external service:

• File: health-check/main.go, lines 13–15

db := database.CreateDynamoDB()
checkRequestAllowed(db, utils.ConcurrencyDisablingLambda)

Health checks should remain lightweight and reliable. Please consider removing the DynamoDB call from this endpoint and moving concurrency/rate‐limit enforcement to API Gateway, a proxy layer, or a dedicated middleware.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Verify rate limiting behavior with tests

The rate limiting implementation is a significant change to the health check functionality. However, there don't appear to be any tests validating this behavior. Consider adding unit or integration tests.

🏁 Script executed:

#!/bin/bash
# Look for test files related to health check and rate limiting
find . -name "*_test.go" | xargs grep -l "health\|rate\|limit"

Length of output: 92

🏁 Script executed:

#!/bin/bash
# Check for rate limiting tests in health-check/main_test.go
grep -R -n "checkRequestAllowed\|ConcurrencyDisablingLambda\|RateLimit\|limit" health-check/main_test.go

Length of output: 106

Add tests for health‑check rate‑limiting behavior

The new checkRequestAllowed logic for rate limiting isn’t covered by any existing tests. Please add unit (or integration) tests to validate both allowed and disallowed scenarios:

  • File to update: health-check/main_test.go
  • Cover calls to checkRequestAllowed(db, utils.ConcurrencyDisablingLambda)
  • Include cases for:
    • Requests under the limit (should succeed)
    • Requests exceeding the limit (should be blocked)
  • Optionally, mock the database or rate‑limiter state to simulate edge conditions

return events.APIGatewayProxyResponse{
Body: "Server health is good!!",
StatusCode: 200,
Expand Down