feat(astro): Add Sentry middleware (#9445)

Lms24 · web-flow · commit 9a92e6be8e55 · 2023-11-07T11:04:14.000+01:00
Add a Sentry middleware handler for Astro [middleware](https://docs.astro.build/en/guides/middleware/). This handler * Creates a new span/transaction via `startSpan` * To create a parameterized transaction name, we interpolate the route name from the raw URL and the `params` object. * Continues a trace from tracing headers * Captures an error with Astro SDK-specific `handled` data * Optionally allows: * setting the client IP from the client request (only in SSR mode) * attaching request headers to `event.metadata.request`
diff --git a/packages/astro/README.md b/packages/astro/README.md
@@ -31,7 +31,7 @@ Install the Sentry Astro SDK with the `astro` CLI:
 npx astro add @sentry/astro
 ```
 
-Complete the setup by adding your DSN and source maps upload configuration:
+Add your DSN and source maps upload configuration:
 
 ```javascript
 import { defineConfig } from "astro/config";
@@ -56,6 +56,22 @@ Follow [this guide](https://docs.sentry.io/product/accounts/auth-tokens/#organiz
 SENTRY_AUTH_TOKEN="your-token"
 ```
 
+Complete the setup by adding the Sentry middleware to your `src/middleware.js` file:
+
+```javascript
+// src/middleware.js
+import { sequence } from "astro:middleware";
+import * as Sentry from "@sentry/astro";
+
+export const onRequest = sequence(
+  Sentry.sentryMiddleware(),
+  // Add your other handlers after sentryMiddleware
+);
+```
+
+This middleware creates server-side spans to monitor performance on the server for page load and endpoint requests.
+
+
 ## Configuration
 
 Check out our docs for configuring your SDK setup:
diff --git a/packages/astro/src/index.server.ts b/packages/astro/src/index.server.ts
@@ -62,5 +62,6 @@ export {
 export * from '@sentry/node';
 
 export { init } from './server/sdk';
+export { handleRequest } from './server/middleware';
 
 export default sentryAstro;
diff --git a/packages/astro/src/server/middleware.ts b/packages/astro/src/server/middleware.ts
@@ -0,0 +1,123 @@
+import { captureException, configureScope, startSpan } from '@sentry/node';
+import { addExceptionMechanism, objectify, stripUrlQueryAndFragment, tracingContextFromHeaders } from '@sentry/utils';
+import type { APIContext, MiddlewareResponseHandler } from 'astro';
+
+type MiddlewareOptions = {
+  /**
+   * If true, the client IP will be attached to the event by calling `setUser`.
+   * Only set this to `true` if you're fine with collecting potentially personally identifiable information (PII).
+   *
+   * This will only work if your app is configured for SSR
+   *
+   * @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 {
+  // In case we have a primitive, wrap it in the equivalent wrapper class (string -> String, etc.) so that we can
+  // store a seen flag on it.
+  const objectifiedErr = objectify(e);
+
+  captureException(objectifiedErr, scope => {
+    scope.addEventProcessor(event => {
+      addExceptionMechanism(event, {
+        type: 'astro',
+        handled: false,
+        data: {
+          function: 'astroMiddleware',
+        },
+      });
+      return event;
+    });
+
+    return scope;
+  });
+
+  return objectifiedErr;
+}
+
+export const handleRequest: (options?: MiddlewareOptions) => MiddlewareResponseHandler = (
+  options = { trackClientIp: false, trackHeaders: false },
+) => {
+  return async (ctx, next) => {
+    const method = ctx.request.method;
+    const headers = ctx.request.headers;
+
+    const { dynamicSamplingContext, traceparentData, propagationContext } = tracingContextFromHeaders(
+      headers.get('sentry-trace') || undefined,
+      headers.get('baggage'),
+    );
+
+    const allHeaders: Record<string, string> = {};
+    headers.forEach((value, key) => {
+      allHeaders[key] = value;
+    });
+
+    configureScope(scope => {
+      scope.setPropagationContext(propagationContext);
+
+      if (options.trackClientIp) {
+        scope.setUser({ ip_address: ctx.clientAddress });
+      }
+    });
+
+    try {
+      // storing res in a variable instead of directly returning is necessary to
+      // invoke the catch block if next() throws
+      const res = await startSpan(
+        {
+          name: `${method} ${interpolateRouteFromUrlAndParams(ctx.url.pathname, ctx.params)}`,
+          op: `http.server.${method.toLowerCase()}`,
+          origin: 'auto.http.astro',
+          status: 'ok',
+          ...traceparentData,
+          metadata: {
+            source: 'route',
+            dynamicSamplingContext: traceparentData && !dynamicSamplingContext ? {} : dynamicSamplingContext,
+          },
+          data: {
+            method,
+            url: stripUrlQueryAndFragment(ctx.url.href),
+            ...(ctx.url.search && { 'http.query': ctx.url.search }),
+            ...(ctx.url.hash && { 'http.fragment': ctx.url.hash }),
+            ...(options.trackHeaders && { headers: allHeaders }),
+          },
+        },
+        async span => {
+          const res = await next();
+          if (span && res.status) {
+            span.setHttpStatus(res.status);
+          }
+          return res;
+        },
+      );
+      return res;
+    } catch (e) {
+      sendErrorToSentry(e);
+      throw e;
+    }
+    // TODO: flush if serveless (first extract function)
+  };
+};
+
+/**
+ * Interpolates the route from the URL and the passed params.
+ * Best we can do to get a route name instead of a raw URL.
+ *
+ * exported for testing
+ */
+export function interpolateRouteFromUrlAndParams(rawUrl: string, params: APIContext['params']): string {
+  return Object.entries(params).reduce((interpolateRoute, value) => {
+    const [paramId, paramValue] = value;
+    return interpolateRoute.replace(new RegExp(`(/|-)${paramValue}(/|-|$)`), `$1[${paramId}]$2`);
+  }, rawUrl);
+}
diff --git a/packages/astro/test/server/middleware.test.ts b/packages/astro/test/server/middleware.test.ts
@@ -0,0 +1,210 @@
+import * as SentryNode from '@sentry/node';
+import * as SentryUtils from '@sentry/utils';
+import { vi } from 'vitest';
+
+import { handleRequest, interpolateRouteFromUrlAndParams } from '../../src/server/middleware';
+
+describe('sentryMiddleware', () => {
+  const startSpanSpy = vi.spyOn(SentryNode, 'startSpan');
+
+  afterEach(() => {
+    vi.clearAllMocks();
+  });
+
+  it('creates a span for an incoming request', async () => {
+    const middleware = handleRequest();
+    const ctx = {
+      request: {
+        method: 'GET',
+        url: '/users/123/details',
+        headers: new Headers(),
+      },
+      url: new URL('https://myDomain.io/users/123/details'),
+      params: {
+        id: '123',
+      },
+    };
+    const nextResult = Promise.resolve({ status: 200 });
+    const next = vi.fn(() => nextResult);
+
+    // @ts-expect-error, a partial ctx object is fine here
+    const resultFromNext = middleware(ctx, next);
+
+    expect(startSpanSpy).toHaveBeenCalledWith(
+      {
+        data: {
+          method: 'GET',
+          url: 'https://mydomain.io/users/123/details',
+        },
+        metadata: {
+          source: 'route',
+        },
+        name: 'GET /users/[id]/details',
+        op: 'http.server.get',
+        origin: 'auto.http.astro',
+        status: 'ok',
+      },
+      expect.any(Function), // the `next` function
+    );
+
+    expect(next).toHaveBeenCalled();
+    expect(resultFromNext).toStrictEqual(nextResult);
+  });
+
+  it('throws and sends an error to sentry if `next()` throws', async () => {
+    const scope = {
+      addEventProcessor: vi.fn().mockImplementation(cb => cb({})),
+    };
+    // @ts-expect-error, just testing the callback, this is okay for this test
+    const captureExceptionSpy = vi.spyOn(SentryNode, 'captureException').mockImplementation((ex, cb) => cb(scope));
+    const addExMechanismSpy = vi.spyOn(SentryUtils, 'addExceptionMechanism');
+
+    const middleware = handleRequest();
+    const ctx = {
+      request: {
+        method: 'GET',
+        url: '/users',
+        headers: new Headers(),
+      },
+      url: new URL('https://myDomain.io/users/'),
+      params: {},
+    };
+
+    const error = new Error('Something went wrong');
+
+    const next = vi.fn(() => {
+      throw error;
+    });
+
+    // @ts-expect-error, a partial ctx object is fine here
+    await expect(async () => middleware(ctx, next)).rejects.toThrowError();
+
+    expect(captureExceptionSpy).toHaveBeenCalledWith(error, expect.any(Function));
+    expect(scope.addEventProcessor).toHaveBeenCalledTimes(1);
+    expect(addExMechanismSpy).toHaveBeenCalledWith(
+      {}, // the mocked event
+      {
+        handled: false,
+        type: 'astro',
+        data: { function: 'astroMiddleware' },
+      },
+    );
+  });
+
+  it('attaches tracing headers', async () => {
+    const scope = { setUser: vi.fn(), setPropagationContext: vi.fn() };
+    // @ts-expect-error, only passing a partial Scope object
+    const configureScopeSpy = vi.spyOn(SentryNode, 'configureScope').mockImplementation(cb => cb(scope));
+
+    const middleware = handleRequest();
+    const ctx = {
+      request: {
+        method: 'GET',
+        url: '/users',
+        headers: new Headers({
+          'sentry-trace': '12345678901234567890123456789012-1234567890123456-1',
+          baggage: 'sentry-release=1.0.0',
+        }),
+      },
+      params: {},
+      url: new URL('https://myDomain.io/users/'),
+    };
+    const next = vi.fn();
+
+    // @ts-expect-error, a partial ctx object is fine here
+    await middleware(ctx, next);
+
+    expect(configureScopeSpy).toHaveBeenCalledTimes(1);
+    expect(scope.setPropagationContext).toHaveBeenCalledWith({
+      dsc: {
+        release: '1.0.0',
+      },
+      parentSpanId: '1234567890123456',
+      sampled: true,
+      spanId: expect.any(String),
+      traceId: '12345678901234567890123456789012',
+    });
+
+    expect(startSpanSpy).toHaveBeenCalledWith(
+      expect.objectContaining({
+        metadata: {
+          source: 'route',
+          dynamicSamplingContext: {
+            release: '1.0.0',
+          },
+        },
+        parentSampled: true,
+        parentSpanId: '1234567890123456',
+        traceId: '12345678901234567890123456789012',
+      }),
+      expect.any(Function), // the `next` function
+    );
+  });
+
+  it('attaches client IP and request headers if options are set', async () => {
+    const scope = { setUser: vi.fn(), setPropagationContext: vi.fn() };
+    // @ts-expect-error, only passing a partial Scope object
+    const configureScopeSpy = vi.spyOn(SentryNode, 'configureScope').mockImplementation(cb => cb(scope));
+
+    const middleware = handleRequest({ trackClientIp: true, trackHeaders: true });
+    const ctx = {
+      request: {
+        method: 'GET',
+        url: '/users',
+        headers: new Headers({
+          'some-header': 'some-value',
+        }),
+      },
+      clientAddress: '192.168.0.1',
+      params: {},
+      url: new URL('https://myDomain.io/users/'),
+    };
+    const next = vi.fn();
+
+    // @ts-expect-error, a partial ctx object is fine here
+    await middleware(ctx, next);
+
+    expect(configureScopeSpy).toHaveBeenCalledTimes(1);
+    expect(scope.setUser).toHaveBeenCalledWith({ ip_address: '192.168.0.1' });
+
+    expect(startSpanSpy).toHaveBeenCalledWith(
+      expect.objectContaining({
+        data: expect.objectContaining({
+          headers: {
+            'some-header': 'some-value',
+          },
+        }),
+      }),
+      expect.any(Function), // the `next` function
+    );
+  });
+});
+
+describe('interpolateRouteFromUrlAndParams', () => {
+  it.each([
+    ['/foo/bar', {}, '/foo/bar'],
+    ['/users/123', { id: '123' }, '/users/[id]'],
+    ['/users/123', { id: '123', foo: 'bar' }, '/users/[id]'],
+    ['/lang/en-US', { lang: 'en', region: 'US' }, '/lang/[lang]-[region]'],
+    ['/lang/en-US/posts', { lang: 'en', region: 'US' }, '/lang/[lang]-[region]/posts'],
+  ])('interpolates route from URL and params %s', (rawUrl, params, expectedRoute) => {
+    expect(interpolateRouteFromUrlAndParams(rawUrl, params)).toEqual(expectedRoute);
+  });
+
+  it('handles params across multiple URL segments in catchall routes', () => {
+    // Ideally, Astro would let us know that this is a catchall route so we can make the param [...catchall] but it doesn't
+    expect(
+      interpolateRouteFromUrlAndParams('/someroute/catchall-123/params/foo/bar', {
+        catchall: 'catchall-123/params/foo',
+        params: 'foo',
+      }),
+    ).toEqual('/someroute/[catchall]/bar');
+  });
+
+  it("doesn't replace partially matching route segments", () => {
+    const rawUrl = '/usernames/username';
+    const params = { name: 'username' };
+    const expectedRoute = '/usernames/[name]';
+    expect(interpolateRouteFromUrlAndParams(rawUrl, params)).toEqual(expectedRoute);
+  });
+});
