From c2e7bf2e8e5e21669e9132335fe52877f0017f04 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 12 Feb 2024 11:50:50 +0100 Subject: [PATCH 1/3] ref(core): make remaining client methods required This makes the following required: * `captureSession` * `getSdkMetadata` * `addEventProcessor` * `getEventProcessor` * `getIntegrationByName` * `addIntegration` * `init` * `captureAggregratMetrics` --- packages/core/src/exports.ts | 2 +- packages/core/src/integration.ts | 4 ++-- packages/core/src/metrics/aggregator.ts | 2 +- .../core/src/metrics/browser-aggregator.ts | 10 ++++---- packages/core/src/sdk.ts | 17 +------------- packages/core/src/utils/prepareEvent.ts | 2 +- packages/node-experimental/src/sdk/hub.ts | 2 +- packages/node-experimental/src/sdk/init.ts | 23 +++++++++---------- packages/node/src/sdk.ts | 2 +- .../src/setupEventContextTrace.ts | 4 ---- packages/replay/src/integration.ts | 2 +- .../replay/src/util/addGlobalListeners.ts | 8 ++----- packages/replay/src/util/getReplay.ts | 4 +--- .../replay/src/util/prepareReplayEvent.ts | 2 +- packages/types/src/client.ts | 20 +++++++--------- 15 files changed, 37 insertions(+), 67 deletions(-) diff --git a/packages/core/src/exports.ts b/packages/core/src/exports.ts index 1e5c6e5503c5..968aef4692de 100644 --- a/packages/core/src/exports.ts +++ b/packages/core/src/exports.ts @@ -466,7 +466,7 @@ function _sendSessionUpdate(): void { // TODO (v8): Remove currentScope and only use the isolation scope(?). // For v7 though, we can't "soft-break" people using getCurrentHub().getScope().setSession() const session = currentScope.getSession() || isolationScope.getSession(); - if (session && client && client.captureSession) { + if (session && client) { client.captureSession(session); } } diff --git a/packages/core/src/integration.ts b/packages/core/src/integration.ts index c95dce4b2f79..80400455363e 100644 --- a/packages/core/src/integration.ts +++ b/packages/core/src/integration.ts @@ -145,7 +145,7 @@ export function setupIntegration(client: Client, integration: Integration, integ client.on('preprocessEvent', (event, hint) => callback(event, hint, client)); } - if (client.addEventProcessor && typeof integration.processEvent === 'function') { + if (typeof integration.processEvent === 'function') { const callback = integration.processEvent.bind(integration) as typeof integration.processEvent; const processor = Object.assign((event: Event, hint: EventHint) => callback(event, hint, client), { @@ -162,7 +162,7 @@ export function setupIntegration(client: Client, integration: Integration, integ export function addIntegration(integration: Integration): void { const client = getClient(); - if (!client || !client.addIntegration) { + if (!client) { DEBUG_BUILD && logger.warn(`Cannot add integration "${integration.name}" because no SDK Client is available.`); return; } diff --git a/packages/core/src/metrics/aggregator.ts b/packages/core/src/metrics/aggregator.ts index 6a49fda5918b..945a2f59d6d0 100644 --- a/packages/core/src/metrics/aggregator.ts +++ b/packages/core/src/metrics/aggregator.ts @@ -153,7 +153,7 @@ export class MetricsAggregator implements MetricsAggregatorBase { * @param flushedBuckets */ private _captureMetrics(flushedBuckets: MetricBucket): void { - if (flushedBuckets.size > 0 && this._client.captureAggregateMetrics) { + if (flushedBuckets.size > 0) { // TODO(@anonrig): Optimization opportunity. // This copy operation can be avoided if we store the key in the bucketItem. const buckets = Array.from(flushedBuckets).map(([, bucketItem]) => bucketItem); diff --git a/packages/core/src/metrics/browser-aggregator.ts b/packages/core/src/metrics/browser-aggregator.ts index 5b5c81353024..2493cdb03ede 100644 --- a/packages/core/src/metrics/browser-aggregator.ts +++ b/packages/core/src/metrics/browser-aggregator.ts @@ -74,11 +74,11 @@ export class BrowserMetricsAggregator implements MetricsAggregator { if (this._buckets.size === 0) { return; } - if (this._client.captureAggregateMetrics) { - // TODO(@anonrig): Use Object.values() when we support ES6+ - const metricBuckets = Array.from(this._buckets).map(([, bucketItem]) => bucketItem); - this._client.captureAggregateMetrics(metricBuckets); - } + + // TODO(@anonrig): Use Object.values() when we support ES6+ + const metricBuckets = Array.from(this._buckets).map(([, bucketItem]) => bucketItem); + this._client.captureAggregateMetrics(metricBuckets); + this._buckets.clear(); } diff --git a/packages/core/src/sdk.ts b/packages/core/src/sdk.ts index f15f6c976e56..f5cb480a1eaf 100644 --- a/packages/core/src/sdk.ts +++ b/packages/core/src/sdk.ts @@ -35,7 +35,7 @@ export function initAndBind( const client = new clientClass(options); setCurrentClient(client); - initializeClient(client); + client.init(); } /** @@ -49,18 +49,3 @@ export function setCurrentClient(client: Client): void { top.client = client; top.scope.setClient(client); } - -/** - * Initialize the client for the current scope. - * Make sure to call this after `setCurrentClient()`. - */ -function initializeClient(client: Client): void { - if (client.init) { - client.init(); - // TODO v8: Remove this fallback - // eslint-disable-next-line deprecation/deprecation - } else if (client.setupIntegrations) { - // eslint-disable-next-line deprecation/deprecation - client.setupIntegrations(); - } -} diff --git a/packages/core/src/utils/prepareEvent.ts b/packages/core/src/utils/prepareEvent.ts index 17be8c5354de..5f5aef7003d2 100644 --- a/packages/core/src/utils/prepareEvent.ts +++ b/packages/core/src/utils/prepareEvent.ts @@ -75,7 +75,7 @@ export function prepareEvent( addExceptionMechanism(prepared, hint.mechanism); } - const clientEventProcessors = client && client.getEventProcessors ? client.getEventProcessors() : []; + const clientEventProcessors = client ? client.getEventProcessors() : []; // This should be the last thing called, since we want that // {@link Hub.addEventProcessor} gets the finished prepared event. diff --git a/packages/node-experimental/src/sdk/hub.ts b/packages/node-experimental/src/sdk/hub.ts index f3cd1f2a7d13..a0e74d8454af 100644 --- a/packages/node-experimental/src/sdk/hub.ts +++ b/packages/node-experimental/src/sdk/hub.ts @@ -145,7 +145,7 @@ function _sendSessionUpdate(): void { const client = getClient(); const session = scope.getSession(); - if (session && client.captureSession) { + if (session) { client.captureSession(session); } } diff --git a/packages/node-experimental/src/sdk/init.ts b/packages/node-experimental/src/sdk/init.ts index d617845b9e2e..d15c28dc3f98 100644 --- a/packages/node-experimental/src/sdk/init.ts +++ b/packages/node-experimental/src/sdk/init.ts @@ -86,19 +86,18 @@ export function init(options: NodeExperimentalOptions | undefined = {}): void { if (options.spotlight) { const client = getClient(); - if (client.addIntegration) { - // force integrations to be setup even if no DSN was set - // If they have already been added before, they will be ignored anyhow - const integrations = client.getOptions().integrations; - for (const integration of integrations) { - client.addIntegration(integration); - } - client.addIntegration( - spotlightIntegration({ - sidecarUrl: typeof options.spotlight === 'string' ? options.spotlight : undefined, - }), - ); + + // force integrations to be setup even if no DSN was set + // If they have already been added before, they will be ignored anyhow + const integrations = client.getOptions().integrations; + for (const integration of integrations) { + client.addIntegration(integration); } + client.addIntegration( + spotlightIntegration({ + sidecarUrl: typeof options.spotlight === 'string' ? options.spotlight : undefined, + }), + ); } // Always init Otel, even if tracing is disabled, because we need it for trace propagation & the HTTP integration diff --git a/packages/node/src/sdk.ts b/packages/node/src/sdk.ts index 14aff2fef1fe..6aa6b0499ca2 100644 --- a/packages/node/src/sdk.ts +++ b/packages/node/src/sdk.ts @@ -176,7 +176,7 @@ export function init(options: NodeOptions = {}): void { if (options.spotlight) { const client = getClient(); - if (client && client.addIntegration) { + if (client) { // force integrations to be setup even if no DSN was set // If they have already been added before, they will be ignored anyhow const integrations = client.getOptions().integrations; diff --git a/packages/opentelemetry/src/setupEventContextTrace.ts b/packages/opentelemetry/src/setupEventContextTrace.ts index c55fc27ed52f..fb41ce463af5 100644 --- a/packages/opentelemetry/src/setupEventContextTrace.ts +++ b/packages/opentelemetry/src/setupEventContextTrace.ts @@ -5,10 +5,6 @@ import { spanHasParentId } from './utils/spanTypes'; /** Ensure the `trace` context is set on all events. */ export function setupEventContextTrace(client: Client): void { - if (!client.addEventProcessor) { - return; - } - client.addEventProcessor(event => { const span = getActiveSpan(); if (!span) { diff --git a/packages/replay/src/integration.ts b/packages/replay/src/integration.ts index 3434888f6574..c52cccd51dd2 100644 --- a/packages/replay/src/integration.ts +++ b/packages/replay/src/integration.ts @@ -367,7 +367,7 @@ Sentry.init({ replaysOnErrorSampleRate: ${errorSampleRate} })`, /* eslint-disable @typescript-eslint/no-non-null-assertion */ try { const client = getClient()!; - const canvasIntegration = client.getIntegrationByName!('ReplayCanvas') as Integration & { + const canvasIntegration = client.getIntegrationByName('ReplayCanvas') as Integration & { getOptions(): ReplayCanvasIntegrationOptions; }; if (!canvasIntegration) { diff --git a/packages/replay/src/util/addGlobalListeners.ts b/packages/replay/src/util/addGlobalListeners.ts index 797a6c3cfa96..f8efc4897b4e 100644 --- a/packages/replay/src/util/addGlobalListeners.ts +++ b/packages/replay/src/util/addGlobalListeners.ts @@ -25,12 +25,8 @@ export function addGlobalListeners(replay: ReplayContainer): void { // Tag all (non replay) events that get sent to Sentry with the current // replay ID so that we can reference them later in the UI - const eventProcessor = handleGlobalEventListener(replay); - if (client && client.addEventProcessor) { - client.addEventProcessor(eventProcessor); - } else { - addEventProcessor(eventProcessor); - } + const eventProcessor = handleGlobalEventListener(replay, !hasHooks(client)); + addEventProcessor(eventProcessor); // If a custom client has no hooks yet, we continue to use the "old" implementation if (client) { diff --git a/packages/replay/src/util/getReplay.ts b/packages/replay/src/util/getReplay.ts index 278505f15338..75d794da8e69 100644 --- a/packages/replay/src/util/getReplay.ts +++ b/packages/replay/src/util/getReplay.ts @@ -7,7 +7,5 @@ import type { replayIntegration } from '../integration'; // eslint-disable-next-line deprecation/deprecation export function getReplay(): ReturnType | undefined { const client = getClient(); - return ( - client && client.getIntegrationByName && client.getIntegrationByName>('Replay') - ); + return client && client.getIntegrationByName>('Replay'); } diff --git a/packages/replay/src/util/prepareReplayEvent.ts b/packages/replay/src/util/prepareReplayEvent.ts index e564f0509326..dfa5e8bbd620 100644 --- a/packages/replay/src/util/prepareReplayEvent.ts +++ b/packages/replay/src/util/prepareReplayEvent.ts @@ -46,7 +46,7 @@ export async function prepareReplayEvent({ preparedEvent.platform = preparedEvent.platform || 'javascript'; // extract the SDK name because `client._prepareEvent` doesn't add it to the event - const metadata = client.getSdkMetadata && client.getSdkMetadata(); + const metadata = client.getSdkMetadata(); const { name, version } = (metadata && metadata.sdk) || {}; preparedEvent.sdk = { diff --git a/packages/types/src/client.ts b/packages/types/src/client.ts index ea50bd5a67f3..7da5386f40f1 100644 --- a/packages/types/src/client.ts +++ b/packages/types/src/client.ts @@ -65,7 +65,7 @@ export interface Client { * * @param session Session to be delivered */ - captureSession?(session: Session): void; + captureSession(session: Session): void; /** * Create a cron monitor check in and send it to Sentry. This method is not available on all clients. @@ -89,7 +89,7 @@ export interface Client { * * TODO (v8): Make this a required method. */ - getSdkMetadata?(): SdkMetadata | undefined; + getSdkMetadata(): SdkMetadata | undefined; /** * Returns the transport that is used by the client. @@ -121,17 +121,13 @@ export interface Client { /** * Adds an event processor that applies to any event processed by this client. - * - * TODO (v8): Make this a required method. */ - addEventProcessor?(eventProcessor: EventProcessor): void; + addEventProcessor(eventProcessor: EventProcessor): void; /** * Get all added event processors for this client. - * - * TODO (v8): Make this a required method. */ - getEventProcessors?(): EventProcessor[]; + getEventProcessors(): EventProcessor[]; /** * Returns the client's instance of the given integration class, it any. @@ -140,7 +136,7 @@ export interface Client { getIntegration(integration: IntegrationClass): T | null; /** Get the instance of the integration with the given name on the client, if it was added. */ - getIntegrationByName?(name: string): T | undefined; + getIntegrationByName(name: string): T | undefined; /** * Add an integration to the client. @@ -150,7 +146,7 @@ export interface Client { * * TODO (v8): Make this a required method. * */ - addIntegration?(integration: Integration): void; + addIntegration(integration: Integration): void; /** * This is an internal function to setup all integrations that should run on the client. @@ -162,7 +158,7 @@ export interface Client { * Initialize this client. * Call this after the client was set on a scope. */ - init?(): void; + init(): void; /** Creates an {@link Event} from all inputs to `captureException` and non-primitive inputs to `captureMessage`. */ // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -191,7 +187,7 @@ export interface Client { * * @experimental This API is experimental and might experience breaking changes */ - captureAggregateMetrics?(metricBucketItems: Array): void; + captureAggregateMetrics(metricBucketItems: Array): void; // HOOKS // TODO(v8): Make the hooks non-optional. From 2c271432183b35f8ad6568a6b737a0976d44aa78 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 12 Feb 2024 11:59:02 +0100 Subject: [PATCH 2/3] Apply suggestions from code review Co-authored-by: Lukas Stracke --- packages/types/src/client.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/types/src/client.ts b/packages/types/src/client.ts index 7da5386f40f1..5a0ddcb38cb8 100644 --- a/packages/types/src/client.ts +++ b/packages/types/src/client.ts @@ -87,7 +87,6 @@ export interface Client { /** * @inheritdoc * - * TODO (v8): Make this a required method. */ getSdkMetadata(): SdkMetadata | undefined; @@ -144,7 +143,6 @@ export interface Client { * In most cases, this should not be necessary, and you're better off just passing the integrations via `integrations: []` at initialization time. * However, if you find the need to conditionally load & add an integration, you can use `addIntegration` to do so. * - * TODO (v8): Make this a required method. * */ addIntegration(integration: Integration): void; From d79216a703816ea8ff106adfe4f960c9e3be7c75 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 12 Feb 2024 13:26:09 +0100 Subject: [PATCH 3/3] fix --- packages/replay/src/util/addGlobalListeners.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/replay/src/util/addGlobalListeners.ts b/packages/replay/src/util/addGlobalListeners.ts index f8efc4897b4e..9b57e7dafec8 100644 --- a/packages/replay/src/util/addGlobalListeners.ts +++ b/packages/replay/src/util/addGlobalListeners.ts @@ -25,7 +25,7 @@ export function addGlobalListeners(replay: ReplayContainer): void { // Tag all (non replay) events that get sent to Sentry with the current // replay ID so that we can reference them later in the UI - const eventProcessor = handleGlobalEventListener(replay, !hasHooks(client)); + const eventProcessor = handleGlobalEventListener(replay); addEventProcessor(eventProcessor); // If a custom client has no hooks yet, we continue to use the "old" implementation