From f0cd873232a4da7dd5cd98b864496e64a5430bac Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Fri, 15 Mar 2024 10:47:46 -0400 Subject: [PATCH 1/8] feat(v8): Remove deprecated integration methods on client --- .../replay/canvas/manualSnapshot/test.ts | 2 +- packages/core/src/baseclient.ts | 24 ----- packages/core/src/hub.ts | 3 +- packages/node-experimental/src/sdk/hub.ts | 102 ++++++++++++++++++ .../node/src/integrations/undici/index.ts | 9 +- .../opentelemetry/src/custom/getCurrentHub.ts | 3 +- 6 files changed, 108 insertions(+), 35 deletions(-) create mode 100644 packages/node-experimental/src/sdk/hub.ts diff --git a/dev-packages/browser-integration-tests/suites/replay/canvas/manualSnapshot/test.ts b/dev-packages/browser-integration-tests/suites/replay/canvas/manualSnapshot/test.ts index 583ff672e590..e90020780d73 100644 --- a/dev-packages/browser-integration-tests/suites/replay/canvas/manualSnapshot/test.ts +++ b/dev-packages/browser-integration-tests/suites/replay/canvas/manualSnapshot/test.ts @@ -31,7 +31,7 @@ sentryTest('can manually snapshot canvas', async ({ getLocalTestUrl, page, brows expect(incrementalSnapshots).toEqual([]); await page.evaluate(() => { - (window as any).Sentry.getClient().getIntegrationById('ReplayCanvas').snapshot(); + (window as any).Sentry.getClient().getIntegrationByName('ReplayCanvas').snapshot(); }); const { incrementalSnapshots: incrementalSnapshotsManual } = getReplayRecordingContent(await reqPromise3); diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 178a37aff5c0..f6e040b53a1d 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -15,7 +15,6 @@ import type { EventProcessor, FeedbackEvent, Integration, - IntegrationClass, Outcome, ParameterizedString, SdkMetadata, @@ -317,16 +316,6 @@ export abstract class BaseClient implements Client { } } - /** - * 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.getIntegrationByName(integrationId); - } - /** * Gets an installed integration by its name. * @@ -336,19 +325,6 @@ export abstract class BaseClient implements Client { 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(integration: IntegrationClass): T | null { - try { - return (this._integrations[integration.id] as T) || null; - } catch (_oO) { - DEBUG_BUILD && logger.warn(`Cannot retrieve integration ${integration.id} from the current Client`); - return null; - } - } - /** * @inheritDoc */ diff --git a/packages/core/src/hub.ts b/packages/core/src/hub.ts index ce3f974f735f..98c4f1f70b6a 100644 --- a/packages/core/src/hub.ts +++ b/packages/core/src/hub.ts @@ -428,8 +428,7 @@ export class Hub implements HubInterface { const client = this.getClient(); if (!client) return null; try { - // eslint-disable-next-line deprecation/deprecation - return client.getIntegration(integration); + return client.getIntegrationByName(integration.id) || null; } catch (_oO) { DEBUG_BUILD && logger.warn(`Cannot retrieve integration ${integration.id} from the current Hub`); return null; diff --git a/packages/node-experimental/src/sdk/hub.ts b/packages/node-experimental/src/sdk/hub.ts new file mode 100644 index 000000000000..dc8f921fbf4e --- /dev/null +++ b/packages/node-experimental/src/sdk/hub.ts @@ -0,0 +1,102 @@ +import type { Client, EventHint, Hub, Integration, IntegrationClass, Scope, SeverityLevel } from '@sentry/types'; + +import { + addBreadcrumb, + captureEvent, + endSession, + getCurrentScope, + getIsolationScope, + setContext, + setExtra, + setExtras, + setTag, + setTags, + setUser, + startSession, + withScope, +} from '@sentry/core'; +import { getClient } from './api'; + +/** + * This is for legacy reasons, and returns a proxy object instead of a hub to be used. + * @deprecated Use the methods directly. + */ +export function getCurrentHub(): Hub { + return { + isOlderThan(_version: number): boolean { + return false; + }, + + bindClient(client: Client): void { + const scope = getCurrentScope(); + scope.setClient(client); + }, + + pushScope(): Scope { + // TODO: This does not work and is actually deprecated + return getCurrentScope(); + }, + + popScope(): boolean { + // TODO: This does not work and is actually deprecated + return false; + }, + + withScope, + getClient: () => getClient() as C | undefined, + getScope: getCurrentScope, + getIsolationScope, + captureException: (exception: unknown, hint?: EventHint) => { + return getCurrentScope().captureException(exception, hint); + }, + captureMessage: (message: string, level?: SeverityLevel, hint?: EventHint) => { + return getCurrentScope().captureMessage(message, level, hint); + }, + captureEvent, + addBreadcrumb, + setUser, + setTags, + setTag, + setExtra, + setExtras, + setContext, + + getIntegration(integration: IntegrationClass): T | null { + return getClient().getIntegrationByName(integration.id) || null; + }, + + startSession, + + endSession, + + captureSession(endSession?: boolean): void { + // both send the update and pull the session from the scope + if (endSession) { + // eslint-disable-next-line deprecation/deprecation + return this.endSession(); + } + + // only send the update + _sendSessionUpdate(); + }, + + shouldSendDefaultPii(): boolean { + const client = getClient(); + const options = client.getOptions(); + return Boolean(options.sendDefaultPii); + }, + }; +} + +/** + * Sends the current Session on the scope + */ +function _sendSessionUpdate(): void { + const scope = getCurrentScope(); + const client = getClient(); + + const session = scope.getSession(); + if (session) { + client.captureSession(session); + } +} diff --git a/packages/node/src/integrations/undici/index.ts b/packages/node/src/integrations/undici/index.ts index 76d94947ee2d..540776adf8d8 100644 --- a/packages/node/src/integrations/undici/index.ts +++ b/packages/node/src/integrations/undici/index.ts @@ -170,8 +170,7 @@ export class Undici implements Integration { } private _onRequestCreate = (message: unknown): void => { - // eslint-disable-next-line deprecation/deprecation - if (!getClient()?.getIntegration(Undici)) { + if (!getClient()?.getIntegrationByName('Undici')) { return; } @@ -229,8 +228,7 @@ export class Undici implements Integration { }; private _onRequestEnd = (message: unknown): void => { - // eslint-disable-next-line deprecation/deprecation - if (!getClient()?.getIntegration(Undici)) { + if (!getClient()?.getIntegrationByName('Undici')) { return; } @@ -269,8 +267,7 @@ export class Undici implements Integration { }; private _onRequestError = (message: unknown): void => { - // eslint-disable-next-line deprecation/deprecation - if (!getClient()?.getIntegration(Undici)) { + if (!getClient()?.getIntegrationByName('Undici')) { return; } diff --git a/packages/opentelemetry/src/custom/getCurrentHub.ts b/packages/opentelemetry/src/custom/getCurrentHub.ts index 4c257e037e38..74ed4c3cc1a7 100644 --- a/packages/opentelemetry/src/custom/getCurrentHub.ts +++ b/packages/opentelemetry/src/custom/getCurrentHub.ts @@ -62,8 +62,7 @@ export function getCurrentHub(): Hub { setContext, getIntegration(integration: IntegrationClass): T | null { - // eslint-disable-next-line deprecation/deprecation - return getClient()?.getIntegration(integration) || null; + return getClient()?.getIntegrationByName(integration) || null; }, startSession, From bd6817f23edca0d3054b826914c3d33bbed922c1 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Fri, 15 Mar 2024 10:55:04 -0400 Subject: [PATCH 2/8] build fix --- packages/node-experimental/src/sdk/hub.ts | 102 ---------------------- packages/types/src/client.ts | 8 +- 2 files changed, 1 insertion(+), 109 deletions(-) delete mode 100644 packages/node-experimental/src/sdk/hub.ts diff --git a/packages/node-experimental/src/sdk/hub.ts b/packages/node-experimental/src/sdk/hub.ts deleted file mode 100644 index dc8f921fbf4e..000000000000 --- a/packages/node-experimental/src/sdk/hub.ts +++ /dev/null @@ -1,102 +0,0 @@ -import type { Client, EventHint, Hub, Integration, IntegrationClass, Scope, SeverityLevel } from '@sentry/types'; - -import { - addBreadcrumb, - captureEvent, - endSession, - getCurrentScope, - getIsolationScope, - setContext, - setExtra, - setExtras, - setTag, - setTags, - setUser, - startSession, - withScope, -} from '@sentry/core'; -import { getClient } from './api'; - -/** - * This is for legacy reasons, and returns a proxy object instead of a hub to be used. - * @deprecated Use the methods directly. - */ -export function getCurrentHub(): Hub { - return { - isOlderThan(_version: number): boolean { - return false; - }, - - bindClient(client: Client): void { - const scope = getCurrentScope(); - scope.setClient(client); - }, - - pushScope(): Scope { - // TODO: This does not work and is actually deprecated - return getCurrentScope(); - }, - - popScope(): boolean { - // TODO: This does not work and is actually deprecated - return false; - }, - - withScope, - getClient: () => getClient() as C | undefined, - getScope: getCurrentScope, - getIsolationScope, - captureException: (exception: unknown, hint?: EventHint) => { - return getCurrentScope().captureException(exception, hint); - }, - captureMessage: (message: string, level?: SeverityLevel, hint?: EventHint) => { - return getCurrentScope().captureMessage(message, level, hint); - }, - captureEvent, - addBreadcrumb, - setUser, - setTags, - setTag, - setExtra, - setExtras, - setContext, - - getIntegration(integration: IntegrationClass): T | null { - return getClient().getIntegrationByName(integration.id) || null; - }, - - startSession, - - endSession, - - captureSession(endSession?: boolean): void { - // both send the update and pull the session from the scope - if (endSession) { - // eslint-disable-next-line deprecation/deprecation - return this.endSession(); - } - - // only send the update - _sendSessionUpdate(); - }, - - shouldSendDefaultPii(): boolean { - const client = getClient(); - const options = client.getOptions(); - return Boolean(options.sendDefaultPii); - }, - }; -} - -/** - * Sends the current Session on the scope - */ -function _sendSessionUpdate(): void { - const scope = getCurrentScope(); - const client = getClient(); - - const session = scope.getSession(); - if (session) { - client.captureSession(session); - } -} diff --git a/packages/types/src/client.ts b/packages/types/src/client.ts index a764023a9b0e..6be08e8652e1 100644 --- a/packages/types/src/client.ts +++ b/packages/types/src/client.ts @@ -7,7 +7,7 @@ import type { DynamicSamplingContext, Envelope } from './envelope'; import type { Event, EventHint } from './event'; import type { EventProcessor } from './eventprocessor'; import type { FeedbackEvent } from './feedback'; -import type { Integration, IntegrationClass } from './integration'; +import type { Integration } from './integration'; import type { ClientOptions } from './options'; import type { ParameterizedString } from './parameterize'; import type { Scope } from './scope'; @@ -127,12 +127,6 @@ export interface Client { */ getEventProcessors(): EventProcessor[]; - /** - * Returns the client's instance of the given integration class, it any. - * @deprecated Use `getIntegrationByName()` instead. - */ - 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; From 3c5707844443905de36a037b488a41d3e6331b03 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Fri, 15 Mar 2024 11:34:05 -0400 Subject: [PATCH 3/8] fix --- packages/opentelemetry/src/custom/getCurrentHub.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/opentelemetry/src/custom/getCurrentHub.ts b/packages/opentelemetry/src/custom/getCurrentHub.ts index 74ed4c3cc1a7..81ccc46b6ddb 100644 --- a/packages/opentelemetry/src/custom/getCurrentHub.ts +++ b/packages/opentelemetry/src/custom/getCurrentHub.ts @@ -62,7 +62,7 @@ export function getCurrentHub(): Hub { setContext, getIntegration(integration: IntegrationClass): T | null { - return getClient()?.getIntegrationByName(integration) || null; + return getClient()?.getIntegrationByName(integration.id) || null; }, startSession, From f2d967af91f2c08b27aaa72bf763d2e3dbadf21b Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Fri, 15 Mar 2024 11:53:18 -0400 Subject: [PATCH 4/8] make main --- packages/sveltekit/src/server/index.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/sveltekit/src/server/index.ts b/packages/sveltekit/src/server/index.ts index 5bd1be392657..0bb2f86e82f3 100644 --- a/packages/sveltekit/src/server/index.ts +++ b/packages/sveltekit/src/server/index.ts @@ -20,8 +20,6 @@ export { getIsolationScope, Hub, NodeClient, - // eslint-disable-next-line deprecation/deprecation - makeMain, setCurrentClient, Scope, SDK_VERSION, From b8d7905d23647d7d32d2f5378bc6541d547909f6 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Fri, 15 Mar 2024 12:21:01 -0400 Subject: [PATCH 5/8] disable flaky test --- MIGRATION.md | 2 ++ .../fetch/captureResponseBody/test.ts | 4 +++- packages/core/test/lib/base.test.ts | 3 --- packages/utils/src/time.ts | 8 -------- 4 files changed, 5 insertions(+), 12 deletions(-) diff --git a/MIGRATION.md b/MIGRATION.md index 0df7e781fe30..b1e951a63283 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -355,6 +355,8 @@ Removed top-level exports: `tracingOrigins`, `MetricsAggregator`, `metricsAggreg `Sentry.configureScope`, `Span`, `spanStatusfromHttpCode`, `makeMain`, `lastEventId`, `pushScope`, `popScope`, `addGlobalEventProcessor`, `timestampWithMs`, `addExtensionMethods` +Remove util exports: `timestampWithMs` + - [Deprecation of `Hub` and `getCurrentHub()`](./MIGRATION.md#deprecate-hub) - [Removal of class-based integrations](./MIGRATION.md#removal-of-class-based-integrations) - [`tracingOrigins` option replaced with `tracePropagationTargets`](./MIGRATION.md#tracingorigins-has-been-replaced-by-tracepropagationtargets) diff --git a/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseBody/test.ts b/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseBody/test.ts index c4607fa9cbf7..5e58b63218ef 100644 --- a/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseBody/test.ts +++ b/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseBody/test.ts @@ -254,7 +254,9 @@ sentryTest('captures non-text response body', async ({ getLocalTestPath, page, b ]); }); -sentryTest('does not capture response body when URL does not match', async ({ getLocalTestPath, page }) => { +// This test is flaky +// See: https://github.com/getsentry/sentry-javascript/issues/11136 +sentryTest.skip('does not capture response body when URL does not match', async ({ getLocalTestPath, page }) => { if (shouldSkipReplayTest()) { sentryTest.skip(); } diff --git a/packages/core/test/lib/base.test.ts b/packages/core/test/lib/base.test.ts index 6c7c8aa0d63d..6be7b6e097a2 100644 --- a/packages/core/test/lib/base.test.ts +++ b/packages/core/test/lib/base.test.ts @@ -42,9 +42,6 @@ jest.mock('@sentry/utils', () => { truncate(str: string): string { return str; }, - timestampWithMs(): number { - return 2020; - }, dateTimestampInSeconds(): number { return 2020; }, diff --git a/packages/utils/src/time.ts b/packages/utils/src/time.ts index 16c7058ae68c..df67493888e3 100644 --- a/packages/utils/src/time.ts +++ b/packages/utils/src/time.ts @@ -68,14 +68,6 @@ function createUnixTimestampInSecondsFunc(): () => number { */ export const timestampInSeconds = createUnixTimestampInSecondsFunc(); -/** - * Re-exported with an old name for backwards-compatibility. - * TODO (v8): Remove this - * - * @deprecated Use `timestampInSeconds` instead. - */ -export const timestampWithMs = timestampInSeconds; - /** * Internal helper to store what is the source of browserPerformanceTimeOrigin below. For debugging only. */ From 1b7cd0271c17a28cd2f0cbf40016e4a584abacfa Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Fri, 15 Mar 2024 12:39:04 -0400 Subject: [PATCH 6/8] skip another flaky test --- .../fetch/captureResponseSize/test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseSize/test.ts b/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseSize/test.ts index ad3aafe34562..955c94fa4e18 100644 --- a/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseSize/test.ts +++ b/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseSize/test.ts @@ -188,7 +188,9 @@ sentryTest('captures response size without Content-Length header', async ({ getL ]); }); -sentryTest('captures response size from non-text response body', async ({ getLocalTestPath, page }) => { +// This test is flaky so it's skipped for now +// See https://github.com/getsentry/sentry-javascript/issues/11137 +sentryTest.skip('captures response size from non-text response body', async ({ getLocalTestPath, page }) => { if (shouldSkipReplayTest()) { sentryTest.skip(); } From 46ed1a8095f3c2f1fa5253f477083a29bcbb7bed Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Fri, 15 Mar 2024 13:03:10 -0400 Subject: [PATCH 7/8] skip more flaky tests --- .../fetch/captureResponseHeaders/test.ts | 4 +++- .../suites/tracing/metrics/handlers-lcp/test.ts | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseHeaders/test.ts b/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseHeaders/test.ts index 8f098627c120..b7fe6aace1a4 100644 --- a/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseHeaders/test.ts +++ b/dev-packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseHeaders/test.ts @@ -157,7 +157,9 @@ sentryTest('captures response headers', async ({ getLocalTestPath, page }) => { ]); }); -sentryTest('does not capture response headers if URL does not match', async ({ getLocalTestPath, page }) => { +// This test is flaky so it's skipped for now +// See https://github.com/getsentry/sentry-javascript/issues/11139 +sentryTest.skip('does not capture response headers if URL does not match', async ({ getLocalTestPath, page }) => { if (shouldSkipReplayTest()) { sentryTest.skip(); } diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/handlers-lcp/test.ts b/dev-packages/browser-integration-tests/suites/tracing/metrics/handlers-lcp/test.ts index a2801f4e4016..1a056cf12114 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/metrics/handlers-lcp/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/handlers-lcp/test.ts @@ -7,7 +7,9 @@ import { getFirstSentryEnvelopeRequest, shouldSkipTracingTest } from '../../../. const bundle = process.env.PW_BUNDLE || ''; -sentryTest( +// This test is flaky so it's skipped for now +// See https://github.com/getsentry/sentry-javascript/issues/11138 +sentryTest.skip( 'should capture metrics for LCP instrumentation handlers', async ({ browserName, getLocalTestPath, page }) => { // This uses a utility that is not exported in CDN bundles From 8a95219b6f5bc3c8603f3a5a1fce179d04e3985c Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Fri, 15 Mar 2024 13:11:16 -0400 Subject: [PATCH 8/8] use large runner for playwright tests --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 27aae6342cef..fb95a14527e8 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -611,7 +611,7 @@ jobs: name: Playwright (${{ matrix.bundle }}${{ matrix.shard && format(' {0}/{1}', matrix.shard, matrix.shards) || ''}}) Tests needs: [job_get_metadata, job_build] if: needs.job_get_metadata.outputs.changed_browser_integration == 'true' || github.event_name != 'pull_request' - runs-on: ubuntu-20.04 + runs-on: ubuntu-20.04-large-js timeout-minutes: 25 strategy: fail-fast: false