Skip to content

Commit

Permalink
feat(nextjs): Make tracing package treeshakable (#5166)
Browse files Browse the repository at this point in the history
  • Loading branch information
lforst authored and AbhiPrasad committed May 30, 2022
1 parent e76c247 commit 0e1f578
Show file tree
Hide file tree
Showing 8 changed files with 70 additions and 28 deletions.
2 changes: 2 additions & 0 deletions MIGRATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,8 @@ For our efforts to reduce bundle size of the SDK we had to remove and refactor p
- Removed `eventStatusFromHttpCode` to save on bundle size.
- Replace `BrowserTracing` `maxTransactionDuration` option with `finalTimeout` option
- Removed `ignoreSentryErrors` option from AWS lambda SDK. Errors originating from the SDK will now *always* be caught internally.
- Removed `Integrations.BrowserTracing` export from `@sentry/nextjs`. Please import `BrowserTracing` from `@sentry/nextjs` directly.
- Removed static `id` property from `BrowserTracing` integration.

## Sentry Angular SDK Changes

Expand Down
17 changes: 16 additions & 1 deletion packages/ember/addon/instance-initializers/sentry-performance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,22 @@ export async function instrumentForPerformance(appInstance: ApplicationInstance)
}),
];

if (isTesting() && Sentry.getCurrentHub()?.getIntegration(tracing.Integrations.BrowserTracing)) {
class FakeBrowserTracingClass {
static id = tracing.BROWSER_TRACING_INTEGRATION_ID;
public name = FakeBrowserTracingClass.id;
setupOnce() {
// noop - We're just faking this class for a lookup
}
}

if (
isTesting() &&
Sentry.getCurrentHub()?.getIntegration(
// This is a temporary hack because the BrowserTracing integration cannot have a static `id` field for tree
// shaking reasons. However, `getIntegration` needs that field.
FakeBrowserTracingClass,
)
) {
// Initializers are called more than once in tests, causing the integrations to not be setup correctly.
return;
}
Expand Down
35 changes: 19 additions & 16 deletions packages/nextjs/src/index.client.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { configureScope, init as reactInit, Integrations as BrowserIntegrations } from '@sentry/react';
import { configureScope, init as reactInit, Integrations } from '@sentry/react';
import { BrowserTracing, defaultRequestInstrumentationOptions } from '@sentry/tracing';
import { EventProcessor } from '@sentry/types';

Expand All @@ -10,12 +10,8 @@ import { addIntegration, UserIntegrations } from './utils/userIntegrations';
export * from '@sentry/react';
export { nextRouterInstrumentation } from './performance/client';

export const Integrations = { ...BrowserIntegrations, BrowserTracing };
export { Integrations };

// This is already exported as part of `Integrations` above (and for the moment will remain so for
// backwards compatibility), but that interferes with treeshaking, so we also export it separately
// here.
//
// Previously we expected users to import `BrowserTracing` like this:
//
// import { Integrations } from '@sentry/nextjs';
Expand All @@ -28,16 +24,23 @@ export const Integrations = { ...BrowserIntegrations, BrowserTracing };
// const instance = new BrowserTracing();
export { BrowserTracing };

// Treeshakable guard to remove all code related to tracing
declare const __SENTRY_TRACING__: boolean;

/** Inits the Sentry NextJS SDK on the browser with the React SDK. */
export function init(options: NextjsOptions): void {
buildMetadata(options, ['nextjs', 'react']);
options.environment = options.environment || process.env.NODE_ENV;

// Only add BrowserTracing if a tracesSampleRate or tracesSampler is set
const integrations =
options.tracesSampleRate === undefined && options.tracesSampler === undefined
? options.integrations
: createClientIntegrations(options.integrations);
let integrations = options.integrations;

// Guard below evaluates to true unless __SENTRY_TRACING__ is text-replaced with "false"
if (typeof __SENTRY_TRACING__ === 'undefined' || __SENTRY_TRACING__) {
// Only add BrowserTracing if a tracesSampleRate or tracesSampler is set
if (options.tracesSampleRate !== undefined || options.tracesSampler !== undefined) {
integrations = createClientIntegrations(options.integrations);
}
}

reactInit({
...options,
Expand All @@ -53,12 +56,12 @@ export function init(options: NextjsOptions): void {
});
}

const defaultBrowserTracingIntegration = new BrowserTracing({
tracingOrigins: [...defaultRequestInstrumentationOptions.tracingOrigins, /^(api\/)/],
routingInstrumentation: nextRouterInstrumentation,
});

function createClientIntegrations(integrations?: UserIntegrations): UserIntegrations {
const defaultBrowserTracingIntegration = new BrowserTracing({
tracingOrigins: [...defaultRequestInstrumentationOptions.tracingOrigins, /^(api\/)/],
routingInstrumentation: nextRouterInstrumentation,
});

if (integrations) {
return addIntegration(defaultBrowserTracingIntegration, integrations, {
BrowserTracing: { keyPath: 'options.routingInstrumentation', value: nextRouterInstrumentation },
Expand Down
12 changes: 7 additions & 5 deletions packages/tracing/src/browser/browsertracing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import {
} from './request';
import { instrumentRoutingWithDefaults } from './router';

export const BROWSER_TRACING_INTEGRATION_ID = 'BrowserTracing';

/** Options for Browser Tracing integration */
export interface BrowserTracingOptions extends RequestInstrumentationOptions {
/**
Expand Down Expand Up @@ -111,18 +113,18 @@ const DEFAULT_BROWSER_TRACING_OPTIONS = {
* any routing library. This integration uses {@see IdleTransaction} to create transactions.
*/
export class BrowserTracing implements Integration {
/**
* @inheritDoc
*/
public static id: string = 'BrowserTracing';
// This class currently doesn't have a static `id` field like the other integration classes, because it prevented
// @sentry/tracing from being treeshaken. Tree shakers do not like static fields, because they behave like side effects.
// TODO: Come up with a better plan, than using static fields on integration classes, and use that plan on all
// integrations.

/** Browser Tracing integration options */
public options: BrowserTracingOptions;

/**
* @inheritDoc
*/
public name: string = BrowserTracing.id;
public name: string = BROWSER_TRACING_INTEGRATION_ID;

private _getCurrentHub?: () => Hub;

Expand Down
2 changes: 1 addition & 1 deletion packages/tracing/src/browser/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
export type { RequestInstrumentationOptions } from './request';

export { BrowserTracing } from './browsertracing';
export { BrowserTracing, BROWSER_TRACING_INTEGRATION_ID } from './browsertracing';
export { instrumentOutgoingRequests, defaultRequestInstrumentationOptions } from './request';
12 changes: 9 additions & 3 deletions packages/tracing/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export { Integrations };
// const instance = new BrowserTracing();
//
// For an example of of the new usage of BrowserTracing, see @sentry/nextjs index.client.ts
export { BrowserTracing } from './browser';
export { BrowserTracing, BROWSER_TRACING_INTEGRATION_ID } from './browser';

export { Span, spanStatusfromHttpCode } from './span';
// eslint-disable-next-line deprecation/deprecation
Expand All @@ -32,8 +32,14 @@ export { instrumentOutgoingRequests, defaultRequestInstrumentationOptions } from
export { IdleTransaction } from './idletransaction';
export { startIdleTransaction } from './hubextensions';

// We are patching the global object with our hub extension methods
addExtensionMethods();
// Treeshakable guard to remove all code related to tracing
declare const __SENTRY_TRACING__: boolean;

// Guard for tree
if (typeof __SENTRY_TRACING__ === 'undefined' || __SENTRY_TRACING__) {
// We are patching the global object with our hub extension methods
addExtensionMethods();
}

export { addExtensionMethods };

Expand Down
9 changes: 8 additions & 1 deletion packages/tracing/test/index.bundle.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,14 @@ import { Integrations } from '../src/index.bundle';
describe('Integrations export', () => {
it('is exported correctly', () => {
Object.keys(Integrations).forEach(key => {
expect(Integrations[key as keyof typeof Integrations].id).toStrictEqual(expect.any(String));
// Skip BrowserTracing because it doesn't have a static id field.
if (key === 'BrowserTracing') {
return;
}

expect(Integrations[key as keyof Omit<typeof Integrations, 'BrowserTracing'>].id).toStrictEqual(
expect.any(String),
);
});
});
});
9 changes: 8 additions & 1 deletion packages/tracing/test/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,14 @@ describe('index', () => {
describe('Integrations', () => {
it('is exported correctly', () => {
Object.keys(Integrations).forEach(key => {
expect(Integrations[key as keyof typeof Integrations].id).toStrictEqual(expect.any(String));
// Skip BrowserTracing because it doesn't have a static id field.
if (key === 'BrowserTracing') {
return;
}

expect(Integrations[key as keyof Omit<typeof Integrations, 'BrowserTracing'>].id).toStrictEqual(
expect.any(String),
);
});
});

Expand Down

0 comments on commit 0e1f578

Please sign in to comment.