Skip to content

Commit 9ccb8c8

Browse files
msonnbcodex
andcommitted
refactor(core): Remove beforeSendSpan null return warning
Co-Authored-By: GPT-6 <codex@openai.com>
1 parent b0a7cc6 commit 9ccb8c8

3 files changed

Lines changed: 11 additions & 71 deletions

File tree

‎packages/core/src/tracing/spans/beforeSendSpan.ts‎

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import { DEBUG_BUILD } from '../../debug-build';
22
import type { BeforeSendStaticSpanCallback, BeforeSendStreamedSpanCallback } from '../../types/options';
33
import type { SpanJSON, StreamedSpanJSON } from '../../types/span';
44
import { addNonEnumerableProperty } from '../../utils/object';
5-
import { consoleSandbox } from '../../utils/debug-logger';
65
import { safeCallback } from '../../utils/safeCallback';
76

87
/**
@@ -57,7 +56,6 @@ export function isStaticBeforeSendSpanCallback(callback: unknown): callback is B
5756
return !!callback && typeof callback === 'function' && '_static' in callback && !!callback._static;
5857
}
5958

60-
let hasShownSpanDropWarning = false;
6159
/**
6260
* Apply a user-provided beforeSendSpan callback to a span JSON.
6361
*/
@@ -72,18 +70,5 @@ export function applyBeforeSendSpanCallback<T extends StreamedSpanJSON | SpanJSO
7270
() => beforeSendSpan(span),
7371
() => span,
7472
);
75-
if (modifiedSpan) {
76-
return modifiedSpan;
77-
}
78-
79-
if (!hasShownSpanDropWarning) {
80-
consoleSandbox(() => {
81-
// eslint-disable-next-line no-console
82-
console.warn(
83-
'[Sentry] Returning null from `beforeSendSpan` is disallowed. To drop certain spans, configure the respective integrations directly or use `ignoreSpans`.',
84-
);
85-
});
86-
hasShownSpanDropWarning = true;
87-
}
88-
return span;
73+
return modifiedSpan || span;
8974
}

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

Lines changed: 0 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1711,51 +1711,6 @@ describe('Client', () => {
17111711
expect(loggerLogSpy).toBeCalledWith('before send for type `transaction` returned `null`, will not send event.');
17121712
});
17131713

1714-
test('does not discard span and warn when returning null from `beforeSendSpan', () => {
1715-
const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined);
1716-
1717-
// @ts-expect-error - intentionally violating the type signature here
1718-
const beforeSendSpan = withStaticSpan(vi.fn(() => null));
1719-
1720-
const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, beforeSendSpan });
1721-
const client = new TestClient(options);
1722-
1723-
const transaction: Event = {
1724-
transaction: '/dogs/are/great',
1725-
type: 'transaction',
1726-
spans: [
1727-
{
1728-
description: 'first span',
1729-
span_id: '9e15bf99fbe4bc80',
1730-
start_timestamp: 1591603196.637835,
1731-
trace_id: '86f39e84263a4de99c326acab3bfe3bd',
1732-
data: {},
1733-
status: 'ok',
1734-
},
1735-
{
1736-
description: 'second span',
1737-
span_id: 'aa554c1f506b0783',
1738-
start_timestamp: 1591603196.637835,
1739-
trace_id: '86f39e84263a4de99c326acab3bfe3bd',
1740-
data: {},
1741-
status: 'ok',
1742-
},
1743-
],
1744-
};
1745-
client.captureEvent(transaction);
1746-
1747-
expect(beforeSendSpan).toHaveBeenCalledTimes(3);
1748-
const capturedEvent = TestClient.instance!.event!;
1749-
expect(capturedEvent.spans).toHaveLength(2);
1750-
expect(client['_outcomes']).toEqual({});
1751-
1752-
expect(consoleWarnSpy).toHaveBeenCalledTimes(1);
1753-
expect(consoleWarnSpy).toHaveBeenCalledWith(
1754-
'[Sentry] Returning null from `beforeSendSpan` is disallowed. To drop certain spans, configure the respective integrations directly or use `ignoreSpans`.',
1755-
);
1756-
consoleWarnSpy.mockRestore();
1757-
});
1758-
17591714
test("doesn't throw if the `beforeSendSpan` callback throws", () => {
17601715
const debugErrorSpy = vi.spyOn(debugLoggerModule.debug, 'error').mockImplementation(() => undefined);
17611716
const error = new Error('beforeSendSpan is broken');

‎packages/core/test/lib/tracing/spans/captureSpan.test.ts‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -507,8 +507,7 @@ describe('captureSpan', () => {
507507
expect(beforeSendSpan).not.toHaveBeenCalled();
508508
});
509509

510-
it('logs a warning if the beforeSendSpan callback returns null', () => {
511-
const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined);
510+
it('keeps the span if the beforeSendSpan callback returns null', () => {
512511
const beforeSendSpan = vi.fn(() => null as unknown as StreamedSpanJSON);
513512

514513
const client = new TestClient(
@@ -522,16 +521,17 @@ describe('captureSpan', () => {
522521
}),
523522
);
524523

525-
const span = startInactiveSpan({ name: 'my-span', attributes: { 'sentry.op': 'http.client' } });
526-
span.end();
527-
528-
captureSpan(span, client);
524+
const span = withScope(scope => {
525+
scope.setClient(client);
526+
const span = startInactiveSpan({ name: 'my-span', attributes: { 'sentry.op': 'http.client' } });
527+
span.end();
528+
return span;
529+
});
529530

530-
expect(consoleWarnSpy).toHaveBeenCalledWith(
531-
'[Sentry] Returning null from `beforeSendSpan` is disallowed. To drop certain spans, configure the respective integrations directly or use `ignoreSpans`.',
532-
);
531+
const serialized = captureSpan(span, client);
533532

534-
consoleWarnSpy.mockRestore();
533+
expect(serialized.span_id).toBe(span.spanContext().spanId);
534+
expect(serialized.name).toBe('my-span');
535535
});
536536

537537
it('keeps the span and logs an error if the beforeSendSpan callback throws', () => {

0 commit comments

Comments
 (0)