Skip to content

Commit

Permalink
fix(sveltekit): Ensure trace meta tags are always injected (#13231)
Browse files Browse the repository at this point in the history
Removes the incorrect root span check guarding the trace `<meta>` tag
injection. With this fix, trace data is now also injected for Tracing
Without Performance scenarios where there is no active span.

closes #13106
  • Loading branch information
Lms24 authored Oct 8, 2024
1 parent 8b1b64c commit 5929a1b
Show file tree
Hide file tree
Showing 25 changed files with 362 additions and 31 deletions.
1 change: 1 addition & 0 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -915,6 +915,7 @@ jobs:
'sveltekit',
'sveltekit-2',
'sveltekit-2-svelte-5',
'sveltekit-2-twp',
'tanstack-router',
'generic-ts3.8',
'node-fastify',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,38 @@ const META_TAG_PARENT_SPAN_ID = '1234567890123456';
const META_TAG_BAGGAGE =
'sentry-trace_id=12345678901234567890123456789012,sentry-public_key=public,sentry-release=1.0.0,sentry-environment=prod';

sentryTest('error on initial page has traceId from meta tag', async ({ getLocalTestUrl, page }) => {
if (shouldSkipTracingTest()) {
sentryTest.skip();
}

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

const errorEventPromise = getFirstSentryEnvelopeRequest<EventAndTraceHeader>(
page,
undefined,
eventAndTraceHeaderRequestParser,
);

await page.locator('#errorBtn').click();
const [errorEvent, errorTraceHeader] = await errorEventPromise;

expect(errorEvent.type).toEqual(undefined);
expect(errorEvent.contexts?.trace).toEqual({
trace_id: META_TAG_TRACE_ID,
parent_span_id: META_TAG_PARENT_SPAN_ID,
span_id: expect.stringMatching(/^[0-9a-f]{16}$/),
});

expect(errorTraceHeader).toEqual({
environment: 'prod',
public_key: 'public',
release: '1.0.0',
trace_id: META_TAG_TRACE_ID,
});
});

sentryTest('error has new traceId after navigation', async ({ getLocalTestUrl, page }) => {
if (shouldSkipTracingTest()) {
sentryTest.skip();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
.DS_Store
node_modules
/build
/.svelte-kit
/package
.env
.env.*
!.env.example
vite.config.js.timestamp-*
vite.config.ts.timestamp-*
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
@sentry:registry=http://127.0.0.1:4873
@sentry-internal:registry=http://127.0.0.1:4873
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# Tracing Without Performance E2E test app

E2E test app for testing Tracing Without Performance in a (SvelteKit) meta framework scenario

Add tests to this app that specifically test TwP in meta frameworks.
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
{
"name": "sveltekit-2-svelte-5",
"version": "0.0.1",
"private": true,
"scripts": {
"dev": "vite dev",
"build": "vite build",
"preview": "vite preview",
"proxy": "node start-event-proxy.mjs",
"clean": "npx rimraf node_modules pnpm-lock.yaml",
"check": "svelte-kit sync && svelte-check --tsconfig ./tsconfig.json",
"check:watch": "svelte-kit sync && svelte-check --tsconfig ./tsconfig.json --watch",
"test:prod": "TEST_ENV=production playwright test",
"test:build": "pnpm install && npx playwright install && pnpm build",
"test:assert": "pnpm test:prod"
},
"dependencies": {
"@sentry/sveltekit": "latest || *"
},
"devDependencies": {
"@playwright/test": "^1.44.1",
"@sentry-internal/test-utils": "link:../../../test-utils",
"@sentry/types": "latest || *",
"@sentry/utils": "latest || *",
"@sveltejs/adapter-auto": "^3.0.0",
"@sveltejs/kit": "^2.0.0",
"@sveltejs/vite-plugin-svelte": "^3.0.0",
"svelte": "^5.0.0-next.115",
"svelte-check": "^3.6.0",
"tslib": "^2.4.1",
"typescript": "^5.0.0",
"vite": "^5.0.3"
},
"type": "module"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { getPlaywrightConfig } from '@sentry-internal/test-utils';

const config = getPlaywrightConfig({
startCommand: 'pnpm preview --port 3030',
port: 3030,
});

export default config;
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// See https://kit.svelte.dev/docs/types#app
// for information about these interfaces
declare global {
namespace App {
// interface Error {}
// interface Locals {}
// interface PageData {}
// interface PageState {}
// interface Platform {}
}
}

export {};
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<!doctype html>
<html lang="en">
<head>
<meta charset="utf-8" />
<link rel="icon" href="%sveltekit.assets%/favicon.png" />
<meta name="viewport" content="width=device-width, initial-scale=1" />
%sveltekit.head%
</head>
<body data-sveltekit-preload-data="hover">
<div style="display: contents">%sveltekit.body%</div>
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { env } from '$env/dynamic/public';
import * as Sentry from '@sentry/sveltekit';

Sentry.init({
environment: 'qa', // dynamic sampling bias to keep transactions
dsn: env.PUBLIC_E2E_TEST_DSN,
release: '1.0.0',
tunnel: `http://localhost:3031/`, // proxy server
});

const myErrorHandler = ({ error, event }: any) => {
console.error('An error occurred on the client side:', error, event);
};

export const handleError = Sentry.handleErrorWithSentry(myErrorHandler);
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { E2E_TEST_DSN } from '$env/static/private';
import * as Sentry from '@sentry/sveltekit';

Sentry.init({
environment: 'qa', // dynamic sampling bias to keep transactions
dsn: E2E_TEST_DSN,
tunnel: `http://localhost:3031/`, // proxy server
});

// not logging anything to console to avoid noise in the test output
export const handleError = Sentry.handleErrorWithSentry(() => {});

export const handle = Sentry.sentryHandle();
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<h1>Sveltekit E2E Test app</h1>
<div data-sveltekit-preload-data="off">
<slot></slot>
</div>
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<h1>Welcome to SvelteKit 2 with Svelte 5!</h1>
<p>Visit <a href="https://kit.svelte.dev">kit.svelte.dev</a> to read the documentation</p>

<ul>
<li>
<a href="/errors">error route</a>
</li>
</ul>
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import * as Sentry from '@sentry/sveltekit';

export const load = async ({ url }) => {
if (!url.search) {
Sentry.captureException(new Error('No search query provided'));
return {
error: 'No search query provided',
};
}
return {
message: 'hi',
};
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<script lang="ts">
import { onMount } from 'svelte';
export let data;
onMount(() => {
if (data.error) {
throw new Error('Client Error');
}
});
</script>

<p>
Data:
{data.message || 'nothing'}
</p>
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { startEventProxyServer } from '@sentry-internal/test-utils';

startEventProxyServer({
port: 3031,
proxyServerName: 'sveltekit-2-twp',
});
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import adapter from '@sveltejs/adapter-auto';
import { vitePreprocess } from '@sveltejs/vite-plugin-svelte';

/** @type {import('@sveltejs/kit').Config} */
const config = {
// Consult https://kit.svelte.dev/docs/integrations#preprocessors
// for more information about preprocessors
preprocess: vitePreprocess(),

kit: {
// adapter-auto only supports some environments, see https://kit.svelte.dev/docs/adapter-auto for a list.
// If your environment is not supported, or you settled on a specific environment, switch out the adapter.
// See https://kit.svelte.dev/docs/adapters for more information about adapters.
adapter: adapter(),
},
};

export default config;
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
import { expect, test } from '@playwright/test';
import { waitForError } from '@sentry-internal/test-utils';

test('errors on frontend and backend are connected by the same trace', async ({ page }) => {
const clientErrorPromise = waitForError('sveltekit-2-twp', evt => {
return evt.exception?.values?.[0].value === 'Client Error';
});

const serverErrorPromise = waitForError('sveltekit-2-twp', evt => {
return evt.exception?.values?.[0].value === 'No search query provided';
});

await page.goto('/errors');

const clientError = await clientErrorPromise;
const serverError = await serverErrorPromise;

expect(clientError).toMatchObject({
contexts: {
trace: {
parent_span_id: expect.stringMatching(/[a-f0-9]{16}/),
span_id: expect.stringMatching(/[a-f0-9]{16}/),
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
},
},
environment: 'qa',
exception: {
values: [
{
mechanism: {
handled: false,
type: 'onunhandledrejection',
},
stacktrace: expect.any(Object),
type: 'Error',
value: 'Client Error',
},
],
},
level: 'error',
platform: 'javascript',
release: '1.0.0',
timestamp: expect.any(Number),
transaction: '/errors',
});

expect(serverError).toMatchObject({
contexts: {
trace: {
span_id: expect.stringMatching(/[a-f0-9]{16}/),
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
},
},
environment: 'qa',
exception: {
values: [
{
mechanism: {
handled: true,
type: 'generic',
},
stacktrace: {},
},
],
},
platform: 'node',
timestamp: expect.any(Number),
transaction: 'GET /errors',
});

const clientTraceId = clientError.contexts?.trace?.trace_id;
const serverTraceId = serverError.contexts?.trace?.trace_id;

expect(clientTraceId).toBe(serverTraceId);
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { expect, test } from '@playwright/test';

test('Initially loaded page contains trace meta tags from backend trace', async ({ page }) => {
await page.goto('/');

const sentryTraceMetaTag = page.locator('meta[name="sentry-trace"]').first();
const sentryTraceContent = await sentryTraceMetaTag.getAttribute('content');

const baggageMetaTag = page.locator('meta[name="baggage"]').first();
const baggageContent = await baggageMetaTag.getAttribute('content');

// ensure that we do not pass a sampled -1 or -0 flag at the end:
expect(sentryTraceContent).toMatch(/^[a-f0-9]{32}-[a-f0-9]{16}$/);

expect(baggageContent?.length).toBeGreaterThan(0);

const traceId = sentryTraceContent!.split('-')[0];

expect(baggageContent).toContain('sentry-environment=qa');
expect(baggageContent).toContain(`sentry-trace_id=${traceId}`);
// ensure baggage also doesn't contain a sampled flag
expect(baggageContent).not.toContain('sentry-sampled=');
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{
"extends": "./.svelte-kit/tsconfig.json",
"compilerOptions": {
"allowJs": true,
"checkJs": true,
"esModuleInterop": true,
"forceConsistentCasingInFileNames": true,
"resolveJsonModule": true,
"skipLibCheck": true,
"sourceMap": true,
"strict": true,
"moduleResolution": "bundler"
},
// Path aliases are handled by https://kit.svelte.dev/docs/configuration#alias
// except $lib which is handled by https://kit.svelte.dev/docs/configuration#files
//
// If you want to overwrite includes/excludes, make sure to copy over the relevant includes/excludes
// from the referenced tsconfig.json - TypeScript does not merge them in
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { sentrySvelteKit } from '@sentry/sveltekit';
import { sveltekit } from '@sveltejs/kit/vite';
import { defineConfig } from 'vite';

export default defineConfig({
plugins: [
sentrySvelteKit({
autoUploadSourceMaps: false,
}),
sveltekit(),
],
});
Loading

0 comments on commit 5929a1b

Please sign in to comment.