From 53a0a038b78d20f92b80d5ecee5a9099802c7614 Mon Sep 17 00:00:00 2001 From: Peter Wadie Date: Tue, 7 Jul 2026 22:04:15 -0400 Subject: [PATCH 1/4] feat(cloudflare): Instrument Cloudflare rate limiter bindings 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. --- .../instrumentations/worker/instrumentEnv.ts | 18 +++- .../worker/instrumentRateLimit.ts | 45 ++++++++++ packages/cloudflare/src/utils/isBinding.ts | 13 ++- .../instrumentations/instrumentEnv.test.ts | 28 ++++++ .../worker/instrumentRateLimit.test.ts | 86 +++++++++++++++++++ .../cloudflare/test/utils/isBinding.test.ts | 33 ++++++- 6 files changed, 220 insertions(+), 3 deletions(-) create mode 100644 packages/cloudflare/src/instrumentations/worker/instrumentRateLimit.ts create mode 100644 packages/cloudflare/test/instrumentations/worker/instrumentRateLimit.test.ts diff --git a/packages/cloudflare/src/instrumentations/worker/instrumentEnv.ts b/packages/cloudflare/src/instrumentations/worker/instrumentEnv.ts index 393072718ab0..309d7e552612 100644 --- a/packages/cloudflare/src/instrumentations/worker/instrumentEnv.ts +++ b/packages/cloudflare/src/instrumentations/worker/instrumentEnv.ts @@ -1,5 +1,12 @@ import type { CloudflareOptions } from '../../client'; -import { isD1Database, isDurableObjectNamespace, isJSRPC, isQueue, isR2Bucket } from '../../utils/isBinding'; +import { + isD1Database, + isDurableObjectNamespace, + isJSRPC, + isQueue, + isR2Bucket, + isRateLimit, +} from '../../utils/isBinding'; import { instrumentD1 } from './instrumentD1'; import { appendRpcMeta } from '../../utils/rpcMeta'; import { getEffectiveRpcPropagation } from '../../utils/rpcOptions'; @@ -7,6 +14,7 @@ import { instrumentDurableObjectNamespace, STUB_NON_RPC_METHODS } from '../instr import { instrumentFetcher } from './instrumentFetcher'; import { instrumentQueueProducer } from './instrumentQueueProducer'; import { instrumentR2Bucket } from './instrumentR2'; +import { instrumentRateLimit } from './instrumentRateLimit'; function isProxyable(item: unknown): item is object { return item !== null && (typeof item === 'object' || typeof item === 'function'); @@ -23,6 +31,7 @@ const instrumentedBindings = new WeakMap(); * - Service bindings / JSRPC proxies * - Queue producers (via `send` + `sendBatch` duck-typing) * - R2 Buckets (via `head` + `put` + `createMultipartUpload` duck-typing) + * - Rate limiters (via `limit` duck-typing) * * @param env - The Cloudflare env object to instrument * @param options - Optional CloudflareOptions to control RPC trace propagation @@ -68,6 +77,13 @@ export function instrumentEnv>(env: Env, opt return instrumented; } + if (isRateLimit(item)) { + const bindingName = typeof prop === 'string' ? prop : String(prop); + const instrumented = instrumentRateLimit(item, bindingName); + instrumentedBindings.set(item, instrumented); + return instrumented; + } + if (!rpcPropagation) { return item; } diff --git a/packages/cloudflare/src/instrumentations/worker/instrumentRateLimit.ts b/packages/cloudflare/src/instrumentations/worker/instrumentRateLimit.ts new file mode 100644 index 000000000000..66ea3f77c952 --- /dev/null +++ b/packages/cloudflare/src/instrumentations/worker/instrumentRateLimit.ts @@ -0,0 +1,45 @@ +import type { RateLimit, RateLimitOptions, RateLimitOutcome } from '@cloudflare/workers-types'; +import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startSpan } from '@sentry/core'; + +const ORIGIN = 'auto.faas.cloudflare.rate_limit'; +const OP = 'ratelimit'; + +/** + * Wraps a Cloudflare rate limiter binding to create a span on each `limit()` call. + * + * A `success: false` outcome means the request was rate limited. That is an + * expected result rather than an error, so it is recorded as a span attribute + * instead of setting an error status on the span. The rate limit `key` is + * intentionally not recorded because it frequently carries user-identifying + * data (e.g. an IP address or user id). + */ +export function instrumentRateLimit(rateLimit: T, bindingName: string): T { + return new Proxy(rateLimit, { + get(target, prop, receiver) { + if (prop === 'limit') { + const original = Reflect.get(target, prop, receiver) as RateLimit['limit']; + + return function (this: unknown, options: RateLimitOptions): Promise { + return startSpan( + { + op: OP, + name: `rate_limit ${bindingName}`, + attributes: { + 'cloudflare.rate_limit.binding': bindingName, + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: OP, + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: ORIGIN, + }, + }, + async span => { + const outcome = await Reflect.apply(original, target, [options]); + span.setAttribute('cloudflare.rate_limit.success', outcome.success); + return outcome; + }, + ); + }; + } + + return Reflect.get(target, prop, receiver); + }, + }); +} diff --git a/packages/cloudflare/src/utils/isBinding.ts b/packages/cloudflare/src/utils/isBinding.ts index 88832f375f21..c8f6387f3c07 100644 --- a/packages/cloudflare/src/utils/isBinding.ts +++ b/packages/cloudflare/src/utils/isBinding.ts @@ -31,7 +31,7 @@ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ -import type { D1Database, DurableObjectNamespace, Queue, R2Bucket } from '@cloudflare/workers-types'; +import type { D1Database, DurableObjectNamespace, Queue, R2Bucket, RateLimit } from '@cloudflare/workers-types'; /** * Checks if a value is a JSRPC proxy (service binding). @@ -95,3 +95,14 @@ export function isR2Bucket(item: unknown): item is R2Bucket { typeof item.createMultipartUpload === 'function' ); } + +/** + * Duck-type check for RateLimit bindings. + * RateLimit only exposes a single `limit` method. Because that is a fairly + * common method name, this check is intentionally run after the more specific + * binding checks (Queue, R2, D1) in `instrumentEnv`, so those win when a binding + * also happens to expose `limit`. + */ +export function isRateLimit(item: unknown): item is RateLimit { + return item != null && isNotJSRPC(item) && typeof item.limit === 'function'; +} diff --git a/packages/cloudflare/test/instrumentations/instrumentEnv.test.ts b/packages/cloudflare/test/instrumentations/instrumentEnv.test.ts index 9309284abb41..fd99686b91e2 100644 --- a/packages/cloudflare/test/instrumentations/instrumentEnv.test.ts +++ b/packages/cloudflare/test/instrumentations/instrumentEnv.test.ts @@ -256,6 +256,34 @@ describe('instrumentEnv', () => { expect(instrumentDurableObjectNamespace).toHaveBeenCalledWith(doNamespace); }); + it('wraps RateLimit bindings in a proxy and forwards calls', async () => { + const startSpanSpy = vi.spyOn(SentryCore, 'startSpan'); + const limit = vi.fn().mockResolvedValue({ success: true }); + const rateLimiter = { limit }; + const env = { MY_RATE_LIMITER: rateLimiter }; + const instrumented = instrumentEnv(env); + + const wrapped = instrumented.MY_RATE_LIMITER as typeof rateLimiter; + // Wrapped binding is a Proxy, not the original reference + expect(wrapped).not.toBe(rateLimiter); + + const outcome = await wrapped.limit({ key: 'user-123' }); + expect(outcome).toEqual({ success: true }); + expect(limit).toHaveBeenCalledTimes(1); + expect(startSpanSpy).toHaveBeenCalledWith( + expect.objectContaining({ op: 'ratelimit', name: 'rate_limit MY_RATE_LIMITER' }), + expect.any(Function), + ); + }); + + it('caches the wrapped RateLimit binding across repeated access', () => { + const rateLimiter = { limit: vi.fn() }; + const env = { MY_RATE_LIMITER: rateLimiter }; + const instrumented = instrumentEnv(env); + + expect(instrumented.MY_RATE_LIMITER).toBe(instrumented.MY_RATE_LIMITER); + }); + describe('mTLS Fetcher bindings', () => { function createMtlsFetcherProxy(mockFetch: ReturnType) { return new Proxy( diff --git a/packages/cloudflare/test/instrumentations/worker/instrumentRateLimit.test.ts b/packages/cloudflare/test/instrumentations/worker/instrumentRateLimit.test.ts new file mode 100644 index 000000000000..7e3376d05f5f --- /dev/null +++ b/packages/cloudflare/test/instrumentations/worker/instrumentRateLimit.test.ts @@ -0,0 +1,86 @@ +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'); + + describe('limit', () => { + test('forwards the call and returns the outcome', async () => { + const rateLimit = createMockRateLimit(true); + const wrapped = instrumentRateLimit(rateLimit, 'MY_RATE_LIMITER'); + + const outcome = await wrapped.limit({ key: 'user-123' }); + + expect(outcome).toEqual({ success: true }); + expect(rateLimit.limit).toHaveBeenCalledTimes(1); + expect(rateLimit.limit).toHaveBeenCalledWith({ key: 'user-123' }); + }); + + test('returns an unsuccessful (rate-limited) outcome unchanged', async () => { + const wrapped = instrumentRateLimit(createMockRateLimit(false), 'MY_RATE_LIMITER'); + + const outcome = await wrapped.limit({ key: 'user-123' }); + + expect(outcome).toEqual({ success: false }); + }); + + test('starts a span with correct attributes', async () => { + const wrapped = instrumentRateLimit(createMockRateLimit(true), 'MY_RATE_LIMITER'); + await wrapped.limit({ key: 'user-123' }); + + expect(startSpanSpy).toHaveBeenCalledTimes(1); + expect(startSpanSpy).toHaveBeenLastCalledWith( + expect.objectContaining({ + op: 'ratelimit', + name: 'rate_limit MY_RATE_LIMITER', + attributes: expect.objectContaining({ + 'cloudflare.rate_limit.binding': 'MY_RATE_LIMITER', + 'sentry.op': 'ratelimit', + 'sentry.origin': 'auto.faas.cloudflare.rate_limit', + }), + }), + expect.any(Function), + ); + }); + + test('does not record the rate limit key (avoids leaking PII)', async () => { + const wrapped = instrumentRateLimit(createMockRateLimit(true), 'MY_RATE_LIMITER'); + await wrapped.limit({ key: 'super-secret-user-id' }); + + const attributes = startSpanSpy.mock.calls[0]![0].attributes!; + expect(JSON.stringify(attributes)).not.toContain('super-secret-user-id'); + }); + + test('records the outcome success on the span', async () => { + const setAttribute = vi.fn(); + startSpanSpy.mockImplementationOnce(((_options: unknown, callback: (span: unknown) => unknown) => + callback({ setAttribute })) as unknown as typeof SentryCore.startSpan); + + const wrapped = instrumentRateLimit(createMockRateLimit(false), 'MY_RATE_LIMITER'); + await wrapped.limit({ key: 'user-123' }); + + expect(setAttribute).toHaveBeenCalledWith('cloudflare.rate_limit.success', false); + }); + }); + + test('forwards unknown property accesses transparently', () => { + const rateLimit = Object.assign(createMockRateLimit(), { + customMethod: vi.fn().mockReturnValue('hi'), + }) as unknown as RateLimit & { customMethod: () => string }; + const wrapped = instrumentRateLimit(rateLimit, 'MY_RATE_LIMITER') as RateLimit & { customMethod: () => string }; + + expect(wrapped.customMethod()).toBe('hi'); + }); +}); diff --git a/packages/cloudflare/test/utils/isBinding.test.ts b/packages/cloudflare/test/utils/isBinding.test.ts index 28d44fd9936d..3bfba571363b 100644 --- a/packages/cloudflare/test/utils/isBinding.test.ts +++ b/packages/cloudflare/test/utils/isBinding.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest'; -import { isD1Database, isDurableObjectNamespace, isJSRPC, isQueue } from '../../src/utils/isBinding'; +import { isD1Database, isDurableObjectNamespace, isJSRPC, isQueue, isRateLimit } from '../../src/utils/isBinding'; describe('isJSRPC', () => { it('returns false for a plain object', () => { @@ -210,3 +210,34 @@ describe('isD1Database', () => { expect(isD1Database(jsrpcProxy)).toBe(false); }); }); + +describe('isRateLimit', () => { + it('returns true for an object with a limit method', () => { + expect(isRateLimit({ limit: async () => ({ success: true }) })).toBe(true); + }); + + it('returns false when limit is missing', () => { + expect(isRateLimit({ foo: 'bar' })).toBe(false); + }); + + it('returns false when limit is not a function', () => { + expect(isRateLimit({ limit: 'nope' })).toBe(false); + }); + + it('returns false for null and undefined', () => { + expect(isRateLimit(null)).toBe(false); + expect(isRateLimit(undefined)).toBe(false); + }); + + it('returns false for a JSRPC proxy even though it returns a function for limit', () => { + const jsrpcProxy = new Proxy( + {}, + { + get(_target, _prop) { + return () => {}; + }, + }, + ); + expect(isRateLimit(jsrpcProxy)).toBe(false); + }); +}); From d2be6454305e7ed601ac87fed6bde5f4cdbf4253 Mon Sep 17 00:00:00 2001 From: Peter Wadie Date: Tue, 7 Jul 2026 22:20:29 -0400 Subject: [PATCH 2/4] test(cloudflare): Add rate limiter binding integration test 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. --- .../suites/ratelimit/index.ts | 26 ++++++++++ .../suites/ratelimit/test.ts | 48 +++++++++++++++++++ .../suites/ratelimit/wrangler.jsonc | 13 +++++ 3 files changed, 87 insertions(+) create mode 100644 dev-packages/cloudflare-integration-tests/suites/ratelimit/index.ts create mode 100644 dev-packages/cloudflare-integration-tests/suites/ratelimit/test.ts create mode 100644 dev-packages/cloudflare-integration-tests/suites/ratelimit/wrangler.jsonc diff --git a/dev-packages/cloudflare-integration-tests/suites/ratelimit/index.ts b/dev-packages/cloudflare-integration-tests/suites/ratelimit/index.ts new file mode 100644 index 000000000000..a14993d0bc36 --- /dev/null +++ b/dev-packages/cloudflare-integration-tests/suites/ratelimit/index.ts @@ -0,0 +1,26 @@ +import type { RateLimit } from '@cloudflare/workers-types'; +import * as Sentry from '@sentry/cloudflare'; + +interface Env { + SENTRY_DSN: string; + MY_RATE_LIMITER: RateLimit; +} + +export default Sentry.withSentry( + (env: Env) => ({ + dsn: env.SENTRY_DSN, + tracesSampleRate: 1, + }), + { + async fetch(request, env) { + const url = new URL(request.url); + + if (url.pathname === '/ratelimit/limit') { + const outcome = await env.MY_RATE_LIMITER.limit({ key: 'test-key' }); + return new Response(JSON.stringify(outcome)); + } + + return new Response('not found', { status: 404 }); + }, + } as ExportedHandler, +); diff --git a/dev-packages/cloudflare-integration-tests/suites/ratelimit/test.ts b/dev-packages/cloudflare-integration-tests/suites/ratelimit/test.ts new file mode 100644 index 000000000000..24e0f380ed69 --- /dev/null +++ b/dev-packages/cloudflare-integration-tests/suites/ratelimit/test.ts @@ -0,0 +1,48 @@ +import type { Envelope } from '@sentry/core'; +import { expect, it } from 'vitest'; +import { createRunner } from '../../runner'; + +function envelopeItemType(envelope: Envelope): string | undefined { + return envelope[1][0]?.[0]?.type as string | undefined; +} + +function envelopeItem(envelope: Envelope): Record { + return envelope[1][0]![1] as Record; +} + +function findSpans(envelope: Envelope, description: string): Array> { + if (envelopeItemType(envelope) !== 'transaction') return []; + const tx = envelopeItem(envelope); + const spans = (tx.spans as Array>) || []; + return spans.filter(s => s.description === description); +} + +function spanData(span: Record): Record { + return span.data as Record; +} + +it('emits a ratelimit span with the binding name and success outcome', async ({ signal }) => { + const runner = createRunner(__dirname) + .expect((envelope: Envelope) => { + const spans = findSpans(envelope, 'rate_limit MY_RATE_LIMITER'); + expect(spans).toHaveLength(1); + const data = spanData(spans[0]!); + expect({ + op: spans[0]!.op, + description: spans[0]!.description, + 'cloudflare.rate_limit.binding': data['cloudflare.rate_limit.binding'], + 'cloudflare.rate_limit.success': data['cloudflare.rate_limit.success'], + 'sentry.origin': data['sentry.origin'], + }).toEqual({ + op: 'ratelimit', + description: 'rate_limit MY_RATE_LIMITER', + 'cloudflare.rate_limit.binding': 'MY_RATE_LIMITER', + 'cloudflare.rate_limit.success': true, + 'sentry.origin': 'auto.faas.cloudflare.rate_limit', + }); + }) + .start(signal); + + await runner.makeRequest('get', '/ratelimit/limit'); + await runner.completed(); +}); diff --git a/dev-packages/cloudflare-integration-tests/suites/ratelimit/wrangler.jsonc b/dev-packages/cloudflare-integration-tests/suites/ratelimit/wrangler.jsonc new file mode 100644 index 000000000000..2ca5445a6b3d --- /dev/null +++ b/dev-packages/cloudflare-integration-tests/suites/ratelimit/wrangler.jsonc @@ -0,0 +1,13 @@ +{ + "name": "worker-name", + "compatibility_date": "2025-06-17", + "main": "index.ts", + "compatibility_flags": ["nodejs_als"], + "ratelimits": [ + { + "name": "MY_RATE_LIMITER", + "namespace_id": "1001", + "simple": { "limit": 100, "period": 60 }, + }, + ], +} From 2504ba92c4ccd1d423141706bcdaefc45767e005 Mon Sep 17 00:00:00 2001 From: Peter Wadie Date: Mon, 13 Jul 2026 16:44:46 -0400 Subject: [PATCH 3/4] ref(cloudflare): Address review feedback on rate limiter instrumentation Remove the non-standard ratelimit span op and the cloudflare.rate_limit.* attributes (keeping the standard origin), fail-fast in the Proxy handler, and align the integration test with the D1 assertion style plus a rate-limited case. --- .../suites/ratelimit/index.ts | 17 ++++- .../suites/ratelimit/test.ts | 63 +++++++++++-------- .../suites/ratelimit/wrangler.jsonc | 2 +- .../worker/instrumentRateLimit.ts | 46 +++++--------- .../instrumentations/instrumentEnv.test.ts | 2 +- .../worker/instrumentRateLimit.test.ts | 38 +++++------ 6 files changed, 84 insertions(+), 84 deletions(-) diff --git a/dev-packages/cloudflare-integration-tests/suites/ratelimit/index.ts b/dev-packages/cloudflare-integration-tests/suites/ratelimit/index.ts index a14993d0bc36..89f4810b26dd 100644 --- a/dev-packages/cloudflare-integration-tests/suites/ratelimit/index.ts +++ b/dev-packages/cloudflare-integration-tests/suites/ratelimit/index.ts @@ -6,6 +6,10 @@ interface Env { MY_RATE_LIMITER: RateLimit; } +function json(data: unknown): Response { + return new Response(JSON.stringify(data), { headers: { 'content-type': 'application/json' } }); +} + export default Sentry.withSentry( (env: Env) => ({ dsn: env.SENTRY_DSN, @@ -15,9 +19,16 @@ export default Sentry.withSentry( async fetch(request, env) { const url = new URL(request.url); - if (url.pathname === '/ratelimit/limit') { - const outcome = await env.MY_RATE_LIMITER.limit({ key: 'test-key' }); - return new Response(JSON.stringify(outcome)); + if (url.pathname === '/ratelimit/allowed') { + const outcome = await env.MY_RATE_LIMITER.limit({ key: 'allowed-key' }); + return json(outcome); + } + + if (url.pathname === '/ratelimit/blocked') { + // The binding's limit is 1, so the second call within the period is rate limited. + await env.MY_RATE_LIMITER.limit({ key: 'blocked-key' }); + const outcome = await env.MY_RATE_LIMITER.limit({ key: 'blocked-key' }); + return json(outcome); } return new Response('not found', { status: 404 }); diff --git a/dev-packages/cloudflare-integration-tests/suites/ratelimit/test.ts b/dev-packages/cloudflare-integration-tests/suites/ratelimit/test.ts index 24e0f380ed69..3f078ef7ec11 100644 --- a/dev-packages/cloudflare-integration-tests/suites/ratelimit/test.ts +++ b/dev-packages/cloudflare-integration-tests/suites/ratelimit/test.ts @@ -10,39 +10,52 @@ function envelopeItem(envelope: Envelope): Record { return envelope[1][0]![1] as Record; } -function findSpans(envelope: Envelope, description: string): Array> { +function findRateLimitSpans(envelope: Envelope): Array> { if (envelopeItemType(envelope) !== 'transaction') return []; - const tx = envelopeItem(envelope); - const spans = (tx.spans as Array>) || []; - return spans.filter(s => s.description === description); + const spans = (envelopeItem(envelope).spans as Array>) || []; + return spans.filter(s => s.origin === 'auto.faas.cloudflare.rate_limit'); } -function spanData(span: Record): Record { - return span.data as Record; -} +it('instruments an allowed rate limiter call automatically via env', async ({ signal }) => { + const runner = createRunner(__dirname) + .ignore('event') + .expect((envelope: Envelope) => { + expect(envelopeItemType(envelope)).toBe('transaction'); + const event = envelopeItem(envelope); + + expect(event.spans).toEqual([ + { + data: { + 'sentry.origin': 'auto.faas.cloudflare.rate_limit', + }, + description: 'rate_limit MY_RATE_LIMITER', + origin: 'auto.faas.cloudflare.rate_limit', + parent_span_id: expect.any(String), + span_id: expect.any(String), + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + trace_id: expect.any(String), + }, + ]); + }) + .start(signal); + + const response = await runner.makeRequest('get', '/ratelimit/allowed'); + expect(response).toEqual({ success: true }); + await runner.completed(); +}); -it('emits a ratelimit span with the binding name and success outcome', async ({ signal }) => { +it('instruments a rate-limited call automatically via env', async ({ signal }) => { const runner = createRunner(__dirname) + .ignore('event') .expect((envelope: Envelope) => { - const spans = findSpans(envelope, 'rate_limit MY_RATE_LIMITER'); - expect(spans).toHaveLength(1); - const data = spanData(spans[0]!); - expect({ - op: spans[0]!.op, - description: spans[0]!.description, - 'cloudflare.rate_limit.binding': data['cloudflare.rate_limit.binding'], - 'cloudflare.rate_limit.success': data['cloudflare.rate_limit.success'], - 'sentry.origin': data['sentry.origin'], - }).toEqual({ - op: 'ratelimit', - description: 'rate_limit MY_RATE_LIMITER', - 'cloudflare.rate_limit.binding': 'MY_RATE_LIMITER', - 'cloudflare.rate_limit.success': true, - 'sentry.origin': 'auto.faas.cloudflare.rate_limit', - }); + expect(envelopeItemType(envelope)).toBe('transaction'); + // Both `limit()` calls on the blocked endpoint are instrumented. + expect(findRateLimitSpans(envelope)).toHaveLength(2); }) .start(signal); - await runner.makeRequest('get', '/ratelimit/limit'); + const response = await runner.makeRequest('get', '/ratelimit/blocked'); + expect(response).toEqual({ success: false }); await runner.completed(); }); diff --git a/dev-packages/cloudflare-integration-tests/suites/ratelimit/wrangler.jsonc b/dev-packages/cloudflare-integration-tests/suites/ratelimit/wrangler.jsonc index 2ca5445a6b3d..97ed092cdc9b 100644 --- a/dev-packages/cloudflare-integration-tests/suites/ratelimit/wrangler.jsonc +++ b/dev-packages/cloudflare-integration-tests/suites/ratelimit/wrangler.jsonc @@ -7,7 +7,7 @@ { "name": "MY_RATE_LIMITER", "namespace_id": "1001", - "simple": { "limit": 100, "period": 60 }, + "simple": { "limit": 1, "period": 60 }, }, ], } diff --git a/packages/cloudflare/src/instrumentations/worker/instrumentRateLimit.ts b/packages/cloudflare/src/instrumentations/worker/instrumentRateLimit.ts index 66ea3f77c952..0b05a99f3518 100644 --- a/packages/cloudflare/src/instrumentations/worker/instrumentRateLimit.ts +++ b/packages/cloudflare/src/instrumentations/worker/instrumentRateLimit.ts @@ -1,45 +1,31 @@ import type { RateLimit, RateLimitOptions, RateLimitOutcome } from '@cloudflare/workers-types'; -import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startSpan } from '@sentry/core'; +import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startSpan } from '@sentry/core'; const ORIGIN = 'auto.faas.cloudflare.rate_limit'; -const OP = 'ratelimit'; /** * Wraps a Cloudflare rate limiter binding to create a span on each `limit()` call. - * - * A `success: false` outcome means the request was rate limited. That is an - * expected result rather than an error, so it is recorded as a span attribute - * instead of setting an error status on the span. The rate limit `key` is - * intentionally not recorded because it frequently carries user-identifying - * data (e.g. an IP address or user id). */ export function instrumentRateLimit(rateLimit: T, bindingName: string): T { return new Proxy(rateLimit, { get(target, prop, receiver) { - if (prop === 'limit') { - const original = Reflect.get(target, prop, receiver) as RateLimit['limit']; - - return function (this: unknown, options: RateLimitOptions): Promise { - return startSpan( - { - op: OP, - name: `rate_limit ${bindingName}`, - attributes: { - 'cloudflare.rate_limit.binding': bindingName, - [SEMANTIC_ATTRIBUTE_SENTRY_OP]: OP, - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: ORIGIN, - }, - }, - async span => { - const outcome = await Reflect.apply(original, target, [options]); - span.setAttribute('cloudflare.rate_limit.success', outcome.success); - return outcome; - }, - ); - }; + if (prop !== 'limit') { + return Reflect.get(target, prop, receiver); } - return Reflect.get(target, prop, receiver); + const original = Reflect.get(target, prop, receiver) as RateLimit['limit']; + + return function (this: unknown, options: RateLimitOptions): Promise { + return startSpan( + { + name: `rate_limit ${bindingName}`, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: ORIGIN, + }, + }, + () => Reflect.apply(original, target, [options]), + ); + }; }, }); } diff --git a/packages/cloudflare/test/instrumentations/instrumentEnv.test.ts b/packages/cloudflare/test/instrumentations/instrumentEnv.test.ts index fd99686b91e2..40c575898de0 100644 --- a/packages/cloudflare/test/instrumentations/instrumentEnv.test.ts +++ b/packages/cloudflare/test/instrumentations/instrumentEnv.test.ts @@ -271,7 +271,7 @@ describe('instrumentEnv', () => { expect(outcome).toEqual({ success: true }); expect(limit).toHaveBeenCalledTimes(1); expect(startSpanSpy).toHaveBeenCalledWith( - expect.objectContaining({ op: 'ratelimit', name: 'rate_limit MY_RATE_LIMITER' }), + expect.objectContaining({ name: 'rate_limit MY_RATE_LIMITER' }), expect.any(Function), ); }); diff --git a/packages/cloudflare/test/instrumentations/worker/instrumentRateLimit.test.ts b/packages/cloudflare/test/instrumentations/worker/instrumentRateLimit.test.ts index 7e3376d05f5f..61062355da77 100644 --- a/packages/cloudflare/test/instrumentations/worker/instrumentRateLimit.test.ts +++ b/packages/cloudflare/test/instrumentations/worker/instrumentRateLimit.test.ts @@ -1,6 +1,7 @@ import type { RateLimit } from '@cloudflare/workers-types'; import * as SentryCore from '@sentry/core'; -import { beforeEach, describe, expect, test, vi } from 'vitest'; +import type { MockInstance } from 'vitest'; +import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'; import { instrumentRateLimit } from '../../../src/instrumentations/worker/instrumentRateLimit'; function createMockRateLimit(success = true): RateLimit { @@ -10,11 +11,15 @@ function createMockRateLimit(success = true): RateLimit { } describe('instrumentRateLimit', () => { + let startSpanSpy: MockInstance; + beforeEach(() => { - vi.clearAllMocks(); + startSpanSpy = vi.spyOn(SentryCore, 'startSpan'); }); - const startSpanSpy = vi.spyOn(SentryCore, 'startSpan'); + afterEach(() => { + vi.restoreAllMocks(); + }); describe('limit', () => { test('forwards the call and returns the outcome', async () => { @@ -36,21 +41,18 @@ describe('instrumentRateLimit', () => { expect(outcome).toEqual({ success: false }); }); - test('starts a span with correct attributes', async () => { + test('starts a span with the binding name and origin', async () => { const wrapped = instrumentRateLimit(createMockRateLimit(true), 'MY_RATE_LIMITER'); await wrapped.limit({ key: 'user-123' }); expect(startSpanSpy).toHaveBeenCalledTimes(1); expect(startSpanSpy).toHaveBeenLastCalledWith( - expect.objectContaining({ - op: 'ratelimit', + { name: 'rate_limit MY_RATE_LIMITER', - attributes: expect.objectContaining({ - 'cloudflare.rate_limit.binding': 'MY_RATE_LIMITER', - 'sentry.op': 'ratelimit', + attributes: { 'sentry.origin': 'auto.faas.cloudflare.rate_limit', - }), - }), + }, + }, expect.any(Function), ); }); @@ -59,19 +61,7 @@ describe('instrumentRateLimit', () => { const wrapped = instrumentRateLimit(createMockRateLimit(true), 'MY_RATE_LIMITER'); await wrapped.limit({ key: 'super-secret-user-id' }); - const attributes = startSpanSpy.mock.calls[0]![0].attributes!; - expect(JSON.stringify(attributes)).not.toContain('super-secret-user-id'); - }); - - test('records the outcome success on the span', async () => { - const setAttribute = vi.fn(); - startSpanSpy.mockImplementationOnce(((_options: unknown, callback: (span: unknown) => unknown) => - callback({ setAttribute })) as unknown as typeof SentryCore.startSpan); - - const wrapped = instrumentRateLimit(createMockRateLimit(false), 'MY_RATE_LIMITER'); - await wrapped.limit({ key: 'user-123' }); - - expect(setAttribute).toHaveBeenCalledWith('cloudflare.rate_limit.success', false); + expect(JSON.stringify(startSpanSpy.mock.calls[0]![0])).not.toContain('super-secret-user-id'); }); }); From 0f63dd770a4b9016e06df32ba39c0e3dc1ea7260 Mon Sep 17 00:00:00 2001 From: Peter Wadie Date: Mon, 13 Jul 2026 19:12:43 -0400 Subject: [PATCH 4/4] test(cloudflare): Filter rate limiter spans by data['sentry.origin'] --- .../cloudflare-integration-tests/suites/ratelimit/test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dev-packages/cloudflare-integration-tests/suites/ratelimit/test.ts b/dev-packages/cloudflare-integration-tests/suites/ratelimit/test.ts index 3f078ef7ec11..30653c6e943d 100644 --- a/dev-packages/cloudflare-integration-tests/suites/ratelimit/test.ts +++ b/dev-packages/cloudflare-integration-tests/suites/ratelimit/test.ts @@ -13,7 +13,9 @@ function envelopeItem(envelope: Envelope): Record { function findRateLimitSpans(envelope: Envelope): Array> { if (envelopeItemType(envelope) !== 'transaction') return []; const spans = (envelopeItem(envelope).spans as Array>) || []; - return spans.filter(s => s.origin === 'auto.faas.cloudflare.rate_limit'); + return spans.filter( + s => (s.data as Record | undefined)?.['sentry.origin'] === 'auto.faas.cloudflare.rate_limit', + ); } it('instruments an allowed rate limiter call automatically via env', async ({ signal }) => {