Skip to content

Commit b44ca2d

Browse files
fix: suppress canceled rate limit logs (#21)
1 parent 42e4d10 commit b44ca2d

3 files changed

Lines changed: 61 additions & 0 deletions

File tree

internal/middleware/ratelimit.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
package middleware
22

33
import (
4+
"context"
45
"crypto/sha256"
56
"encoding/hex"
7+
"errors"
68
"fmt"
79
"log/slog"
810
"net"
@@ -39,6 +41,10 @@ func RateLimit(store kv.Store, cfg config.RateLimit, logger *slog.Logger) Middle
3941
key := rateLimitKey(ip, cfg.Window, now)
4042
count, err := store.Increment(r.Context(), key, cfg.Window+time.Second)
4143
if err != nil {
44+
if errors.Is(err, context.Canceled) {
45+
trace.SpanFromContext(r.Context()).SetAttributes(observability.RateLimitAttrs{Outcome: observability.RateLimitOutcomeCanceled, Limit: cfg.Requests, Window: cfg.Window}.Attributes()...)
46+
return
47+
}
4248
trace.SpanFromContext(r.Context()).SetAttributes(observability.RateLimitAttrs{Outcome: observability.RateLimitOutcomeStoreError, Limit: cfg.Requests, Window: cfg.Window}.Attributes()...)
4349
logger.ErrorContext(r.Context(), "rate limit check failed", slog.Any("error", err), slog.String("client_ip", ip))
4450
http.Error(w, http.StatusText(http.StatusServiceUnavailable), http.StatusServiceUnavailable)

internal/middleware/ratelimit_test.go

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,11 @@ package middleware
33
import (
44
"context"
55
"errors"
6+
"log/slog"
67
"net/http"
78
"net/http/httptest"
89
"strconv"
10+
"sync/atomic"
911
"testing"
1012
"time"
1113

@@ -78,6 +80,33 @@ func TestRateLimitFailsClosedOnStoreError(t *testing.T) {
7880
}
7981
}
8082

83+
func TestRateLimitDoesNotLogCanceledRequestsAsStoreErrors(t *testing.T) {
84+
t.Parallel()
85+
86+
var errorLogs atomic.Int64
87+
logger := slog.New(countingErrorHandler{count: &errorLogs})
88+
store := incrementFunc(func(ctx context.Context, _ string, _ time.Duration) (int64, error) {
89+
return 0, ctx.Err()
90+
})
91+
handler := RateLimit(store, config.RateLimit{Enabled: true, Requests: 2, Window: time.Minute}, logger)(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
92+
w.WriteHeader(http.StatusNoContent)
93+
}))
94+
ctx, cancel := context.WithCancel(t.Context())
95+
cancel()
96+
req, err := http.NewRequestWithContext(ctx, http.MethodPost, "http://example.test/mcp", nil)
97+
if err != nil {
98+
t.Fatal(err)
99+
}
100+
req.RemoteAddr = "203.0.113.10:1234"
101+
102+
rec := httptest.NewRecorder()
103+
handler.ServeHTTP(rec, req)
104+
105+
if got := errorLogs.Load(); got != 0 {
106+
t.Fatalf("error logs = %d, want 0", got)
107+
}
108+
}
109+
81110
func TestClientIPPrefersFlyHeaderAndNormalizes(t *testing.T) {
82111
t.Parallel()
83112

@@ -114,3 +143,28 @@ func (f incrementFunc) Set(context.Context, string, []byte, time.Duration) error
114143
func (f incrementFunc) Increment(ctx context.Context, key string, ttl time.Duration) (int64, error) {
115144
return f(ctx, key, ttl)
116145
}
146+
147+
type countingErrorHandler struct {
148+
count *atomic.Int64
149+
}
150+
151+
var _ slog.Handler = countingErrorHandler{}
152+
153+
func (h countingErrorHandler) Enabled(_ context.Context, level slog.Level) bool {
154+
return level >= slog.LevelError
155+
}
156+
157+
func (h countingErrorHandler) Handle(_ context.Context, record slog.Record) error {
158+
if record.Level >= slog.LevelError {
159+
h.count.Add(1)
160+
}
161+
return nil
162+
}
163+
164+
func (h countingErrorHandler) WithAttrs([]slog.Attr) slog.Handler {
165+
return h
166+
}
167+
168+
func (h countingErrorHandler) WithGroup(string) slog.Handler {
169+
return h
170+
}

internal/observability/attrs.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,7 @@ const (
128128
RateLimitOutcomeAllowed RateLimitOutcome = "allowed"
129129
RateLimitOutcomeLimited RateLimitOutcome = "limited"
130130
RateLimitOutcomeStoreError RateLimitOutcome = "store_error"
131+
RateLimitOutcomeCanceled RateLimitOutcome = "canceled"
131132
RateLimitOutcomeDisabled RateLimitOutcome = "disabled"
132133
RateLimitOutcomeSkipped RateLimitOutcome = "skipped"
133134
)

0 commit comments

Comments
 (0)