Skip to content

Commit

Permalink
feat(core): Drop spans if beforeSendSpan returns null
Browse files Browse the repository at this point in the history
  • Loading branch information
andreiborza committed May 16, 2024
1 parent 85df82e commit 8698a17
Show file tree
Hide file tree
Showing 7 changed files with 142 additions and 15 deletions.
10 changes: 9 additions & 1 deletion packages/core/src/baseclient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import type {
Span,
SpanAttributes,
SpanContextData,
SpanJSON,
StartSpanOptions,
TransactionEvent,
Transport,
Expand Down Expand Up @@ -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) {
Expand Down
15 changes: 12 additions & 3 deletions packages/core/src/envelope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import type {
SessionEnvelope,
SessionItem,
SpanEnvelope,
SpanItem,
SpanJSON,
} from '@sentry/types';
import {
Expand Down Expand Up @@ -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<DynamicSamplingContext>): dsc is DynamicSamplingContext {
return !!dsc.trace_id && !!dsc.public_key;
}
Expand All @@ -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<SpanEnvelope>(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<SpanEnvelope>(headers, items) : null;
}
13 changes: 10 additions & 3 deletions packages/core/src/tracing/sentrySpan.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down Expand Up @@ -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;
}

Expand Down
32 changes: 32 additions & 0 deletions packages/core/test/lib/base.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
28 changes: 23 additions & 5 deletions packages/core/test/lib/envelope.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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: {
Expand All @@ -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),
});
Expand All @@ -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',
Expand Down Expand Up @@ -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',
Expand All @@ -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();
});
});
53 changes: 53 additions & 0 deletions packages/core/test/lib/tracing/sentrySpan.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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', () => {
Expand Down
6 changes: 3 additions & 3 deletions packages/types/src/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -285,11 +285,11 @@ export interface ClientOptions<TO extends BaseTransportOptions = BaseTransportOp
/**
* 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.
* Returning `null` will cause this span to be dropped.
* @param span The span generated by the SDK.
* @returns A new span that will be sent | null.
*/
beforeSendSpan?: (span: SpanJSON) => SpanJSON;
beforeSendSpan?: (span: SpanJSON) => SpanJSON | null;

/**
* An event-processing callback for transaction events, guaranteed to be invoked after all other event
Expand Down

0 comments on commit 8698a17

Please sign in to comment.