Skip to content

feat(sveltekit): Automatically add BrowserTracing integration #7528

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

Merged
merged 3 commits into from
Mar 21, 2023
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/nextjs/src/client/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@ import type { BrowserOptions } from '@sentry/react';
import { configureScope, init as reactInit, Integrations } from '@sentry/react';
import { BrowserTracing, defaultRequestInstrumentationOptions } from '@sentry/tracing';
import type { EventProcessor } from '@sentry/types';
import { addOrUpdateIntegration } from '@sentry/utils';

import { devErrorSymbolicationEventProcessor } from '../common/devErrorSymbolicationEventProcessor';
import { getVercelEnv } from '../common/getVercelEnv';
import { buildMetadata } from '../common/metadata';
import { addOrUpdateIntegration } from '../common/userIntegrations';
import { nextRouterInstrumentation } from './performance';
import { applyTunnelRouteOption } from './tunnelRoute';

Expand Down
5 changes: 2 additions & 3 deletions packages/nextjs/src/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,14 @@ import { RewriteFrames } from '@sentry/integrations';
import type { NodeOptions } from '@sentry/node';
import { configureScope, getCurrentHub, init as nodeInit, Integrations } from '@sentry/node';
import type { EventProcessor } from '@sentry/types';
import { escapeStringForRegex, logger } from '@sentry/utils';
import type { IntegrationWithExclusionOption } from '@sentry/utils';
import { addOrUpdateIntegration, escapeStringForRegex, logger } from '@sentry/utils';
import * as domainModule from 'domain';
import * as path from 'path';

import { devErrorSymbolicationEventProcessor } from '../common/devErrorSymbolicationEventProcessor';
import { getVercelEnv } from '../common/getVercelEnv';
import { buildMetadata } from '../common/metadata';
import type { IntegrationWithExclusionOption } from '../common/userIntegrations';
import { addOrUpdateIntegration } from '../common/userIntegrations';
import { isBuild } from './utils/isBuild';

export * from '@sentry/node';
Expand Down
2 changes: 1 addition & 1 deletion packages/nextjs/test/clientSdk.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@ import * as SentryReact from '@sentry/react';
import { WINDOW } from '@sentry/react';
import { Integrations as TracingIntegrations } from '@sentry/tracing';
import type { Integration } from '@sentry/types';
import type { UserIntegrationsFunction } from '@sentry/utils';
import { logger } from '@sentry/utils';
import { JSDOM } from 'jsdom';

import { init, Integrations, nextRouterInstrumentation } from '../src/client';
import type { UserIntegrationsFunction } from '../src/common/userIntegrations';

const { BrowserTracing } = TracingIntegrations;

Expand Down
1 change: 1 addition & 0 deletions packages/sveltekit/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
"@sentry/svelte": "7.44.1",
"@sentry/types": "7.44.1",
"@sentry/utils": "7.44.1",
"@sentry-internal/tracing": "7.44.1",
"magic-string": "^0.30.0"
},
"devDependencies": {
Expand Down
33 changes: 32 additions & 1 deletion packages/sveltekit/src/client/sdk.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,49 @@
import { defaultRequestInstrumentationOptions } from '@sentry-internal/tracing';
import { hasTracingEnabled } from '@sentry/core';
import type { BrowserOptions } from '@sentry/svelte';
import { configureScope, init as initSvelteSdk } from '@sentry/svelte';
import { BrowserTracing, configureScope, init as initSvelteSdk } from '@sentry/svelte';
import { addOrUpdateIntegration } from '@sentry/utils';

import { applySdkMetadata } from '../common/metadata';

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

/**
*
* @param options
*/
export function init(options: BrowserOptions): void {
applySdkMetadata(options, ['sveltekit', 'svelte']);

addClientIntegrations(options);

initSvelteSdk(options);

configureScope(scope => {
scope.setTag('runtime', 'browser');
});
}

function addClientIntegrations(options: BrowserOptions): void {
let integrations = options.integrations || [];

// 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({
tracePropagationTargets: [...defaultRequestInstrumentationOptions.tracePropagationTargets],
// TODO: Add SvelteKit router instrumentations
// routingInstrumentation: sveltekitRoutingInstrumentation,
});

integrations = addOrUpdateIntegration(defaultBrowserTracingIntegration, integrations, {
// TODO: Add SvelteKit router instrumentations
// options.routingInstrumentation: sveltekitRoutingInstrumentation,
});
}
}

