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(svelte)!: Disable component update tracking by default #15265

Merged
merged 6 commits into from
Feb 4, 2025
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
Original file line number Diff line number Diff line change
Expand Up @@ -109,30 +109,6 @@ test.describe('client-specific performance events', () => {
op: 'ui.svelte.init',
origin: 'auto.ui.svelte',
}),
expect.objectContaining({
data: { 'sentry.op': 'ui.svelte.update', 'sentry.origin': 'auto.ui.svelte' },
description: '<components/+page>',
op: 'ui.svelte.update',
origin: 'auto.ui.svelte',
}),
expect.objectContaining({
data: { 'sentry.op': 'ui.svelte.update', 'sentry.origin': 'auto.ui.svelte' },
description: '<Component1>',
op: 'ui.svelte.update',
origin: 'auto.ui.svelte',
}),
expect.objectContaining({
data: { 'sentry.op': 'ui.svelte.update', 'sentry.origin': 'auto.ui.svelte' },
description: '<Component2>',
op: 'ui.svelte.update',
origin: 'auto.ui.svelte',
}),
expect.objectContaining({
data: { 'sentry.op': 'ui.svelte.update', 'sentry.origin': 'auto.ui.svelte' },
description: '<Component3>',
op: 'ui.svelte.update',
origin: 'auto.ui.svelte',
}),
]),
);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import Component2 from "./Component2.svelte";
import Component3 from "./Component3.svelte";

Sentry.trackComponent({componentName: 'components/+page'})
Sentry.trackComponent({componentName: 'components/+page', trackUpdates: true})

</script>
<h2>Demonstrating Component Tracking</h2>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import Component2 from "./Component2.svelte";
import {trackComponent} from '@sentry/sveltekit';

trackComponent({componentName: 'Component1'});
trackComponent({componentName: 'Component1', trackUpdates: true});

</script>
<h3>Howdy, I'm component 1</h3>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import Component3 from "./Component3.svelte";
import {trackComponent} from '@sentry/sveltekit';

trackComponent({componentName: 'Component2'});
trackComponent({componentName: 'Component2', trackUpdates: true});
</script>
<h3>Howdy, I'm component 2</h3>

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<script lang="ts">
import * as Sentry from '@sentry/sveltekit';
Sentry.trackComponent({componentName: 'Component3'});
Sentry.trackComponent({componentName: 'Component3', trackUpdates: true});
</script>

<h3>Howdy, I'm component 3</h3>
33 changes: 13 additions & 20 deletions packages/svelte/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import type { PreprocessorGroup } from 'svelte/types/compiler/preprocess';
import { componentTrackingPreprocessor, defaultComponentTrackingOptions } from './preprocessors';
import type { SentryPreprocessorGroup, SentrySvelteConfigOptions, SvelteConfig } from './types';

