Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 1.8k
feat(tanstackstart-react): Add file exclude to configure middleware auto-instrumentation#19007
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
b877f5140a7ba43660c729a8026be985b3ef7f4c22092858fce8dbc7330d7f8b262eaa14873c3a910011c12298725e86bc1b2c92fb94322ae7f8e23df42f7140ba4ec9912468fc0632a478e1a207f4f30d41a45057da296e806bead9421fc529a1e6bfc47ee555c71e1778822File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,10 @@ | ||
| import { stringMatchesSomePattern } from '@sentry/core'; | ||
| import * as path from 'path'; | ||
| import type { Plugin } from 'vite'; | ||
| type AutoInstrumentMiddlewareOptions = { | ||
| enabled?: boolean; | ||
| debug?: boolean; | ||
| exclude?: Array<string | RegExp>; | ||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe it would make sense to add some defaults here. E.g. excluding the test files if this is a very common usecase
| ||
| }; | ||
| type WrapResult = { | ||
| @@ -87,25 +89,44 @@ function applyWrap( | ||
| }; | ||
| } | ||
| /** | ||
| * Checks if a file should be skipped from auto-instrumentation based on exclude patterns. | ||
| */ | ||
| export function shouldSkipFile(id: string, exclude: Array<string | RegExp> | undefined, debug: boolean): boolean { | ||
| // file doesn't match exclude patterns, don't skip | ||
| if (!exclude || exclude.length === 0 || !stringMatchesSomePattern(id, exclude)) { | ||
| return false; | ||
| } | ||
| // file matches exclude patterns, skip | ||
| if (debug) { | ||
| // eslint-disable-next-line no-console | ||
| console.log(`[Sentry] Skipping auto-instrumentation for excluded file: ${id}`); | ||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. l: I think this should be rather MemberAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this needs to be console.log since it happens during build not runtime | ||
| } | ||
| return true; | ||
| } | ||
nicohrubec marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| /** | ||
| * A Vite plugin that automatically instruments TanStack Start middlewares: | ||
| * - `requestMiddleware` and `functionMiddleware` arrays in `createStart()` | ||
| * - `middleware` arrays in `createFileRoute()` route definitions | ||
| */ | ||
| export function makeAutoInstrumentMiddlewarePlugin(options: AutoInstrumentMiddlewareOptions = {}): Plugin { | ||
| const { enabled = true, debug = false } = options; | ||
| const { debug = false, exclude } = options; | ||
| return { | ||
| name: 'sentry-tanstack-middleware-auto-instrument', | ||
| enforce: 'pre', | ||
| transform(code, id) { | ||
| if (!enabled) { | ||
| // Skip if not a TS/TSX file | ||
| const fileExtension = path.extname(id); | ||
| if (!['.ts', '.tsx'].includes(fileExtension)) { | ||
nicohrubec marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| return null; | ||
| } | ||
| // Skip if not a TS/JS file | ||
| if (!/\.(ts|tsx|js|jsx|mjs|mts)$/.test(id)) { | ||
| // Skip if file matches exclude patterns | ||
| if (shouldSkipFile(id, exclude, debug)) { | ||
| return null; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -8,17 +8,34 @@ import { makeAddSentryVitePlugin, makeEnableSourceMapsVitePlugin } from './sourc | ||
| */ | ||
| export interface SentryTanstackStartOptions extends BuildTimeOptionsBase { | ||
| /** | ||
| * If this flag is `true`, the Sentry plugins will automatically instrument TanStack Start middlewares. | ||
| * Configure automatic middleware instrumentation. | ||
| * | ||
| * This wraps global middlewares (`requestMiddleware` and `functionMiddleware`) in `createStart()` with Sentry | ||
| * instrumentation to capture performance data. | ||
| * - Set to `false` to disable automatic middleware instrumentation entirely. | ||
| * - Set to `true` (default) to enable for all middleware files. | ||
| * - Set to an object with `exclude` to enable but exclude specific files. | ||
| * | ||
| * Set to `false` to disable automatic middleware instrumentation if you prefer to wrap middlewares manually | ||
| * using `wrapMiddlewaresWithSentry`. | ||
| * The `exclude` option takes an array of strings or regular expressions matched | ||
| * against the full file path. String patterns match as substrings. | ||
| * | ||
| * @default true | ||
| * | ||
| * @example | ||
| * // Disable completely | ||
| * sentryTanstackStart({ autoInstrumentMiddleware: false }) | ||
| * | ||
| * @example | ||
| * // Enable with exclusions | ||
| * sentryTanstackStart({ | ||
| * autoInstrumentMiddleware: { | ||
| * exclude: ['/routes/admin/', /\.test\.ts$/], | ||
| * }, | ||
| * }) | ||
| */ | ||
| autoInstrumentMiddleware?: boolean; | ||
| autoInstrumentMiddleware?: | ||
| | boolean | ||
| | { | ||
| exclude?: Array<string | RegExp>; | ||
| }; | ||
| } | ||
| /** | ||
| @@ -54,8 +71,17 @@ export function sentryTanstackStart(options: SentryTanstackStartOptions = {}): P | ||
| const plugins: Plugin[] = [...makeAddSentryVitePlugin(options)]; | ||
| // middleware auto-instrumentation | ||
| if (options.autoInstrumentMiddleware !== false) { | ||
| plugins.push(makeAutoInstrumentMiddlewarePlugin({ enabled: true, debug: options.debug })); | ||
| const autoInstrumentConfig = options.autoInstrumentMiddleware; | ||
| const isDisabled = autoInstrumentConfig === false; | ||
| const excludePatterns = typeof autoInstrumentConfig === 'object' ? autoInstrumentConfig.exclude : undefined; | ||
sentry[bot] marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| if (!isDisabled) { | ||
| plugins.push( | ||
| makeAutoInstrumentMiddlewarePlugin({ | ||
| debug: options.debug, | ||
| exclude: excludePatterns, | ||
| }), | ||
| ); | ||
| } | ||
| // source maps | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -4,6 +4,7 @@ import { | ||
| addSentryImport, | ||
| arrayToObjectShorthand, | ||
| makeAutoInstrumentMiddlewarePlugin, | ||
| shouldSkipFile, | ||
| wrapGlobalMiddleware, | ||
| wrapRouteMiddleware, | ||
| wrapServerFnMiddleware, | ||
| @@ -29,20 +30,13 @@ export const Route = createFileRoute('/foo')({ | ||
| }); | ||
| `; | ||
| it('does not instrument non-TS/JS files', () => { | ||
| it('does not instrument non-TS/TSX files', () => { | ||
| const plugin = makeAutoInstrumentMiddlewarePlugin() as PluginWithTransform; | ||
| const result = plugin.transform(createStartFile, '/app/start.css'); | ||
| expect(result).toBeNull(); | ||
| }); | ||
| it('does not instrument when enabled is false', () => { | ||
| const plugin = makeAutoInstrumentMiddlewarePlugin({ enabled: false }) as PluginWithTransform; | ||
| const result = plugin.transform(createStartFile, '/app/start.ts'); | ||
| expect(result).toBeNull(); | ||
| }); | ||
| it('does not instrument files without createStart or createFileRoute', () => { | ||
| const plugin = makeAutoInstrumentMiddlewarePlugin() as PluginWithTransform; | ||
| const code = "export const foo = 'bar';"; | ||
| @@ -97,6 +91,14 @@ createStart(() => ({ requestMiddleware: [getMiddleware()] })); | ||
| consoleWarnSpy.mockRestore(); | ||
| }); | ||
| it('does not instrument files matching exclude patterns', () => { | ||
| const plugin = makeAutoInstrumentMiddlewarePlugin({ | ||
| exclude: ['/routes/admin/'], | ||
| }) as PluginWithTransform; | ||
| const result = plugin.transform(createStartFile, '/app/routes/admin/start.ts'); | ||
| expect(result).toBeNull(); | ||
| }); | ||
nicohrubec marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| }); | ||
| describe('wrapGlobalMiddleware', () => { | ||
| @@ -445,6 +447,37 @@ describe('addSentryImport', () => { | ||
| }); | ||
| }); | ||
| describe('shouldSkipFile', () => { | ||
| it('returns false when exclude is undefined', () => { | ||
| expect(shouldSkipFile('/app/start.ts', undefined, false)).toBe(false); | ||
| }); | ||
| it('returns false when exclude is empty array', () => { | ||
| expect(shouldSkipFile('/app/start.ts', [], false)).toBe(false); | ||
| }); | ||
| it('returns false when file does not match any pattern', () => { | ||
| expect(shouldSkipFile('/app/start.ts', ['/admin/', /\.test\.ts$/], false)).toBe(false); | ||
| }); | ||
| it('returns true when file matches string pattern', () => { | ||
| expect(shouldSkipFile('/app/routes/admin/start.ts', ['/admin/'], false)).toBe(true); | ||
| }); | ||
| it('returns true when file matches regex pattern', () => { | ||
| expect(shouldSkipFile('/app/start.test.ts', [/\.test\.ts$/], false)).toBe(true); | ||
| }); | ||
| it('logs debug message when skipping file', () => { | ||
| const consoleLogSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); | ||
| shouldSkipFile('/app/routes/admin/start.ts', ['/admin/'], true); | ||
| expect(consoleLogSpy).toHaveBeenCalledWith( | ||
| expect.stringContaining('Skipping auto-instrumentation for excluded file'), | ||
| ); | ||
| consoleLogSpy.mockRestore(); | ||
| }); | ||
| }); | ||
| describe('arrayToObjectShorthand', () => { | ||
| it('converts single identifier', () => { | ||
| expect(arrayToObjectShorthand('foo')).toBe('{ foo }'); | ||
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was an unnecessary option since we just don't add this plugin if it's not enabled, so I removed it