options.integrations = integrations;
}
75 changes: 74 additions & 1 deletion packages/sveltekit/test/client/sdk.test.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import { getCurrentHub } from '@sentry/core';
import type { BrowserClient } from '@sentry/svelte';
import * as SentrySvelte from '@sentry/svelte';
import { SDK_VERSION, WINDOW } from '@sentry/svelte';
import { vi } from 'vitest';

import { init } from '../../src/client/sdk';
import { BrowserTracing, init } from '../../src/client';

const svelteInit = vi.spyOn(SentrySvelte, 'init');

Expand Down Expand Up @@ -47,5 +48,77 @@ describe('Sentry client SDK', () => {
// @ts-ignore need access to protected _tags attribute
expect(currentScope._tags).toEqual({ runtime: 'browser' });
});

describe('automatically added integrations', () => {
it.each([
['tracesSampleRate', { tracesSampleRate: 0 }],
['tracesSampler', { tracesSampler: () => 1.0 }],
['enableTracing', { enableTracing: true }],
])('adds the BrowserTracing integration if tracing is enabled via %s', (_, tracingOptions) => {
init({
dsn: 'https://[email protected]/1337',
...tracingOptions,
});

const integrationsToInit = svelteInit.mock.calls[0][0].integrations;
const browserTracing = (getCurrentHub().getClient() as BrowserClient)?.getIntegrationById('BrowserTracing');

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

it.each([
['enableTracing', { enableTracing: false }],
['no tracing option set', {}],
])("doesn't add the BrowserTracing integration if tracing is disabled via %s", (_, tracingOptions) => {
init({
dsn: 'https://[email protected]/1337',
...tracingOptions,
});

const integrationsToInit = svelteInit.mock.calls[0][0].integrations;
const browserTracing = (getCurrentHub().getClient() as BrowserClient)?.getIntegrationById('BrowserTracing');

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

it("doesn't add the BrowserTracing integration if `__SENTRY_TRACING__` is set to false", () => {
// This is the closest we can get to unit-testing the `__SENTRY_TRACING__` tree-shaking guard
// IRL, the code to add the integration would most likely be removed by the bundler.

globalThis.__SENTRY_TRACING__ = false;

init({
dsn: 'https://[email protected]/1337',
enableTracing: true,
});

const integrationsToInit = svelteInit.mock.calls[0][0].integrations;
const browserTracing = (getCurrentHub().getClient() as BrowserClient)?.getIntegrationById('BrowserTracing');

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

delete globalThis.__SENTRY_TRACING__;
});

// TODO: this test is only meaningful once we have a routing instrumentation which we always want to add
// to a user-provided BrowserTracing integration (see NextJS SDK)
it.skip('Merges the user-provided BrowserTracing integration with the automatically added one', () => {
init({
dsn: 'https://[email protected]/1337',
integrations: [new BrowserTracing({ tracePropagationTargets: ['myDomain.com'] })],
enableTracing: true,
});

const integrationsToInit = svelteInit.mock.calls[0][0].integrations;
const browserTracing = (getCurrentHub().getClient() as BrowserClient)?.getIntegrationById('BrowserTracing');

expect(integrationsToInit).toContainEqual(expect.objectContaining({ name: 'BrowserTracing' }));
expect(browserTracing).toBeDefined();
expect((browserTracing as BrowserTracing).options.tracePropagationTargets).toEqual(['myDomain.com']);
});
});
});
});
1 change: 1 addition & 0 deletions packages/utils/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,4 @@ export * from './clientreport';
export * from './ratelimit';
export * from './baggage';
export * from './url';
export * from './userIntegrations';
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,6 @@ import type { Integration } from '@sentry/types';

export type UserIntegrationsFunction = (integrations: Integration[]) => Integration[];
export type UserIntegrations = Integration[] | UserIntegrationsFunction;

type ForcedIntegrationOptions = {
[keyPath: string]: unknown;
};

export type IntegrationWithExclusionOption = Integration & {
/**
* Allow the user to exclude this integration by not returning it from a function provided as the `integrations` option
Expand All @@ -16,6 +11,10 @@ export type IntegrationWithExclusionOption = Integration & {
allowExclusionByUser?: boolean;
};

type ForcedIntegrationOptions = {
[keyPath: string]: unknown;
};

/**
* Recursively traverses an object to update an existing nested key.
* Note: The provided key path must include existing properties,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
import type {
IntegrationWithExclusionOption as Integration,
UserIntegrations,
} from '../../src/common/userIntegrations';
import { addOrUpdateIntegration } from '../../src/common/userIntegrations';
import type { IntegrationWithExclusionOption as Integration, UserIntegrations } from '../src/userIntegrations';
import { addOrUpdateIntegration } from '../src/userIntegrations';

type MockIntegrationOptions = {
name: string;
Expand Down