ref(astro): Put request as SDK processing metadata instead of span data

Author: mydea

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<string, unknown> & {
 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<string, string> = {};
-
-      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();

packages/astro/test/server/middleware.test.ts

@@ -19,13 +19,15 @@ describe('sentryMiddleware', () => {
     return {} as Span | undefined;
   });
   const setUserMock = vi.fn();
+  const setSDKProcessingMetadataMock = vi.fn();
 
   beforeEach(() => {
     vi.spyOn(SentryNode, 'getCurrentScope').mockImplementation(() => {
       return {
         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 <meta> tags into the HTML of a pageload response', async () => {