Skip to content

Commit 305f4f4

Browse files
mydeakrystofwoldrich
authored andcommitted
feat(core): Capture # of dropped spans through beforeSendTransaction (#13022)
Fixes #12849. This is a bit tricky because `beforeSendTransaction` can return a promise, so in order to avoid dealing with this I put the # of spans on the sdk metadata, and then compare that afterwards.
1 parent eeae3cf commit 305f4f4

File tree

2 files changed

+88
-7
lines changed

2 files changed

+88
-7
lines changed

packages/core/src/baseclient.ts

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -419,21 +419,21 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
419419
/**
420420
* @inheritDoc
421421
*/
422-
public recordDroppedEvent(reason: EventDropReason, category: DataCategory, _event?: Event): void {
423-
// Note: we use `event` in replay, where we overwrite this hook.
424-
422+
public recordDroppedEvent(reason: EventDropReason, category: DataCategory, eventOrCount?: Event | number): void {
425423
if (this._options.sendClientReports) {
424+
// TODO v9: We do not need the `event` passed as third argument anymore, and can possibly remove this overload
425+
// If event is passed as third argument, we assume this is a count of 1
426+
const count = typeof eventOrCount === 'number' ? eventOrCount : 1;
427+
426428
// We want to track each category (error, transaction, session, replay_event) separately
427429
// but still keep the distinction between different type of outcomes.
428430
// We could use nested maps, but it's much easier to read and type this way.
429431
// A correct type for map-based implementation if we want to go that route
430432
// would be `Partial<Record<SentryRequestType, Partial<Record<Outcome, number>>>>`
431433
// With typescript 4.1 we could even use template literal types
432434
const key = `${reason}:${category}`;
433-
DEBUG_BUILD && logger.log(`Adding outcome: "${key}"`);
434-
435-
// The following works because undefined + 1 === NaN and NaN is falsy
436-
this._outcomes[key] = this._outcomes[key] + 1 || 1;
435+
DEBUG_BUILD && logger.log(`Recording outcome: "${key}"${count > 1 ? ` (${count} times)` : ''}`);
436+
this._outcomes[key] = (this._outcomes[key] || 0) + count;
437437
}
438438
}
439439

@@ -778,6 +778,12 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
778778
.then(processedEvent => {
779779
if (processedEvent === null) {
780780
this.recordDroppedEvent('before_send', dataCategory, event);
781+
if (isTransaction) {
782+
const spans = event.spans || [];
783+
// the transaction itself counts as one span, plus all the child spans that are added
784+
const spanCount = 1 + spans.length;
785+
this.recordDroppedEvent('before_send', 'span', spanCount);
786+
}
781787
throw new SentryError(`${beforeSendLabel} returned \`null\`, will not send event.`, 'log');
782788
}
783789

@@ -786,6 +792,18 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
786792
this._updateSessionFromEvent(session, processedEvent);
787793
}
788794

795+
if (isTransaction) {
796+
const spanCountBefore =
797+
(processedEvent.sdkProcessingMetadata && processedEvent.sdkProcessingMetadata.spanCountBeforeProcessing) ||
798+
0;
799+
const spanCountAfter = processedEvent.spans ? processedEvent.spans.length : 0;
800+
801+
const droppedSpanCount = spanCountBefore - spanCountAfter;
802+
if (droppedSpanCount > 0) {
803+
this.recordDroppedEvent('before_send', 'span', droppedSpanCount);
804+
}
805+
}
806+
789807
// None of the Sentry built event processor will update transaction name,
790808
// so if the transaction name has been changed by an event processor, we know
791809
// it has to come from custom event processor added by a user

packages/core/test/lib/base.test.ts

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1004,6 +1004,69 @@ describe('BaseClient', () => {
10041004
expect(TestClient.instance!.event!.transaction).toBe('/adopt/dont/shop');
10051005
});
10061006

1007+
test('calls `beforeSendTransaction` and drops spans', () => {
1008+
const beforeSendTransaction = jest.fn(event => {
1009+
event.spans = [{ span_id: 'span5', trace_id: 'trace1', start_timestamp: 1234 }];
1010+
return event;
1011+
});
1012+
const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, beforeSendTransaction });
1013+
const client = new TestClient(options);
1014+
1015+
client.captureEvent({
1016+
transaction: '/dogs/are/great',
1017+
type: 'transaction',
1018+
spans: [
1019+
{ span_id: 'span1', trace_id: 'trace1', start_timestamp: 1234 },
1020+
{ span_id: 'span2', trace_id: 'trace1', start_timestamp: 1234 },
1021+
{ span_id: 'span3', trace_id: 'trace1', start_timestamp: 1234 },
1022+
],
1023+
});
1024+
1025+
expect(beforeSendTransaction).toHaveBeenCalled();
1026+
expect(TestClient.instance!.event!.spans?.length).toBe(1);
1027+
1028+
expect(client['_outcomes']).toEqual({ 'before_send:span': 2 });
1029+
});
1030+
1031+
test('calls `beforeSendSpan` and uses the modified spans', () => {
1032+
expect.assertions(3);
1033+
1034+
const beforeSendSpan = jest.fn(span => {
1035+
span.data = { version: 'bravo' };
1036+
return span;
1037+
});
1038+
1039+
const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, beforeSendSpan });
1040+
const client = new TestClient(options);
1041+
const transaction: Event = {
1042+
transaction: '/cats/are/great',
1043+
type: 'transaction',
1044+
spans: [
1045+
{
1046+
description: 'first span',
1047+
span_id: '9e15bf99fbe4bc80',
1048+
start_timestamp: 1591603196.637835,
1049+
trace_id: '86f39e84263a4de99c326acab3bfe3bd',
1050+
},
1051+
{
1052+
description: 'second span',
1053+
span_id: 'aa554c1f506b0783',
1054+
start_timestamp: 1591603196.637835,
1055+
trace_id: '86f39e84263a4de99c326acab3bfe3bd',
1056+
},
1057+
],
1058+
};
1059+
1060+
client.captureEvent(transaction);
1061+
1062+
expect(beforeSendSpan).toHaveBeenCalledTimes(2);
1063+
const capturedEvent = TestClient.instance!.event!;
1064+
for (const [idx, span] of capturedEvent.spans!.entries()) {
1065+
const originalSpan = transaction.spans![idx];
1066+
expect(span).toEqual({ ...originalSpan, data: { version: 'bravo' } });
1067+
}
1068+
});
1069+
10071070
test('calls `beforeSend` and discards the event', () => {
10081071
expect.assertions(4);
10091072

0 commit comments

Comments
 (0)