Skip to content

Commit 9698d03

Browse files
authored
feat(core): Deprecate beforeSendTransaction and ignoreTransactions and log warnings if used with streaming (#22908)
This PR: 1. Deprecates `beforeSendTransaction` and `ignoreTransactions` 2. Logs a console warning (intentionally not gated with `debug: true`) to warn users if they use any of the two options with span streaming enabled I intentionally didn't update the migration guide. this is handled in #22643. more details in #22856 closes #22856 closes #20279
1 parent 57fddd7 commit 9698d03

8 files changed

Lines changed: 230 additions & 3 deletions

File tree

.size-limit.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,7 @@ module.exports = [
286286
path: createCDNPath('bundle.logs.metrics.min.js'),
287287
gzip: false,
288288
brotli: false,
289-
limit: '100 KB',
289+
limit: '101 KB',
290290
disablePlugins: ['@size-limit/esbuild'],
291291
},
292292
{

AGENTS.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,8 @@ Uses **Git Flow** (see `docs/gitflow.md`).
134134
- Never expose secrets or keys
135135
- When modifying files, cover all occurrences (including `src/` and `test/`)
136136
- Comments explain **why**, never **what** — never add a comment that restates what the code does or describes the change being made; only comment when the reasoning isn't obvious from the code itself
137+
- Do not use `expect(someSpy.mock.calls[0]?.[0])` or similar constructs to check what a spy was called with.
138+
Instead use `expect(someSpy).toHaveBeenCalledWith(...)` or derivatives for a more readable and less brittle test assertion.
137139

138140
## Lazy Loading Is a Last Resort
139141

packages/bundler-plugins/src/core/sentry/telemetry.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ export function createSentryInstance(
4545
return event;
4646
},
4747

48+
// Deprecated, but still applied because this client runs on the static trace lifecycle.
49+
// oxlint-disable-next-line typescript/no-deprecated
4850
beforeSendTransaction: event => {
4951
delete event.server_name; // Server name might contain PII
5052
return event;

packages/core/src/client.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ import { reparentChildSpans, shouldIgnoreSpan } from './utils/should-ignore-span
5454
import { showSpanDropWarning } from './utils/spanUtils';
5555
import { safeUnref } from './utils/timer';
5656
import { convertSpanJsonToTransactionEvent, convertTransactionEventToSpanJson } from './utils/transactionEvent';
57+
import { maybeWarnAboutIgnoredTransactionOptions } from './utils/warnAboutIgnoredTransactionOptions';
5758
import { resolveDataCollectionOptions } from './utils/data-collection/resolveDataCollectionOptions';
5859

5960
const ALREADY_SEEN_ERROR = "Not capturing exception because it's already been captured.";
@@ -507,6 +508,8 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
507508
this._options.integrations.some(({ name }) => name.startsWith('Spotlight'))
508509
) {
509510
this._setupIntegrations();
511+
512+
maybeWarnAboutIgnoredTransactionOptions(this._options);
510513
}
511514
}
512515

@@ -1653,7 +1656,12 @@ function processBeforeSend(
16531656
event: Event,
16541657
hint: EventHint,
16551658
): PromiseLike<Event | null> | Event | null {
1656-
const { beforeSend, beforeSendTransaction, ignoreSpans } = options;
1659+
const {
1660+
beforeSend,
1661+
ignoreSpans,
1662+
// oxlint-disable-next-line typescript/no-deprecated
1663+
beforeSendTransaction,
1664+
} = options;
16571665
const beforeSendSpan = !isStreamedBeforeSendSpanCallback(options.beforeSendSpan) && options.beforeSendSpan;
16581666

16591667
let processedEvent = event;

packages/core/src/types/options.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -337,10 +337,19 @@ export interface ClientOptions<TO extends BaseTransportOptions = BaseTransportOp
337337
* A pattern for transaction names which should not be sent to Sentry.
338338
* By default, all transactions will be sent.
339339
*
340+
* Important: This option is ignored by default! It only has an effect if {@link traceLifecycle} is set to `'static'`.
341+
* Use {@link ignoreSpans} instead, which works with both trace lifecycles.
342+
*
340343
* Behavior of the `ignoreTransactions` option is controlled by the `Sentry.eventFiltersIntegration` integration.
341344
* If the event filters integration is not installed, the `ignoreTransactions` option will not have any effect.
342345
*
343346
* @default []
347+
*
348+
* @deprecated This option only has an effect if {@link traceLifecycle} is set to `'static'`. With span streaming
349+
* (`traceLifecycle: 'stream'`, the default), the SDK ignores it. Use {@link ignoreSpans} instead, which works with both
350+
* trace lifecycles. `ignoreTransactions` will be removed in v12.
351+
*
352+
* @see {@link ClientOptions.ignoreSpans}
344353
*/
345354
ignoreTransactions?: Array<string | RegExp>;
346355

@@ -637,12 +646,20 @@ export interface ClientOptions<TO extends BaseTransportOptions = BaseTransportOp
637646
* An event-processing callback for transaction events, guaranteed to be invoked after all other event
638647
* processors. This allows an event to be modified or dropped before it's sent.
639648
*
649+
* Important: This callback is ignored by default! It only runs if {@link traceLifecycle} is set to `'static'`.
650+
*
640651
* Note that you must return a valid event from this callback. If you do not wish to modify the event, simply return
641652
* it at the end. Returning `null` will cause the event to be dropped.
642653
*
643-
* @param event The error or message event generated by the SDK.
654+
* @param event The transaction event generated by the SDK.
644655
* @param hint Event metadata useful for processing.
645656
* @returns A new event that will be sent | null.
657+
*
658+
* @deprecated This option only has an effect if {@link traceLifecycle} is set to `'static'`. With span streaming
659+
* (`traceLifecycle: 'stream'`, the default), the SDK ignores it. Use {@link beforeSendSpan} instead, which works with both
660+
* trace lifecycles. `beforeSendTransaction` will be removed in v12 of the SDK.
661+
*
662+
* @see {@link ClientOptions.beforeSendSpan}
646663
*/
647664
beforeSendTransaction?: (
648665
event: TransactionEvent,
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
import type { ClientOptions } from '../types/options';
2+
import { consoleSandbox } from './debug-logger';
3+
4+
/**
5+
* Warns about `beforeSendTransaction` and `ignoreTransactions` being ignored because span streaming is enabled.
6+
*
7+
* Both options are tied to the transaction event that span streaming no longer produces, so they silently
8+
* stop taking effect when users upgrade. Since that's easy to miss, this warning bypasses the `debug` logger
9+
* (which is opt-in and stripped from non-debug bundles) and writes to the console directly.
10+
*/
11+
export function maybeWarnAboutIgnoredTransactionOptions(options: ClientOptions): void {
12+
if (
13+
options.traceLifecycle !== 'stream' ||
14+
// oxlint-disable-next-line typescript/no-deprecated
15+
!(options.beforeSendTransaction || options.ignoreTransactions?.length)
16+
) {
17+
return;
18+
}
19+
20+
consoleSandbox(() => {
21+
// oxlint-disable-next-line no-console
22+
console.warn(
23+
"[Sentry] `beforeSendTransaction` and `ignoreTransactions` are ignored with `traceLifecycle: 'stream'` (enabled by default). Use `beforeSendSpan` and `ignoreSpans` instead, or set `traceLifecycle: 'static'`.",
24+
);
25+
});
26+
}

packages/core/test/lib/client.test.ts

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,93 @@ describe('Client', () => {
9696
});
9797
});
9898

99+
describe('init() / transaction option warnings', () => {
100+
test('warns about transaction options ignored by span streaming', () => {
101+
const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined);
102+
103+
const options = getDefaultTestClientOptions({
104+
dsn: PUBLIC_DSN,
105+
traceLifecycle: 'stream',
106+
beforeSendTransaction: event => event,
107+
ignoreTransactions: ['/healthcheck'],
108+
});
109+
new TestClient(options).init();
110+
111+
expect(consoleWarnSpy).toHaveBeenCalledTimes(1);
112+
expect(consoleWarnSpy).toHaveBeenCalledWith(
113+
expect.stringContaining(
114+
"`beforeSendTransaction` and `ignoreTransactions` are ignored with `traceLifecycle: 'stream'` (enabled by default).",
115+
),
116+
);
117+
consoleWarnSpy.mockRestore();
118+
});
119+
120+
test('stays silent when the client is disabled because no DSN was provided', () => {
121+
const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined);
122+
123+
const options = getDefaultTestClientOptions({
124+
traceLifecycle: 'stream',
125+
beforeSendTransaction: event => event,
126+
ignoreTransactions: ['/healthcheck'],
127+
});
128+
new TestClient(options).init();
129+
130+
expect(consoleWarnSpy).not.toHaveBeenCalled();
131+
consoleWarnSpy.mockRestore();
132+
});
133+
134+
test('stays silent when the client is disabled via `enabled: false`', () => {
135+
const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined);
136+
137+
const options = getDefaultTestClientOptions({
138+
dsn: PUBLIC_DSN,
139+
enabled: false,
140+
traceLifecycle: 'stream',
141+
beforeSendTransaction: event => event,
142+
});
143+
new TestClient(options).init();
144+
145+
expect(consoleWarnSpy).not.toHaveBeenCalled();
146+
consoleWarnSpy.mockRestore();
147+
});
148+
149+
test('warns without a DSN when Spotlight is enabled, since spans are still sent', () => {
150+
const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined);
151+
152+
const options = getDefaultTestClientOptions({
153+
traceLifecycle: 'stream',
154+
beforeSendTransaction: event => event,
155+
integrations: [{ name: 'SpotlightBrowser' }],
156+
});
157+
new TestClient(options).init();
158+
159+
expect(consoleWarnSpy).toHaveBeenCalledTimes(1);
160+
consoleWarnSpy.mockRestore();
161+
});
162+
163+
test('stays silent when an integration falls back to the static trace lifecycle during setup', () => {
164+
const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined);
165+
166+
const options = getDefaultTestClientOptions({
167+
dsn: PUBLIC_DSN,
168+
traceLifecycle: 'stream',
169+
beforeSendTransaction: event => event,
170+
integrations: [
171+
{
172+
name: 'FallsBackToStatic',
173+
setup: client => {
174+
client.getOptions().traceLifecycle = 'static';
175+
},
176+
},
177+
],
178+
});
179+
new TestClient(options).init();
180+
181+
expect(consoleWarnSpy).not.toHaveBeenCalled();
182+
consoleWarnSpy.mockRestore();
183+
});
184+
});
185+
99186
describe('getOptions()', () => {
100187
test('returns the options', () => {
101188
expect.assertions(1);
Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
2+
import type { ClientOptions } from '../../../src/types/options';
3+
import * as debugLoggerModule from '../../../src/utils/debug-logger';
4+
import { maybeWarnAboutIgnoredTransactionOptions } from '../../../src/utils/warnAboutIgnoredTransactionOptions';
5+
6+
describe('maybeWarnAboutIgnoredTransactionOptions', () => {
7+
let consoleWarnSpy: ReturnType<typeof vi.spyOn>;
8+
9+
beforeEach(() => {
10+
consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined);
11+
vi.spyOn(debugLoggerModule, 'consoleSandbox').mockImplementation(cb => cb());
12+
});
13+
14+
afterEach(() => {
15+
vi.restoreAllMocks();
16+
});
17+
18+
function options(overrides: Partial<ClientOptions>): ClientOptions {
19+
return { integrations: [], transport: () => ({}) as never, stackParser: () => [], ...overrides } as ClientOptions;
20+
}
21+
22+
it('warns when `beforeSendTransaction` is set and span streaming is enabled', () => {
23+
maybeWarnAboutIgnoredTransactionOptions(
24+
options({ traceLifecycle: 'stream', beforeSendTransaction: event => event }),
25+
);
26+
27+
expect(consoleWarnSpy).toHaveBeenCalledTimes(1);
28+
expect(consoleWarnSpy.mock.calls[0]?.[0]).toContain('`beforeSendTransaction` and `ignoreTransactions`');
29+
});
30+
31+
it('warns when `ignoreTransactions` is set and span streaming is enabled', () => {
32+
maybeWarnAboutIgnoredTransactionOptions(
33+
options({ traceLifecycle: 'stream', ignoreTransactions: ['/healthcheck'] }),
34+
);
35+
36+
expect(consoleWarnSpy).toHaveBeenCalledTimes(1);
37+
expect(consoleWarnSpy.mock.calls[0]?.[0]).toContain('`beforeSendTransaction` and `ignoreTransactions`');
38+
});
39+
40+
it('warns only once when both options are set', () => {
41+
maybeWarnAboutIgnoredTransactionOptions(
42+
options({
43+
traceLifecycle: 'stream',
44+
beforeSendTransaction: event => event,
45+
ignoreTransactions: ['/healthcheck'],
46+
}),
47+
);
48+
49+
expect(consoleWarnSpy).toHaveBeenCalledTimes(1);
50+
});
51+
52+
it('points at the replacement options and the opt-out', () => {
53+
maybeWarnAboutIgnoredTransactionOptions(
54+
options({ traceLifecycle: 'stream', beforeSendTransaction: event => event }),
55+
);
56+
57+
const message = consoleWarnSpy.mock.calls[0]?.[0];
58+
expect(message).toContain('`beforeSendSpan` and `ignoreSpans`');
59+
expect(message).toContain("`traceLifecycle: 'static'`");
60+
});
61+
62+
it('does not warn when the trace lifecycle is static', () => {
63+
maybeWarnAboutIgnoredTransactionOptions(
64+
options({
65+
traceLifecycle: 'static',
66+
beforeSendTransaction: event => event,
67+
ignoreTransactions: ['/healthcheck'],
68+
}),
69+
);
70+
71+
expect(consoleWarnSpy).not.toHaveBeenCalled();
72+
});
73+
74+
it('does not warn when neither option is set', () => {
75+
maybeWarnAboutIgnoredTransactionOptions(options({ traceLifecycle: 'stream' }));
76+
77+
expect(consoleWarnSpy).not.toHaveBeenCalled();
78+
});
79+
80+
it('does not warn for an empty `ignoreTransactions` array', () => {
81+
maybeWarnAboutIgnoredTransactionOptions(options({ traceLifecycle: 'stream', ignoreTransactions: [] }));
82+
83+
expect(consoleWarnSpy).not.toHaveBeenCalled();
84+
});
85+
});

0 commit comments

Comments
 (0)