Repository navigation
feat(cloudflare): Add incoming request span options to httpServerIntegration
#25582
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| import * as Sentry from '@sentry/cloudflare'; | ||
|
|
||
| interface Env { | ||
| SENTRY_DSN: string; | ||
| } | ||
|
|
||
| // A plain worker. The `@sentry/cloudflare/vite` plugin wraps it with `withSentry` at build time, | ||
| // with the options from `instrument.server.ts`. | ||
| export default { | ||
| async fetch(request: Request): Promise<Response> { | ||
| const { pathname } = new URL(request.url); | ||
|
|
||
| if (pathname === '/health') { | ||
| Sentry.captureMessage('Health check'); | ||
| return new Response('ok'); | ||
| } | ||
|
|
||
| if (pathname === '/favicon.ico') { | ||
| Sentry.captureMessage('Static asset'); | ||
| return new Response('icon'); | ||
| } | ||
|
|
||
| if (pathname === '/health/stream') { | ||
| // No span for this request. The message is captured after the handler returns, while the | ||
| // body streams, so it is only sent if the SDK waits for the stream to finish. | ||
| const encoder = new TextEncoder(); | ||
| const body = new ReadableStream<Uint8Array>({ | ||
| async start(controller) { | ||
| controller.enqueue(encoder.encode('a')); | ||
| await new Promise(resolve => setTimeout(resolve, 50)); | ||
| Sentry.captureMessage('Streamed after handler return'); | ||
| controller.enqueue(encoder.encode('b')); | ||
| controller.close(); | ||
| }, | ||
| }); | ||
| return new Response(body, { headers: { 'content-type': 'text/event-stream' } }); | ||
| } | ||
|
|
||
| return new Response('ok', { status: 201 }); | ||
| }, | ||
| } satisfies ExportedHandler<Env>; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| import { defineCloudflareOptions, httpServerIntegration } from '@sentry/cloudflare'; | ||
|
|
||
| export default defineCloudflareOptions((env: { SENTRY_DSN: string }) => ({ | ||
| dsn: env.SENTRY_DSN, | ||
| tracesSampleRate: 1.0, | ||
| traceLifecycle: 'static', | ||
| integrations: [ | ||
| httpServerIntegration({ | ||
| ignoreIncomingRequests: urlPath => urlPath.startsWith('/health'), | ||
| onSpanCreated: (span, request) => { | ||
| span.setAttribute('test.on_span_created', new URL(request.url).pathname); | ||
| }, | ||
| onSpanEnd: (span, _request, response) => { | ||
| span.setAttribute('test.on_span_end', response.status); | ||
| }, | ||
| }), | ||
| ], | ||
| })); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| import type { Envelope, Event } from '@sentry/core'; | ||
| import { expect, it } from 'vitest'; | ||
| import { createRunner } from '../../../runner'; | ||
|
|
||
| function itemOf(envelope: Envelope): { type: string; event: Event } { | ||
| const [header, event] = envelope[1][0] as [{ type: string }, Event]; | ||
| return { type: header.type, event }; | ||
| } | ||
|
|
||
| function messageEvent(message: string): (envelope: Envelope) => void { | ||
| return envelope => { | ||
| const { type, event } = itemOf(envelope); | ||
| expect(type).toBe('event'); | ||
| expect(event.message).toBe(message); | ||
| }; | ||
| } | ||
|
|
||
| // The `onSpanCreated` and `onSpanEnd` hooks of the worker set these attributes. | ||
| function apiTransaction(envelope: Envelope): void { | ||
| const { type, event } = itemOf(envelope); | ||
| expect(type).toBe('transaction'); | ||
| expect(event.transaction).toBe('GET /api'); | ||
| expect(event.contexts?.trace?.data).toEqual( | ||
| expect.objectContaining({ | ||
| 'test.on_span_created': '/api', | ||
| 'test.on_span_end': 201, | ||
| }), | ||
| ); | ||
| } | ||
|
|
||
| // Each test makes a request without a span, then a request with one. With `failOnUnexpected`, a | ||
| // transaction for the first request fails the test, as it arrives before the second one. | ||
|
|
||
| it('does not create a span for a request that ignoreIncomingRequests matches', async ({ signal }) => { | ||
| const runner = createRunner(__dirname) | ||
| .unordered() | ||
| .failOnUnexpected() | ||
| .expect(messageEvent('Health check')) | ||
| .expect(apiTransaction) | ||
| .start(signal); | ||
|
|
||
| await runner.makeRequest('get', '/health'); | ||
| await runner.makeRequest('get', '/api'); | ||
| await runner.completed(); | ||
| }); | ||
|
|
||
| it('does not create a span for a static asset request by default', async ({ signal }) => { | ||
| const runner = createRunner(__dirname) | ||
| .unordered() | ||
| .failOnUnexpected() | ||
| .expect(messageEvent('Static asset')) | ||
| .expect(apiTransaction) | ||
| .start(signal); | ||
|
|
||
| await runner.makeRequest('get', '/favicon.ico'); | ||
| await runner.makeRequest('get', '/api'); | ||
| await runner.completed(); | ||
| }); | ||
|
|
||
| it('sends events from a streamed response without a span after the stream finishes', async ({ signal }) => { | ||
| const runner = createRunner(__dirname) | ||
| .unordered() | ||
| .failOnUnexpected() | ||
| .expect(messageEvent('Streamed after handler return')) | ||
| .expect(apiTransaction) | ||
| .start(signal); | ||
|
|
||
| expect(await runner.makeRequest<string>('get', '/health/stream')).toBe('ab'); | ||
| await runner.makeRequest('get', '/api'); | ||
| await runner.completed(); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| import { cloudflare } from '@cloudflare/vite-plugin'; | ||
| import { sentryCloudflareVitePlugin } from '@sentry/cloudflare/vite'; | ||
| import { defineConfig } from 'vite'; | ||
|
|
||
| export default defineConfig({ | ||
| plugins: [cloudflare(), sentryCloudflareVitePlugin()], | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| { | ||
| "$schema": "../../../node_modules/wrangler/config-schema.json", | ||
| "name": "cloudflare-integrations-http-server-spans", | ||
| // `main` points at the source entry. The Sentry Vite plugin builds from it, and the runner serves | ||
| // the built output. | ||
| "main": "index.ts", | ||
| "compatibility_date": "2025-06-17", | ||
| "compatibility_flags": ["nodejs_compat"], | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,15 @@ | ||
| import type { Client, IntegrationFn, MaxRequestBodySize } from '@sentry/core'; | ||
| import { captureBodyFromWinterCGRequest, defineIntegration, getIsolationScope } from '@sentry/core'; | ||
| import type { Client, IntegrationFn, MaxRequestBodySize, Span } from '@sentry/core'; | ||
| import { | ||
| captureBodyFromWinterCGRequest, | ||
| defineIntegration, | ||
| getIsolationScope, | ||
| hasSpansEnabled, | ||
| parseStringToURLObject, | ||
| safeCallback, | ||
| } from '@sentry/core'; | ||
| import type { AssertTrue, CheckServerRequestOptions } from '@sentry/core/server'; | ||
| import { isStaticAssetRequest } from '@sentry/core/server'; | ||
| import { DEBUG_BUILD } from '../debug-build'; | ||
|
|
||
| const INTEGRATION_NAME = 'HttpServer' as const; | ||
|
|
||
|
|
@@ -39,6 +48,42 @@ export interface HttpServerIntegrationOptions { | |
| * ``` | ||
| */ | ||
| ignoreRequestBody?: (url: string, request: Request) => boolean; | ||
|
|
||
| /** | ||
| * Do not create spans for incoming requests whose URL path makes the given callback return `true`. | ||
| * | ||
| * `urlPath` is the path of the request, including the query string. `request` is the incoming | ||
| * {@link Request}. Errors thrown by the handler are still captured. | ||
| */ | ||
| ignoreIncomingRequests?: (urlPath: string, request: Request) => boolean; | ||
|
|
||
| /** | ||
| * Whether to automatically ignore common static asset requests (favicon.ico, robots.txt, etc.) | ||
| * when creating spans. | ||
| * | ||
| * @default `true` | ||
| */ | ||
| ignoreStaticAssets?: boolean; | ||
|
|
||
| /** | ||
| * Whether to create spans for incoming requests. | ||
| * Errors are still captured and request data is still attached when this is `false`. | ||
| * Defaults to the client's tracing configuration (`hasSpansEnabled`). | ||
| */ | ||
| spans?: boolean; | ||
|
|
||
| /** | ||
| * A hook that can be used to mutate the span for incoming requests. | ||
| * This is triggered after the span is created, before the request handler runs. | ||
| */ | ||
| onSpanCreated?: (span: Span, request: Request) => void; | ||
|
|
||
| /** | ||
| * A hook that can be used to mutate the span one last time before it ends. | ||
| * For a streamed response, this runs when the stream finishes. | ||
| * It is not called when the request handler throws. | ||
| */ | ||
| onSpanEnd?: (span: Span, request: Request, response: Response) => void; | ||
| } | ||
|
|
||
| // Checks these options against the shared `ServerRequestOptions`. A new | ||
|
|
@@ -49,30 +94,27 @@ type _ServerRequestOptionsCheck = AssertTrue< | |
| HttpServerIntegrationOptions, | ||
| Request, | ||
| Response, | ||
| // Not supported yet. | ||
| | 'spans' | ||
| | 'ignoreIncomingRequests' | ||
| | 'ignoreStaticAssets' | ||
| | 'onSpanCreated' | ||
| | 'onSpanEnd' | ||
| // Not supported. A worker has no long-lived process to run the timer | ||
| // that flushes session aggregates. | ||
| | 'sessions' | ||
| | 'sessionFlushingDelayMS' | ||
| 'sessions' | 'sessionFlushingDelayMS' | ||
| > | ||
| >; | ||
|
|
||
| interface HttpServerIntegrationInstance { | ||
| interface HttpServerIntegrationInstance extends HttpServerIntegrationOptions { | ||
| name: string; | ||
| maxRequestBodySize: MaxRequestBodySize | undefined; | ||
| ignoreRequestBody?: (url: string, request: Request) => boolean; | ||
| } | ||
|
|
||
| const _httpServerIntegration = ((options: HttpServerIntegrationOptions = {}): HttpServerIntegrationInstance => { | ||
| return { | ||
| name: INTEGRATION_NAME, | ||
| maxRequestBodySize: options.maxRequestBodySize, | ||
| ignoreRequestBody: options.ignoreRequestBody, | ||
| ignoreIncomingRequests: options.ignoreIncomingRequests, | ||
| ignoreStaticAssets: options.ignoreStaticAssets ?? true, | ||
| spans: options.spans, | ||
| onSpanCreated: options.onSpanCreated, | ||
| onSpanEnd: options.onSpanEnd, | ||
| }; | ||
| }) satisfies IntegrationFn; | ||
|
|
||
|
|
@@ -122,10 +164,75 @@ export async function captureIncomingRequestBody(client: Client, request: Reques | |
| return; | ||
| } | ||
|
|
||
| if (integration.ignoreRequestBody?.(request.url, request)) { | ||
| const ignoreRequestBody = integration.ignoreRequestBody; | ||
| if ( | ||
| ignoreRequestBody && | ||
| // If the filter throws, skip the body so it cannot leak data the filter was meant to hide. | ||
| safeCallback( | ||
| DEBUG_BUILD ? 'Error in `ignoreRequestBody`, skipping the body:' : '', | ||
| () => ignoreRequestBody(request.url, request), | ||
| () => true, | ||
| ) | ||
| ) { | ||
| return; | ||
| } | ||
|
|
||
| const isolationScope = getIsolationScope(); | ||
| await captureBodyFromWinterCGRequest(request, isolationScope, effectiveBodySize); | ||
| } | ||
|
|
||
| /** | ||
| * Whether to create a span for the incoming request, based on the HttpServer integration config. | ||
| * Called internally by `wrapRequestHandler`. | ||
| */ | ||
| export function shouldCreateIncomingRequestSpan(client: Client | undefined, request: Request): boolean { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same answer, I guess. We should either do this for all 3, or none of them, but Node already behaves this way, and it seems low risk. In fact, for Cloudflare, this affects even less traffic, since typically the Worker typically won't be responsible for serving static files anyway, I would imagine? In any event, we should definitely keep the machinery, and either leave the default on all three true (to match node) or false (to prevent a behavior change, and flip to true in v12). Probably the sensible way to approach this is another deliberate PR on top of (or after) this stack, which sets the default to false, if that's how we want to go. |
||
| if (!client) { | ||
| return false; | ||
| } | ||
|
|
||
| const integration = client.getIntegrationByName<HttpServerIntegrationInstance>(INTEGRATION_NAME); | ||
| if (!(integration?.spans ?? hasSpansEnabled(client.getOptions()))) { | ||
| return false; | ||
| } | ||
|
|
||
| const urlObject = parseStringToURLObject(request.url); | ||
| const urlPath = urlObject ? `${urlObject.pathname}${urlObject.search}` : '/'; | ||
| if (integration?.ignoreStaticAssets && request.method === 'GET' && isStaticAssetRequest(urlPath)) { | ||
| return false; | ||
| } | ||
|
|
||
| const ignoreIncomingRequests = integration?.ignoreIncomingRequests; | ||
| return !( | ||
| ignoreIncomingRequests && | ||
| safeCallback( | ||
| DEBUG_BUILD ? 'Error in `ignoreIncomingRequests`, creating the span:' : '', | ||
| () => ignoreIncomingRequests(urlPath, request), | ||
| () => false, | ||
| ) | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * The `onSpanCreated` and `onSpanEnd` hooks of the HttpServer integration, guarded so that a hook | ||
| * that throws does not fail the request. Called internally by `wrapRequestHandler`. | ||
| */ | ||
| export function getIncomingRequestSpanHooks(client: Client | undefined): { | ||
| onSpanCreated: (span: Span, request: Request) => void; | ||
| onSpanEnd: (span: Span, request: Request, response: Response) => void; | ||
| } { | ||
| const integration = client?.getIntegrationByName<HttpServerIntegrationInstance>(INTEGRATION_NAME); | ||
| return { | ||
| onSpanCreated: (span, request) => | ||
| safeCallback( | ||
| DEBUG_BUILD ? 'Error in `onSpanCreated`:' : '', | ||
| () => integration?.onSpanCreated?.(span, request), | ||
| () => undefined, | ||
| ), | ||
| onSpanEnd: (span, request, response) => | ||
| safeCallback( | ||
| DEBUG_BUILD ? 'Error in `onSpanEnd`:' : '', | ||
| () => integration?.onSpanEnd?.(span, request, response), | ||
| () => undefined, | ||
| ), | ||
| }; | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.