From b7bd95fd11cae4639fbd9a691fc19df79c47b42d Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Fri, 22 Mar 2024 10:36:00 -0400 Subject: [PATCH] ref(v8): change integration.setupOnce signature (#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`! --- packages/core/src/integration.ts | 18 ++----- packages/core/test/lib/sdk.test.ts | 8 +-- packages/core/test/mocks/integration.ts | 6 +-- packages/feedback/src/core/integration.ts | 4 +- packages/feedback/src/modal/integration.ts | 4 +- .../feedback/src/screenshot/integration.ts | 4 +- .../src/integrations/anr/index.ts | 6 +-- packages/node/src/integrations/http.ts | 13 ++--- .../node/src/integrations/undici/index.ts | 13 ++--- .../test/integrations/contextlines.test.ts | 8 ++- packages/node/test/integrations/http.test.ts | 51 +++++++------------ .../src/node/integrations/express.ts | 2 +- .../src/node/integrations/mongo.ts | 4 +- packages/types/src/index.ts | 2 +- packages/types/src/integration.ts | 51 ++----------------- 15 files changed, 53 insertions(+), 141 deletions(-) diff --git a/packages/core/src/integration.ts b/packages/core/src/integration.ts index ccf1f86cff18..237a086b92bb 100644 --- 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( * 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: Fn): (...args: Parameters) => IntegrationFnResult { +export function defineIntegration(fn: Fn): (...args: Parameters) => Integration { return fn; } diff --git a/packages/core/test/lib/sdk.test.ts b/packages/core/test/lib/sdk.test.ts index 0117585d05ab..ecc7d18483e5 100644 --- 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 index dbee06380d1c..ce012923030e 100644 --- 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 index 489bfd32728d..befc2cc03495 100644 --- 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 index 67f2c327db5b..fa034650f023 100644 --- 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 index 93fb5b605e04..3b959b971d70 100644 --- 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 index 8e28e0eb3535..c1bc03bc5a86 100644 --- 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 = {}) => { // 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) => IntegrationFnResult & AnrInternal; +type AnrReturn = (options?: Partial) => 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 index a8ca8b832455..bc4571f35af0 100644 --- 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()?.getOptions(); + const clientOptions = getCurrentHub().getClient()?.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 index 540776adf8d8..2a0ea01fe234 100644 --- 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) => { // 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 index dcc50f34c9a4..67f3ad793fbc 100644 --- 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 { - await (contextLines as IntegrationFnResult & { processEvent: (event: Event) => Promise }).processEvent({ + await (contextLines as Integration & { processEvent: (event: Event) => Promise }).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 index 00e642ae6e10..3b1fa4904f04 100644 --- 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 index 430a72faa5a3..26c82a2ef8dd 100644 --- 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; diff --git a/packages/tracing-internal/src/node/integrations/mongo.ts b/packages/tracing-internal/src/node/integrations/mongo.ts index 7c6f1bf97159..b85e0f3ae03a 100644 --- a/packages/tracing-internal/src/node/integrations/mongo.ts +++ b/packages/tracing-internal/src/node/integrations/mongo.ts @@ -1,6 +1,6 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startInactiveSpan } from '@sentry/core'; import { getClient } from '@sentry/core'; -import type { EventProcessor, SpanAttributes, StartSpanOptions } from '@sentry/types'; +import type { SpanAttributes, StartSpanOptions } from '@sentry/types'; import { fill, isThenable, loadModule, logger } from '@sentry/utils'; import { DEBUG_BUILD } from '../../common/debug-build'; @@ -139,7 +139,7 @@ export class Mongo implements LazyLoadedIntegration { /** * @inheritDoc */ - public setupOnce(_: (callback: EventProcessor) => void): void { + public setupOnce(): void { const pkg = this.loadDependency(); if (!pkg) { diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 3c6c3a6fe8a4..9b8009de4fc4 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -52,7 +52,7 @@ export type { EventProcessor } from './eventprocessor'; export type { Exception } from './exception'; export type { Extra, Extras } from './extra'; export type { Hub } from './hub'; -export type { Integration, IntegrationClass, IntegrationFn, IntegrationFnResult } from './integration'; +export type { Integration, IntegrationClass, IntegrationFn } from './integration'; export type { Mechanism } from './mechanism'; export type { ExtractedNodeRequestData, HttpHeaderValue, Primitive, WorkerLocation } from './misc'; export type { ClientOptions, Options } from './options'; diff --git a/packages/types/src/integration.ts b/packages/types/src/integration.ts index 543238ccc07b..af54d389a3fd 100644 --- a/packages/types/src/integration.ts +++ b/packages/types/src/integration.ts @@ -1,7 +1,5 @@ import type { Client } from './client'; import type { Event, EventHint } from './event'; -import type { EventProcessor } from './eventprocessor'; -import type { Hub } from './hub'; /** Integration Class Interface */ export interface IntegrationClass { @@ -13,9 +11,8 @@ export interface IntegrationClass { new (...args: any[]): T; } -/** Integration interface. - * This is more or less the same as `Integration`, but with a slimmer `setupOnce` siganture. */ -export interface IntegrationFnResult { +/** Integration interface */ +export interface Integration { /** * The name of the integration. */ @@ -60,46 +57,4 @@ export interface IntegrationFnResult { * An integration in function form. * This is expected to return an integration. */ -export type IntegrationFn = (...rest: any[]) => IntegrationFnResult; - -/** Integration interface */ -export interface Integration { - /** - * The name of the integration. - */ - name: string; - - /** - * This hook is only called once, even if multiple clients are created. - * It does not receives any arguments, and should only use for e.g. global monkey patching and similar things. - */ - setupOnce?(addGlobalEventProcessor: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void; - - /** - * Set up an integration for the given client. - * Receives the client as argument. - * - * Whenever possible, prefer this over `setupOnce`, as that is only run for the first client, - * whereas `setup` runs for each client. Only truly global things (e.g. registering global handlers) - * should be done in `setupOnce`. - */ - setup?(client: Client): void; - - /** - * This hook is triggered after `setupOnce()` and `setup()` have been called for all integrations. - * You can use it if it is important that all other integrations have been run before. - */ - afterAllSetup?(client: Client): void; - - /** - * An optional hook that allows to preprocess an event _before_ it is passed to all other event processors. - */ - preprocessEvent?(event: Event, hint: EventHint | undefined, client: Client): void; - - /** - * An optional hook that allows to process an event. - * Return `null` to drop the event, or mutate the event & return it. - * This receives the client that the integration was installed for as third argument. - */ - processEvent?(event: Event, hint: EventHint, client: Client): Event | null | PromiseLike; -} +export type IntegrationFn = (...rest: any[]) => Integration;