Skip to content
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
42 changes: 42 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,48 @@

- "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott

## 10.8.0

### Important Changes

- **feat(sveltekit): Add Compatibility for builtin SvelteKit Tracing ([#17423](https://github.com/getsentry/sentry-javascript/pull/17423))**

Comment on lines +9 to +12
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

m: leftover

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This release makes the `@sentry/sveltekit` SDK compatible with SvelteKit's native [observability support](https://svelte.dev/docs/kit/observability) introduced in SvelteKit version `2.31.0`.
If you enable both, instrumentation and tracing, the SDK will now initialize early enough to set up additional instrumentation like database queries and it will pick up spans emitted from SvelteKit.

We will follow up with docs how to set up the SDK soon.
For now, If you're on SvelteKit version `2.31.0` or newer, you can easily opt into the new feature:

1. Enable [experimental tracing and instrumentation support](https://svelte.dev/docs/kit/observability) in `svelte.config.js`:
2. Move your `Sentry.init()` call from `src/hooks.server.(js|ts)` to the new `instrumentation.server.(js|ts)` file:

```ts
// instrumentation.server.ts
import * as Sentry from '@sentry/sveltekit';

Sentry.init({
dsn: '...',
// rest of your config
});
```

The rest of your Sentry config in `hooks.server.ts` (`sentryHandle` and `handleErrorWithSentry`) should stay the same.

If you prefer to stay on the hooks-file based config for now, the SDK will continue to work as previously.

Thanks to the Svelte team and @elliott-with-the-longest-name-on-github for implementing observability support and for reviewing our PR!

### Other Changes

- fix(react): Avoid multiple name updates on navigation spans ([#17438](https://github.com/getsentry/sentry-javascript/pull/17438))

<details>
<summary> <strong>Internal Changes</strong> </summary>

- test(profiling): Add tests for current state of profiling ([#17470](https://github.com/getsentry/sentry-javascript/pull/17470))

</details>

## 10.7.0

### Important Changes
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import * as Sentry from '@sentry/browser';
import { browserProfilingIntegration } from '@sentry/browser';

window.Sentry = Sentry;

Sentry.init({
dsn: 'https://[email protected]/1337',
integrations: [browserProfilingIntegration()],
tracesSampleRate: 1,
profilesSampleRate: 1,
});

function fibonacci(n) {
if (n <= 1) {
return n;
}
return fibonacci(n - 1) + fibonacci(n - 2);
}

await Sentry.startSpanManual({ name: 'root-fibonacci-2', parentSpan: null, forceTransaction: true }, async span => {
fibonacci(30);

// Timeout to prevent flaky tests. Integration samples every 20ms, if function is too fast it might not get sampled
await new Promise(resolve => setTimeout(resolve, 21));
span.end();
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
import { expect } from '@playwright/test';
import type { Event, Profile } from '@sentry/core';
import { sentryTest } from '../../../utils/fixtures';
import {
properEnvelopeRequestParser,
shouldSkipTracingTest,
waitForTransactionRequestOnUrl,
} from '../../../utils/helpers';

sentryTest(
'does not send profile envelope when document-policy is not set',
async ({ page, getLocalTestUrl, browserName }) => {
if (shouldSkipTracingTest() || browserName !== 'chromium') {
// Profiling only works when tracing is enabled
sentryTest.skip();
}

const url = await getLocalTestUrl({ testDir: __dirname });

const req = await waitForTransactionRequestOnUrl(page, url);
const transactionEvent = properEnvelopeRequestParser<Event>(req, 0);
const profileEvent = properEnvelopeRequestParser<Profile>(req, 1);

expect(transactionEvent).toBeDefined();
expect(profileEvent).toBeUndefined();
},
);

sentryTest('sends profile envelope in legacy mode', async ({ page, getLocalTestUrl, browserName }) => {
if (shouldSkipTracingTest() || browserName !== 'chromium') {
// Profiling only works when tracing is enabled
sentryTest.skip();
}

const url = await getLocalTestUrl({ testDir: __dirname, responseHeaders: { 'Document-Policy': 'js-profiling' } });

const req = await waitForTransactionRequestOnUrl(page, url);
const profileEvent = properEnvelopeRequestParser<Profile>(req, 1);
expect(profileEvent).toBeDefined();

const profile = profileEvent.profile;
expect(profileEvent.profile).toBeDefined();

expect(profile.samples).toBeDefined();
expect(profile.stacks).toBeDefined();
expect(profile.frames).toBeDefined();
expect(profile.thread_metadata).toBeDefined();

// Samples
expect(profile.samples.length).toBeGreaterThanOrEqual(2);
for (const sample of profile.samples) {
expect(typeof sample.elapsed_since_start_ns).toBe('string');
expect(sample.elapsed_since_start_ns).toMatch(/^\d+$/); // Numeric string
expect(parseInt(sample.elapsed_since_start_ns, 10)).toBeGreaterThanOrEqual(0);

expect(typeof sample.stack_id).toBe('number');
expect(sample.stack_id).toBeGreaterThanOrEqual(0);
expect(sample.thread_id).toBe('0'); // Should be main thread
}

// Stacks
expect(profile.stacks.length).toBeGreaterThan(0);
for (const stack of profile.stacks) {
expect(Array.isArray(stack)).toBe(true);
for (const frameIndex of stack) {
expect(typeof frameIndex).toBe('number');
expect(frameIndex).toBeGreaterThanOrEqual(0);
expect(frameIndex).toBeLessThan(profile.frames.length);
}
}

// Frames
expect(profile.frames.length).toBeGreaterThan(0);
for (const frame of profile.frames) {
expect(frame).toHaveProperty('function');
expect(frame).toHaveProperty('abs_path');
expect(frame).toHaveProperty('lineno');
expect(frame).toHaveProperty('colno');

expect(typeof frame.function).toBe('string');
expect(typeof frame.abs_path).toBe('string');
expect(typeof frame.lineno).toBe('number');
expect(typeof frame.colno).toBe('number');
}

const functionNames = profile.frames.map(frame => frame.function).filter(name => name !== '');

if ((process.env.PW_BUNDLE || '').endsWith('min')) {
// Function names are minified in minified bundles
expect(functionNames.length).toBeGreaterThan(0);
expect((functionNames as string[]).every(name => name?.length > 0)).toBe(true); // Just make sure they're not empty strings
} else {
expect(functionNames).toEqual(
expect.arrayContaining([
'_startRootSpan',
'withScope',
'createChildOrRootSpan',
'startSpanManual',
'startProfileForSpan',
'startJSSelfProfile',
]),
);
}

expect(profile.thread_metadata).toHaveProperty('0');
expect(profile.thread_metadata['0']).toHaveProperty('name');
expect(profile.thread_metadata['0'].name).toBe('main');

// Test that profile duration makes sense (should be > 20ms based on test setup)
const startTime = parseInt(profile.samples[0].elapsed_since_start_ns, 10);
const endTime = parseInt(profile.samples[profile.samples.length - 1].elapsed_since_start_ns, 10);
const durationNs = endTime - startTime;
const durationMs = durationNs / 1_000_000; // Convert ns to ms

// Should be at least 20ms based on our setTimeout(21) in the test
expect(durationMs).toBeGreaterThan(20);
});
13 changes: 11 additions & 2 deletions dev-packages/browser-integration-tests/utils/fixtures.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export type TestFixtures = {
skipRouteHandler?: boolean;
skipDsnRouteHandler?: boolean;
handleLazyLoadedFeedback?: boolean;
responseHeaders?: Record<string, string>;
}) => Promise<string>;
forceFlushReplay: () => Promise<string>;
enableConsole: () => void;
Expand All @@ -59,7 +60,13 @@ const sentryTest = base.extend<TestFixtures>({

getLocalTestUrl: ({ page }, use) => {
return use(
async ({ testDir, skipRouteHandler = false, skipDsnRouteHandler = false, handleLazyLoadedFeedback = false }) => {
async ({
testDir,
skipRouteHandler = false,
skipDsnRouteHandler = false,
handleLazyLoadedFeedback = false,
responseHeaders = {},
}) => {
const pagePath = `${TEST_HOST}/index.html`;

const tmpDir = path.join(testDir, 'dist', crypto.randomUUID());
Expand All @@ -86,7 +93,9 @@ const sentryTest = base.extend<TestFixtures>({
const file = route.request().url().split('/').pop();
const filePath = path.resolve(tmpDir, `./${file}`);

return fs.existsSync(filePath) ? route.fulfill({ path: filePath }) : route.continue();
return fs.existsSync(filePath)
? route.fulfill({ path: filePath, headers: responseHeaders })
: route.continue();
});

if (handleLazyLoadedFeedback) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ const IMPORTED_INTEGRATION_CDN_BUNDLE_PATHS: Record<string, string> = {
feedbackIntegration: 'feedback',
moduleMetadataIntegration: 'modulemetadata',
graphqlClientIntegration: 'graphqlclient',
browserProfilingIntegration: 'browserprofiling',
// technically, this is not an integration, but let's add it anyway for simplicity
makeMultiplexedTransport: 'multiplexedtransport',
};
Expand Down
9 changes: 7 additions & 2 deletions dev-packages/browser-integration-tests/utils/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export const envelopeParser = (request: Request | null): unknown[] => {
});
};

// Rather use the `properEnvelopeRequestParser`, as the `envelopeParser` does not follow the envelope spec.
export const envelopeRequestParser = <T = SentryEvent>(request: Request | null, envelopeIndex = 2): T => {
return envelopeParser(request)[envelopeIndex] as T;
};
Expand Down Expand Up @@ -79,8 +80,12 @@ function getEventAndTraceHeader(envelope: EventEnvelope): EventAndTraceHeader {
return [event, trace];
}

export const properEnvelopeRequestParser = <T = SentryEvent>(request: Request | null, envelopeIndex = 1): T => {
return properEnvelopeParser(request)[0]?.[envelopeIndex] as T;
export const properEnvelopeRequestParser = <T = SentryEvent>(
request: Request | null,
envelopeItemIndex: number,
envelopeIndex = 1, // 1 is usually the payload of the envelope (0 is the header)
): T => {
return properEnvelopeParser(request)[envelopeItemIndex]?.[envelopeIndex] as T;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Envelope Request Parser Parameter Shift

The properEnvelopeRequestParser function's signature changed, adding envelopeItemIndex as the second parameter. This redefines the meaning of the second argument for existing callers, shifting it from envelopeIndex to envelopeItemIndex. Consequently, the function now accesses [envelopeItemIndex]?.[1] instead of [0]?.[envelopeIndex], which will cause existing code to extract incorrect data or fail.

Fix in Cursor Fix in Web

};

export const properFullEnvelopeRequestParser = <T extends Envelope>(request: Request | null): T => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,12 @@ const router = sentryCreateBrowserRouter(
lazyChildren: () => import('./pages/InnerLazyRoutes').then(module => module.someMoreNestedRoutes),
},
},
{
path: '/another-lazy',
handle: {
lazyChildren: () => import('./pages/AnotherLazyRoutes').then(module => module.anotherNestedRoutes),
},
},
{
path: '/static',
element: <>Hello World</>,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import React from 'react';
import { Link } from 'react-router-dom';

export const anotherNestedRoutes = [
{
path: 'sub',
children: [
{
index: true,
element: (
<div id="another-lazy-route">
Another Lazy Route
<Link to="/lazy/inner/999/888/777" id="navigate-to-inner">
Navigate to Inner Lazy Route
</Link>
</div>
),
},
{
path: ':id',
children: [
{
index: true,
element: <div id="another-lazy-route-with-id">Another Lazy Route with ID</div>,
},
{
path: ':subId',
element: (
<div id="another-lazy-route-deep">
Another Deep Lazy Route
<Link to="/lazy/inner/111/222/333" id="navigate-to-inner-from-deep">
Navigate to Inner from Deep
</Link>
</div>
),
},
],
},
],
},
];
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,14 @@ const Index = () => {
<Link to="/lazy/inner/123/456/789" id="navigation">
navigate
</Link>
<br />
<Link to="/another-lazy/sub" id="navigation-to-another">
Navigate to Another Lazy Route
</Link>
<br />
<Link to="/another-lazy/sub/555/666" id="navigation-to-another-deep">
Navigate to Another Deep Lazy Route
</Link>
</>
);
};
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React from 'react';
import { Link } from 'react-router-dom';

export const someMoreNestedRoutes = [
{
Expand All @@ -24,9 +25,15 @@ export const someMoreNestedRoutes = [
},
{
path: ':someAnotherId',
element: <div id="innermost-lazy-route">
Rendered
</div>,
element: (
<div id="innermost-lazy-route">
Rendered
<br />
<Link to="/another-lazy/sub/888/999" id="navigate-to-another-from-inner">
Navigate to Another Lazy Route
</Link>
</div>
),
},
],
},
Expand Down
Loading
Loading