diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 7b9db9fbb908..d29c7f61af47 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -24,6 +24,7 @@ import type { Span, SpanAttributes, SpanContextData, + SpanJSON, StartSpanOptions, TransactionEvent, Transport, @@ -904,7 +905,14 @@ function processBeforeSend( if (isTransactionEvent(event)) { if (event.spans && beforeSendSpan) { - event.spans = event.spans.map(span => beforeSendSpan(span)); + const processedSpans: SpanJSON[] = []; + for (const span of event.spans) { + const processedSpan = beforeSendSpan(span); + if (processedSpan) { + processedSpans.push(processedSpan); + } + } + event.spans = processedSpans; } if (beforeSendTransaction) { diff --git a/packages/core/src/envelope.ts b/packages/core/src/envelope.ts index d2a97eb766aa..ac336b52aed6 100644 --- a/packages/core/src/envelope.ts +++ b/packages/core/src/envelope.ts @@ -12,6 +12,7 @@ import type { SessionEnvelope, SessionItem, SpanEnvelope, + SpanItem, SpanJSON, } from '@sentry/types'; import { @@ -98,8 +99,9 @@ export function createEventEnvelope( * Create envelope from Span item. * * Takes an optional client and runs spans through `beforeSendSpan` if available. + * @returns A SpanEnvelope or null if all spans have been dropped */ -export function createSpanEnvelope(spans: SentrySpan[], client?: Client): SpanEnvelope { +export function createSpanEnvelope(spans: SentrySpan[], client?: Client): SpanEnvelope | null { function dscHasRequiredProps(dsc: Partial): dsc is DynamicSamplingContext { return !!dsc.trace_id && !!dsc.public_key; } @@ -118,7 +120,14 @@ export function createSpanEnvelope(spans: SentrySpan[], client?: Client): SpanEn const convertToSpanJSON = beforeSendSpan ? (span: SentrySpan) => beforeSendSpan(spanToJSON(span) as SpanJSON) : (span: SentrySpan) => spanToJSON(span); - const items = spans.map(span => createSpanEnvelopeItem(convertToSpanJSON(span))); - return createEnvelope(headers, items); + const items: SpanItem[] = []; + for (const span of spans) { + const spanJson = convertToSpanJSON(span); + if (spanJson) { + items.push(createSpanEnvelopeItem(spanJson)); + } + } + + return items.length > 0 ? createEnvelope(headers, items) : null; } diff --git a/packages/core/src/tracing/sentrySpan.ts b/packages/core/src/tracing/sentrySpan.ts index 92401a032c29..0e35405570a0 100644 --- a/packages/core/src/tracing/sentrySpan.ts +++ b/packages/core/src/tracing/sentrySpan.ts @@ -97,12 +97,12 @@ export class SentrySpan implements Span { this._events = []; + this._isStandaloneSpan = spanContext.isStandalone; + // If the span is already ended, ensure we finalize the span immediately if (this._endTime) { this._onSpanEnded(); } - - this._isStandaloneSpan = spanContext.isStandalone; } /** @inheritdoc */ @@ -259,7 +259,14 @@ export class SentrySpan implements Span { // if this is a standalone span, we send it immediately if (this._isStandaloneSpan) { - sendSpanEnvelope(createSpanEnvelope([this], client)); + const spanEnvelope = createSpanEnvelope([this], client); + if (spanEnvelope) { + sendSpanEnvelope(spanEnvelope); + } else { + if (client) { + client.recordDroppedEvent('before_send', 'span'); + } + } return; } diff --git a/packages/core/test/lib/base.test.ts b/packages/core/test/lib/base.test.ts index 135897390b36..1ee834e0d8ae 100644 --- a/packages/core/test/lib/base.test.ts +++ b/packages/core/test/lib/base.test.ts @@ -1119,6 +1119,38 @@ describe('BaseClient', () => { expect(loggerWarnSpy).toBeCalledWith('before send for type `transaction` returned `null`, will not send event.'); }); + test('calls `beforeSendSpan` and discards the span', () => { + expect.assertions(2); + + const beforeSendSpan = jest.fn(() => null); + const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, beforeSendSpan }); + const client = new TestClient(options); + + const transaction: Event = { + transaction: '/cats/are/great', + type: 'transaction', + spans: [ + { + description: 'first span', + span_id: '9e15bf99fbe4bc80', + start_timestamp: 1591603196.637835, + trace_id: '86f39e84263a4de99c326acab3bfe3bd', + }, + { + description: 'second span', + span_id: 'aa554c1f506b0783', + start_timestamp: 1591603196.637835, + trace_id: '86f39e84263a4de99c326acab3bfe3bd', + }, + ], + }; + client.captureEvent(transaction); + + expect(beforeSendSpan).toHaveBeenCalledTimes(2); + const capturedEvent = TestClient.instance!.event!; + expect(capturedEvent.spans).toHaveLength(0); + }); + test('calls `beforeSend` and logs info about invalid return value', () => { const invalidValues = [undefined, false, true, [], 1]; expect.assertions(invalidValues.length * 3); diff --git a/packages/core/test/lib/envelope.test.ts b/packages/core/test/lib/envelope.test.ts index 7d5a2c5f7740..a06fbad236f0 100644 --- a/packages/core/test/lib/envelope.test.ts +++ b/packages/core/test/lib/envelope.test.ts @@ -109,7 +109,7 @@ describe('createSpanEnvelope', () => { const spanEnvelope = createSpanEnvelope([span]); - const spanItem = spanEnvelope[1][0][1]; + const spanItem = spanEnvelope![1][0][1]; expect(spanItem).toEqual({ data: { 'sentry.origin': 'manual', @@ -131,7 +131,7 @@ describe('createSpanEnvelope', () => { new SentrySpan({ name: 'test', attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'custom' } }), ]); - const spanEnvelopeHeaders = spanEnvelope[0]; + const spanEnvelopeHeaders = spanEnvelope![0]; expect(spanEnvelopeHeaders).toEqual({ sent_at: expect.any(String), trace: { @@ -152,7 +152,7 @@ describe('createSpanEnvelope', () => { const spanEnvelope = createSpanEnvelope([new SentrySpan()]); - const spanEnvelopeHeaders = spanEnvelope[0]; + const spanEnvelopeHeaders = spanEnvelope![0]; expect(spanEnvelopeHeaders).toEqual({ sent_at: expect.any(String), }); @@ -174,7 +174,7 @@ describe('createSpanEnvelope', () => { expect(beforeSendSpan).toHaveBeenCalled(); - const spanItem = spanEnvelope[1][0][1]; + const spanItem = spanEnvelope![1][0][1]; expect(spanItem).toEqual({ data: { 'sentry.origin': 'manual', @@ -209,7 +209,7 @@ describe('createSpanEnvelope', () => { expect(beforeSendSpan).toHaveBeenCalled(); - const spanItem = spanEnvelope[1][0][1]; + const spanItem = spanEnvelope![1][0][1]; expect(spanItem).toEqual({ data: { 'sentry.origin': 'manual', @@ -224,4 +224,22 @@ describe('createSpanEnvelope', () => { trace_id: expect.stringMatching(/^[0-9a-f]{32}$/), }); }); + + it('calls `beforeSendSpan` and discards the envelope', () => { + const beforeSendSpan = jest.fn(() => null); + const options = getDefaultTestClientOptions({ dsn: 'https://domain/123', beforeSendSpan }); + const client = new TestClient(options); + + const span = new SentrySpan({ + name: 'test', + isStandalone: true, + startTimestamp: 1, + endTimestamp: 2, + }); + + const spanEnvelope = createSpanEnvelope([span], client); + + expect(beforeSendSpan).toHaveBeenCalled(); + expect(spanEnvelope).toBeNull(); + }); }); diff --git a/packages/core/test/lib/tracing/sentrySpan.test.ts b/packages/core/test/lib/tracing/sentrySpan.test.ts index 13d52149bb8b..dbc895d34469 100644 --- a/packages/core/test/lib/tracing/sentrySpan.test.ts +++ b/packages/core/test/lib/tracing/sentrySpan.test.ts @@ -2,6 +2,8 @@ import { timestampInSeconds } from '@sentry/utils'; import { SentrySpan } from '../../../src/tracing/sentrySpan'; import { SPAN_STATUS_ERROR } from '../../../src/tracing/spanstatus'; import { TRACE_FLAG_NONE, TRACE_FLAG_SAMPLED, spanToJSON } from '../../../src/utils/spanUtils'; +import { TestClient, getDefaultTestClientOptions } from '../../mocks/client'; +import { setCurrentClient } from '../../../src'; describe('SentrySpan', () => { describe('name', () => { @@ -88,6 +90,57 @@ describe('SentrySpan', () => { span.end(); expect(spanToJSON(span).timestamp).toBeGreaterThan(1); }); + + test('sends the span if `beforeSendSpan` does not modify the span ', () => { + const beforeSendSpan = jest.fn(span => span) + const client = new TestClient( + getDefaultTestClientOptions({ + dsn: 'https://username@domain/123', + enableSend: true, + beforeSendSpan, + }) + ); + setCurrentClient(client) + + // @ts-expect-error Accessing private transport API + const mockSend = jest.spyOn(client._transport, 'send'); + const span = new SentrySpan({ + name: 'test', + isStandalone: true, + startTimestamp: 1, + endTimestamp: 2, + sampled: true, + }); + span.end(); + expect(mockSend).toHaveBeenCalled() + }); + + test('does not send the span if `beforeSendSpan` drops the span', () => { + const beforeSendSpan = jest.fn(() => null) + const client = new TestClient( + getDefaultTestClientOptions({ + dsn: 'https://username@domain/123', + enableSend: true, + beforeSendSpan, + }) + ); + setCurrentClient(client) + + const recordDroppedEventSpy = jest.spyOn(client, 'recordDroppedEvent'); + // @ts-expect-error Accessing private transport API + const mockSend = jest.spyOn(client._transport, 'send'); + const span = new SentrySpan({ + name: 'test', + isStandalone: true, + startTimestamp: 1, + endTimestamp: 2, + sampled: true, + }); + span.end(); + + expect(mockSend).not.toHaveBeenCalled(); + expect(recordDroppedEventSpy).toHaveBeenCalledWith('before_send', 'span') + }); }); describe('end', () => { diff --git a/packages/types/src/options.ts b/packages/types/src/options.ts index 204c283e56ce..26a043647dcc 100644 --- a/packages/types/src/options.ts +++ b/packages/types/src/options.ts @@ -285,11 +285,11 @@ export interface ClientOptions SpanJSON; + beforeSendSpan?: (span: SpanJSON) => SpanJSON | null; /** * An event-processing callback for transaction events, guaranteed to be invoked after all other event