-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
feat(react-router): Drop low-quality transactions via ignoreSpans
#20514
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
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| import * as Sentry from '@sentry/react-router'; | ||
|
|
||
| export async function loader() { | ||
| await Sentry.flush(2000); | ||
| return new Response(null, { status: 204 }); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| import { expect, test } from '@playwright/test'; | ||
| import { waitForTransaction } from '@sentry-internal/test-utils'; | ||
| import { APP_NAME } from '../constants'; | ||
|
|
||
| test.describe('low-quality transaction filter', () => { | ||
| test('does not send a server transaction for /__manifest? requests', async ({ page }) => { | ||
| const serverTxns: Array<{ transaction?: string }> = []; | ||
|
|
||
| const navigationPromise = waitForTransaction(APP_NAME, async transactionEvent => { | ||
| return ( | ||
| transactionEvent.transaction === '/performance/ssr' && transactionEvent.contexts?.trace?.op === 'navigation' | ||
| ); | ||
| }); | ||
|
|
||
| waitForTransaction(APP_NAME, async evt => { | ||
| serverTxns.push(evt); | ||
| return false; | ||
| }); | ||
|
|
||
| await page.goto('/performance'); | ||
| await page.waitForTimeout(1000); | ||
| await page.getByRole('link', { name: 'SSR Page' }).click(); | ||
|
|
||
| await navigationPromise; | ||
|
|
||
| // Force the server to flush any in-flight transactions before we assert | ||
| await page.evaluate(() => fetch('/__sentry-flush')); | ||
|
|
||
| expect(serverTxns.some(t => t.transaction?.match(/GET \/__manifest\?/))).toBe(false); | ||
|
nicohrubec marked this conversation as resolved.
|
||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,37 +1,25 @@ | ||
| import { type Client, debug, defineIntegration, type Event, type EventHint } from '@sentry/core'; | ||
| import type { Client, IntegrationFn } from '@sentry/core'; | ||
| import { defineIntegration } from '@sentry/core'; | ||
| import type { NodeOptions } from '@sentry/node'; | ||
|
|
||
| const LOW_QUALITY_TRANSACTIONS_REGEXES = [ | ||
|
mydea marked this conversation as resolved.
|
||
| /GET \/node_modules\//, | ||
| /GET \/favicon\.ico/, | ||
| /GET \/@id\//, | ||
| /GET \/__manifest\?/, | ||
|
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. Bug: The regex Suggested FixUpdate the regular expression to not expect a query string. Change Prompt for AI AgentDid we get this right? 👍 / 👎 to inform future reviews. |
||
| ]; | ||
|
|
||
| // TODO(v11): Remove the `_options` parameter (unused and only kept for back-compat with the previous signature) | ||
| const _lowQualityTransactionsFilterIntegration = ((_options?: NodeOptions) => ({ | ||
|
nicohrubec marked this conversation as resolved.
|
||
| name: 'LowQualityTransactionsFilter', | ||
| beforeSetup(client) { | ||
| const opts = client.getOptions(); | ||
| opts.ignoreSpans = [...(opts.ignoreSpans || []), ...LOW_QUALITY_TRANSACTIONS_REGEXES]; | ||
| }, | ||
| })) satisfies IntegrationFn; | ||
|
|
||
| /** | ||
| * Integration that filters out noisy http transactions such as requests to node_modules, favicon.ico, @id/ | ||
| * | ||
| * Integration that filters out noisy http transactions such as requests to node_modules, favicon.ico, @id/, __manifest. | ||
| * Adds regex entries to `ignoreSpans` so the filter applies in both static and streaming trace lifecycles. | ||
| */ | ||
|
|
||
| function _lowQualityTransactionsFilterIntegration(options: NodeOptions): { | ||
| name: string; | ||
| processEvent: (event: Event, hint: EventHint, client: Client) => Event | null; | ||
| } { | ||
| const matchedRegexes = [/GET \/node_modules\//, /GET \/favicon\.ico/, /GET \/@id\//, /GET \/__manifest\?/]; | ||
|
|
||
| return { | ||
| name: 'LowQualityTransactionsFilter', | ||
|
|
||
| processEvent(event: Event, _hint: EventHint, _client: Client): Event | null { | ||
| if (event.type !== 'transaction' || !event.transaction) { | ||
| return event; | ||
| } | ||
|
|
||
| const transaction = event.transaction; | ||
|
|
||
| if (matchedRegexes.some(regex => transaction.match(regex))) { | ||
| options.debug && debug.log('[ReactRouter] Filtered node_modules transaction:', event.transaction); | ||
| return null; | ||
| } | ||
|
|
||
| return event; | ||
| }, | ||
| }; | ||
| } | ||
|
|
||
| export const lowQualityTransactionsFilterIntegration = defineIntegration((options: NodeOptions) => | ||
| _lowQualityTransactionsFilterIntegration(options), | ||
| ); | ||
| export const lowQualityTransactionsFilterIntegration = defineIntegration(_lowQualityTransactionsFilterIntegration); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,67 +1,60 @@ | ||
| import type { Event, EventType } from '@sentry/core'; | ||
| import * as SentryCore from '@sentry/core'; | ||
| import * as SentryNode from '@sentry/node'; | ||
| import { afterEach, describe, expect, it, vi } from 'vitest'; | ||
| import type { Client, ClientOptions } from '@sentry/core'; | ||
| import { shouldIgnoreSpan } from '@sentry/core'; | ||
| import { describe, expect, it } from 'vitest'; | ||
| import { lowQualityTransactionsFilterIntegration } from '../../src/server/integration/lowQualityTransactionsFilterIntegration'; | ||
|
|
||
| const debugLoggerLogSpy = vi.spyOn(SentryCore.debug, 'log').mockImplementation(() => {}); | ||
|
|
||
| describe('Low Quality Transactions Filter Integration', () => { | ||
| afterEach(() => { | ||
| vi.clearAllMocks(); | ||
| SentryNode.getGlobalScope().clear(); | ||
| function makeMockClient(initial: Partial<ClientOptions> = {}): Client { | ||
| const options = { ...initial } as ClientOptions; | ||
| return { getOptions: () => options } as Client; | ||
| } | ||
|
|
||
| function setupIntegrationAndGetIgnoreSpans(initial: Partial<ClientOptions> = {}) { | ||
| const integration = lowQualityTransactionsFilterIntegration({}); | ||
| const client = makeMockClient(initial); | ||
| integration.beforeSetup!(client); | ||
| return client.getOptions().ignoreSpans!; | ||
| } | ||
|
|
||
| describe('lowQualityTransactionsFilterIntegration', () => { | ||
| it('appends the low-quality regexes to ignoreSpans', () => { | ||
| expect(setupIntegrationAndGetIgnoreSpans()).toEqual([ | ||
| /GET \/node_modules\//, | ||
| /GET \/favicon\.ico/, | ||
| /GET \/@id\//, | ||
| /GET \/__manifest\?/, | ||
| ]); | ||
| }); | ||
|
|
||
| describe('integration functionality', () => { | ||
| describe('filters out low quality transactions', () => { | ||
| it.each([ | ||
| ['node_modules requests', 'GET /node_modules/some-package/index.js'], | ||
| ['favicon.ico requests', 'GET /favicon.ico'], | ||
| ['@id/ requests', 'GET /@id/some-id'], | ||
| ['manifest requests', 'GET /__manifest?p=%2Fperformance%2Fserver-action'], | ||
| ])('%s', (description, transaction) => { | ||
| const integration = lowQualityTransactionsFilterIntegration({ debug: true }); | ||
| const event = { | ||
| type: 'transaction' as EventType, | ||
| transaction, | ||
| } as Event; | ||
|
|
||
| const result = integration.processEvent!(event, {}, {} as SentryCore.Client); | ||
|
|
||
| expect(result).toBeNull(); | ||
|
|
||
| expect(debugLoggerLogSpy).toHaveBeenCalledWith('[ReactRouter] Filtered node_modules transaction:', transaction); | ||
| }); | ||
| }); | ||
|
|
||
| describe('allows high quality transactions', () => { | ||
| it.each([ | ||
| ['normal page requests', 'GET /api/users'], | ||
| ['API endpoints', 'POST /data'], | ||
| ['app routes', 'GET /projects/123'], | ||
| ])('%s', (description, transaction) => { | ||
| const integration = lowQualityTransactionsFilterIntegration({}); | ||
| const event = { | ||
| type: 'transaction' as EventType, | ||
| transaction, | ||
| } as Event; | ||
|
|
||
| const result = integration.processEvent!(event, {}, {} as SentryCore.Client); | ||
| it('preserves user-provided ignoreSpans entries', () => { | ||
| expect(setupIntegrationAndGetIgnoreSpans({ ignoreSpans: [/keep-me/] })).toEqual([ | ||
| /keep-me/, | ||
| /GET \/node_modules\//, | ||
| /GET \/favicon\.ico/, | ||
| /GET \/@id\//, | ||
| /GET \/__manifest\?/, | ||
| ]); | ||
| }); | ||
|
|
||
| expect(result).toEqual(event); | ||
| }); | ||
| describe('drops low-quality transactions', () => { | ||
| it.each([ | ||
| ['node_modules requests', 'GET /node_modules/some-package/index.js'], | ||
| ['favicon.ico requests', 'GET /favicon.ico'], | ||
| ['@id/ requests', 'GET /@id/some-id'], | ||
| ['manifest requests', 'GET /__manifest?p=%2Fperformance%2Fserver-action'], | ||
| ])('%s', (_label, name) => { | ||
| const ignoreSpans = setupIntegrationAndGetIgnoreSpans(); | ||
| expect(shouldIgnoreSpan({ description: name, op: 'http.server' }, ignoreSpans)).toBe(true); | ||
| }); | ||
| }); | ||
|
|
||
| it('does not affect non-transaction events', () => { | ||
| const integration = lowQualityTransactionsFilterIntegration({}); | ||
| const event = { | ||
| type: 'error' as EventType, | ||
| transaction: 'GET /node_modules/some-package/index.js', | ||
| } as Event; | ||
|
|
||
| const result = integration.processEvent!(event, {}, {} as SentryCore.Client); | ||
|
|
||
| expect(result).toEqual(event); | ||
| describe('keeps high-quality transactions', () => { | ||
| it.each([ | ||
| ['normal page requests', 'GET /api/users'], | ||
| ['API endpoints', 'POST /data'], | ||
| ['app routes', 'GET /projects/123'], | ||
| ])('%s', (_label, name) => { | ||
| const ignoreSpans = setupIntegrationAndGetIgnoreSpans(); | ||
| expect(shouldIgnoreSpan({ description: name, op: 'http.server' }, ignoreSpans)).toBe(false); | ||
| }); | ||
| }); | ||
| }); |
Uh oh!
There was an error while loading. Please reload this page.