Skip to content

Commit

Permalink
feat(svelte)!: Disable component update tracking by default (#15265)
Browse files Browse the repository at this point in the history
Due to a change in the lifecycle of Svelte
components in Svelte 5 (using Rune mode), our SDK can no longer leverage
the `(before|after)Update` hooks to track component update spans.
For v9, this patch therefore disables update tracking by default.
  • Loading branch information
Lms24 authored Feb 4, 2025
1 parent 3c4df06 commit bb7237a
Show file tree
Hide file tree
Showing 13 changed files with 57 additions and 95 deletions.
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

0 comments on commit bb7237a

Please sign in to comment.