diff --git a/MIGRATION.md b/MIGRATION.md index 57598adfc59d..0ab76f759ab7 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -1,5 +1,22 @@ # Upgrading from 7.x to 8.x +## Removal of `trackHeaders` option for Astro middleware + +Instead of opting-in via the middleware config, you can configure if headers should be captured via +`requestDataIntegration` options, which defaults to `true` but can be disabled like this: + +``` +Sentry.init({ + integrations: [ + Sentry.requestDataIntegration({ + include: { + headers: false + }, + }), + ], +}); +``` + ## Removal of Client-Side health check transaction filters The SDK no longer filters out health check transactions by default. Instead, they are sent to Sentry but still dropped diff --git a/packages/astro/src/server/middleware.ts b/packages/astro/src/server/middleware.ts index 20da1f7d407c..b5c0e599754a 100644 --- a/packages/astro/src/server/middleware.ts +++ b/packages/astro/src/server/middleware.ts @@ -8,7 +8,7 @@ import { startSpan, withIsolationScope, } from '@sentry/node-experimental'; -import type { Client, Scope, Span } from '@sentry/types'; +import type { Client, Scope, Span, SpanAttributes } from '@sentry/types'; import { addNonEnumerableProperty, objectify, stripUrlQueryAndFragment } from '@sentry/utils'; import type { APIContext, MiddlewareResponseHandler } from 'astro'; @@ -27,15 +27,6 @@ type MiddlewareOptions = { * @default false (recommended) */ trackClientIp?: boolean; - - /** - * If true, the headers from the request will be attached to the event by calling `setExtra`. - * - * Only set this to `true` if you're fine with collecting potentially personally identifiable information (PII). - * - * @default false (recommended) - */ - trackHeaders?: boolean; }; function sendErrorToSentry(e: unknown): unknown { @@ -63,7 +54,6 @@ type AstroLocalsWithSentry = Record & { export const handleRequest: (options?: MiddlewareOptions) => MiddlewareResponseHandler = options => { const handlerOptions = { trackClientIp: false, - trackHeaders: false, ...options, }; @@ -100,13 +90,9 @@ async function instrumentRequest( baggage: headers.get('baggage'), }, async () => { - const allHeaders: Record = {}; - - if (options.trackHeaders) { - headers.forEach((value, key) => { - allHeaders[key] = value; - }); - } + // We store this on the current scope, not isolation scope, + // because we may have multiple requests nested inside each other + getCurrentScope().setSDKProcessingMetadata({ request: ctx.request }); if (options.trackClientIp) { getCurrentScope().setUser({ ip_address: ctx.clientAddress }); @@ -117,22 +103,27 @@ async function instrumentRequest( const source = interpolatedRoute ? 'route' : 'url'; // storing res in a variable instead of directly returning is necessary to // invoke the catch block if next() throws + + const attributes: SpanAttributes = { + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.astro', + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source, + method, + url: stripUrlQueryAndFragment(ctx.url.href), + }; + + if (ctx.url.search) { + attributes['http.query'] = ctx.url.search; + } + + if (ctx.url.hash) { + attributes['http.fragment'] = ctx.url.hash; + } + const res = await startSpan( { - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.astro', - }, + attributes, name: `${method} ${interpolatedRoute || ctx.url.pathname}`, op: 'http.server', - status: 'ok', - data: { - method, - url: stripUrlQueryAndFragment(ctx.url.href), - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source, - ...(ctx.url.search && { 'http.query': ctx.url.search }), - ...(ctx.url.hash && { 'http.fragment': ctx.url.hash }), - ...(options.trackHeaders && { headers: allHeaders }), - }, }, async span => { const originalResponse = await next(); diff --git a/packages/astro/test/server/middleware.test.ts b/packages/astro/test/server/middleware.test.ts index eb0ca9a9daba..cd5caec43c48 100644 --- a/packages/astro/test/server/middleware.test.ts +++ b/packages/astro/test/server/middleware.test.ts @@ -19,6 +19,7 @@ describe('sentryMiddleware', () => { return {} as Span | undefined; }); const setUserMock = vi.fn(); + const setSDKProcessingMetadataMock = vi.fn(); beforeEach(() => { vi.spyOn(SentryNode, 'getCurrentScope').mockImplementation(() => { @@ -26,6 +27,7 @@ describe('sentryMiddleware', () => { setUser: setUserMock, setPropagationContext: vi.fn(), getSpan: getSpanMock, + setSDKProcessingMetadata: setSDKProcessingMetadataMock, } as any; }); vi.spyOn(SentryNode, 'getActiveSpan').mockImplementation(getSpanMock); @@ -60,15 +62,12 @@ describe('sentryMiddleware', () => { { attributes: { 'sentry.origin': 'auto.http.astro', - }, - data: { method: 'GET', url: 'https://mydomain.io/users/123/details', [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', }, name: 'GET /users/[id]/details', op: 'http.server', - status: 'ok', }, expect.any(Function), // the `next` function ); @@ -97,15 +96,12 @@ describe('sentryMiddleware', () => { { attributes: { 'sentry.origin': 'auto.http.astro', - }, - data: { method: 'GET', url: 'http://localhost:1234/a%xx', [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', }, name: 'GET a%xx', op: 'http.server', - status: 'ok', }, expect.any(Function), // the `next` function ); @@ -142,8 +138,8 @@ describe('sentryMiddleware', () => { }); }); - it('attaches client IP and request headers if options are set', async () => { - const middleware = handleRequest({ trackClientIp: true, trackHeaders: true }); + it('attaches client IP if `trackClientIp=true`', async () => { + const middleware = handleRequest({ trackClientIp: true }); const ctx = { request: { method: 'GET', @@ -162,17 +158,36 @@ describe('sentryMiddleware', () => { await middleware(ctx, next); expect(setUserMock).toHaveBeenCalledWith({ ip_address: '192.168.0.1' }); + }); - expect(startSpanSpy).toHaveBeenCalledWith( - expect.objectContaining({ - data: expect.objectContaining({ - headers: { - 'some-header': 'some-value', - }, + it('attaches request as SDK processing metadata', async () => { + const middleware = handleRequest({}); + const ctx = { + request: { + method: 'GET', + url: '/users', + headers: new Headers({ + 'some-header': 'some-value', }), - }), - expect.any(Function), // the `next` function - ); + }, + clientAddress: '192.168.0.1', + params: {}, + url: new URL('https://myDomain.io/users/'), + }; + const next = vi.fn(() => nextResult); + + // @ts-expect-error, a partial ctx object is fine here + await middleware(ctx, next); + + expect(setSDKProcessingMetadataMock).toHaveBeenCalledWith({ + request: { + method: 'GET', + url: '/users', + headers: new Headers({ + 'some-header': 'some-value', + }), + }, + }); }); it('injects tracing tags into the HTML of a pageload response', async () => {