From 4fe3ec8d16bfbd3def008bc0a532f591d4072d87 Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Fri, 22 May 2026 14:05:26 +0200 Subject: [PATCH 1/2] migrate --- .../pagesRouterRoutingInstrumentation.ts | 14 ++- .../common/utils/addHeadersAsAttributes.ts | 9 +- .../common/utils/setUrlProcessingMetadata.ts | 8 +- .../common/withServerActionInstrumentation.ts | 4 +- .../pagesRouterInstrumentation.test.ts | 6 ++ .../test/utils/addHeadersAsAttributes.test.ts | 63 +++++++++++ .../utils/setUrlProcessingMetadata.test.ts | 101 ++++++++++++++++++ 7 files changed, 199 insertions(+), 6 deletions(-) create mode 100644 packages/nextjs/test/utils/addHeadersAsAttributes.test.ts create mode 100644 packages/nextjs/test/utils/setUrlProcessingMetadata.test.ts diff --git a/packages/nextjs/src/client/routing/pagesRouterRoutingInstrumentation.ts b/packages/nextjs/src/client/routing/pagesRouterRoutingInstrumentation.ts index a1bec4eac04a..100939d046e6 100644 --- a/packages/nextjs/src/client/routing/pagesRouterRoutingInstrumentation.ts +++ b/packages/nextjs/src/client/routing/pagesRouterRoutingInstrumentation.ts @@ -1,5 +1,6 @@ import type { Client, TransactionSource } from '@sentry/core'; import { + _INTERNAL_filterKeyValueData, browserPerformanceTimeOrigin, debug, parseBaggageHeader, @@ -129,7 +130,7 @@ export function pagesRouterInstrumentPageLoad(client: Client): void { [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.nextjs.pages_router_instrumentation', [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: route ? 'route' : 'url', - ...(params && client.getOptions().sendDefaultPii && { ...params }), + ...(params && getFilteredRouteParams(params, client)), }, }, { sentryTrace, baggage }, @@ -185,6 +186,17 @@ function getNextRouteFromPathname(pathname: string): string | undefined { }); } +function getFilteredRouteParams(params: ParsedUrlQuery, client: Client): Record { + const { queryParams } = client.getDataCollectionOptions(); + const stringParams: Record = {}; + for (const [key, value] of Object.entries(params)) { + if (typeof value === 'string') { + stringParams[key] = value; + } + } + return _INTERNAL_filterKeyValueData(stringParams, queryParams); +} + /** * Converts a Next.js style route to a regular expression that matches on pathnames (no query params or URL fragments). * diff --git a/packages/nextjs/src/common/utils/addHeadersAsAttributes.ts b/packages/nextjs/src/common/utils/addHeadersAsAttributes.ts index 8d4b0eca3724..162828c0ac2a 100644 --- a/packages/nextjs/src/common/utils/addHeadersAsAttributes.ts +++ b/packages/nextjs/src/common/utils/addHeadersAsAttributes.ts @@ -12,12 +12,19 @@ export function addHeadersAsAttributes( return {}; } + const client = getClient(); + const { httpHeaders } = client?.getDataCollectionOptions() ?? { httpHeaders: { request: false, response: false } }; + + if (httpHeaders.request === false) { + return {}; + } + const headersDict: Record = headers instanceof Headers || (typeof headers === 'object' && 'get' in headers) ? winterCGHeadersToDict(headers as Headers) : headers; - const headerAttributes = httpHeadersToSpanAttributes(headersDict, getClient()?.getOptions().sendDefaultPii ?? false); + const headerAttributes = httpHeadersToSpanAttributes(headersDict, httpHeaders.request === true); if (span) { span.setAttributes(headerAttributes); diff --git a/packages/nextjs/src/common/utils/setUrlProcessingMetadata.ts b/packages/nextjs/src/common/utils/setUrlProcessingMetadata.ts index 0c7e0c3b33f2..a05039e1ef00 100644 --- a/packages/nextjs/src/common/utils/setUrlProcessingMetadata.ts +++ b/packages/nextjs/src/common/utils/setUrlProcessingMetadata.ts @@ -11,9 +11,13 @@ export function setUrlProcessingMetadata(event: Event): void { return; } - // Only add URL if sendDefaultPii is enabled, as URLs may contain PII const client = getClient(); - if (!client?.getOptions().sendDefaultPii) { + if (!client) { + return; + } + + // todo(v11): Replace with a dataCollection gate once URL collection is covered by the spec. + if (!client.getOptions().sendDefaultPii) { return; } diff --git a/packages/nextjs/src/common/withServerActionInstrumentation.ts b/packages/nextjs/src/common/withServerActionInstrumentation.ts index 5a2c884a8f85..a1ff9ac12ffe 100644 --- a/packages/nextjs/src/common/withServerActionInstrumentation.ts +++ b/packages/nextjs/src/common/withServerActionInstrumentation.ts @@ -70,7 +70,7 @@ async function withServerActionInstrumentationImplementation> { return withIsolationScope(async isolationScope => { - const sendDefaultPii = getClient()?.getOptions().sendDefaultPii; + const shouldRecordResponse = getClient()?.getDataCollectionOptions().httpBodies.includes('outgoingResponse'); let sentryTraceHeader; let baggageHeader; @@ -138,7 +138,7 @@ async function withServerActionInstrumentationImplementation { 'sentry.op': 'pageload', 'sentry.origin': 'auto.pageload.nextjs.pages_router_instrumentation', 'sentry.source': 'route', + user: 'lforst', + id: '1337', + q: '42', }, }, ], @@ -190,6 +193,9 @@ describe('pagesRouterInstrumentPageLoad', () => { const client = { emit, getOptions: () => ({}), + getDataCollectionOptions: () => ({ + queryParams: { deny: [] }, + }), } as unknown as Client; pagesRouterInstrumentPageLoad(client); diff --git a/packages/nextjs/test/utils/addHeadersAsAttributes.test.ts b/packages/nextjs/test/utils/addHeadersAsAttributes.test.ts new file mode 100644 index 000000000000..639a08a23b52 --- /dev/null +++ b/packages/nextjs/test/utils/addHeadersAsAttributes.test.ts @@ -0,0 +1,63 @@ +import * as SentryCore from '@sentry/core'; +import { describe, expect, it, vi } from 'vitest'; +import { addHeadersAsAttributes } from '../../src/common/utils/addHeadersAsAttributes'; + +describe('addHeadersAsAttributes', () => { + it('returns empty object when headers are undefined', () => { + expect(addHeadersAsAttributes(undefined)).toEqual({}); + }); + + it('returns empty object when httpHeaders.request is false', () => { + vi.spyOn(SentryCore, 'getClient').mockReturnValue({ + getDataCollectionOptions: () => ({ + httpHeaders: { request: false, response: true }, + }), + } as unknown as SentryCore.Client); + + const result = addHeadersAsAttributes({ 'content-type': 'application/json' }); + expect(result).toEqual({}); + }); + + it('includes all headers with sensitive filtering when httpHeaders.request is true', () => { + vi.spyOn(SentryCore, 'getClient').mockReturnValue({ + getDataCollectionOptions: () => ({ + httpHeaders: { request: true, response: true }, + }), + } as unknown as SentryCore.Client); + + const result = addHeadersAsAttributes({ + 'content-type': 'application/json', + accept: 'text/html', + }); + + expect(result).toMatchObject({ + 'http.request.header.content_type': 'application/json', + 'http.request.header.accept': 'text/html', + }); + }); + + it('applies stricter PII filtering when httpHeaders.request is a deny list', () => { + vi.spyOn(SentryCore, 'getClient').mockReturnValue({ + getDataCollectionOptions: () => ({ + httpHeaders: { request: { deny: [] }, response: true }, + }), + } as unknown as SentryCore.Client); + + const result = addHeadersAsAttributes({ + 'content-type': 'application/json', + accept: 'text/html', + }); + + expect(result).toMatchObject({ + 'http.request.header.content_type': 'application/json', + 'http.request.header.accept': 'text/html', + }); + }); + + it('returns empty object when no client is available', () => { + vi.spyOn(SentryCore, 'getClient').mockReturnValue(undefined); + + const result = addHeadersAsAttributes({ 'content-type': 'application/json' }); + expect(result).toEqual({}); + }); +}); diff --git a/packages/nextjs/test/utils/setUrlProcessingMetadata.test.ts b/packages/nextjs/test/utils/setUrlProcessingMetadata.test.ts new file mode 100644 index 000000000000..d41d728d3a55 --- /dev/null +++ b/packages/nextjs/test/utils/setUrlProcessingMetadata.test.ts @@ -0,0 +1,101 @@ +import type { Event } from '@sentry/core'; +import * as SentryCore from '@sentry/core'; +import { describe, expect, it, vi } from 'vitest'; +import { setUrlProcessingMetadata } from '../../src/common/utils/setUrlProcessingMetadata'; + +function makeTransactionEvent(overrides?: Partial): Event { + return { + type: 'transaction', + contexts: { + trace: { + op: 'http.server', + data: { + 'next.route': '/api/users/[id]', + 'http.target': '/api/users/123', + }, + }, + }, + sdkProcessingMetadata: { + capturedSpanIsolationScope: { + getScopeData: () => ({ + sdkProcessingMetadata: { + normalizedRequest: { + headers: { + 'x-forwarded-proto': 'https', + host: 'example.com', + }, + }, + }, + }), + }, + }, + ...overrides, + }; +} + +describe('setUrlProcessingMetadata', () => { + it('skips non-transaction events', () => { + const event = makeTransactionEvent({ type: undefined }); + setUrlProcessingMetadata(event); + // No error thrown, nothing changed + }); + + it('skips when sendDefaultPii is false', () => { + vi.spyOn(SentryCore, 'getClient').mockReturnValue({ + getOptions: () => ({ sendDefaultPii: false }), + } as unknown as SentryCore.Client); + + const scopeData = { + sdkProcessingMetadata: { + normalizedRequest: { + headers: { host: 'example.com' }, + }, + }, + }; + + const event = makeTransactionEvent({ + sdkProcessingMetadata: { + capturedSpanIsolationScope: { getScopeData: () => scopeData }, + }, + }); + + setUrlProcessingMetadata(event); + expect(scopeData.sdkProcessingMetadata.normalizedRequest).not.toHaveProperty('url'); + }); + + it('adds URL when sendDefaultPii is true', () => { + vi.spyOn(SentryCore, 'getClient').mockReturnValue({ + getOptions: () => ({ sendDefaultPii: true }), + } as unknown as SentryCore.Client); + + const scopeData = { + sdkProcessingMetadata: { + normalizedRequest: { + headers: { + 'x-forwarded-proto': 'https', + host: 'example.com', + }, + }, + }, + }; + + const event: Event = { + type: 'transaction', + contexts: { + trace: { + op: 'http.server', + data: { + 'next.route': '/api/users/[id]', + 'http.target': '/api/users/123', + }, + }, + }, + sdkProcessingMetadata: { + capturedSpanIsolationScope: { getScopeData: () => scopeData }, + }, + }; + + setUrlProcessingMetadata(event); + expect(scopeData.sdkProcessingMetadata.normalizedRequest.url).toBe('https://example.com/api/users/123'); + }); +}); From 266b2930144d6742fd2ab1434cf908c652d1e948 Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Thu, 28 May 2026 13:31:37 +0200 Subject: [PATCH 2/2] rm gating for url --- .../pagesRouterRoutingInstrumentation.ts | 14 +--- .../common/utils/setUrlProcessingMetadata.ts | 5 -- .../pagesRouterInstrumentation.test.ts | 9 +-- .../utils/setUrlProcessingMetadata.test.ts | 69 ++++--------------- 4 files changed, 17 insertions(+), 80 deletions(-) diff --git a/packages/nextjs/src/client/routing/pagesRouterRoutingInstrumentation.ts b/packages/nextjs/src/client/routing/pagesRouterRoutingInstrumentation.ts index 100939d046e6..0b9146b26f5b 100644 --- a/packages/nextjs/src/client/routing/pagesRouterRoutingInstrumentation.ts +++ b/packages/nextjs/src/client/routing/pagesRouterRoutingInstrumentation.ts @@ -1,6 +1,5 @@ import type { Client, TransactionSource } from '@sentry/core'; import { - _INTERNAL_filterKeyValueData, browserPerformanceTimeOrigin, debug, parseBaggageHeader, @@ -130,7 +129,7 @@ export function pagesRouterInstrumentPageLoad(client: Client): void { [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.nextjs.pages_router_instrumentation', [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: route ? 'route' : 'url', - ...(params && getFilteredRouteParams(params, client)), + ...(params && { ...params }), }, }, { sentryTrace, baggage }, @@ -186,17 +185,6 @@ function getNextRouteFromPathname(pathname: string): string | undefined { }); } -function getFilteredRouteParams(params: ParsedUrlQuery, client: Client): Record { - const { queryParams } = client.getDataCollectionOptions(); - const stringParams: Record = {}; - for (const [key, value] of Object.entries(params)) { - if (typeof value === 'string') { - stringParams[key] = value; - } - } - return _INTERNAL_filterKeyValueData(stringParams, queryParams); -} - /** * Converts a Next.js style route to a regular expression that matches on pathnames (no query params or URL fragments). * diff --git a/packages/nextjs/src/common/utils/setUrlProcessingMetadata.ts b/packages/nextjs/src/common/utils/setUrlProcessingMetadata.ts index a05039e1ef00..61add752008a 100644 --- a/packages/nextjs/src/common/utils/setUrlProcessingMetadata.ts +++ b/packages/nextjs/src/common/utils/setUrlProcessingMetadata.ts @@ -16,11 +16,6 @@ export function setUrlProcessingMetadata(event: Event): void { return; } - // todo(v11): Replace with a dataCollection gate once URL collection is covered by the spec. - if (!client.getOptions().sendDefaultPii) { - return; - } - const traceData = event.contexts.trace.data; // Get the route from trace data diff --git a/packages/nextjs/test/performance/pagesRouterInstrumentation.test.ts b/packages/nextjs/test/performance/pagesRouterInstrumentation.test.ts index 10f4f29a5cf6..d8b7dbf243fd 100644 --- a/packages/nextjs/test/performance/pagesRouterInstrumentation.test.ts +++ b/packages/nextjs/test/performance/pagesRouterInstrumentation.test.ts @@ -112,9 +112,9 @@ describe('pagesRouterInstrumentPageLoad', () => { it.each([ [ - 'https://example.com/lforst/posts/1337?q=42', + 'https://example.com/chargome/posts/1337?q=42', '/[user]/posts/[id]', - { user: 'lforst', id: '1337', q: '42' }, + { user: 'chargome', id: '1337', q: '42' }, { pageProps: { _sentryTraceData: 'c82b8554881b4d28ad977de04a4fb40a-a755953cd3394d5f-1', @@ -128,7 +128,7 @@ describe('pagesRouterInstrumentPageLoad', () => { 'sentry.op': 'pageload', 'sentry.origin': 'auto.pageload.nextjs.pages_router_instrumentation', 'sentry.source': 'route', - user: 'lforst', + user: 'chargome', id: '1337', q: '42', }, @@ -193,9 +193,6 @@ describe('pagesRouterInstrumentPageLoad', () => { const client = { emit, getOptions: () => ({}), - getDataCollectionOptions: () => ({ - queryParams: { deny: [] }, - }), } as unknown as Client; pagesRouterInstrumentPageLoad(client); diff --git a/packages/nextjs/test/utils/setUrlProcessingMetadata.test.ts b/packages/nextjs/test/utils/setUrlProcessingMetadata.test.ts index d41d728d3a55..a170fbaa8a71 100644 --- a/packages/nextjs/test/utils/setUrlProcessingMetadata.test.ts +++ b/packages/nextjs/test/utils/setUrlProcessingMetadata.test.ts @@ -3,71 +3,17 @@ import * as SentryCore from '@sentry/core'; import { describe, expect, it, vi } from 'vitest'; import { setUrlProcessingMetadata } from '../../src/common/utils/setUrlProcessingMetadata'; -function makeTransactionEvent(overrides?: Partial): Event { - return { - type: 'transaction', - contexts: { - trace: { - op: 'http.server', - data: { - 'next.route': '/api/users/[id]', - 'http.target': '/api/users/123', - }, - }, - }, - sdkProcessingMetadata: { - capturedSpanIsolationScope: { - getScopeData: () => ({ - sdkProcessingMetadata: { - normalizedRequest: { - headers: { - 'x-forwarded-proto': 'https', - host: 'example.com', - }, - }, - }, - }), - }, - }, - ...overrides, - }; -} - describe('setUrlProcessingMetadata', () => { it('skips non-transaction events', () => { - const event = makeTransactionEvent({ type: undefined }); + const event: Event = { type: undefined }; setUrlProcessingMetadata(event); - // No error thrown, nothing changed }); - it('skips when sendDefaultPii is false', () => { + it('adds URL without sendDefaultPii', () => { vi.spyOn(SentryCore, 'getClient').mockReturnValue({ getOptions: () => ({ sendDefaultPii: false }), } as unknown as SentryCore.Client); - const scopeData = { - sdkProcessingMetadata: { - normalizedRequest: { - headers: { host: 'example.com' }, - }, - }, - }; - - const event = makeTransactionEvent({ - sdkProcessingMetadata: { - capturedSpanIsolationScope: { getScopeData: () => scopeData }, - }, - }); - - setUrlProcessingMetadata(event); - expect(scopeData.sdkProcessingMetadata.normalizedRequest).not.toHaveProperty('url'); - }); - - it('adds URL when sendDefaultPii is true', () => { - vi.spyOn(SentryCore, 'getClient').mockReturnValue({ - getOptions: () => ({ sendDefaultPii: true }), - } as unknown as SentryCore.Client); - const scopeData = { sdkProcessingMetadata: { normalizedRequest: { @@ -98,4 +44,15 @@ describe('setUrlProcessingMetadata', () => { setUrlProcessingMetadata(event); expect(scopeData.sdkProcessingMetadata.normalizedRequest.url).toBe('https://example.com/api/users/123'); }); + + it('skips when no client is available', () => { + vi.spyOn(SentryCore, 'getClient').mockReturnValue(undefined); + + const event: Event = { + type: 'transaction', + contexts: { trace: { op: 'http.server', data: { 'next.route': '/test' } } }, + }; + + setUrlProcessingMetadata(event); + }); });