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(sveltekit): Update default integration handling & deprecate addOrUpdateIntegration #10263

Merged
merged 3 commits into from
Jan 23, 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
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