Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { type RouteConfig, index, prefix, route } from '@react-router/dev/routes

export default [
index('routes/home.tsx'),
route('__sentry-flush', 'routes/sentry-flush.tsx'),
...prefix('errors', [
route('client', 'routes/errors/client.tsx'),
route('client/:client-param', 'routes/errors/client-param.tsx'),
Expand Down
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;
});
Comment thread
cursor[bot] marked this conversation as resolved.

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);
Comment thread
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';

Check warning on line 1 in packages/react-router/src/server/integration/lowQualityTransactionsFilterIntegration.ts

View workflow job for this annotation

GitHub Actions / Lint

eslint(no-unused-vars)

Type 'Client' is imported but never used.
import { defineIntegration } from '@sentry/core';
import type { NodeOptions } from '@sentry/node';

const LOW_QUALITY_TRANSACTIONS_REGEXES = [
Comment thread
mydea marked this conversation as resolved.
/GET \/node_modules\//,
/GET \/favicon\.ico/,
/GET \/@id\//,
/GET \/__manifest\?/,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The regex /GET \/__manifest\?/ will not match in the streaming path because query strings are stripped from the URL before matching, rendering the filter ineffective.
Severity: MEDIUM

Suggested Fix

Update the regular expression to not expect a query string. Change /GET \/__manifest\?/ to /GET \/__manifest/ to ensure it matches the span description after the query string has been removed.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location:
packages/react-router/src/server/integration/lowQualityTransactionsFilterIntegration.ts#L9

Potential issue: The regular expression `/GET \/__manifest\?/` is used to filter
low-quality transactions. This expression requires a literal `?` character to match.
However, the logic for the streaming path uses `inferSpanData()`, which strips query
strings and fragments from URLs before the regex is applied. For a request like `GET
/__manifest?p=...`, the description becomes `GET /__manifest`, which will not be matched
by the regex. This causes the filter to silently fail for all transactions processed via
the streaming path, defeating one of the primary goals of the change.

Did 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) => ({
Comment thread
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);
});
});
});
Loading