From 61646479d84f2ef2add6cd6eb18660f6cdc1b59e Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 3 Nov 2023 12:17:35 +0100 Subject: [PATCH 1/4] feat(astro): Add Sentry middleware --- packages/astro/README.md | 18 +- packages/astro/src/index.server.ts | 1 + packages/astro/src/server/middleware.ts | 121 ++++++++++ packages/astro/test/server/middleware.test.ts | 210 ++++++++++++++++++ 4 files changed, 349 insertions(+), 1 deletion(-) create mode 100644 packages/astro/src/server/middleware.ts create mode 100644 packages/astro/test/server/middleware.test.ts diff --git a/packages/astro/README.md b/packages/astro/README.md index 4470d854c717..df68adfa1037 100644 --- 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 index 2d174277b0af..daa1e50722b4 100644 --- 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 { sentryMiddleware } from './server/middleware'; export default sentryAstro; diff --git a/packages/astro/src/server/middleware.ts b/packages/astro/src/server/middleware.ts new file mode 100644 index 000000000000..be543054e624 --- /dev/null +++ b/packages/astro/src/server/middleware.ts @@ -0,0 +1,121 @@ +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). + * + * @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 sentryMiddleware: (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 = {}; + 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 new file mode 100644 index 000000000000..382ee96613b3 --- /dev/null +++ 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 { interpolateRouteFromUrlAndParams, sentryMiddleware } 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 = sentryMiddleware(); + 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 = sentryMiddleware(); + 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 = sentryMiddleware(); + 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 = sentryMiddleware({ 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); + }); +}); From 52204e3351b547787ea625f9116f275a5a1e7a70 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 3 Nov 2023 12:22:18 +0100 Subject: [PATCH 2/4] doc cleanup --- packages/astro/src/server/middleware.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/astro/src/server/middleware.ts b/packages/astro/src/server/middleware.ts index be543054e624..c05133848546 100644 --- a/packages/astro/src/server/middleware.ts +++ b/packages/astro/src/server/middleware.ts @@ -7,6 +7,8 @@ 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; From c0a8dcf13da33fe2381fa68c87150de538cf391a Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 6 Nov 2023 12:07:52 +0100 Subject: [PATCH 3/4] s/sentryMiddleware/handleRequest --- packages/astro/src/index.server.ts | 2 +- packages/astro/src/server/middleware.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/astro/src/index.server.ts b/packages/astro/src/index.server.ts index daa1e50722b4..5ec649c81584 100644 --- a/packages/astro/src/index.server.ts +++ b/packages/astro/src/index.server.ts @@ -62,6 +62,6 @@ export { export * from '@sentry/node'; export { init } from './server/sdk'; -export { sentryMiddleware } from './server/middleware'; +export { handleRequest } from './server/middleware'; export default sentryAstro; diff --git a/packages/astro/src/server/middleware.ts b/packages/astro/src/server/middleware.ts index c05133848546..ff4ac1d44c78 100644 --- a/packages/astro/src/server/middleware.ts +++ b/packages/astro/src/server/middleware.ts @@ -45,7 +45,7 @@ function sendErrorToSentry(e: unknown): unknown { return objectifiedErr; } -export const sentryMiddleware: (options?: MiddlewareOptions) => MiddlewareResponseHandler = ( +export const handleRequest: (options?: MiddlewareOptions) => MiddlewareResponseHandler = ( options = { trackClientIp: false, trackHeaders: false }, ) => { return async (ctx, next) => { From cb49f96844b0abc7088ffb787d5edb10ec36dc14 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 7 Nov 2023 10:34:00 +0100 Subject: [PATCH 4/4] fix unit tests after rename --- packages/astro/test/server/middleware.test.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/astro/test/server/middleware.test.ts b/packages/astro/test/server/middleware.test.ts index 382ee96613b3..af42348a94b9 100644 --- a/packages/astro/test/server/middleware.test.ts +++ b/packages/astro/test/server/middleware.test.ts @@ -2,7 +2,7 @@ import * as SentryNode from '@sentry/node'; import * as SentryUtils from '@sentry/utils'; import { vi } from 'vitest'; -import { interpolateRouteFromUrlAndParams, sentryMiddleware } from '../../src/server/middleware'; +import { handleRequest, interpolateRouteFromUrlAndParams } from '../../src/server/middleware'; describe('sentryMiddleware', () => { const startSpanSpy = vi.spyOn(SentryNode, 'startSpan'); @@ -12,7 +12,7 @@ describe('sentryMiddleware', () => { }); it('creates a span for an incoming request', async () => { - const middleware = sentryMiddleware(); + const middleware = handleRequest(); const ctx = { request: { method: 'GET', @@ -59,7 +59,7 @@ describe('sentryMiddleware', () => { const captureExceptionSpy = vi.spyOn(SentryNode, 'captureException').mockImplementation((ex, cb) => cb(scope)); const addExMechanismSpy = vi.spyOn(SentryUtils, 'addExceptionMechanism'); - const middleware = sentryMiddleware(); + const middleware = handleRequest(); const ctx = { request: { method: 'GET', @@ -96,7 +96,7 @@ describe('sentryMiddleware', () => { // @ts-expect-error, only passing a partial Scope object const configureScopeSpy = vi.spyOn(SentryNode, 'configureScope').mockImplementation(cb => cb(scope)); - const middleware = sentryMiddleware(); + const middleware = handleRequest(); const ctx = { request: { method: 'GET', @@ -146,7 +146,7 @@ describe('sentryMiddleware', () => { // @ts-expect-error, only passing a partial Scope object const configureScopeSpy = vi.spyOn(SentryNode, 'configureScope').mockImplementation(cb => cb(scope)); - const middleware = sentryMiddleware({ trackClientIp: true, trackHeaders: true }); + const middleware = handleRequest({ trackClientIp: true, trackHeaders: true }); const ctx = { request: { method: 'GET',