From 6b7d1316325d31ba0b866b476e68e93c9649a491 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 23 Jul 2024 17:08:10 +0200 Subject: [PATCH 1/4] feat(core): Capture # of dropped spans through `beforeSendTransaction` --- packages/core/src/baseclient.ts | 36 +++++++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 5122b66d3267..de4677b154c0 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -371,10 +371,12 @@ export abstract class BaseClient implements Client { /** * @inheritDoc */ - public recordDroppedEvent(reason: EventDropReason, category: DataCategory, _event?: Event): void { - // Note: we use `event` in replay, where we overwrite this hook. - + public recordDroppedEvent(reason: EventDropReason, category: DataCategory, eventOrCount?: Event | number): void { if (this._options.sendClientReports) { + // TODO v9: We do not need the `event` passed as third argument anymore, and can possibly remove this overload + // If event is passed as third argument, we assume this is a count of 1 + const count = typeof eventOrCount === 'number' ? eventOrCount : 1; + // We want to track each category (error, transaction, session, replay_event) separately // but still keep the distinction between different type of outcomes. // We could use nested maps, but it's much easier to read and type this way. @@ -382,9 +384,8 @@ export abstract class BaseClient implements Client { // would be `Partial>>>` // With typescript 4.1 we could even use template literal types const key = `${reason}:${category}`; - DEBUG_BUILD && logger.log(`Adding outcome: "${key}"`); - - this._outcomes[key] = (this._outcomes[key] || 0) + 1; + DEBUG_BUILD && logger.log(`Recording outcome: "${key}"${count > 1 ? ` (${count} times)` : ''}`); + this._outcomes[key] = (this._outcomes[key] || 0) + count; } } @@ -794,11 +795,11 @@ export abstract class BaseClient implements Client { .then(processedEvent => { if (processedEvent === null) { this.recordDroppedEvent('before_send', dataCategory, event); - if (isTransactionEvent(event)) { + if (isTransaction) { const spans = event.spans || []; // the transaction itself counts as one span, plus all the child spans that are added const spanCount = 1 + spans.length; - this._outcomes['span'] = (this._outcomes['span'] || 0) + spanCount; + this.recordDroppedEvent('before_send', 'span', spanCount); } throw new SentryError(`${beforeSendLabel} returned \`null\`, will not send event.`, 'log'); } @@ -808,6 +809,18 @@ export abstract class BaseClient implements Client { this._updateSessionFromEvent(session, processedEvent); } + if (isTransaction) { + const spanCountBefore = + (processedEvent.sdkProcessingMetadata && processedEvent.sdkProcessingMetadata.spanCountBeforeProcessing) || + 0; + const spanCountAfter = processedEvent.spans ? processedEvent.spans.length : 0; + + const droppedSpanCount = spanCountBefore - spanCountAfter; + if (droppedSpanCount > 0) { + this.recordDroppedEvent('before_send', 'span', droppedSpanCount); + } + } + // None of the Sentry built event processor will update transaction name, // so if the transaction name has been changed by an event processor, we know // it has to come from custom event processor added by a user @@ -973,6 +986,13 @@ function processBeforeSend( } if (beforeSendTransaction) { + // We store the # of spans before processing in SDK metadata, + // so we can compare it afterwards to determine how many spans were dropped + const spanCountBefore = event.spans ? event.spans.length : 0; + event.sdkProcessingMetadata = { + ...event.sdkProcessingMetadata, + spanCountBeforeProcessing: spanCountBefore, + }; return beforeSendTransaction(event, hint); } } From 495dbd773aeff13517ff5e288c3d384a52685bf1 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 23 Jul 2024 17:19:55 +0200 Subject: [PATCH 2/4] small ref --- packages/core/src/baseclient.ts | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index de4677b154c0..64410360e51d 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -986,13 +986,15 @@ function processBeforeSend( } if (beforeSendTransaction) { - // We store the # of spans before processing in SDK metadata, - // so we can compare it afterwards to determine how many spans were dropped - const spanCountBefore = event.spans ? event.spans.length : 0; - event.sdkProcessingMetadata = { - ...event.sdkProcessingMetadata, - spanCountBeforeProcessing: spanCountBefore, - }; + if (event.spans) { + // We store the # of spans before processing in SDK metadata, + // so we can compare it afterwards to determine how many spans were dropped + const spanCountBefore = event.spans.length; + event.sdkProcessingMetadata = { + ...event.sdkProcessingMetadata, + spanCountBeforeProcessing: spanCountBefore, + }; + } return beforeSendTransaction(event, hint); } } From 08c0ff763ecfa4130c4dd9579a9373ea80ed09c6 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 24 Jul 2024 10:20:32 +0200 Subject: [PATCH 3/4] add tests --- packages/core/test/lib/base.test.ts | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/packages/core/test/lib/base.test.ts b/packages/core/test/lib/base.test.ts index 3fa10828e32a..026b2b3479bc 100644 --- a/packages/core/test/lib/base.test.ts +++ b/packages/core/test/lib/base.test.ts @@ -1042,6 +1042,30 @@ describe('BaseClient', () => { expect(TestClient.instance!.event!.transaction).toBe('/adopt/dont/shop'); }); + test('calls `beforeSendTransaction` and drops spans', () => { + const beforeSendTransaction = jest.fn(event => { + event.spans = [{ span_id: 'span5', trace_id: 'trace1', start_timestamp: 1234 }]; + return event; + }); + const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, beforeSendTransaction }); + const client = new TestClient(options); + + client.captureEvent({ + transaction: '/dogs/are/great', + type: 'transaction', + spans: [ + { span_id: 'span1', trace_id: 'trace1', start_timestamp: 1234 }, + { span_id: 'span2', trace_id: 'trace1', start_timestamp: 1234 }, + { span_id: 'span3', trace_id: 'trace1', start_timestamp: 1234 }, + ], + }); + + expect(beforeSendTransaction).toHaveBeenCalled(); + expect(TestClient.instance!.event!.spans?.length).toBe(1); + + expect(client['_outcomes']).toEqual({ 'before_send:span': 2 }); + }); + test('calls `beforeSendSpan` and uses the modified spans', () => { expect.assertions(3); @@ -1120,8 +1144,6 @@ describe('BaseClient', () => { }); 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); @@ -1149,6 +1171,7 @@ describe('BaseClient', () => { expect(beforeSendSpan).toHaveBeenCalledTimes(2); const capturedEvent = TestClient.instance!.event!; expect(capturedEvent.spans).toHaveLength(0); + expect(client['_outcomes']).toEqual({ 'before_send:span': 2 }); }); test('calls `beforeSend` and logs info about invalid return value', () => { @@ -2017,6 +2040,7 @@ describe('BaseClient', () => { describe('hook removal with `on`', () => { it('should return a cleanup function that, when executed, unregisters a hook', async () => { + jest.useFakeTimers(); expect.assertions(8); const client = new TestClient( From f6ddc1c85db7613ee484362e716c213fdf49a74e Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 24 Jul 2024 10:51:35 +0200 Subject: [PATCH 4/4] adjust size limits --- .size-limit.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.size-limit.js b/.size-limit.js index 2e7899cb934a..97a8376b65da 100644 --- a/.size-limit.js +++ b/.size-limit.js @@ -48,14 +48,14 @@ module.exports = [ path: 'packages/browser/build/npm/esm/index.js', import: createImport('init', 'browserTracingIntegration', 'replayIntegration', 'replayCanvasIntegration'), gzip: true, - limit: '76 KB', + limit: '78 KB', }, { name: '@sentry/browser (incl. Tracing, Replay, Feedback)', path: 'packages/browser/build/npm/esm/index.js', import: createImport('init', 'browserTracingIntegration', 'replayIntegration', 'feedbackIntegration'), gzip: true, - limit: '89 KB', + limit: '90 KB', }, { name: '@sentry/browser (incl. Tracing, Replay, Feedback, metrics)',