Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(v8): Remove deprecated integration methods on client #11134

Merged
merged 8 commits into from
Mar 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions MIGRATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
24 changes: 0 additions & 24 deletions packages/core/src/baseclient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import type {
EventProcessor,
FeedbackEvent,
Integration,
IntegrationClass,
Outcome,
ParameterizedString,
SdkMetadata,
Expand Down Expand Up @@ -317,16 +316,6 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
}
}

/**
* 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.
*
Expand All @@ -336,19 +325,6 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
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<T extends Integration>(integration: IntegrationClass<T>): 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
*/
Expand Down
3 changes: 1 addition & 2 deletions packages/core/src/hub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<T>(integration.id) || null;
} catch (_oO) {
DEBUG_BUILD && logger.warn(`Cannot retrieve integration ${integration.id} from the current Hub`);
return null;
Expand Down
3 changes: 0 additions & 3 deletions packages/core/test/lib/base.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,6 @@ jest.mock('@sentry/utils', () => {
truncate(str: string): string {
return str;
},
timestampWithMs(): number {
return 2020;
},
dateTimestampInSeconds(): number {
return 2020;
},
Expand Down
9 changes: 3 additions & 6 deletions packages/node/src/integrations/undici/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

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

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

Expand Down
3 changes: 1 addition & 2 deletions packages/opentelemetry/src/custom/getCurrentHub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,7 @@ export function getCurrentHub(): Hub {
setContext,

getIntegration<T extends Integration>(integration: IntegrationClass<T>): T | null {
// eslint-disable-next-line deprecation/deprecation
return getClient()?.getIntegration(integration) || null;
return getClient()?.getIntegrationByName<T>(integration.id) || null;
},

startSession,
Expand Down
2 changes: 0 additions & 2 deletions packages/sveltekit/src/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ export {
getIsolationScope,
Hub,
NodeClient,
// eslint-disable-next-line deprecation/deprecation
makeMain,
setCurrentClient,
Scope,
SDK_VERSION,
Expand Down
8 changes: 1 addition & 7 deletions packages/types/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -127,12 +127,6 @@ export interface Client<O extends ClientOptions = ClientOptions> {
*/
getEventProcessors(): EventProcessor[];

/**
* Returns the client's instance of the given integration class, it any.
* @deprecated Use `getIntegrationByName()` instead.
*/
getIntegration<T extends Integration>(integration: IntegrationClass<T>): T | null;

/** Get the instance of the integration with the given name on the client, if it was added. */
getIntegrationByName<T extends Integration = Integration>(name: string): T | undefined;

Expand Down
8 changes: 0 additions & 8 deletions packages/utils/src/time.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down