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

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, which should also save
some bytes.

This was not really deprecated before, but is also more an intimate API
so I do not expect this to affect users too much. Also, deprecating
arguments like this does not really work well with eslint anyhow, so
even if we would deprecate it it is unlikely users would find out
anyhow.

Author: mydea

docs/migration/v8-to-v9.md

@@ -199,6 +199,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`
 

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

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;
-}

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', () => {

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();
 

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({});
   }