Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,6 @@ Sentry.init({
release: '1.0',
tracesSampleRate: 1.0,
transport: loggingTransport,
dataCollection: { httpBodies: [] },
integrations: [Sentry.httpIntegration({ maxIncomingRequestBodySize: 'small' })],
});
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,34 @@ describe('express with httpIntegration and not defined maxIncomingRequestBodySiz
});
});

describe('express with httpIntegration, disabled httpBodies, and explicit maxIncomingRequestBodySize', () => {
afterAll(() => {
cleanupChildProcesses();
});

createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument-small.mjs', (createRunner, test) => {
test('captures request bodies because the explicit size overrides dataCollection.httpBodies', async () => {
const runner = createRunner()
.expect({
transaction: {
transaction: 'POST /test-body-size',
request: {
data: JSON.stringify(generatePayload(MAX_SMALL)),
},
},
})
.start();

await runner.makeRequest('post', '/test-body-size', {
headers: { 'Content-Type': 'application/json' },
data: JSON.stringify(generatePayload(MAX_SMALL)),
});

await runner.completed();
});
});
});

describe('express with httpIntegration and maxIncomingRequestBodySize: "none"', () => {
afterAll(() => {
cleanupChildProcesses();
Expand Down
15 changes: 8 additions & 7 deletions packages/cloudflare/src/integrations/httpServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,14 @@ export interface HttpServerIntegrationOptions {

interface HttpServerIntegrationInstance {
name: string;
maxRequestBodySize: MaxRequestBodySize;
maxRequestBodySize: MaxRequestBodySize | undefined;
ignoreRequestBody?: (url: string, request: Request) => boolean;
}

const _httpServerIntegration = ((options: HttpServerIntegrationOptions = {}): HttpServerIntegrationInstance => {
return {
name: INTEGRATION_NAME,
maxRequestBodySize: options.maxRequestBodySize ?? 'medium',
maxRequestBodySize: options.maxRequestBodySize,
ignoreRequestBody: options.ignoreRequestBody,
};
}) satisfies IntegrationFn;
Expand Down Expand Up @@ -85,11 +85,12 @@ export async function captureIncomingRequestBody(client: Client, request: Reques
return;
}

// TODO(v11): Gate incoming request body capture on `dataCollection.httpBodies` (capture only when
// `'incomingRequest'` is listed) instead of defaulting to `'medium'`.
const maxRequestBodySize = integration.maxRequestBodySize;
const configuredBodySize = integration.maxRequestBodySize;
const effectiveBodySize: MaxRequestBodySize =
configuredBodySize ??
(client.getDataCollectionOptions().httpBodies.includes('incomingRequest') ? 'medium' : 'none');

if (maxRequestBodySize === 'none') {
if (effectiveBodySize === 'none') {
return;
}

Expand All @@ -104,5 +105,5 @@ export async function captureIncomingRequestBody(client: Client, request: Reques
}

const isolationScope = getIsolationScope();
await captureBodyFromWinterCGRequest(request, isolationScope, maxRequestBodySize);
await captureBodyFromWinterCGRequest(request, isolationScope, effectiveBodySize);
}
68 changes: 63 additions & 5 deletions packages/cloudflare/test/request.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { beforeAll, beforeEach, describe, expect, onTestFinished, test, vi } fro
import { setAsyncLocalStorageAsyncContextStrategy } from '../src/async';
import type { CloudflareOptions } from '../src/client';
import { CloudflareClient } from '../src/client';
import { httpServerIntegration } from '../src/integrations/httpServer';
import { wrapRequestHandler } from '../src/request';

const MOCK_OPTIONS: CloudflareOptions = {
Expand Down Expand Up @@ -206,18 +207,14 @@ describe('withSentry', () => {
expect(sentryEvent.contexts?.culture).toEqual({ timezone: 'UTC' });
});

// TODO(v11): Body capture should be gated on `dataCollection.httpBodies` (only capture when
// `'incomingRequest'` is listed). Until then we keep the historical behavior of capturing
// incoming request bodies by default at `'medium'`, consistent with the Node SDK.
test('captures request body with default integration (medium size)', async () => {
test('captures request body by default (all body types included by default)', async () => {
let sentryEvent: Event = {};
const context = createMockExecutionContext();

await wrapRequestHandler(
{
options: {
...MOCK_OPTIONS,
// Default integrations include httpServerIntegration with 'medium' default
beforeSend(event) {
sentryEvent = event;
return null;
Expand All @@ -241,6 +238,67 @@ describe('withSentry', () => {
);
});

test('does not capture request body when dataCollection.httpBodies excludes incomingRequest', async () => {
let sentryEvent: Event = {};
const context = createMockExecutionContext();

await wrapRequestHandler(
{
options: {
...MOCK_OPTIONS,
dataCollection: { httpBodies: [] },
beforeSend(event) {
sentryEvent = event;
return null;
},
},
request: new Request('https://example.com', {
method: 'POST',
headers: { 'content-type': 'application/json' },
body: JSON.stringify({ username: 'test', data: 'value' }),
}),
context,
},
() => {
SentryCore.captureMessage('request body');
return new Response('test');
},
);

expect(sentryEvent.sdkProcessingMetadata?.normalizedRequest?.data).toBeUndefined();
});

test('explicit maxRequestBodySize overrides dataCollection.httpBodies', async () => {
let sentryEvent: Event = {};
const context = createMockExecutionContext();

await wrapRequestHandler(
{
options: {
...MOCK_OPTIONS,
dataCollection: { httpBodies: [] },
integrations: [httpServerIntegration({ maxRequestBodySize: 'medium' })],
beforeSend(event) {
sentryEvent = event;
return null;
},
},
request: new Request('https://example.com', {
method: 'POST',
headers: { 'content-type': 'application/json' },
body: JSON.stringify({ key: 'value' }),
}),
context,
},
() => {
SentryCore.captureMessage('request body');
return new Response('test');
},
);

expect(sentryEvent.sdkProcessingMetadata?.normalizedRequest?.data).toEqual(JSON.stringify({ key: 'value' }));
});

// TODO(v11): Cookies should be attached (subject to denylist filtering) by default. Until then we keep the
// historical Cloudflare behavior of not attaching cookies unless the user explicitly opts in.
test('does not capture cookies by default', async () => {
Expand Down
10 changes: 7 additions & 3 deletions packages/core/src/integrations/http/server-subscription.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,14 +108,18 @@ export function instrumentServer(options: HttpInstrumentationOptions, server: Ht
const url = request.url || '/';
const normalizedRequest = httpRequestToRequestData(request);
const {
maxRequestBodySize = 'medium',
maxRequestBodySize: configuredBodySize,
ignoreRequestBody,
sessions = true,
sessionFlushingDelayMS = 60_000,
} = options;

if (maxRequestBodySize !== 'none' && !ignoreRequestBody?.(url, request)) {
patchRequestToCaptureBody(request, isolationScope, maxRequestBodySize, INTEGRATION_NAME);
const effectiveBodySize =
configuredBodySize ??
(client.getDataCollectionOptions().httpBodies.includes('incomingRequest') ? 'medium' : 'none');

if (effectiveBodySize !== 'none' && !ignoreRequestBody?.(url, request)) {
patchRequestToCaptureBody(request, isolationScope, effectiveBodySize, INTEGRATION_NAME);
}

// Update the isolation scope, isolate this request
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ export function defaultPiiToCollectionOptions(sendDefaultPii?: boolean): Resolve
userInfo: false,
cookies: { deny: PII_HEADER_SNIPPETS },
httpHeaders: { request: { deny: PII_HEADER_SNIPPETS }, response: { deny: PII_HEADER_SNIPPETS } },
httpBodies: [],
// Request bodies were governed by maxRequestBodySize, not sendDefaultPii, so preserve that behavior.
httpBodies: ['incomingRequest', 'outgoingRequest', 'incomingResponse', 'outgoingResponse'],
urlQueryParams: { deny: PII_HEADER_SNIPPETS },
// The GraphQL document has literal values redacted at collection time, so it was historically
// always attached regardless of `sendDefaultPii`; keep it on to preserve that behavior.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ describe('defaultPiiToCollectionOptions', () => {
request: { deny: ['forwarded', '-ip', 'remote-', 'via', '-user'] },
response: { deny: ['forwarded', '-ip', 'remote-', 'via', '-user'] },
},
httpBodies: [],
httpBodies: ['incomingRequest', 'outgoingRequest', 'incomingResponse', 'outgoingResponse'],
urlQueryParams: { deny: ['forwarded', '-ip', 'remote-', 'via', '-user'] },
graphQL: { document: true, variables: true },
genAI: { inputs: true, outputs: true },
Expand All @@ -43,7 +43,7 @@ describe('defaultPiiToCollectionOptions', () => {
request: { deny: ['forwarded', '-ip', 'remote-', 'via', '-user'] },
response: { deny: ['forwarded', '-ip', 'remote-', 'via', '-user'] },
},
httpBodies: [],
httpBodies: ['incomingRequest', 'outgoingRequest', 'incomingResponse', 'outgoingResponse'],
urlQueryParams: { deny: ['forwarded', '-ip', 'remote-', 'via', '-user'] },
graphQL: { document: true, variables: true },
genAI: { inputs: true, outputs: true },
Expand All @@ -61,7 +61,7 @@ describe('defaultPiiToCollectionOptions', () => {
request: { deny: ['forwarded', '-ip', 'remote-', 'via', '-user'] },
response: { deny: ['forwarded', '-ip', 'remote-', 'via', '-user'] },
},
httpBodies: [],
httpBodies: ['incomingRequest', 'outgoingRequest', 'incomingResponse', 'outgoingResponse'],
urlQueryParams: { deny: ['forwarded', '-ip', 'remote-', 'via', '-user'] },
graphQL: { document: true, variables: true },
genAI: { inputs: true, outputs: true },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ describe('resolveDataCollectionOptions', () => {

// sendDefaultPii undefined → restrictive bridge (backward compat; userInfo defaults to true only when dataCollection is set)
expect(result.userInfo).toBe(false);
expect(result.httpBodies).toEqual([]);
expect(result.httpBodies).toEqual(['incomingRequest', 'outgoingRequest', 'incomingResponse', 'outgoingResponse']);
expect(result.genAI).toEqual({ inputs: true, outputs: true });
// GraphQL documents are redacted at collection time, so they stay on to preserve legacy behavior.
expect(result.graphQL).toEqual({ document: true, variables: true });
Expand Down Expand Up @@ -59,7 +59,7 @@ describe('resolveDataCollectionOptions', () => {
const result = resolveDataCollectionOptions({ sendDefaultPii: false });

expect(result.userInfo).toBe(false);
expect(result.httpBodies).toEqual([]);
expect(result.httpBodies).toEqual(['incomingRequest', 'outgoingRequest', 'incomingResponse', 'outgoingResponse']);
expect(result.genAI).toEqual({ inputs: true, outputs: true });
expect(result.graphQL).toEqual({ document: true, variables: true });
expect(result.databaseQueryData).toBe(false);
Expand Down
2 changes: 1 addition & 1 deletion packages/deno/src/integrations/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ const _denoHttpIntegration = ((options: DenoHttpIntegrationOptions = {}) => {
spans: options.spans,
ignoreStaticAssets: options.ignoreStaticAssets,
ignoreIncomingRequests: options.ignoreIncomingRequests,
maxRequestBodySize: options.maxRequestBodySize ?? 'medium',
maxRequestBodySize: options.maxRequestBodySize,
ignoreRequestBody: options.ignoreRequestBody,
onSpanCreated: options.onIncomingSpanCreated,
onSpanEnd: options.onIncomingSpanEnd,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ const _httpServerIntegration = ((options: HttpServerIntegrationOptions = {}) =>
const _options = {
sessions: options.sessions ?? true,
sessionFlushingDelayMS: options.sessionFlushingDelayMS ?? 60_000,
maxRequestBodySize: options.maxRequestBodySize ?? 'medium',
maxRequestBodySize: options.maxRequestBodySize,
// Server spans are created by `httpServerSpansIntegration` via the
// `httpServerRequest` client event + `_startSpanCallback`, not by the
// core subscription helper. Explicitly opt out so the helper does not
Expand Down
Loading