ref(v8): change integration.setupOnce signature (getsentry#11238)

Make `integration.setupOnce` accept no arguments. This will allow us to easily remove `addGlobalEventProcessor` which is deprecated API. This also means we can remove `IntegrationFnResult`, as the type signature of the functional and class based integrations are now the same. Next up - remove `addGlobalEventProcessor`!
cadesalaberry · Apr 19, 2024 · 95473a9 · 95473a9
1 parent a4b66e1
commit 95473a9
Show file tree

Hide file tree

Showing 15 changed files with 53 additions and 141 deletions.
diff --git a/packages/core/src/integration.ts b/packages/core/src/integration.ts
@@ -1,19 +1,8 @@
-import type {
-  Client,
-  Event,
-  EventHint,
-  Integration,
-  IntegrationClass,
-  IntegrationFn,
-  IntegrationFnResult,
-  Options,
-} from '@sentry/types';
+import type { Client, Event, EventHint, Integration, IntegrationClass, IntegrationFn, Options } from '@sentry/types';
 import { arrayify, logger } from '@sentry/utils';
 import { getClient } from './currentScopes';
 
 import { DEBUG_BUILD } from './debug-build';
-import { addGlobalEventProcessor } from './eventProcessors';
-import { getCurrentHub } from './hub';
 
 declare module '@sentry/types' {
   interface Integration {
@@ -130,8 +119,7 @@ export function setupIntegration(client: Client, integration: Integration, integ
 
   // `setupOnce` is only called the first time
   if (installedIntegrations.indexOf(integration.name) === -1 && typeof integration.setupOnce === 'function') {
-    // eslint-disable-next-line deprecation/deprecation
-    integration.setupOnce(addGlobalEventProcessor, getCurrentHub);
+    integration.setupOnce();
     installedIntegrations.push(integration.name);
   }
 
@@ -203,6 +191,6 @@ export function convertIntegrationFnToClass<Fn extends IntegrationFn>(
  * Define an integration function that can be used to create an integration instance.
  * Note that this by design hides the implementation details of the integration, as they are considered internal.
  */
-export function defineIntegration<Fn extends IntegrationFn>(fn: Fn): (...args: Parameters<Fn>) => IntegrationFnResult {
+export function defineIntegration<Fn extends IntegrationFn>(fn: Fn): (...args: Parameters<Fn>) => Integration {
   return fn;
 }
diff --git a/packages/core/test/lib/sdk.test.ts b/packages/core/test/lib/sdk.test.ts
@@ -1,4 +1,4 @@
-import type { Client, Integration, IntegrationFnResult } from '@sentry/types';
+import type { Client, Integration } from '@sentry/types';
 import { captureCheckIn, getCurrentScope, setCurrentClient } from '../../src';
 
 import { installedIntegrations } from '../../src/integration';
@@ -43,20 +43,20 @@ describe('SDK', () => {
         name: 'integration1',
         setupOnce: jest.fn(() => list.push('setupOnce1')),
         afterAllSetup: jest.fn(() => list.push('afterAllSetup1')),
-      } satisfies IntegrationFnResult;
+      } satisfies Integration;
 
       const integration2 = {
         name: 'integration2',
         setupOnce: jest.fn(() => list.push('setupOnce2')),
         setup: jest.fn(() => list.push('setup2')),
         afterAllSetup: jest.fn(() => list.push('afterAllSetup2')),
-      } satisfies IntegrationFnResult;
+      } satisfies Integration;
 
       const integration3 = {
         name: 'integration3',
         setupOnce: jest.fn(() => list.push('setupOnce3')),
         setup: jest.fn(() => list.push('setup3')),
-      } satisfies IntegrationFnResult;
+      } satisfies Integration;
 
       const integrations: Integration[] = [integration1, integration2, integration3];
       const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, integrations });

diff --git a/packages/core/test/mocks/integration.ts b/packages/core/test/mocks/integration.ts
@@ -1,4 +1,4 @@
-import type { Event, EventProcessor, Integration } from '@sentry/types';
+import type { Client, Event, EventProcessor, Integration } from '@sentry/types';
 
 import { getClient, getCurrentScope } from '../../src';
 
@@ -27,8 +27,8 @@ export class AddAttachmentTestIntegration implements Integration {
 
   public name: string = 'AddAttachmentTestIntegration';
 
-  public setupOnce(addGlobalEventProcessor: (callback: EventProcessor) => void): void {
-    addGlobalEventProcessor((event, hint) => {
+  public setup(client: Client): void {
+    client.addEventProcessor((event, hint) => {
       hint.attachments = [...(hint.attachments || []), { filename: 'integration.file', data: 'great content!' }];
       return event;
     });

diff --git a/packages/feedback/src/core/integration.ts b/packages/feedback/src/core/integration.ts
@@ -1,5 +1,5 @@
 import { defineIntegration, getClient } from '@sentry/core';
-import type { IntegrationFn, IntegrationFnResult } from '@sentry/types';
+import type { Integration, IntegrationFn } from '@sentry/types';
 import { isBrowser, logger } from '@sentry/utils';
 import {
   ACTOR_LABEL,
@@ -42,7 +42,7 @@ interface PublicFeedbackIntegration {
   closeDialog: () => void;
   removeWidget: () => void;
 }
-export type IFeedbackIntegration = IntegrationFnResult & PublicFeedbackIntegration;
+export type IFeedbackIntegration = Integration & PublicFeedbackIntegration;
 
 export const _feedbackIntegration = (({
   // FeedbackGeneralConfiguration

diff --git a/packages/feedback/src/modal/integration.ts b/packages/feedback/src/modal/integration.ts
@@ -1,5 +1,5 @@
 import { defineIntegration } from '@sentry/core';
-import type { IntegrationFn, IntegrationFnResult } from '@sentry/types';
+import type { Integration, IntegrationFn } from '@sentry/types';
 import { createDialog } from './createDialog';
 
 interface PublicFeedbackModalIntegration {
@@ -8,7 +8,7 @@ interface PublicFeedbackModalIntegration {
 
 const INTEGRATION_NAME = 'FeedbackModal';
 
-export type IFeedbackModalIntegration = IntegrationFnResult & PublicFeedbackModalIntegration;
+export type IFeedbackModalIntegration = Integration & PublicFeedbackModalIntegration;
 
 export const _feedbackModalIntegration = (() => {
   return {

diff --git a/packages/feedback/src/screenshot/integration.ts b/packages/feedback/src/screenshot/integration.ts
@@ -1,5 +1,5 @@
 import { defineIntegration } from '@sentry/core';
-import type { IntegrationFn, IntegrationFnResult } from '@sentry/types';
+import type { Integration, IntegrationFn } from '@sentry/types';
 import { createInput } from './createInput';
 
 interface PublicFeedbackScreenshotIntegration {
@@ -8,7 +8,7 @@ interface PublicFeedbackScreenshotIntegration {
 
 const INTEGRATION_NAME = 'FeedbackScreenshot';
 
-export type IFeedbackScreenshotIntegration = IntegrationFnResult & PublicFeedbackScreenshotIntegration;
+export type IFeedbackScreenshotIntegration = Integration & PublicFeedbackScreenshotIntegration;
 
 export const _feedbackScreenshotIntegration = (() => {
   return {

diff --git a/packages/node-experimental/src/integrations/anr/index.ts b/packages/node-experimental/src/integrations/anr/index.ts
@@ -1,5 +1,5 @@
 import { defineIntegration, getCurrentScope } from '@sentry/core';
-import type { Contexts, Event, EventHint, IntegrationFn, IntegrationFnResult } from '@sentry/types';
+import type { Contexts, Event, EventHint, Integration, IntegrationFn } from '@sentry/types';
 import { logger } from '@sentry/utils';
 import * as inspector from 'inspector';
 import { Worker } from 'worker_threads';
@@ -69,10 +69,10 @@ const _anrIntegration = ((options: Partial<AnrIntegrationOptions> = {}) => {
       // This allows us to call into all integrations to fetch the full context
       setImmediate(() => this.startWorker());
     },
-  } as IntegrationFnResult & AnrInternal;
+  } as Integration & AnrInternal;
 }) satisfies IntegrationFn;
 
-type AnrReturn = (options?: Partial<AnrIntegrationOptions>) => IntegrationFnResult & AnrInternal;
+type AnrReturn = (options?: Partial<AnrIntegrationOptions>) => Integration & AnrInternal;
 
 export const anrIntegration = defineIntegration(_anrIntegration) as AnrReturn;
 

diff --git a/packages/node/src/integrations/http.ts b/packages/node/src/integrations/http.ts
@@ -1,7 +1,6 @@
 /* eslint-disable max-lines */
 import type * as http from 'http';
 import type * as https from 'https';
-import type { Hub } from '@sentry/core';
 import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startInactiveSpan } from '@sentry/core';
 import { defineIntegration, getIsolationScope, hasTracingEnabled } from '@sentry/core';
 import {
@@ -18,10 +17,8 @@ import {
 } from '@sentry/core';
 import type {
   ClientOptions,
-  EventProcessor,
   Integration,
   IntegrationFn,
-  IntegrationFnResult,
   SanitizedRequestData,
   TracePropagationTargets,
 } from '@sentry/types';
@@ -124,9 +121,8 @@ const _httpIntegration = ((options: HttpIntegrationOptions = {}) => {
             shouldCreateSpanForRequest,
           }),
   };
-
   // eslint-disable-next-line deprecation/deprecation
-  return new Http(convertedOptions) as unknown as IntegrationFnResult;
+  return new Http(convertedOptions) as unknown as Integration;
 }) satisfies IntegrationFn;
 
 /**
@@ -169,12 +165,9 @@ export class Http implements Integration {
   /**
    * @inheritDoc
    */
-  public setupOnce(
-    _addGlobalEventProcessor: (callback: EventProcessor) => void,
-    setupOnceGetCurrentHub: () => Hub,
-  ): void {
+  public setupOnce(): void {
     // eslint-disable-next-line deprecation/deprecation
-    const clientOptions = setupOnceGetCurrentHub().getClient<NodeClient>()?.getOptions();
+    const clientOptions = getCurrentHub().getClient<NodeClient>()?.getOptions();
 
     // If `tracing` is not explicitly set, we default this based on whether or not tracing is enabled.
     // But for compatibility, we only do that if `enableIfHasTracingEnabled` is set.

diff --git a/packages/node/src/integrations/undici/index.ts b/packages/node/src/integrations/undici/index.ts
@@ -13,14 +13,7 @@ import {
   setHttpStatus,
   spanToTraceHeader,
 } from '@sentry/core';
-import type {
-  EventProcessor,
-  Integration,
-  IntegrationFn,
-  IntegrationFnResult,
-  Span,
-  SpanAttributes,
-} from '@sentry/types';
+import type { Integration, IntegrationFn, Span, SpanAttributes } from '@sentry/types';
 import {
   LRUMap,
   dynamicSamplingContextToSentryBaggageHeader,
@@ -82,7 +75,7 @@ export interface UndiciOptions {
 
 const _nativeNodeFetchintegration = ((options?: Partial<UndiciOptions>) => {
   // eslint-disable-next-line deprecation/deprecation
-  return new Undici(options) as unknown as IntegrationFnResult;
+  return new Undici(options) as unknown as Integration;
 }) satisfies IntegrationFn;
 
 export const nativeNodeFetchintegration = defineIntegration(_nativeNodeFetchintegration);
@@ -125,7 +118,7 @@ export class Undici implements Integration {
   /**
    * @inheritDoc
    */
-  public setupOnce(_addGlobalEventProcessor: (callback: EventProcessor) => void): void {
+  public setupOnce(): void {
     // Requires Node 16+ to use the diagnostics_channel API.
     if (NODE_VERSION.major < 16) {
       return;

diff --git a/packages/node/test/integrations/contextlines.test.ts b/packages/node/test/integrations/contextlines.test.ts
@@ -1,5 +1,5 @@
 import * as fs from 'fs';
-import type { Event, IntegrationFnResult, StackFrame } from '@sentry/types';
+import type { Event, Integration, StackFrame } from '@sentry/types';
 import { parseStackFrames } from '@sentry/utils';
 
 import { contextLinesIntegration } from '../../src';
@@ -9,10 +9,10 @@ import { getError } from '../helper/error';
 
 describe('ContextLines', () => {
   let readFileSpy: jest.SpyInstance;
-  let contextLines: IntegrationFnResult;
+  let contextLines: Integration;
 
   async function addContext(frames: StackFrame[]): Promise<void> {
-    await (contextLines as IntegrationFnResult & { processEvent: (event: Event) => Promise<Event> }).processEvent({
+    await (contextLines as Integration & { processEvent: (event: Event) => Promise<Event> }).processEvent({
       exception: { values: [{ stacktrace: { frames } }] },
     });
   }
@@ -101,7 +101,6 @@ describe('ContextLines', () => {
     });
 
     test('parseStack with no context', async () => {
-      // eslint-disable-next-line deprecation/deprecation
       contextLines = contextLinesIntegration({ frameContextLines: 0 });
 
       expect.assertions(1);
@@ -114,7 +113,6 @@ describe('ContextLines', () => {
 
   test('does not attempt to readfile multiple times if it fails', async () => {
     expect.assertions(1);
-    // eslint-disable-next-line deprecation/deprecation
     contextLines = contextLinesIntegration();
 
     readFileSpy.mockImplementation(() => {

diff --git a/packages/node/test/integrations/http.test.ts b/packages/node/test/integrations/http.test.ts
@@ -1,7 +1,6 @@
 import * as http from 'http';
 import * as https from 'https';
-import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, getSpanDescendants, startSpan } from '@sentry/core';
-import type { Hub } from '@sentry/core';
+import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, getSpanDescendants, makeMain, startSpan } from '@sentry/core';
 import { getCurrentHub, getIsolationScope, setCurrentClient } from '@sentry/core';
 import { Transaction } from '@sentry/core';
 import { getCurrentScope, setUser, spanToJSON, startInactiveSpan } from '@sentry/core';
@@ -346,7 +345,11 @@ describe('tracing', () => {
       setCurrentClient(client);
       client.init();
       // eslint-disable-next-line deprecation/deprecation
-      return getCurrentHub();
+      const hub = getCurrentHub();
+      // eslint-disable-next-line deprecation/deprecation
+      makeMain(hub);
+
+      return hub;
     }
 
     function createTransactionAndPutOnScope() {
@@ -365,12 +368,9 @@ describe('tracing', () => {
         // eslint-disable-next-line deprecation/deprecation
         const httpIntegration = new HttpIntegration({ tracing: true });
 
-        const hub = createHub({ shouldCreateSpanForRequest: () => false });
+        createHub({ shouldCreateSpanForRequest: () => false });
 
-        httpIntegration.setupOnce(
-          () => undefined,
-          () => hub as Hub,
-        );
+        httpIntegration.setupOnce();
 
         const transaction = createTransactionAndPutOnScope();
 
@@ -412,12 +412,9 @@ describe('tracing', () => {
           // eslint-disable-next-line deprecation/deprecation
           const httpIntegration = new HttpIntegration({ tracing: true });
 
-          const hub = createHub({ tracePropagationTargets });
+          createHub({ tracePropagationTargets });
 
-          httpIntegration.setupOnce(
-            () => undefined,
-            () => hub as Hub,
-          );
+          httpIntegration.setupOnce();
 
           createTransactionAndPutOnScope();
 
@@ -445,12 +442,9 @@ describe('tracing', () => {
           // eslint-disable-next-line deprecation/deprecation
           const httpIntegration = new HttpIntegration({ tracing: true });
 
-          const hub = createHub({ tracePropagationTargets });
+          createHub({ tracePropagationTargets });
 
-          httpIntegration.setupOnce(
-            () => undefined,
-            () => hub as Hub,
-          );
+          httpIntegration.setupOnce();
 
           createTransactionAndPutOnScope();
 
@@ -474,12 +468,9 @@ describe('tracing', () => {
           },
         });
 
-        const hub = createHub();
+        createHub();
 
-        httpIntegration.setupOnce(
-          () => undefined,
-          () => hub as Hub,
-        );
+        httpIntegration.setupOnce();
 
         const transaction = createTransactionAndPutOnScope();
 
@@ -521,12 +512,9 @@ describe('tracing', () => {
           // eslint-disable-next-line deprecation/deprecation
           const httpIntegration = new HttpIntegration({ tracing: { tracePropagationTargets } });
 
-          const hub = createHub();
+          createHub();
 
-          httpIntegration.setupOnce(
-            () => undefined,
-            () => hub as Hub,
-          );
+          httpIntegration.setupOnce();
 
           createTransactionAndPutOnScope();
 
@@ -554,12 +542,9 @@ describe('tracing', () => {
           // eslint-disable-next-line deprecation/deprecation
           const httpIntegration = new HttpIntegration({ tracing: { tracePropagationTargets } });
 
-          const hub = createHub();
+          createHub();
 
-          httpIntegration.setupOnce(
-            () => undefined,
-            () => hub as Hub,
-          );
+          httpIntegration.setupOnce();
 
           createTransactionAndPutOnScope();
 

diff --git a/packages/tracing-internal/src/node/integrations/express.ts b/packages/tracing-internal/src/node/integrations/express.ts
@@ -121,7 +121,7 @@ export class Express implements Integration {
   /**
    * @inheritDoc
    */
-  public setupOnce(_: unknown): void {
+  public setupOnce(): void {
     if (!this._router) {
       DEBUG_BUILD && logger.error('ExpressIntegration is missing an Express instance');
       return;