const DEFAULT_SENTRY_OPTIONS: SentrySvelteConfigOptions = {
const defaultSentryOptions: SentrySvelteConfigOptions = {
componentTracking: defaultComponentTrackingOptions,
};

Expand All @@ -20,32 +20,25 @@ export function withSentryConfig(
sentryOptions?: SentrySvelteConfigOptions,
): SvelteConfig {
const mergedOptions = {
...DEFAULT_SENTRY_OPTIONS,
...defaultSentryOptions,
...sentryOptions,
componentTracking: {
...defaultSentryOptions.componentTracking,
...sentryOptions?.componentTracking,
},
};

const originalPreprocessors = getOriginalPreprocessorArray(originalConfig);

// Map is insertion-order-preserving. It's important to add preprocessors
// to this map in the right order we want to see them being executed.
// see: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map
const sentryPreprocessors = new Map<string, SentryPreprocessorGroup>();

const shouldTrackComponents = mergedOptions.componentTracking?.trackComponents;
if (shouldTrackComponents) {
const firstPassPreproc: SentryPreprocessorGroup = componentTrackingPreprocessor(mergedOptions.componentTracking);
sentryPreprocessors.set(firstPassPreproc.sentryId || '', firstPassPreproc);
// Bail if users already added the preprocessor
if (originalPreprocessors.find((p: PreprocessorGroup) => !!(p as SentryPreprocessorGroup).sentryId)) {
return originalConfig;
}

// We prioritize user-added preprocessors, so we don't insert sentry processors if they
// have already been added by users.
originalPreprocessors.forEach((p: SentryPreprocessorGroup) => {
if (p.sentryId) {
sentryPreprocessors.delete(p.sentryId);
}
});

const mergedPreprocessors = [...sentryPreprocessors.values(), ...originalPreprocessors];
const mergedPreprocessors = [...originalPreprocessors];
if (mergedOptions.componentTracking.trackComponents) {
mergedPreprocessors.unshift(componentTrackingPreprocessor(mergedOptions.componentTracking));
}

return {
...originalConfig,
Expand Down
5 changes: 0 additions & 5 deletions packages/svelte/src/constants.ts

This file was deleted.

19 changes: 12 additions & 7 deletions packages/svelte/src/performance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/browser';
import type { Span } from '@sentry/core';
import { afterUpdate, beforeUpdate, onMount } from 'svelte';

import { startInactiveSpan } from '@sentry/core';
import { DEFAULT_COMPONENT_NAME, UI_SVELTE_INIT, UI_SVELTE_UPDATE } from './constants';
import { logger, startInactiveSpan } from '@sentry/core';
import type { TrackComponentOptions } from './types';

const defaultTrackComponentOptions: {
Expand All @@ -12,7 +11,7 @@ const defaultTrackComponentOptions: {
componentName?: string;
} = {
trackInit: true,
trackUpdates: true,
trackUpdates: false,
};

/**
Expand All @@ -29,21 +28,27 @@ export function trackComponent(options?: TrackComponentOptions): void {

const customComponentName = mergedOptions.componentName;

const componentName = `<${customComponentName || DEFAULT_COMPONENT_NAME}>`;
const componentName = `<${customComponentName || 'Svelte Component'}>`;

if (mergedOptions.trackInit) {
recordInitSpan(componentName);
}

if (mergedOptions.trackUpdates) {
recordUpdateSpans(componentName);
try {
recordUpdateSpans(componentName);
} catch {
logger.warn(
"Cannot track component updates. This is likely because you're using Svelte 5 in Runes mode. Set `trackUpdates: false` in `withSentryConfig` or `trackComponent` to disable this warning.",
);
}
}
}

function recordInitSpan(componentName: string): void {
const initSpan = startInactiveSpan({
onlyIfParent: true,
op: UI_SVELTE_INIT,
op: 'ui.svelte.init',
name: componentName,
attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.svelte' },
});
Expand All @@ -58,7 +63,7 @@ function recordUpdateSpans(componentName: string): void {
beforeUpdate(() => {
updateSpan = startInactiveSpan({
onlyIfParent: true,
op: UI_SVELTE_UPDATE,
op: 'ui.svelte.update',
name: componentName,
attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.svelte' },
});
Expand Down
2 changes: 1 addition & 1 deletion packages/svelte/src/preprocessors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import type { ComponentTrackingInitOptions, SentryPreprocessorGroup, TrackCompon
export const defaultComponentTrackingOptions: Required<ComponentTrackingInitOptions> = {
trackComponents: true,
trackInit: true,
trackUpdates: true,
trackUpdates: false,
};

export const FIRST_PASS_COMPONENT_TRACKING_PREPROC_ID = 'FIRST_PASS_COMPONENT_TRACKING_PREPROCESSOR';
Expand Down
7 changes: 5 additions & 2 deletions packages/svelte/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,18 @@ export type SpanOptions = {
* onMount lifecycle hook. This span tells how long it takes a component
* to be created and inserted into the DOM.
*
* Defaults to true if component tracking is enabled
* @default `true` if component tracking is enabled
*/
trackInit?: boolean;

/**
* If true, a span is recorded between a component's beforeUpdate and afterUpdate
* lifecycle hooks.
*
* Defaults to true if component tracking is enabled
* Caution: Component updates can only be tracked in Svelte versions prior to version 5
* or in Svelte 5 in legacy mode (i.e. without Runes).
*
* @default `false` if component tracking is enabled
*/
trackUpdates?: boolean;
};
Expand Down
2 changes: 1 addition & 1 deletion packages/svelte/test/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ describe('withSentryConfig', () => {

const wrappedConfig = withSentryConfig(originalConfig);

expect(wrappedConfig).toEqual({ ...originalConfig, preprocess: [sentryPreproc] });
expect(wrappedConfig).toEqual({ ...originalConfig });
});

it('handles multiple wraps correctly by only adding our preprocessors once', () => {
Expand Down
40 changes: 15 additions & 25 deletions packages/svelte/test/performance.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import { getClient, getCurrentScope, getIsolationScope, init, startSpan } from '

import type { TransactionEvent } from '@sentry/core';

// @ts-expect-error svelte import
import DummyComponent from './components/Dummy.svelte';

const PUBLIC_DSN = 'https://username@domain/123';
Expand Down Expand Up @@ -37,7 +36,7 @@ describe('Sentry.trackComponent()', () => {
});
});

it('creates init and update spans on component initialization', async () => {
it('creates init spans on component initialization by default', async () => {
startSpan({ name: 'outer' }, span => {
expect(span).toBeDefined();
render(DummyComponent, { props: { options: {} } });
Expand All @@ -47,7 +46,7 @@ describe('Sentry.trackComponent()', () => {

expect(transactions).toHaveLength(1);
const transaction = transactions[0]!;
expect(transaction.spans).toHaveLength(2);
expect(transaction.spans).toHaveLength(1);

const rootSpanId = transaction.contexts?.trace?.span_id;
expect(rootSpanId).toBeDefined();
Expand All @@ -68,29 +67,14 @@ describe('Sentry.trackComponent()', () => {
timestamp: expect.any(Number),
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
});

expect(transaction.spans![1]).toEqual({
data: {
'sentry.op': 'ui.svelte.update',
'sentry.origin': 'auto.ui.svelte',
},
description: '<Svelte Component>',
op: 'ui.svelte.update',
origin: 'auto.ui.svelte',
parent_span_id: rootSpanId,
span_id: expect.stringMatching(/[a-f0-9]{16}/),
start_timestamp: expect.any(Number),
timestamp: expect.any(Number),
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
});
});

it('creates an update span, when the component is updated', async () => {
it('creates an update span, if `trackUpdates` is `true`', async () => {
startSpan({ name: 'outer' }, async span => {
expect(span).toBeDefined();

// first we create the component
const { component } = render(DummyComponent, { props: { options: {} } });
const { component } = render(DummyComponent, { props: { options: { trackUpdates: true } } });

// then trigger an update
// (just changing the trackUpdates prop so that we trigger an update. #
Expand Down Expand Up @@ -175,7 +159,7 @@ describe('Sentry.trackComponent()', () => {
startSpan({ name: 'outer' }, span => {
expect(span).toBeDefined();

render(DummyComponent, { props: { options: { trackInit: false } } });
render(DummyComponent, { props: { options: { trackInit: false, trackUpdates: true } } });
});

await getClient()?.flush();
Expand Down Expand Up @@ -206,7 +190,13 @@ describe('Sentry.trackComponent()', () => {
expect(span).toBeDefined();

render(DummyComponent, {
props: { options: { componentName: 'CustomComponentName' } },
props: {
options: {
componentName: 'CustomComponentName',
// enabling updates to check for both span names in one test
trackUpdates: true,
},
},
});
});

Expand All @@ -220,7 +210,7 @@ describe('Sentry.trackComponent()', () => {
expect(transaction.spans![1]?.description).toEqual('<CustomComponentName>');
});

it("doesn't do anything, if there's no ongoing transaction", async () => {
it("doesn't do anything, if there's no ongoing parent span", async () => {
render(DummyComponent, {
props: { options: { componentName: 'CustomComponentName' } },
});
Expand All @@ -230,11 +220,11 @@ describe('Sentry.trackComponent()', () => {
expect(transactions).toHaveLength(0);
});

it("doesn't record update spans, if there's no ongoing root span at that time", async () => {
it("doesn't record update spans, if there's no ongoing parent span at that time", async () => {
const component = startSpan({ name: 'outer' }, span => {
expect(span).toBeDefined();

const { component } = render(DummyComponent, { props: { options: {} } });
const { component } = render(DummyComponent, { props: { options: { trackUpdates: true } } });
return component;
});

Expand Down
12 changes: 6 additions & 6 deletions packages/svelte/test/preprocessors.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ function expectComponentCodeToBeModified(
preprocessedComponents.forEach(cmp => {
const expectedFunctionCallOptions = {
trackInit: options?.trackInit ?? true,
trackUpdates: options?.trackUpdates ?? true,
trackUpdates: options?.trackUpdates ?? false,
componentName: cmp.name,
};
const expectedFunctionCall = `trackComponent(${JSON.stringify(expectedFunctionCallOptions)});\n`;
Expand Down Expand Up @@ -115,7 +115,7 @@ describe('componentTrackingPreprocessor', () => {

expect(cmp2?.newCode).toEqual(cmp2?.originalCode);

expectComponentCodeToBeModified([cmp1!, cmp3!], { trackInit: true, trackUpdates: true });
expectComponentCodeToBeModified([cmp1!, cmp3!], { trackInit: true, trackUpdates: false });
});

it('doesnt inject the function call to the same component more than once', () => {
Expand Down Expand Up @@ -149,7 +149,7 @@ describe('componentTrackingPreprocessor', () => {
return { ...cmp, newCode: res.code, map: res.map };
});

expectComponentCodeToBeModified([cmp11!, cmp2!], { trackInit: true, trackUpdates: true });
expectComponentCodeToBeModified([cmp11!, cmp2!], { trackInit: true });
expect(cmp12!.newCode).toEqual(cmp12!.originalCode);
});

Expand Down Expand Up @@ -228,7 +228,7 @@ describe('componentTrackingPreprocessor', () => {

expect(processedCode.code).toEqual(
'<script>import { trackComponent } from "@sentry/svelte";\n' +
'trackComponent({"trackInit":true,"trackUpdates":true,"componentName":"Cmp1"});\n\n' +
'trackComponent({"trackInit":true,"trackUpdates":false,"componentName":"Cmp1"});\n\n' +
'</script>\n' +
"<p>I'm just a plain component</p>\n" +
'<style>p{margin-top:10px}</style>',
Expand All @@ -248,7 +248,7 @@ describe('componentTrackingPreprocessor', () => {

expect(processedCode.code).toEqual(
'<script>import { trackComponent } from "@sentry/svelte";\n' +
'trackComponent({"trackInit":true,"trackUpdates":true,"componentName":"Cmp2"});\n' +
'trackComponent({"trackInit":true,"trackUpdates":false,"componentName":"Cmp2"});\n' +
"console.log('hi');</script>\n" +
"<p>I'm a component with a script</p>\n" +
'<style>p{margin-top:10px}</style>',
Expand All @@ -267,7 +267,7 @@ describe('componentTrackingPreprocessor', () => {

expect(processedCode.code).toEqual(
'<script>import { trackComponent } from "@sentry/svelte";\n' +
'trackComponent({"trackInit":true,"trackUpdates":true,"componentName":"unknown"});\n' +
'trackComponent({"trackInit":true,"trackUpdates":false,"componentName":"unknown"});\n' +
"console.log('hi');</script>",
);
});
Expand Down
Loading