Skip to content

Commit

Permalink
ref(v8): change integration.setupOnce signature (#11238)
Browse files Browse the repository at this point in the history
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`!
  • Loading branch information
AbhiPrasad authored and c298lee committed Mar 23, 2024
1 parent cb45cbc commit df518bf
Show file tree
Hide file tree
Showing 15 changed files with 53 additions and 141 deletions.
18 changes: 3 additions & 15 deletions packages/core/src/integration.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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;
}
8 changes: 4 additions & 4 deletions packages/core/test/lib/sdk.test.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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 });
Expand Down
6 changes: 3 additions & 3 deletions packages/core/test/mocks/integration.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -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;
});
Expand Down
4 changes: 2 additions & 2 deletions packages/feedback/src/core/integration.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -42,7 +42,7 @@ interface PublicFeedbackIntegration {
closeDialog: () => void;
removeWidget: () => void;
}
export type IFeedbackIntegration = IntegrationFnResult & PublicFeedbackIntegration;
export type IFeedbackIntegration = Integration & PublicFeedbackIntegration;

export const _feedbackIntegration = (({
// FeedbackGeneralConfiguration
Expand Down
4 changes: 2 additions & 2 deletions packages/feedback/src/modal/integration.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -8,7 +8,7 @@ interface PublicFeedbackModalIntegration {

const INTEGRATION_NAME = 'FeedbackModal';

export type IFeedbackModalIntegration = IntegrationFnResult & PublicFeedbackModalIntegration;
export type IFeedbackModalIntegration = Integration & PublicFeedbackModalIntegration;

export const _feedbackModalIntegration = (() => {
return {
Expand Down
4 changes: 2 additions & 2 deletions packages/feedback/src/screenshot/integration.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -8,7 +8,7 @@ interface PublicFeedbackScreenshotIntegration {

const INTEGRATION_NAME = 'FeedbackScreenshot';

export type IFeedbackScreenshotIntegration = IntegrationFnResult & PublicFeedbackScreenshotIntegration;
export type IFeedbackScreenshotIntegration = Integration & PublicFeedbackScreenshotIntegration;

export const _feedbackScreenshotIntegration = (() => {
return {
Expand Down
6 changes: 3 additions & 3 deletions packages/node-experimental/src/integrations/anr/index.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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;

Expand Down
13 changes: 3 additions & 10 deletions packages/node/src/integrations/http.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -18,10 +17,8 @@ import {
} from '@sentry/core';
import type {
ClientOptions,
EventProcessor,
Integration,
IntegrationFn,
IntegrationFnResult,
SanitizedRequestData,
TracePropagationTargets,
} from '@sentry/types';
Expand Down Expand Up @@ -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;

/**
Expand Down Expand Up @@ -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.
Expand Down
13 changes: 3 additions & 10 deletions packages/node/src/integrations/undici/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down
8 changes: 3 additions & 5 deletions packages/node/test/integrations/contextlines.test.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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 } }] },
});
}
Expand Down Expand Up @@ -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);
Expand All @@ -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(() => {
Expand Down
51 changes: 18 additions & 33 deletions packages/node/test/integrations/http.test.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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() {
Expand All @@ -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();

Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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();

Expand All @@ -474,12 +468,9 @@ describe('tracing', () => {
},
});

const hub = createHub();
createHub();

httpIntegration.setupOnce(
() => undefined,
() => hub as Hub,
);
httpIntegration.setupOnce();

const transaction = createTransactionAndPutOnScope();

Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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();

Expand Down
2 changes: 1 addition & 1 deletion packages/tracing-internal/src/node/integrations/express.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading

0 comments on commit df518bf

Please sign in to comment.