feat(cloudflare): Instrument Cloudflare rate limiter bindings#22035
feat(cloudflare): Instrument Cloudflare rate limiter bindings#22035PeterWadie wants to merge 2 commits into
Conversation
Automatically wraps limit() calls on Cloudflare rate limiter bindings in a span, mirroring the existing R2/Queue/D1 binding instrumentation. The rate-limited outcome is recorded via a span attribute rather than an error status, and the rate limit key is not recorded to avoid leaking PII.
PeterWadie
requested review from
JPeer264,
andreiborza and
mydea
and removed request for
a team
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 53a0a03. Configure here.
There was a problem hiding this comment.
Pull request overview
Adds first-class tracing support for Cloudflare Workers RateLimit bindings in @sentry/cloudflare. The implementation follows the existing env-binding model (D1 / Queue / R2) by detecting the binding on env access, proxy-wrapping it, and creating a span around each limit() call while recording the outcome as a span attribute (without capturing the key to avoid PII).
Changes:
- Add
isRateLimitduck-typing to detect RateLimit bindings (limitmethod + not JSRPC). - Instrument
envaccess to wrap detected RateLimit bindings and cache the wrapped proxy. - Introduce
instrumentRateLimitwhich creates aratelimitspan perlimit()call and recordscloudflare.rate_limit.success.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/cloudflare/src/utils/isBinding.ts | Adds isRateLimit duck-type guard for RateLimit bindings. |
| packages/cloudflare/src/instrumentations/worker/instrumentRateLimit.ts | New instrumentation proxy that wraps limit() with a span and records the success outcome. |
| packages/cloudflare/src/instrumentations/worker/instrumentEnv.ts | Wires RateLimit detection into env proxying + caching alongside existing binding instrumentation. |
| packages/cloudflare/test/utils/isBinding.test.ts | Adds unit coverage for isRateLimit behavior (including JSRPC proxy exclusion). |
| packages/cloudflare/test/instrumentations/worker/instrumentRateLimit.test.ts | Adds unit tests for span creation/attributes, forwarding behavior, and avoiding key/PII capture. |
| packages/cloudflare/test/instrumentations/instrumentEnv.test.ts | Adds tests for env detection, wrapping, forwarding, and caching of RateLimit bindings. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Adds an integration suite that exercises a real rate limiter binding through wrangler and asserts the emitted ratelimit span and its attributes, matching the coverage of the R2 and Queue binding instrumentations.
| import type { RateLimit } from '@cloudflare/workers-types'; | ||
| import * as SentryCore from '@sentry/core'; | ||
| import { beforeEach, describe, expect, test, vi } from 'vitest'; | ||
| import { instrumentRateLimit } from '../../../src/instrumentations/worker/instrumentRateLimit'; | ||
|
|
||
| function createMockRateLimit(success = true): RateLimit { | ||
| return { | ||
| limit: vi.fn().mockResolvedValue({ success }), | ||
| } as unknown as RateLimit; | ||
| } | ||
|
|
||
| describe('instrumentRateLimit', () => { | ||
| beforeEach(() => { | ||
| vi.clearAllMocks(); | ||
| }); | ||
|
|
||
| const startSpanSpy = vi.spyOn(SentryCore, 'startSpan'); | ||
|
|
Adds automatic tracing for Cloudflare rate limiter bindings, mirroring the existing R2, Queue, and D1 binding instrumentation. When a
RateLimitbinding is accessed onenv, itslimit()calls are wrapped in a span and the rate-limit outcome is recorded as a span attribute.Details
instrumentRateLimitwraps the binding in aProxyand starts aratelimitspan aroundlimit(), with acloudflare.rate_limit.bindingattribute.success: falseresult means the request was rate limited. Since that is an expected outcome rather than an error, it is recorded via acloudflare.rate_limit.successattribute instead of setting an error status on the span.keyis intentionally not recorded, as it commonly contains user-identifying data (e.g. an IP address or user id).limitduck-type inisBinding, wired intoinstrumentEnvafter the more specific Queue/R2/D1 checks so those win when a binding also happens to exposelimit.envdetection/caching.I went with
ratelimitfor the span op andauto.faas.cloudflare.rate_limitfor the origin to stay consistent with the existing Cloudflare binding instrumentations. Happy to adjust the op/attribute naming to match your conventions.Fixes #20871