Skip to content

Commit

Permalink
feat(sveltekit): Update default integration handling & deprecate `add…
Browse files Browse the repository at this point in the history
…OrUpdateIntegration` (#10263)

This updates the last usage of `addOrUpdateIntegration` and deprecates
it.
  • Loading branch information
mydea committed Jan 23, 2024
1 parent a682534 commit 62b0c4d
Show file tree
Hide file tree
Showing 11 changed files with 253 additions and 190 deletions.
14 changes: 14 additions & 0 deletions packages/sveltekit/src/client/browserTracingIntegration.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { BrowserTracing as OriginalBrowserTracing } from '@sentry/svelte';
import { svelteKitRoutingInstrumentation } from './router';

/**
* A custom BrowserTracing integration for Sveltekit.
*/
export class BrowserTracing extends OriginalBrowserTracing {
public constructor(options?: ConstructorParameters<typeof OriginalBrowserTracing>[0]) {
super({
routingInstrumentation: svelteKitRoutingInstrumentation,
...options,
});
}
}
67 changes: 49 additions & 18 deletions packages/sveltekit/src/client/sdk.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import { applySdkMetadata, hasTracingEnabled } from '@sentry/core';
import type { BrowserOptions } from '@sentry/svelte';
import { BrowserTracing, WINDOW, getCurrentScope, init as initSvelteSdk } from '@sentry/svelte';
import { addOrUpdateIntegration } from '@sentry/utils';
import { getDefaultIntegrations as getDefaultSvelteIntegrations } from '@sentry/svelte';
import { WINDOW, getCurrentScope, init as initSvelteSdk } from '@sentry/svelte';
import type { Integration } from '@sentry/types';

import { svelteKitRoutingInstrumentation } from './router';
import { BrowserTracing } from './browserTracingIntegration';

type WindowWithSentryFetchProxy = typeof WINDOW & {
_sentryFetchProxy?: typeof fetch;
Expand All @@ -18,15 +19,20 @@ declare const __SENTRY_TRACING__: boolean;
* @param options Configuration options for the SDK.
*/
export function init(options: BrowserOptions): void {
applySdkMetadata(options, 'sveltekit', ['sveltekit', 'svelte']);
const opts = {
defaultIntegrations: getDefaultIntegrations(options),
...options,
};

addClientIntegrations(options);
applySdkMetadata(opts, 'sveltekit', ['sveltekit', 'svelte']);

fixBrowserTracingIntegration(opts);

// 1. Switch window.fetch to our fetch proxy we injected earlier
const actualFetch = switchToFetchProxy();

// 2. Initialize the SDK which will instrument our proxy
initSvelteSdk(options);
initSvelteSdk(opts);

// 3. Restore the original fetch now that our proxy is instrumented
if (actualFetch) {
Expand All @@ -36,24 +42,49 @@ export function init(options: BrowserOptions): void {
getCurrentScope().setTag('runtime', 'browser');
}

function addClientIntegrations(options: BrowserOptions): void {
let integrations = options.integrations || [];
// TODO v8: Remove this again
// We need to handle BrowserTracing passed to `integrations` that comes from `@sentry/tracing`, not `@sentry/sveltekit` :(
function fixBrowserTracingIntegration(options: BrowserOptions): void {
const { integrations } = options;
if (!integrations) {
return;
}

if (Array.isArray(integrations)) {
options.integrations = maybeUpdateBrowserTracingIntegration(integrations);
} else {
options.integrations = defaultIntegrations => {
const userFinalIntegrations = integrations(defaultIntegrations);

return maybeUpdateBrowserTracingIntegration(userFinalIntegrations);
};
}
}

function maybeUpdateBrowserTracingIntegration(integrations: Integration[]): Integration[] {
const browserTracing = integrations.find(integration => integration.name === 'BrowserTracing');
// If BrowserTracing was added, but it is not our forked version,
// replace it with our forked version with the same options
if (browserTracing && !(browserTracing instanceof BrowserTracing)) {
const options: ConstructorParameters<typeof BrowserTracing>[0] = (browserTracing as BrowserTracing).options;
// This option is overwritten by the custom integration
delete options.routingInstrumentation;
integrations[integrations.indexOf(browserTracing)] = new BrowserTracing(options);
}

return integrations;
}

// This evaluates to true unless __SENTRY_TRACING__ is text-replaced with "false",
// in which case everything inside will get treeshaken away
function getDefaultIntegrations(options: BrowserOptions): Integration[] | undefined {
// This evaluates to true unless __SENTRY_TRACING__ is text-replaced with "false", in which case everything inside
// will get treeshaken away
if (typeof __SENTRY_TRACING__ === 'undefined' || __SENTRY_TRACING__) {
if (hasTracingEnabled(options)) {
const defaultBrowserTracingIntegration = new BrowserTracing({
routingInstrumentation: svelteKitRoutingInstrumentation,
});

integrations = addOrUpdateIntegration(defaultBrowserTracingIntegration, integrations, {
'options.routingInstrumentation': svelteKitRoutingInstrumentation,
});
return [...getDefaultSvelteIntegrations(options), new BrowserTracing()];
}
}

options.integrations = integrations;
return undefined;
}

/**
Expand Down
77 changes: 77 additions & 0 deletions packages/sveltekit/src/server/rewriteFramesIntegration.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
import { defineIntegration } from '@sentry/core';
import { rewriteFramesIntegration as originalRewriteFramesIntegration } from '@sentry/integrations';
import type { IntegrationFn, StackFrame } from '@sentry/types';
import { GLOBAL_OBJ, basename, escapeStringForRegex, join } from '@sentry/utils';
import { WRAPPED_MODULE_SUFFIX } from '../vite/autoInstrument';
import type { GlobalWithSentryValues } from '../vite/injectGlobalValues';

type StackFrameIteratee = (frame: StackFrame) => StackFrame;
interface RewriteFramesOptions {
root?: string;
prefix?: string;
iteratee?: StackFrameIteratee;
}

export const customRewriteFramesIntegration = ((options?: RewriteFramesOptions) => {
return originalRewriteFramesIntegration({
iteratee: rewriteFramesIteratee,
...options,
});
}) satisfies IntegrationFn;

export const rewriteFramesIntegration = defineIntegration(customRewriteFramesIntegration);

/**
* A custom iteratee function for the `RewriteFrames` integration.
*
* Does the same as the default iteratee, but also removes the `module` property from the
* frame to improve issue grouping.
*
* For some reason, our stack trace processing pipeline isn't able to resolve the bundled
* module name to the original file name correctly, leading to individual error groups for
* each module. Removing the `module` field makes the grouping algorithm fall back to the
* `filename` field, which is correctly resolved and hence grouping works as expected.
*
* Exported for tests only.
*/
export function rewriteFramesIteratee(frame: StackFrame): StackFrame {
if (!frame.filename) {
return frame;
}
const globalWithSentryValues: GlobalWithSentryValues = GLOBAL_OBJ;
const svelteKitBuildOutDir = globalWithSentryValues.__sentry_sveltekit_output_dir;
const prefix = 'app:///';

// Check if the frame filename begins with `/` or a Windows-style prefix such as `C:\`
const isWindowsFrame = /^[a-zA-Z]:\\/.test(frame.filename);
const startsWithSlash = /^\//.test(frame.filename);
if (isWindowsFrame || startsWithSlash) {
const filename = isWindowsFrame
? frame.filename
.replace(/^[a-zA-Z]:/, '') // remove Windows-style prefix
.replace(/\\/g, '/') // replace all `\\` instances with `/`
: frame.filename;

let strippedFilename;
if (svelteKitBuildOutDir) {
strippedFilename = filename.replace(
// eslint-disable-next-line @sentry-internal/sdk/no-regexp-constructor -- not end user input + escaped anyway
new RegExp(`^.*${escapeStringForRegex(join(svelteKitBuildOutDir, 'server'))}/`),
'',
);
} else {
strippedFilename = basename(filename);
}
frame.filename = `${prefix}${strippedFilename}`;
}

delete frame.module;

// In dev-mode, the WRAPPED_MODULE_SUFFIX is still present in the frame's file name.
// We need to remove it to make sure that the frame's filename matches the actual file
if (frame.filename.endsWith(WRAPPED_MODULE_SUFFIX)) {
frame.filename = frame.filename.slice(0, -WRAPPED_MODULE_SUFFIX.length);
}

return frame;
}
22 changes: 8 additions & 14 deletions packages/sveltekit/src/server/sdk.ts
Original file line number Diff line number Diff line change
@@ -1,29 +1,23 @@
import { applySdkMetadata, getCurrentScope } from '@sentry/core';
import { RewriteFrames } from '@sentry/integrations';
import type { NodeOptions } from '@sentry/node';
import { getDefaultIntegrations as getDefaultNodeIntegrations } from '@sentry/node';
import { init as initNodeSdk } from '@sentry/node';
import { addOrUpdateIntegration } from '@sentry/utils';

import { rewriteFramesIteratee } from './utils';
import { rewriteFramesIntegration } from './rewriteFramesIntegration';

/**
*
* @param options
*/
export function init(options: NodeOptions): void {
applySdkMetadata(options, 'sveltekit', ['sveltekit', 'node']);
const opts = {
defaultIntegrations: [...getDefaultNodeIntegrations(options), rewriteFramesIntegration()],
...options,
};

addServerIntegrations(options);
applySdkMetadata(opts, 'sveltekit', ['sveltekit', 'node']);

initNodeSdk(options);
initNodeSdk(opts);

getCurrentScope().setTag('runtime', 'node');
}

function addServerIntegrations(options: NodeOptions): void {
options.integrations = addOrUpdateIntegration(
// eslint-disable-next-line deprecation/deprecation
new RewriteFrames({ iteratee: rewriteFramesIteratee }),
options.integrations || [],
);
}
58 changes: 1 addition & 57 deletions packages/sveltekit/src/server/utils.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
import { flush } from '@sentry/node';
import type { StackFrame } from '@sentry/types';
import { GLOBAL_OBJ, basename, escapeStringForRegex, join, logger, tracingContextFromHeaders } from '@sentry/utils';
import { logger, tracingContextFromHeaders } from '@sentry/utils';
import type { RequestEvent } from '@sveltejs/kit';

import { DEBUG_BUILD } from '../common/debug-build';
import { WRAPPED_MODULE_SUFFIX } from '../vite/autoInstrument';
import type { GlobalWithSentryValues } from '../vite/injectGlobalValues';

/**
* Takes a request event and extracts traceparent and DSC data
Expand All @@ -19,59 +16,6 @@ export function getTracePropagationData(event: RequestEvent): ReturnType<typeof
return tracingContextFromHeaders(sentryTraceHeader, baggageHeader);
}

/**
* A custom iteratee function for the `RewriteFrames` integration.
*
* Does the same as the default iteratee, but also removes the `module` property from the
* frame to improve issue grouping.
*
* For some reason, our stack trace processing pipeline isn't able to resolve the bundled
* module name to the original file name correctly, leading to individual error groups for
* each module. Removing the `module` field makes the grouping algorithm fall back to the
* `filename` field, which is correctly resolved and hence grouping works as expected.
*/
export function rewriteFramesIteratee(frame: StackFrame): StackFrame {
if (!frame.filename) {
return frame;
}
const globalWithSentryValues: GlobalWithSentryValues = GLOBAL_OBJ;
const svelteKitBuildOutDir = globalWithSentryValues.__sentry_sveltekit_output_dir;
const prefix = 'app:///';

// Check if the frame filename begins with `/` or a Windows-style prefix such as `C:\`
const isWindowsFrame = /^[a-zA-Z]:\\/.test(frame.filename);
const startsWithSlash = /^\//.test(frame.filename);
if (isWindowsFrame || startsWithSlash) {
const filename = isWindowsFrame
? frame.filename
.replace(/^[a-zA-Z]:/, '') // remove Windows-style prefix
.replace(/\\/g, '/') // replace all `\\` instances with `/`
: frame.filename;

let strippedFilename;
if (svelteKitBuildOutDir) {
strippedFilename = filename.replace(
// eslint-disable-next-line @sentry-internal/sdk/no-regexp-constructor -- not end user input + escaped anyway
new RegExp(`^.*${escapeStringForRegex(join(svelteKitBuildOutDir, 'server'))}/`),
'',
);
} else {
strippedFilename = basename(filename);
}
frame.filename = `${prefix}${strippedFilename}`;
}

delete frame.module;

// In dev-mode, the WRAPPED_MODULE_SUFFIX is still present in the frame's file name.
// We need to remove it to make sure that the frame's filename matches the actual file
if (frame.filename.endsWith(WRAPPED_MODULE_SUFFIX)) {
frame.filename = frame.filename.slice(0, -WRAPPED_MODULE_SUFFIX.length);
}

return frame;
}

/** Flush the event queue to ensure that events get sent to Sentry before the response is finished and the lambda ends */
export async function flushIfServerless(): Promise<void> {
const platformSupportsStreaming = !process.env.LAMBDA_TASK_ROOT && !process.env.VERCEL;
Expand Down
12 changes: 0 additions & 12 deletions packages/sveltekit/test/client/sdk.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,7 @@ describe('Sentry client SDK', () => {
...tracingOptions,
});

const integrationsToInit = svelteInit.mock.calls[0][0].integrations;
const browserTracing = getClient<BrowserClient>()?.getIntegrationByName('BrowserTracing');

expect(integrationsToInit).toContainEqual(expect.objectContaining({ name: 'BrowserTracing' }));
expect(browserTracing).toBeDefined();
});

Expand All @@ -77,10 +74,7 @@ describe('Sentry client SDK', () => {
...tracingOptions,
});

const integrationsToInit = svelteInit.mock.calls[0][0].integrations;
const browserTracing = getClient<BrowserClient>()?.getIntegrationByName('BrowserTracing');

expect(integrationsToInit).not.toContainEqual(expect.objectContaining({ name: 'BrowserTracing' }));
expect(browserTracing).toBeUndefined();
});

Expand All @@ -96,10 +90,7 @@ describe('Sentry client SDK', () => {
enableTracing: true,
});

const integrationsToInit = svelteInit.mock.calls[0][0].integrations;
const browserTracing = getClient<BrowserClient>()?.getIntegrationByName('BrowserTracing');

expect(integrationsToInit).not.toContainEqual(expect.objectContaining({ name: 'BrowserTracing' }));
expect(browserTracing).toBeUndefined();

// @ts-expect-error this is fine in the test
Expand All @@ -113,12 +104,9 @@ describe('Sentry client SDK', () => {
enableTracing: true,
});

const integrationsToInit = svelteInit.mock.calls[0][0].integrations;

const browserTracing = getClient<BrowserClient>()?.getIntegrationByName('BrowserTracing') as BrowserTracing;
const options = browserTracing.options;

expect(integrationsToInit).toContainEqual(expect.objectContaining({ name: 'BrowserTracing' }));
expect(browserTracing).toBeDefined();

// This shows that the user-configured options are still here
Expand Down

0 comments on commit 62b0c4d

Please sign in to comment.