ref(core): Move sentry breadcrumb logic into integration (#6195)

`getIntegrationById` is used to ensure that the `Breadcrumbs` integration is not included in the bundle when it's not used. 

Moving this logic from the `BaseClient` into the integration should save further bundle size when the `Breadcrumbs` integration is not used, especially since `getEventDescription` will no longer be included too.

Author: timfish

packages/browser/src/client.ts

@@ -1,6 +1,6 @@
-import { BaseClient, getCurrentHub, getEnvelopeEndpointWithUrlEncodedAuth, Scope, SDK_VERSION } from '@sentry/core';
+import { BaseClient, getEnvelopeEndpointWithUrlEncodedAuth, Scope, SDK_VERSION } from '@sentry/core';
 import { ClientOptions, Event, EventHint, Options, Severity, SeverityLevel } from '@sentry/types';
-import { createClientReportEnvelope, dsnToString, getEventDescription, logger, serializeEnvelope } from '@sentry/utils';
+import { createClientReportEnvelope, dsnToString, logger, serializeEnvelope } from '@sentry/utils';
 
 import { eventFromException, eventFromMessage } from './eventbuilder';
 import { WINDOW } from './helpers';
@@ -101,27 +101,10 @@ export class BrowserClient extends BaseClient<BrowserClientOptions> {
     // bundles, if it is not used by the SDK.
     // This all sadly is a bit ugly, but we currently don't have a "pre-send" hook on the integrations so we do it this
     // way for now.
-    const breadcrumbIntegration = this.getIntegrationById(BREADCRUMB_INTEGRATION_ID) as Breadcrumbs | null;
-    if (
-      breadcrumbIntegration &&
-      // We check for definedness of `options`, even though it is not strictly necessary, because that access to
-      // `.sentry` below does not throw, in case users provided their own integration with id "Breadcrumbs" that does
-      // not have an`options` field
-      breadcrumbIntegration.options &&
-      breadcrumbIntegration.options.sentry
-    ) {
-      getCurrentHub().addBreadcrumb(
-        {
-          category: `sentry.${event.type === 'transaction' ? 'transaction' : 'event'}`,
-          event_id: event.event_id,
-          level: event.level,
-          message: getEventDescription(event),
-        },
-        {
-          event,
-        },
-      );
-    }
+    const breadcrumbIntegration = this.getIntegrationById(BREADCRUMB_INTEGRATION_ID) as Breadcrumbs | undefined;
+    // We check for definedness of `addSentryBreadcrumb` in case users provided their own integration with id
+    // "Breadcrumbs" that does not have this function.
+    breadcrumbIntegration?.addSentryBreadcrumb?.(event);
 
     super.sendEvent(event, hint);
   }

packages/browser/src/integrations/breadcrumbs.ts

@@ -1,9 +1,10 @@
 /* eslint-disable @typescript-eslint/no-unsafe-member-access */
 /* eslint-disable max-lines */
 import { getCurrentHub } from '@sentry/core';
-import { Integration } from '@sentry/types';
+import { Event, Integration } from '@sentry/types';
 import {
   addInstrumentationHandler,
+  getEventDescription,
   htmlTreeAsString,
   parseUrl,
   safeJoin,
@@ -85,6 +86,25 @@ export class Breadcrumbs implements Integration {
       addInstrumentationHandler('history', _historyBreadcrumb);
     }
   }
+
+  /**
+   * Adds a breadcrumb for Sentry events or transactions if this option is enabled.
+   */
+  public addSentryBreadcrumb(event: Event): void {
+    if (this.options.sentry) {
+      getCurrentHub().addBreadcrumb(
+        {
+          category: `sentry.${event.type === 'transaction' ? 'transaction' : 'event'}`,
+          event_id: event.event_id,
+          level: event.level,
+          message: getEventDescription(event),
+        },
+        {
+          event,
+        },
+      );
+    }
+  }
 }
 
 /**

packages/browser/test/unit/integrations/breadcrumbs.test.ts

@@ -0,0 +1,35 @@
+import { getCurrentHub } from '@sentry/core';
+
+import { Breadcrumbs, BrowserClient, flush, Hub } from '../../../src';
+import { getDefaultBrowserClientOptions } from '../helper/browser-client-options';
+
+const hub = new Hub();
+
+jest.mock('@sentry/core', () => {
+  const original = jest.requireActual('@sentry/core');
+  return {
+    ...original,
+    getCurrentHub: () => hub,
+  };
+});
+
+describe('Breadcrumbs', () => {
+  it('Should add sentry breadcrumb', async () => {
+    const addBreadcrumb = jest.fn();
+    hub.addBreadcrumb = addBreadcrumb;
+
+    const client = new BrowserClient({
+      ...getDefaultBrowserClientOptions(),
+      dsn: 'https://username@domain/123',
+      integrations: [new Breadcrumbs()],
+    });
+
+    getCurrentHub().bindClient(client);
+
+    client.captureMessage('test');
+    await flush(2000);
+
+    expect(addBreadcrumb.mock.calls[0][0].category).toEqual('sentry.event');
+    expect(addBreadcrumb.mock.calls[0][0].message).toEqual('test');
+  });
+});