From a368ce1ecbfc5a434a292e31116782290399de7d Mon Sep 17 00:00:00 2001 From: Andrei Borza Date: Mon, 6 May 2024 17:35:08 +0200 Subject: [PATCH 1/4] feat(core): Add `beforeSendSpan` hook (#11886) --- packages/core/src/baseclient.ts | 12 +++-- packages/core/src/envelope.ts | 18 ++++++- packages/core/src/tracing/sentrySpan.ts | 2 +- packages/core/test/lib/base.test.ts | 71 +++++++++++++++++++++++++ packages/core/test/lib/envelope.test.ts | 67 +++++++++++++++++++++++ packages/types/src/options.ts | 10 ++++ 6 files changed, 174 insertions(+), 6 deletions(-) diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 096b288c8a21..7b9db9fbb908 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -896,14 +896,20 @@ function processBeforeSend( event: Event, hint: EventHint, ): PromiseLike | Event | null { - const { beforeSend, beforeSendTransaction } = options; + const { beforeSend, beforeSendTransaction, beforeSendSpan } = options; if (isErrorEvent(event) && beforeSend) { return beforeSend(event, hint); } - if (isTransactionEvent(event) && beforeSendTransaction) { - return beforeSendTransaction(event, hint); + if (isTransactionEvent(event)) { + if (event.spans && beforeSendSpan) { + event.spans = event.spans.map(span => beforeSendSpan(span)); + } + + if (beforeSendTransaction) { + return beforeSendTransaction(event, hint); + } } return event; diff --git a/packages/core/src/envelope.ts b/packages/core/src/envelope.ts index 2aef194b069e..8cd496ad472a 100644 --- a/packages/core/src/envelope.ts +++ b/packages/core/src/envelope.ts @@ -1,4 +1,5 @@ import type { + Client, DsnComponents, DynamicSamplingContext, Event, @@ -11,6 +12,8 @@ import type { SessionEnvelope, SessionItem, SpanEnvelope, + SpanItem, + SpanJSON, } from '@sentry/types'; import { createEnvelope, @@ -94,8 +97,10 @@ export function createEventEnvelope( /** * Create envelope from Span item. + * + * Takes an optional client and runs spans through `beforeSendSpan` if available. */ -export function createSpanEnvelope(spans: SentrySpan[]): SpanEnvelope { +export function createSpanEnvelope(spans: SentrySpan[], client?: Client): SpanEnvelope { function dscHasRequiredProps(dsc: Partial): dsc is DynamicSamplingContext { return !!dsc.trace_id && !!dsc.public_key; } @@ -109,6 +114,15 @@ export function createSpanEnvelope(spans: SentrySpan[]): SpanEnvelope { sent_at: new Date().toISOString(), ...(dscHasRequiredProps(dsc) && { trace: dsc }), }; - const items = spans.map(span => createSpanEnvelopeItem(spanToJSON(span))); + + const beforeSend = client && client.getOptions().beforeSendSpan; + let items: SpanItem[]; + + if (beforeSend) { + items = spans.map(span => createSpanEnvelopeItem(beforeSend(spanToJSON(span) as SpanJSON))); + } else { + items = spans.map(span => createSpanEnvelopeItem(spanToJSON(span))); + } + return createEnvelope(headers, items); } diff --git a/packages/core/src/tracing/sentrySpan.ts b/packages/core/src/tracing/sentrySpan.ts index e9e503150fcd..92401a032c29 100644 --- a/packages/core/src/tracing/sentrySpan.ts +++ b/packages/core/src/tracing/sentrySpan.ts @@ -259,7 +259,7 @@ export class SentrySpan implements Span { // if this is a standalone span, we send it immediately if (this._isStandaloneSpan) { - sendSpanEnvelope(createSpanEnvelope([this])); + sendSpanEnvelope(createSpanEnvelope([this], client)); return; } diff --git a/packages/core/test/lib/base.test.ts b/packages/core/test/lib/base.test.ts index 3cfb230da5cf..135897390b36 100644 --- a/packages/core/test/lib/base.test.ts +++ b/packages/core/test/lib/base.test.ts @@ -978,6 +978,38 @@ describe('BaseClient', () => { expect(TestClient.instance!.event!.transaction).toBe('/dogs/are/great'); }); + test('calls `beforeSendSpan` and uses original spans without any changes', () => { + expect.assertions(2); + + const beforeSendSpan = jest.fn(span => span); + 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).toEqual(transaction.spans); + }); + test('calls `beforeSend` and uses the modified event', () => { expect.assertions(2); @@ -1010,6 +1042,45 @@ describe('BaseClient', () => { expect(TestClient.instance!.event!.transaction).toBe('/adopt/dont/shop'); }); + test('calls `beforeSendSpan` and uses the modified spans', () => { + expect.assertions(3); + + const beforeSendSpan = jest.fn(span => { + span.data = { version: 'bravo' }; + return span; + }); + + 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!; + for (const [idx, span] of capturedEvent.spans!.entries()) { + const originalSpan = transaction.spans![idx]; + expect(span).toEqual({ ...originalSpan, data: { version: 'bravo' } }); + } + }); + test('calls `beforeSend` and discards the event', () => { expect.assertions(4); diff --git a/packages/core/test/lib/envelope.test.ts b/packages/core/test/lib/envelope.test.ts index f5240e826604..2f378de8da7a 100644 --- a/packages/core/test/lib/envelope.test.ts +++ b/packages/core/test/lib/envelope.test.ts @@ -157,4 +157,71 @@ describe('createSpanEnvelope', () => { sent_at: expect.any(String), }); }); + + it('calls `beforeSendSpan` and uses original span without any changes', () => { + const beforeSendSpan = jest.fn(span => span); + 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(); + + const spanItem = spanEnvelope[1][0][1]; + expect(spanItem).toEqual({ + data: { + 'sentry.origin': 'manual', + }, + description: 'test', + is_segment: true, + origin: 'manual', + span_id: expect.stringMatching(/^[0-9a-f]{16}$/), + segment_id: spanItem.segment_id, + start_timestamp: 1, + timestamp: 2, + trace_id: expect.stringMatching(/^[0-9a-f]{32}$/), + }); + }); + + it('calls `beforeSendSpan` and uses the modified span', () => { + const beforeSendSpan = jest.fn(span => { + span.description = `mutated description: ${span.description}`; + return span; + }); + 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(); + + const spanItem = spanEnvelope[1][0][1]; + expect(spanItem.description).toEqual({ + data: { + 'sentry.origin': 'manual', + }, + description: 'mutated description: test', + is_segment: true, + origin: 'manual', + span_id: expect.stringMatching(/^[0-9a-f]{16}$/), + segment_id: spanItem.segment_id, + start_timestamp: 1, + timestamp: 2, + trace_id: expect.stringMatching(/^[0-9a-f]{32}$/), + }); + }); }); diff --git a/packages/types/src/options.ts b/packages/types/src/options.ts index bf41b16fc595..204c283e56ce 100644 --- a/packages/types/src/options.ts +++ b/packages/types/src/options.ts @@ -3,6 +3,7 @@ import type { ErrorEvent, EventHint, TransactionEvent } from './event'; import type { Integration } from './integration'; import type { CaptureContext } from './scope'; import type { SdkMetadata } from './sdkmetadata'; +import type { SpanJSON } from './span'; import type { StackLineParser, StackParser } from './stacktrace'; import type { TracePropagationTargets } from './tracing'; import type { SamplingContext } from './transaction'; @@ -281,6 +282,15 @@ export interface ClientOptions PromiseLike | ErrorEvent | null; + /** + * An event-processing callback for spans. This allows a span to be modified before it's sent. + * + * Note that you must return a valid span from this callback. If you do not wish to modify the span, simply return + * it at the end. + * @param span The span generated by the SDK. + */ + beforeSendSpan?: (span: SpanJSON) => SpanJSON; + /** * An event-processing callback for transaction events, guaranteed to be invoked after all other event * processors. This allows an event to be modified or dropped before it's sent. From 85df82e9325ab320677c1274bdf31393d7959f0b Mon Sep 17 00:00:00 2001 From: Andrei Borza Date: Tue, 7 May 2024 12:56:40 +0200 Subject: [PATCH 2/4] feat(core): Improve envelope span transformation readability --- packages/core/src/envelope.ts | 14 +++++--------- packages/core/test/lib/envelope.test.ts | 2 +- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/packages/core/src/envelope.ts b/packages/core/src/envelope.ts index 8cd496ad472a..d2a97eb766aa 100644 --- a/packages/core/src/envelope.ts +++ b/packages/core/src/envelope.ts @@ -12,7 +12,6 @@ import type { SessionEnvelope, SessionItem, SpanEnvelope, - SpanItem, SpanJSON, } from '@sentry/types'; import { @@ -115,14 +114,11 @@ export function createSpanEnvelope(spans: SentrySpan[], client?: Client): SpanEn ...(dscHasRequiredProps(dsc) && { trace: dsc }), }; - const beforeSend = client && client.getOptions().beforeSendSpan; - let items: SpanItem[]; - - if (beforeSend) { - items = spans.map(span => createSpanEnvelopeItem(beforeSend(spanToJSON(span) as SpanJSON))); - } else { - items = spans.map(span => createSpanEnvelopeItem(spanToJSON(span))); - } + const beforeSendSpan = client && client.getOptions().beforeSendSpan; + 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); } diff --git a/packages/core/test/lib/envelope.test.ts b/packages/core/test/lib/envelope.test.ts index 2f378de8da7a..7d5a2c5f7740 100644 --- a/packages/core/test/lib/envelope.test.ts +++ b/packages/core/test/lib/envelope.test.ts @@ -210,7 +210,7 @@ describe('createSpanEnvelope', () => { expect(beforeSendSpan).toHaveBeenCalled(); const spanItem = spanEnvelope[1][0][1]; - expect(spanItem.description).toEqual({ + expect(spanItem).toEqual({ data: { 'sentry.origin': 'manual', }, From bf687e4ae1e6e63dbe155cd6898f23a8f622561b Mon Sep 17 00:00:00 2001 From: Andrei Borza Date: Thu, 16 May 2024 14:29:14 +0200 Subject: [PATCH 3/4] feat(core): Drop spans if `beforeSendSpan` returns null --- packages/core/src/baseclient.ts | 10 +++- packages/core/src/envelope.ts | 15 ++++-- packages/core/src/tracing/sentrySpan.ts | 13 +++-- packages/core/test/lib/base.test.ts | 32 +++++++++++ packages/core/test/lib/envelope.test.ts | 28 ++++++++-- .../core/test/lib/tracing/sentrySpan.test.ts | 53 +++++++++++++++++++ packages/types/src/options.ts | 6 +-- 7 files changed, 142 insertions(+), 15 deletions(-) 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..5d6895f34026 100644 --- a/packages/core/test/lib/tracing/sentrySpan.test.ts +++ b/packages/core/test/lib/tracing/sentrySpan.test.ts @@ -1,7 +1,9 @@ import { timestampInSeconds } from '@sentry/utils'; +import { setCurrentClient } from '../../../src'; 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'; 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 From 7fed679a9989338c7edcd528c38ac870b8b9216e Mon Sep 17 00:00:00 2001 From: Andrei Borza Date: Thu, 16 May 2024 15:15:33 +0200 Subject: [PATCH 4/4] feat(core): Undo `createSpanEnvelope` returning null, drop standalone span envelope based on items length --- packages/core/src/envelope.ts | 5 ++--- packages/core/src/tracing/sentrySpan.ts | 21 ++++++++++++------- packages/core/test/lib/envelope.test.ts | 28 +++++-------------------- 3 files changed, 20 insertions(+), 34 deletions(-) diff --git a/packages/core/src/envelope.ts b/packages/core/src/envelope.ts index ac336b52aed6..258216b290d4 100644 --- a/packages/core/src/envelope.ts +++ b/packages/core/src/envelope.ts @@ -99,9 +99,8 @@ 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 | null { +export function createSpanEnvelope(spans: SentrySpan[], client?: Client): SpanEnvelope { function dscHasRequiredProps(dsc: Partial): dsc is DynamicSamplingContext { return !!dsc.trace_id && !!dsc.public_key; } @@ -129,5 +128,5 @@ export function createSpanEnvelope(spans: SentrySpan[], client?: Client): SpanEn } } - return items.length > 0 ? createEnvelope(headers, items) : null; + return createEnvelope(headers, items); } diff --git a/packages/core/src/tracing/sentrySpan.ts b/packages/core/src/tracing/sentrySpan.ts index 0e35405570a0..b2dd49f32b2a 100644 --- a/packages/core/src/tracing/sentrySpan.ts +++ b/packages/core/src/tracing/sentrySpan.ts @@ -259,14 +259,7 @@ export class SentrySpan implements Span { // if this is a standalone span, we send it immediately if (this._isStandaloneSpan) { - const spanEnvelope = createSpanEnvelope([this], client); - if (spanEnvelope) { - sendSpanEnvelope(spanEnvelope); - } else { - if (client) { - client.recordDroppedEvent('before_send', 'span'); - } - } + sendSpanEnvelope(createSpanEnvelope([this], client)); return; } @@ -364,12 +357,24 @@ function isStandaloneSpan(span: Span): boolean { return span instanceof SentrySpan && span.isStandaloneSpan(); } +/** + * Sends a `SpanEnvelope`. + * + * Note: If the envelope's spans are dropped, e.g. via `beforeSendSpan`, + * the envelope will not be sent either. + */ function sendSpanEnvelope(envelope: SpanEnvelope): void { const client = getClient(); if (!client) { return; } + const spanItems = envelope[1]; + if (!spanItems || spanItems.length === 0) { + client.recordDroppedEvent('before_send', 'span'); + return; + } + const transport = client.getTransport(); if (transport) { transport.send(envelope).then(null, reason => { diff --git a/packages/core/test/lib/envelope.test.ts b/packages/core/test/lib/envelope.test.ts index a06fbad236f0..7d5a2c5f7740 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,22 +224,4 @@ 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(); - }); });