feat(core)!: Stop accepting event as argument for recordDroppedEvent

The event was no longer used for some time, instead you can (optionally) pass a count of dropped events as third argument. This has been around since v7, so we can finally clean this up here.
getsentry · Jan 14, 2025 · 1e4a6dc · 1e4a6dc
1 parent f24ee28
commit 1e4a6dc
Show file tree

Hide file tree

Showing 6 changed files with 21 additions and 62 deletions.
diff --git a/docs/migration/v8-to-v9.md b/docs/migration/v8-to-v9.md
@@ -193,6 +193,7 @@ Sentry.init({
 The following changes are unlikely to affect users of the SDK. They are listed here only for completion sake, and to alert users that may be relying on internal behavior.
 
 - `client._prepareEvent()` now requires a currentScope & isolationScope to be passed as last arugments
+- `client.recordDroppedEvent()` no longer accepts an `event` as third argument. The event was no longer used for some time, instead you can (optionally) pass a count of dropped events as third argument.
 
 ### `@sentry/nestjs`
 

diff --git a/packages/core/src/client.ts b/packages/core/src/client.ts
@@ -430,12 +430,8 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
   /**
    * Record on the client that an event got dropped (ie, an event that will not be sent to Sentry).
    */
-  public recordDroppedEvent(reason: EventDropReason, category: DataCategory, eventOrCount?: Event | number): void {
+  public recordDroppedEvent(reason: EventDropReason, category: DataCategory, count: number = 1): 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.
@@ -919,7 +915,7 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
     // Sampling for transaction happens somewhere else
     const parsedSampleRate = typeof sampleRate === 'undefined' ? undefined : parseSampleRate(sampleRate);
     if (isError && typeof parsedSampleRate === 'number' && Math.random() > parsedSampleRate) {
-      this.recordDroppedEvent('sample_rate', 'error', event);
+      this.recordDroppedEvent('sample_rate', 'error');
       return rejectedSyncPromise(
         new SentryError(
           `Discarding event because it's not included in the random sample (sampling rate = ${sampleRate})`,
@@ -933,7 +929,7 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
     return this._prepareEvent(event, hint, currentScope, isolationScope)
       .then(prepared => {
         if (prepared === null) {
-          this.recordDroppedEvent('event_processor', dataCategory, event);
+          this.recordDroppedEvent('event_processor', dataCategory);
           throw new SentryError('An event processor returned `null`, will not send event.', 'log');
         }
 
@@ -947,7 +943,7 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
       })
       .then(processedEvent => {
         if (processedEvent === null) {
-          this.recordDroppedEvent('before_send', dataCategory, event);
+          this.recordDroppedEvent('before_send', dataCategory);
           if (isTransaction) {
             const spans = event.spans || [];
             // the transaction itself counts as one span, plus all the child spans that are added

diff --git a/packages/core/src/transports/base.ts b/packages/core/src/transports/base.ts
@@ -1,17 +1,13 @@
+import { DEBUG_BUILD } from '../debug-build';
 import type {
   Envelope,
   EnvelopeItem,
-  EnvelopeItemType,
-  Event,
   EventDropReason,
-  EventItem,
   InternalBaseTransportOptions,
   Transport,
   TransportMakeRequestResponse,
   TransportRequestExecutor,
 } from '../types-hoist';
-
-import { DEBUG_BUILD } from '../debug-build';
 import {
   createEnvelope,
   envelopeItemTypeToDataCategory,
@@ -49,8 +45,7 @@ export function createTransport(
     forEachEnvelopeItem(envelope, (item, type) => {
       const dataCategory = envelopeItemTypeToDataCategory(type);
       if (isRateLimited(rateLimits, dataCategory)) {
-        const event: Event | undefined = getEventForEnvelopeItem(item, type);
-        options.recordDroppedEvent('ratelimit_backoff', dataCategory, event);
+        options.recordDroppedEvent('ratelimit_backoff', dataCategory);
       } else {
         filteredEnvelopeItems.push(item);
       }
@@ -66,8 +61,7 @@ export function createTransport(
     // Creates client report for each item in an envelope
     const recordEnvelopeLoss = (reason: EventDropReason): void => {
       forEachEnvelopeItem(filteredEnvelope, (item, type) => {
-        const event: Event | undefined = getEventForEnvelopeItem(item, type);
-        options.recordDroppedEvent(reason, envelopeItemTypeToDataCategory(type), event);
+        options.recordDroppedEvent(reason, envelopeItemTypeToDataCategory(type));
       });
     };
 
@@ -107,11 +101,3 @@ export function createTransport(
     flush,
   };
 }
-
-function getEventForEnvelopeItem(item: Envelope[1][number], type: EnvelopeItemType): Event | undefined {
-  if (type !== 'event' && type !== 'transaction') {
-    return undefined;
-  }
-
-  return Array.isArray(item) ? (item as EventItem)[1] : undefined;
-}
diff --git a/packages/core/test/lib/client.test.ts b/packages/core/test/lib/client.test.ts
@@ -1517,9 +1517,7 @@ describe('Client', () => {
       client.captureEvent({ message: 'hello' }, {});
 
       expect(beforeSend).toHaveBeenCalled();
-      expect(recordLostEventSpy).toHaveBeenCalledWith('before_send', 'error', {
-        message: 'hello',
-      });
+      expect(recordLostEventSpy).toHaveBeenCalledWith('before_send', 'error');
     });
 
     test('`beforeSendTransaction` records dropped events', () => {
@@ -1539,10 +1537,7 @@ describe('Client', () => {
       client.captureEvent({ transaction: '/dogs/are/great', type: 'transaction' });
 
       expect(beforeSendTransaction).toHaveBeenCalled();
-      expect(recordLostEventSpy).toHaveBeenCalledWith('before_send', 'transaction', {
-        transaction: '/dogs/are/great',
-        type: 'transaction',
-      });
+      expect(recordLostEventSpy).toHaveBeenCalledWith('before_send', 'transaction');
     });
 
     test('event processor drops error event when it returns `null`', () => {
@@ -1594,9 +1589,7 @@ describe('Client', () => {
 
       client.captureEvent({ message: 'hello' }, {}, scope);
 
-      expect(recordLostEventSpy).toHaveBeenCalledWith('event_processor', 'error', {
-        message: 'hello',
-      });
+      expect(recordLostEventSpy).toHaveBeenCalledWith('event_processor', 'error');
     });
 
     test('event processor records dropped transaction events', () => {
@@ -1612,10 +1605,7 @@ describe('Client', () => {
 
       client.captureEvent({ transaction: '/dogs/are/great', type: 'transaction' }, {}, scope);
 
-      expect(recordLostEventSpy).toHaveBeenCalledWith('event_processor', 'transaction', {
-        transaction: '/dogs/are/great',
-        type: 'transaction',
-      });
+      expect(recordLostEventSpy).toHaveBeenCalledWith('event_processor', 'transaction');
     });
 
     test('mutating transaction name with event processors sets transaction-name-change metadata', () => {
@@ -1704,9 +1694,7 @@ describe('Client', () => {
       const recordLostEventSpy = jest.spyOn(client, 'recordDroppedEvent');
 
       client.captureEvent({ message: 'hello' }, {});
-      expect(recordLostEventSpy).toHaveBeenCalledWith('sample_rate', 'error', {
-        message: 'hello',
-      });
+      expect(recordLostEventSpy).toHaveBeenCalledWith('sample_rate', 'error');
     });
 
     test('captures logger message', () => {

diff --git a/packages/core/test/lib/transports/base.test.ts b/packages/core/test/lib/transports/base.test.ts
@@ -135,9 +135,7 @@ describe('createTransport', () => {
 
         await transport.send(ERROR_ENVELOPE);
         expect(requestExecutor).not.toHaveBeenCalled();
-        expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'error', {
-          event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2',
-        });
+        expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'error');
         requestExecutor.mockClear();
         recordDroppedEventCallback.mockClear();
 
@@ -179,9 +177,7 @@ describe('createTransport', () => {
 
         await transport.send(ERROR_ENVELOPE); // Error envelope should not be sent because of pending rate limit
         expect(requestExecutor).not.toHaveBeenCalled();
-        expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'error', {
-          event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2',
-        });
+        expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'error');
         requestExecutor.mockClear();
         recordDroppedEventCallback.mockClear();
 
@@ -223,23 +219,19 @@ describe('createTransport', () => {
 
         await transport.send(TRANSACTION_ENVELOPE); // Transaction envelope should not be sent because of pending rate limit
         expect(requestExecutor).not.toHaveBeenCalled();
-        expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'transaction', {
-          event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2',
-        });
+        expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'transaction');
         requestExecutor.mockClear();
         recordDroppedEventCallback.mockClear();
 
         await transport.send(ERROR_ENVELOPE); // Error envelope should not be sent because of pending rate limit
         expect(requestExecutor).not.toHaveBeenCalled();
-        expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'error', {
-          event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2',
-        });
+        expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'error');
         requestExecutor.mockClear();
         recordDroppedEventCallback.mockClear();
 
         await transport.send(ATTACHMENT_ENVELOPE); // Attachment envelope should not be sent because of pending rate limit
         expect(requestExecutor).not.toHaveBeenCalled();
-        expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'attachment', undefined);
+        expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'attachment');
         requestExecutor.mockClear();
         recordDroppedEventCallback.mockClear();
 
@@ -287,17 +279,13 @@ describe('createTransport', () => {
 
         await transport.send(TRANSACTION_ENVELOPE); // Transaction envelope should not be sent because of pending rate limit
         expect(requestExecutor).not.toHaveBeenCalled();
-        expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'transaction', {
-          event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2',
-        });
+        expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'transaction');
         requestExecutor.mockClear();
         recordDroppedEventCallback.mockClear();
 
         await transport.send(ERROR_ENVELOPE); // Error envelope should not be sent because of pending rate limit
         expect(requestExecutor).not.toHaveBeenCalled();
-        expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'error', {
-          event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2',
-        });
+        expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'error');
         requestExecutor.mockClear();
         recordDroppedEventCallback.mockClear();
 

diff --git a/packages/replay-internal/src/util/sendReplayRequest.ts b/packages/replay-internal/src/util/sendReplayRequest.ts
@@ -53,7 +53,7 @@ export async function sendReplayRequest({
 
   if (!replayEvent) {
     // Taken from baseclient's `_processEvent` method, where this is handled for errors/transactions
-    client.recordDroppedEvent('event_processor', 'replay', baseEvent);
+    client.recordDroppedEvent('event_processor', 'replay');
     DEBUG_BUILD && logger.info('An event processor returned `null`, will not send event.');
     return resolvedSyncPromise({});
   }