diff --git a/.size-limit.js b/.size-limit.js index d589cfccdc65..6ef49dad2033 100644 --- a/.size-limit.js +++ b/.size-limit.js @@ -286,7 +286,7 @@ module.exports = [ path: createCDNPath('bundle.logs.metrics.min.js'), gzip: false, brotli: false, - limit: '100 KB', + limit: '101 KB', disablePlugins: ['@size-limit/esbuild'], }, { diff --git a/AGENTS.md b/AGENTS.md index 62374358ad61..7ca4e8b264f8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -134,6 +134,8 @@ Uses **Git Flow** (see `docs/gitflow.md`). - Never expose secrets or keys - When modifying files, cover all occurrences (including `src/` and `test/`) - 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 +- Do not use `expect(someSpy.mock.calls[0]?.[0])` or similar constructs to check what a spy was called with. + Instead use `expect(someSpy).toHaveBeenCalledWith(...)` or derivatives for a more readable and less brittle test assertion. ## Lazy Loading Is a Last Resort diff --git a/packages/bundler-plugins/src/core/sentry/telemetry.ts b/packages/bundler-plugins/src/core/sentry/telemetry.ts index 5615ac3e8f6f..962064c383ee 100644 --- a/packages/bundler-plugins/src/core/sentry/telemetry.ts +++ b/packages/bundler-plugins/src/core/sentry/telemetry.ts @@ -45,6 +45,8 @@ export function createSentryInstance( return event; }, + // Deprecated, but still applied because this client runs on the static trace lifecycle. + // oxlint-disable-next-line typescript/no-deprecated beforeSendTransaction: event => { delete event.server_name; // Server name might contain PII return event; diff --git a/packages/core/src/client.ts b/packages/core/src/client.ts index 6076b065cf62..7196fd68f7f9 100644 --- a/packages/core/src/client.ts +++ b/packages/core/src/client.ts @@ -54,6 +54,7 @@ import { reparentChildSpans, shouldIgnoreSpan } from './utils/should-ignore-span import { showSpanDropWarning } from './utils/spanUtils'; import { safeUnref } from './utils/timer'; import { convertSpanJsonToTransactionEvent, convertTransactionEventToSpanJson } from './utils/transactionEvent'; +import { maybeWarnAboutIgnoredTransactionOptions } from './utils/warnAboutIgnoredTransactionOptions'; import { resolveDataCollectionOptions } from './utils/data-collection/resolveDataCollectionOptions'; const ALREADY_SEEN_ERROR = "Not capturing exception because it's already been captured."; @@ -507,6 +508,8 @@ export abstract class Client { this._options.integrations.some(({ name }) => name.startsWith('Spotlight')) ) { this._setupIntegrations(); + + maybeWarnAboutIgnoredTransactionOptions(this._options); } } @@ -1653,7 +1656,12 @@ function processBeforeSend( event: Event, hint: EventHint, ): PromiseLike | Event | null { - const { beforeSend, beforeSendTransaction, ignoreSpans } = options; + const { + beforeSend, + ignoreSpans, + // oxlint-disable-next-line typescript/no-deprecated + beforeSendTransaction, + } = options; const beforeSendSpan = !isStreamedBeforeSendSpanCallback(options.beforeSendSpan) && options.beforeSendSpan; let processedEvent = event; diff --git a/packages/core/src/types/options.ts b/packages/core/src/types/options.ts index e5f938082509..b7730b720c47 100644 --- a/packages/core/src/types/options.ts +++ b/packages/core/src/types/options.ts @@ -343,10 +343,19 @@ export interface ClientOptions; @@ -643,12 +652,20 @@ export interface ClientOptions { + // oxlint-disable-next-line no-console + console.warn( + "[Sentry] `beforeSendTransaction` and `ignoreTransactions` are ignored with `traceLifecycle: 'stream'` (enabled by default). Use `beforeSendSpan` and `ignoreSpans` instead, or set `traceLifecycle: 'static'`.", + ); + }); +} diff --git a/packages/core/test/lib/client.test.ts b/packages/core/test/lib/client.test.ts index 5b0cb752f2c2..d30314073bcf 100644 --- a/packages/core/test/lib/client.test.ts +++ b/packages/core/test/lib/client.test.ts @@ -96,6 +96,93 @@ describe('Client', () => { }); }); + describe('init() / transaction option warnings', () => { + test('warns about transaction options ignored by span streaming', () => { + const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined); + + const options = getDefaultTestClientOptions({ + dsn: PUBLIC_DSN, + traceLifecycle: 'stream', + beforeSendTransaction: event => event, + ignoreTransactions: ['/healthcheck'], + }); + new TestClient(options).init(); + + expect(consoleWarnSpy).toHaveBeenCalledTimes(1); + expect(consoleWarnSpy).toHaveBeenCalledWith( + expect.stringContaining( + "`beforeSendTransaction` and `ignoreTransactions` are ignored with `traceLifecycle: 'stream'` (enabled by default).", + ), + ); + consoleWarnSpy.mockRestore(); + }); + + test('stays silent when the client is disabled because no DSN was provided', () => { + const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined); + + const options = getDefaultTestClientOptions({ + traceLifecycle: 'stream', + beforeSendTransaction: event => event, + ignoreTransactions: ['/healthcheck'], + }); + new TestClient(options).init(); + + expect(consoleWarnSpy).not.toHaveBeenCalled(); + consoleWarnSpy.mockRestore(); + }); + + test('stays silent when the client is disabled via `enabled: false`', () => { + const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined); + + const options = getDefaultTestClientOptions({ + dsn: PUBLIC_DSN, + enabled: false, + traceLifecycle: 'stream', + beforeSendTransaction: event => event, + }); + new TestClient(options).init(); + + expect(consoleWarnSpy).not.toHaveBeenCalled(); + consoleWarnSpy.mockRestore(); + }); + + test('warns without a DSN when Spotlight is enabled, since spans are still sent', () => { + const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined); + + const options = getDefaultTestClientOptions({ + traceLifecycle: 'stream', + beforeSendTransaction: event => event, + integrations: [{ name: 'SpotlightBrowser' }], + }); + new TestClient(options).init(); + + expect(consoleWarnSpy).toHaveBeenCalledTimes(1); + consoleWarnSpy.mockRestore(); + }); + + test('stays silent when an integration falls back to the static trace lifecycle during setup', () => { + const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined); + + const options = getDefaultTestClientOptions({ + dsn: PUBLIC_DSN, + traceLifecycle: 'stream', + beforeSendTransaction: event => event, + integrations: [ + { + name: 'FallsBackToStatic', + setup: client => { + client.getOptions().traceLifecycle = 'static'; + }, + }, + ], + }); + new TestClient(options).init(); + + expect(consoleWarnSpy).not.toHaveBeenCalled(); + consoleWarnSpy.mockRestore(); + }); + }); + describe('getOptions()', () => { test('returns the options', () => { expect.assertions(1); diff --git a/packages/core/test/lib/utils/warnAboutIgnoredTransactionOptions.test.ts b/packages/core/test/lib/utils/warnAboutIgnoredTransactionOptions.test.ts new file mode 100644 index 000000000000..dadae7211872 --- /dev/null +++ b/packages/core/test/lib/utils/warnAboutIgnoredTransactionOptions.test.ts @@ -0,0 +1,85 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import type { ClientOptions } from '../../../src/types/options'; +import * as debugLoggerModule from '../../../src/utils/debug-logger'; +import { maybeWarnAboutIgnoredTransactionOptions } from '../../../src/utils/warnAboutIgnoredTransactionOptions'; + +describe('maybeWarnAboutIgnoredTransactionOptions', () => { + let consoleWarnSpy: ReturnType; + + beforeEach(() => { + consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined); + vi.spyOn(debugLoggerModule, 'consoleSandbox').mockImplementation(cb => cb()); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + function options(overrides: Partial): ClientOptions { + return { integrations: [], transport: () => ({}) as never, stackParser: () => [], ...overrides } as ClientOptions; + } + + it('warns when `beforeSendTransaction` is set and span streaming is enabled', () => { + maybeWarnAboutIgnoredTransactionOptions( + options({ traceLifecycle: 'stream', beforeSendTransaction: event => event }), + ); + + expect(consoleWarnSpy).toHaveBeenCalledTimes(1); + expect(consoleWarnSpy.mock.calls[0]?.[0]).toContain('`beforeSendTransaction` and `ignoreTransactions`'); + }); + + it('warns when `ignoreTransactions` is set and span streaming is enabled', () => { + maybeWarnAboutIgnoredTransactionOptions( + options({ traceLifecycle: 'stream', ignoreTransactions: ['/healthcheck'] }), + ); + + expect(consoleWarnSpy).toHaveBeenCalledTimes(1); + expect(consoleWarnSpy.mock.calls[0]?.[0]).toContain('`beforeSendTransaction` and `ignoreTransactions`'); + }); + + it('warns only once when both options are set', () => { + maybeWarnAboutIgnoredTransactionOptions( + options({ + traceLifecycle: 'stream', + beforeSendTransaction: event => event, + ignoreTransactions: ['/healthcheck'], + }), + ); + + expect(consoleWarnSpy).toHaveBeenCalledTimes(1); + }); + + it('points at the replacement options and the opt-out', () => { + maybeWarnAboutIgnoredTransactionOptions( + options({ traceLifecycle: 'stream', beforeSendTransaction: event => event }), + ); + + const message = consoleWarnSpy.mock.calls[0]?.[0]; + expect(message).toContain('`beforeSendSpan` and `ignoreSpans`'); + expect(message).toContain("`traceLifecycle: 'static'`"); + }); + + it('does not warn when the trace lifecycle is static', () => { + maybeWarnAboutIgnoredTransactionOptions( + options({ + traceLifecycle: 'static', + beforeSendTransaction: event => event, + ignoreTransactions: ['/healthcheck'], + }), + ); + + expect(consoleWarnSpy).not.toHaveBeenCalled(); + }); + + it('does not warn when neither option is set', () => { + maybeWarnAboutIgnoredTransactionOptions(options({ traceLifecycle: 'stream' })); + + expect(consoleWarnSpy).not.toHaveBeenCalled(); + }); + + it('does not warn for an empty `ignoreTransactions` array', () => { + maybeWarnAboutIgnoredTransactionOptions(options({ traceLifecycle: 'stream', ignoreTransactions: [] })); + + expect(consoleWarnSpy).not.toHaveBeenCalled(); + }); +});