feat(core): Add client.getIntegrationByName() (#10130)

mydea · web-flow · commit 5498a78b1b3a · 2024-01-15T13:27:08.000+01:00
And also deprecate both `getIntegration()` as well as
`getIntegrationById()` which is only on the baseclient, but not on the
client type, anyhow :grimace:.

Usage:

```ts
const replay = getClient().getIntegrationByName('Replay');
```

Or, if you want to have an easier time with types:

```ts
const replay = getClient().getIntegrationByName&lt;Replay&gt;('Replay');
```

## Why do we need this

In v8, integrations will not be classes anymore, so you cannot pass a
class definition anymore to `getIntegration()`. We also decided to
remove the static `id` field, so you cannot find an integration via that
way anymore.

`getIntegrationById()` was always just on the baseclient, so not
properly types anyhow. It is also an inconsistent/weird naming because
we will not have an `id` anymore for integrations at all.
diff --git a/MIGRATION.md b/MIGRATION.md
@@ -19,6 +19,16 @@ but in v8 you will not be able to rely on this always existing anymore - any int
 This should not affect most people, but in the case that you are manually calling `integration.setupOnce()` right now,
 make sure to guard it's existence properly.
 
+## Deprecate `getIntegration()` and `getIntegrationById()`
+
+This deprecates `getIntegration()` on both the hub & the client, as well as `getIntegrationById()` on the baseclient.
+Instead, use `getIntegrationByName()`. You can optionally pass an integration generic to make it easier to work with
+typescript:
+
+```ts
+const replay = getClient().getIntegrationByName<Replay>('Replay');
+```
+
 ## Deprecate `Hub`
 
 The `Hub` has been a very important part of the Sentry SDK API up until now. Hubs were the SDK's "unit of concurrency"
diff --git a/dev-packages/browser-integration-tests/loader-suites/loader/onLoad/customBrowserTracing/test.ts b/dev-packages/browser-integration-tests/loader-suites/loader/onLoad/customBrowserTracing/test.ts
@@ -27,7 +27,7 @@ sentryTest('should handle custom added BrowserTracing integration', async ({ get
   expect(eventData.transaction_info?.source).toEqual('url');
 
   const tracePropagationTargets = await page.evaluate(() => {
-    const browserTracing = (window as any).Sentry.getClient().getIntegrationById('BrowserTracing');
+    const browserTracing = (window as any).Sentry.getClient().getIntegrationByName('BrowserTracing');
     return browserTracing.options.tracePropagationTargets;
   });
 
diff --git a/dev-packages/browser-integration-tests/loader-suites/loader/onLoad/customIntegrations/test.ts b/dev-packages/browser-integration-tests/loader-suites/loader/onLoad/customIntegrations/test.ts
@@ -24,14 +24,14 @@ sentryTest('should handle custom added integrations & default integrations', asy
   });
 
   const hasCustomIntegration = await page.evaluate(() => {
-    return !!(window as any).Sentry.getClient().getIntegrationById('CustomIntegration');
+    return !!(window as any).Sentry.getClient().getIntegrationByName('CustomIntegration');
   });
 
   const hasReplay = await page.evaluate(() => {
-    return !!(window as any).Sentry.getClient().getIntegrationById('Replay');
+    return !!(window as any).Sentry.getClient().getIntegrationByName('Replay');
   });
   const hasBrowserTracing = await page.evaluate(() => {
-    return !!(window as any).Sentry.getClient().getIntegrationById('BrowserTracing');
+    return !!(window as any).Sentry.getClient().getIntegrationByName('BrowserTracing');
   });
 
   expect(hasCustomIntegration).toEqual(true);
diff --git a/dev-packages/browser-integration-tests/loader-suites/loader/onLoad/customIntegrationsFunction/test.ts b/dev-packages/browser-integration-tests/loader-suites/loader/onLoad/customIntegrationsFunction/test.ts
@@ -21,14 +21,14 @@ sentryTest(
     });
 
     const hasCustomIntegration = await page.evaluate(() => {
-      return !!(window as any).Sentry.getClient().getIntegrationById('CustomIntegration');
+      return !!(window as any).Sentry.getClient().getIntegrationByName('CustomIntegration');
     });
 
     const hasReplay = await page.evaluate(() => {
-      return !!(window as any).Sentry.getClient().getIntegrationById('Replay');
+      return !!(window as any).Sentry.getClient().getIntegrationByName('Replay');
     });
     const hasBrowserTracing = await page.evaluate(() => {
-      return !!(window as any).Sentry.getClient().getIntegrationById('BrowserTracing');
+      return !!(window as any).Sentry.getClient().getIntegrationByName('BrowserTracing');
     });
 
     expect(hasCustomIntegration).toEqual(true);
diff --git a/dev-packages/browser-integration-tests/loader-suites/loader/onLoad/customReplay/test.ts b/dev-packages/browser-integration-tests/loader-suites/loader/onLoad/customReplay/test.ts
@@ -27,7 +27,7 @@ sentryTest('should handle custom added Replay integration', async ({ getLocalTes
   expect(eventData.segment_id).toBe(0);
 
   const useCompression = await page.evaluate(() => {
-    const replay = (window as any).Sentry.getClient().getIntegrationById('Replay');
+    const replay = (window as any).Sentry.getClient().getIntegrationByName('Replay');
     return replay._replay.getOptions().useCompression;
   });
 
diff --git a/packages/astro/test/client/sdk.test.ts b/packages/astro/test/client/sdk.test.ts
@@ -61,7 +61,7 @@ describe('Sentry client SDK', () => {
         });
 
         const integrationsToInit = browserInit.mock.calls[0][0]?.integrations;
-        const browserTracing = getClient<BrowserClient>()?.getIntegrationById('BrowserTracing');
+        const browserTracing = getClient<BrowserClient>()?.getIntegrationByName('BrowserTracing');
 
         expect(integrationsToInit).toContainEqual(expect.objectContaining({ name: 'BrowserTracing' }));
         expect(browserTracing).toBeDefined();
@@ -77,7 +77,7 @@ describe('Sentry client SDK', () => {
         });
 
         const integrationsToInit = browserInit.mock.calls[0][0]?.integrations;
-        const browserTracing = getClient<BrowserClient>()?.getIntegrationById('BrowserTracing');
+        const browserTracing = getClient<BrowserClient>()?.getIntegrationByName('BrowserTracing');
 
         expect(integrationsToInit).not.toContainEqual(expect.objectContaining({ name: 'BrowserTracing' }));
         expect(browserTracing).toBeUndefined();
@@ -92,7 +92,7 @@ describe('Sentry client SDK', () => {
         });
 
         const integrationsToInit = browserInit.mock.calls[0][0]?.integrations;
-        const browserTracing = getClient<BrowserClient>()?.getIntegrationById('BrowserTracing');
+        const browserTracing = getClient<BrowserClient>()?.getIntegrationByName('BrowserTracing');
 
         expect(integrationsToInit).not.toContainEqual(expect.objectContaining({ name: 'BrowserTracing' }));
         expect(browserTracing).toBeUndefined();
@@ -109,7 +109,7 @@ describe('Sentry client SDK', () => {
 
         const integrationsToInit = browserInit.mock.calls[0][0]?.integrations;
 
-        const browserTracing = getClient<BrowserClient>()?.getIntegrationById('BrowserTracing') as BrowserTracing;
+        const browserTracing = getClient<BrowserClient>()?.getIntegrationByName('BrowserTracing') as BrowserTracing;
         const options = browserTracing.options;
 
         expect(integrationsToInit).toContainEqual(expect.objectContaining({ name: 'BrowserTracing' }));
diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts
@@ -334,13 +334,24 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
    * Gets an installed integration by its `id`.
    *
    * @returns The installed integration or `undefined` if no integration with that `id` was installed.
+   * @deprecated Use `getIntegrationByName()` instead.
    */
   public getIntegrationById(integrationId: string): Integration | undefined {
-    return this._integrations[integrationId];
+    return this.getIntegrationByName(integrationId);
   }
 
   /**
-   * @inheritDoc
+   * Gets an installed integration by its name.
+   *
+   * @returns The installed integration or `undefined` if no integration with that `name` was installed.
+   */
+  public getIntegrationByName<T extends Integration = Integration>(integrationName: string): T | undefined {
+    return this._integrations[integrationName] as T | undefined;
+  }
+
+  /**
+   * Returns the client's instance of the given integration class, it any.
+   * @deprecated Use `getIntegrationByName()` instead.
    */
   public getIntegration<T extends Integration>(integration: IntegrationClass<T>): T | null {
     try {
diff --git a/packages/core/src/hub.ts b/packages/core/src/hub.ts
@@ -472,13 +472,14 @@ export class Hub implements HubInterface {
 
   /**
    * @inheritDoc
-   * @deprecated Use `Sentry.getClient().getIntegration()` instead.
+   * @deprecated Use `Sentry.getClient().getIntegrationByName()` instead.
    */
   public getIntegration<T extends Integration>(integration: IntegrationClass<T>): T | null {
     // eslint-disable-next-line deprecation/deprecation
     const client = this.getClient();
     if (!client) return null;
     try {
+      // eslint-disable-next-line deprecation/deprecation
       return client.getIntegration(integration);
     } catch (_oO) {
       DEBUG_BUILD && logger.warn(`Cannot retrieve integration ${integration.id} from the current Hub`);
diff --git a/packages/core/test/lib/base.test.ts b/packages/core/test/lib/base.test.ts
@@ -1496,7 +1496,7 @@ describe('BaseClient', () => {
       client.setupIntegrations();
 
       expect(Object.keys((client as any)._integrations).length).toEqual(1);
-      expect(client.getIntegration(TestIntegration)).toBeTruthy();
+      expect(client.getIntegrationByName(TestIntegration.id)).toBeTruthy();
     });
 
     test('sets up each integration on `init` call', () => {
@@ -1507,7 +1507,7 @@ describe('BaseClient', () => {
       client.init();
 
       expect(Object.keys((client as any)._integrations).length).toEqual(1);
-      expect(client.getIntegration(TestIntegration)).toBeTruthy();
+      expect(client.getIntegrationByName(TestIntegration.id)).toBeTruthy();
     });
 
     test('skips installation for `setupIntegrations()` if DSN is not provided', () => {
@@ -1519,7 +1519,7 @@ describe('BaseClient', () => {
       client.setupIntegrations();
 
       expect(Object.keys((client as any)._integrations).length).toEqual(0);
-      expect(client.getIntegration(TestIntegration)).toBeFalsy();
+      expect(client.getIntegrationByName(TestIntegration.id)).toBeFalsy();
     });
 
     test('skips installation for `init()` if DSN is not provided', () => {
@@ -1530,7 +1530,7 @@ describe('BaseClient', () => {
       client.init();
 
       expect(Object.keys((client as any)._integrations).length).toEqual(0);
-      expect(client.getIntegration(TestIntegration)).toBeFalsy();
+      expect(client.getIntegrationByName(TestIntegration.id)).toBeFalsy();
     });
 
     test('skips installation for `setupIntegrations()` if `enabled` is set to `false`', () => {
@@ -1546,7 +1546,7 @@ describe('BaseClient', () => {
       client.setupIntegrations();
 
       expect(Object.keys((client as any)._integrations).length).toEqual(0);
-      expect(client.getIntegration(TestIntegration)).toBeFalsy();
+      expect(client.getIntegrationByName(TestIntegration.id)).toBeFalsy();
     });
 
     test('skips installation for `init()` if `enabled` is set to `false`', () => {
@@ -1561,7 +1561,7 @@ describe('BaseClient', () => {
       client.init();
 
       expect(Object.keys((client as any)._integrations).length).toEqual(0);
-      expect(client.getIntegration(TestIntegration)).toBeFalsy();
+      expect(client.getIntegrationByName(TestIntegration.id)).toBeFalsy();
     });
 
     test('skips installation if integrations are already installed', () => {
@@ -1577,7 +1577,7 @@ describe('BaseClient', () => {
       client.setupIntegrations();
 
       expect(Object.keys((client as any)._integrations).length).toEqual(1);
-      expect(client.getIntegration(TestIntegration)).toBeTruthy();
+      expect(client.getIntegrationByName(TestIntegration.id)).toBeTruthy();
       expect(setupIntegrationsHelper).toHaveBeenCalledTimes(1);
 
       // ...but it shouldn't try to install a second time
@@ -1597,7 +1597,7 @@ describe('BaseClient', () => {
       client.init();
 
       expect(Object.keys((client as any)._integrations).length).toEqual(1);
-      expect(client.getIntegration(TestIntegration)).toBeTruthy();
+      expect(client.getIntegrationByName(TestIntegration.id)).toBeTruthy();
       expect(setupIntegrationsHelper).toHaveBeenCalledTimes(1);
 
       client.init();
diff --git a/packages/core/test/mocks/integration.ts b/packages/core/test/mocks/integration.ts
@@ -9,7 +9,7 @@ export class TestIntegration implements Integration {
 
   public setupOnce(): void {
     const eventProcessor: EventProcessor = (event: Event) => {
-      if (!getClient()?.getIntegration(TestIntegration)) {
+      if (!getClient()?.getIntegrationByName?.('TestIntegration')) {
         return event;
       }
 
diff --git a/packages/ember/addon/instance-initializers/sentry-performance.ts b/packages/ember/addon/instance-initializers/sentry-performance.ts
@@ -441,8 +441,8 @@ export async function instrumentForPerformance(appInstance: ApplicationInstance)
 
     if (
       client &&
-      (client as BrowserClient).getIntegrationById &&
-      (client as BrowserClient).getIntegrationById('BrowserTracing')
+      (client as BrowserClient).getIntegrationByName &&
+      (client as BrowserClient).getIntegrationByName('BrowserTracing')
     ) {
       // Initializers are called more than once in tests, causing the integrations to not be setup correctly.
       return;
diff --git a/packages/ember/tests/acceptance/sentry-replay-test.ts b/packages/ember/tests/acceptance/sentry-replay-test.ts
@@ -1,6 +1,6 @@
 import { visit } from '@ember/test-helpers';
 import * as Sentry from '@sentry/ember';
-import type { ReplayContainer } from '@sentry/replay/build/npm/types/types';
+import type { BrowserClient, Replay } from '@sentry/ember';
 import { setupApplicationTest } from 'ember-qunit';
 import { module, test } from 'qunit';
 
@@ -13,10 +13,10 @@ module('Acceptance | Sentry Session Replay', function (hooks) {
   test('Test replay', async function (assert) {
     await visit('/replay');
 
-    const integration = Sentry.getClient()?.getIntegration(Sentry.Replay);
+    const integration = Sentry.getClient<BrowserClient>()?.getIntegrationByName('Replay');
     assert.ok(integration);
 
-    const replay = (integration as Sentry.Replay)['_replay'] as ReplayContainer;
+    const replay = (integration as Sentry.Replay)['_replay'] as Replay['_replay'];
 
     assert.true(replay.isEnabled());
     assert.false(replay.isPaused());
diff --git a/packages/ember/tests/dummy/app/routes/replay.ts b/packages/ember/tests/dummy/app/routes/replay.ts
@@ -5,9 +5,8 @@ import * as Sentry from '@sentry/ember';
 export default class ReplayRoute extends Route {
   public async beforeModel(): Promise<void> {
     const { Replay } = Sentry;
-
-    if (!Sentry.getClient()!.getIntegration(Replay)) {
-      const client = Sentry.getClient() as BrowserClient;
+    const client = Sentry.getClient<BrowserClient>();
+    if (client && !client.getIntegrationByName('Replay')) {
       client.addIntegration(new Replay());
     }
   }
diff --git a/packages/feedback/README.md b/packages/feedback/README.md
@@ -212,7 +212,7 @@ import {Feedback} from '@sentry-internal/feedback';
 
 function MyFeedbackButton() {
     const client = getCurrentHub().getClient<BrowserClient>();
-    const feedback = client?.getIntegration(Feedback);
+    const feedback = client?.getIntegrationByName('Feedback');
 
     // Don't render custom feedback button if Feedback integration not installed
     if (!feedback) {
@@ -258,7 +258,7 @@ import {BrowserClient, getCurrentHub} from '@sentry/react';
 import {Feedback} from '@sentry-internal/feedback';
 
 document.getElementById('my-feedback-form').addEventListener('submit', (event) => {
-  const feedback = getCurrentHub().getClient<BrowserClient>()?.getIntegration(Feedback);
+  const feedback = getCurrentHub().getClient<BrowserClient>()?.getIntegrationByName('Feedback');
   const formData = new FormData(event.currentTarget);
   feedback.sendFeedback(formData);
   event.preventDefault();
diff --git a/packages/integrations/src/offline.ts b/packages/integrations/src/offline.ts
@@ -81,6 +81,7 @@ export class Offline implements Integration {
     }
 
     const eventProcessor: EventProcessor = event => {
+      // eslint-disable-next-line deprecation/deprecation
       if (this.hub && this.hub.getIntegration(Offline)) {
         // cache if we are positively offline
         if ('navigator' in WINDOW && 'onLine' in WINDOW.navigator && !WINDOW.navigator.onLine) {
diff --git a/packages/node-experimental/src/sdk/hub.ts b/packages/node-experimental/src/sdk/hub.ts
@@ -94,6 +94,7 @@ export function getCurrentHub(): Hub {
     },
 
     getIntegration<T extends Integration>(integration: IntegrationClass<T>): T | null {
+      // eslint-disable-next-line deprecation/deprecation
       return getClient().getIntegration(integration);
     },
 
diff --git a/packages/node-experimental/src/sdk/spanProcessor.ts b/packages/node-experimental/src/sdk/spanProcessor.ts
@@ -4,8 +4,8 @@ import type { Span } from '@opentelemetry/sdk-trace-base';
 import { SemanticAttributes } from '@opentelemetry/semantic-conventions';
 import { SentrySpanProcessor, getClient, getSpanFinishScope } from '@sentry/opentelemetry';
 
-import { Http } from '../integrations/http';
-import { NodeFetch } from '../integrations/node-fetch';
+import type { Http } from '../integrations/http';
+import type { NodeFetch } from '../integrations/node-fetch';
 import type { NodeExperimentalClient } from '../types';
 import { getIsolationScope } from './api';
 import { Scope } from './scope';
@@ -33,8 +33,8 @@ export class NodeExperimentalSentrySpanProcessor extends SentrySpanProcessor {
   /** @inheritDoc */
   protected _shouldSendSpanToSentry(span: Span): boolean {
     const client = getClient<NodeExperimentalClient>();
-    const httpIntegration = client ? client.getIntegration(Http) : undefined;
-    const fetchIntegration = client ? client.getIntegration(NodeFetch) : undefined;
+    const httpIntegration = client ? client.getIntegrationByName<Http>('Http') : undefined;
+    const fetchIntegration = client ? client.getIntegrationByName<NodeFetch>('NodeFetch') : undefined;
 
     // If we encounter a client or server span with url & method, we assume this comes from the http instrumentation
     // In this case, if `shouldCreateSpansForRequests` is false, we want to _record_ the span but not _sample_ it,
diff --git a/packages/node-experimental/test/integration/transactions.test.ts b/packages/node-experimental/test/integration/transactions.test.ts
@@ -512,8 +512,8 @@ describe('Integration | Transactions', () => {
 
     const client = getClient() as NodeExperimentalClient;
 
-    jest.spyOn(client, 'getIntegration').mockImplementation(integrationClass => {
-      if (integrationClass.name === 'Http') {
+    jest.spyOn(client, 'getIntegrationByName').mockImplementation(name => {
+      if (name === 'Http') {
         return {
           shouldCreateSpansForRequests: false,
         } as Http;
@@ -576,8 +576,8 @@ describe('Integration | Transactions', () => {
 
     const client = getClient() as NodeExperimentalClient;
 
-    jest.spyOn(client, 'getIntegration').mockImplementation(integrationClass => {
-      if (integrationClass.name === 'NodeFetch') {
+    jest.spyOn(client, 'getIntegrationByName').mockImplementation(name => {
+      if (name === 'NodeFetch') {
         return {
           shouldCreateSpansForRequests: false,
         } as NodeFetch;
diff --git a/packages/node/src/integrations/undici/index.ts b/packages/node/src/integrations/undici/index.ts
@@ -137,6 +137,7 @@ export class Undici implements Integration {
   }
 
   private _onRequestCreate = (message: unknown): void => {
+    // eslint-disable-next-line deprecation/deprecation
     if (!getClient()?.getIntegration(Undici)) {
       return;
     }
@@ -195,6 +196,7 @@ export class Undici implements Integration {
   };
 
   private _onRequestEnd = (message: unknown): void => {
+    // eslint-disable-next-line deprecation/deprecation
     if (!getClient()?.getIntegration(Undici)) {
       return;
     }
@@ -234,6 +236,7 @@ export class Undici implements Integration {
   };
 
   private _onRequestError = (message: unknown): void => {
+    // eslint-disable-next-line deprecation/deprecation
     if (!getClient()?.getIntegration(Undici)) {
       return;
     }
diff --git a/packages/node/test/integrations/undici.test.ts b/packages/node/test/integrations/undici.test.ts
@@ -417,7 +417,7 @@ function setupTestServer() {
 
 function patchUndici(userOptions: Partial<UndiciOptions>): () => void {
   try {
-    const undici = getClient()!.getIntegration(Undici);
+    const undici = getClient()!.getIntegrationByName!('Undici');
     // @ts-expect-error need to access private property
     options = { ...undici._options };
     // @ts-expect-error need to access private property
@@ -428,7 +428,7 @@ function patchUndici(userOptions: Partial<UndiciOptions>): () => void {
 
   return () => {
     try {
-      const undici = getClient()!.getIntegration(Undici);
+      const undici = getClient()!.getIntegrationByName!('Undici');
       // @ts-expect-error Need to override readonly property
       undici!['_options'] = { ...options };
     } catch (_) {
diff --git a/packages/sveltekit/test/client/sdk.test.ts b/packages/sveltekit/test/client/sdk.test.ts
diff --git a/packages/types/src/client.ts b/packages/types/src/client.ts
diff --git a/packages/vercel-edge/src/integrations/wintercg-fetch.ts b/packages/vercel-edge/src/integrations/wintercg-fetch.ts
diff --git a/packages/vercel-edge/test/wintercg-fetch.test.ts b/packages/vercel-edge/test/wintercg-fetch.test.ts
diff --git a/packages/vue/test/integration/init.test.ts b/packages/vue/test/integration/init.test.ts

Original file line number	Diff line number	Diff line change
`@@ -9,7 +9,7 @@ export class TestIntegration implements Integration {`
`9`	`9`
`10`	`10`	`public setupOnce(): void {`
`11`	`11`	`const eventProcessor: EventProcessor = (event: Event) => {`
`12`		`- if (!getClient()?.getIntegration(TestIntegration)) {`
	`12`	`+ if (!getClient()?.getIntegrationByName?.('TestIntegration')) {`
`13`	`13`	`return event;`
`14`	`14`	`}`
`15`	`15`