Skip to content

feat: Add tracing to load, server actions, and handle/resolve #13900

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

Draft
wants to merge 2 commits into
base: elliott/init-tracing
Choose a base branch
from
Draft
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
60 changes: 60 additions & 0 deletions packages/kit/src/core/config/index.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,9 @@
publicPrefix: 'PUBLIC_',
privatePrefix: ''
},
experimental: {
tracing: undefined
},
files: {
assets: join(prefix, 'static'),
hooks: {
Expand Down Expand Up @@ -404,3 +407,60 @@
'The Svelte config file must have a configuration object as its default export. See https://svelte.dev/docs/kit/configuration'
);
});

test('accepts valid tracing values', () => {
assert.doesNotThrow(() => {
validate_config({
kit: {
experimental: {
tracing: 'server'
}
}
});
});

assert.doesNotThrow(() => {
validate_config({
kit: {
experimental: {
tracing: undefined
}
}
});
});
});

test('errors on invalid tracing values', () => {
assert.throws(() => {

Check failure on line 434 in packages/kit/src/core/config/index.spec.js

View workflow job for this annotation

GitHub Actions / test-kit (18, ubuntu-latest, chromium)

src/core/config/index.spec.js > errors on invalid tracing values

AssertionError: expected [Function] to throw error matching /^config\.kit\.tracing should be undef…/ but got 'config.kit.experimental.tracing shoul…' - Expected: /^config\.kit\.tracing should be undefined or "server"$/ + Received: "config.kit.experimental.tracing should be undefined or \"server\"" ❯ src/core/config/index.spec.js:434:9

Check failure on line 434 in packages/kit/src/core/config/index.spec.js

View workflow job for this annotation

GitHub Actions / test-kit-server-side-route-resolution (build)

src/core/config/index.spec.js > errors on invalid tracing values

AssertionError: expected [Function] to throw error matching /^config\.kit\.tracing should be undef…/ but got 'config.kit.experimental.tracing shoul…' - Expected: /^config\.kit\.tracing should be undefined or "server"$/ + Received: "config.kit.experimental.tracing should be undefined or \"server\"" ❯ src/core/config/index.spec.js:434:9

Check failure on line 434 in packages/kit/src/core/config/index.spec.js

View workflow job for this annotation

GitHub Actions / test-kit (22, ubuntu-latest, chromium)

src/core/config/index.spec.js > errors on invalid tracing values

AssertionError: expected [Function] to throw error matching /^config\.kit\.tracing should be undef…/ but got 'config.kit.experimental.tracing shoul…' - Expected: /^config\.kit\.tracing should be undefined or "server"$/ + Received: "config.kit.experimental.tracing should be undefined or \"server\"" ❯ src/core/config/index.spec.js:434:9

Check failure on line 434 in packages/kit/src/core/config/index.spec.js

View workflow job for this annotation

GitHub Actions / test-kit-cross-browser (18, ubuntu-latest, firefox, build)

src/core/config/index.spec.js > errors on invalid tracing values

AssertionError: expected [Function] to throw error matching /^config\.kit\.tracing should be undef…/ but got 'config.kit.experimental.tracing shoul…' - Expected: /^config\.kit\.tracing should be undefined or "server"$/ + Received: "config.kit.experimental.tracing should be undefined or \"server\"" ❯ src/core/config/index.spec.js:434:9

Check failure on line 434 in packages/kit/src/core/config/index.spec.js

View workflow job for this annotation

GitHub Actions / test-kit (20, ubuntu-latest, chromium)

src/core/config/index.spec.js > errors on invalid tracing values

AssertionError: expected [Function] to throw error matching /^config\.kit\.tracing should be undef…/ but got 'config.kit.experimental.tracing shoul…' - Expected: /^config\.kit\.tracing should be undefined or "server"$/ + Received: "config.kit.experimental.tracing should be undefined or \"server\"" ❯ src/core/config/index.spec.js:434:9

Check failure on line 434 in packages/kit/src/core/config/index.spec.js

View workflow job for this annotation

GitHub Actions / test-kit-cross-browser (18, macOS-latest, webkit, build)

src/core/config/index.spec.js > errors on invalid tracing values

AssertionError: expected [Function] to throw error matching /^config\.kit\.tracing should be undef…/ but got 'config.kit.experimental.tracing shoul…' - Expected: /^config\.kit\.tracing should be undefined or "server"$/ + Received: "config.kit.experimental.tracing should be undefined or \"server\"" ❯ src/core/config/index.spec.js:434:9

Check failure on line 434 in packages/kit/src/core/config/index.spec.js

View workflow job for this annotation

GitHub Actions / test-kit-cross-browser (18, windows-latest, chromium, build)

src/core/config/index.spec.js > errors on invalid tracing values

AssertionError: expected [Function] to throw error matching /^config\.kit\.tracing should be undef…/ but got 'config.kit.experimental.tracing shoul…' - Expected: /^config\.kit\.tracing should be undefined or "server"$/ + Received: "config.kit.experimental.tracing should be undefined or \"server\"" ❯ src/core/config/index.spec.js:434:9
validate_config({
kit: {
experimental: {
// @ts-expect-error - given value expected to throw
tracing: true
}
}
});
}, /^config\.kit\.tracing should be undefined or "server"$/);

assert.throws(() => {
validate_config({
kit: {
experimental: {
// @ts-expect-error - given value expected to throw
tracing: false
}
}
});
}, /^config\.kit\.tracing should be undefined or "server"$/);

assert.throws(() => {
validate_config({
kit: {
experimental: {
// @ts-expect-error - given value expected to throw
tracing: 'client'
}
}
});
}, /^config\.kit\.tracing should be undefined or "server"$/);
});
9 changes: 9 additions & 0 deletions packages/kit/src/core/config/options.js
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,15 @@ const options = object(
privatePrefix: string('')
}),

experimental: object({
tracing: validate(undefined, (input, keypath) => {
if (input !== 'server') {
throw new Error(`${keypath} should be undefined or "server"`);
}
return input;
})
}),

files: object({
assets: string('static'),
hooks: object({
Expand Down
6 changes: 6 additions & 0 deletions packages/kit/src/core/sync/write_server.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ import { set_building, set_prerendering } from '__sveltekit/environment';
import { set_assets } from '__sveltekit/paths';
import { set_manifest, set_read_implementation } from '__sveltekit/server';
import { set_private_env, set_public_env, set_safe_public_env } from '${runtime_directory}/shared-server.js';
import { get_tracer, enable_tracing } from '${runtime_directory}/telemetry/get_tracer.js';

if (${s(config.kit.tracing === 'server')}) {
enable_tracing();
}

export const options = {
app_template_contains_nonce: ${template.includes('%sveltekit.nonce%')},
Expand Down Expand Up @@ -60,6 +65,7 @@ export const options = {
.replace(/%sveltekit\.status%/g, '" + status + "')
.replace(/%sveltekit\.error\.message%/g, '" + message + "')}
},
tracer: get_tracer(),
version_hash: ${s(hash(config.kit.version.name))}
};

Expand Down
90 changes: 58 additions & 32 deletions packages/kit/src/exports/hooks/sequence.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
/** @import { Handle, RequestEvent, ResolveOptions } from '@sveltejs/kit' */
/** @import { MaybePromise } from 'types' */
import { with_event } from '../../runtime/app/server/event.js';
import { get_tracer } from '../../runtime/telemetry/get_tracer.js';
import { record_span } from '../../runtime/telemetry/record_span.js';

/**
* A helper function for sequencing multiple `handle` calls in a middleware-like manner.
* The behavior for the `handle` options is as follows:
Expand Down Expand Up @@ -66,56 +72,76 @@
* first post-processing
* ```
*
* @param {...import('@sveltejs/kit').Handle} handlers The chain of `handle` functions
* @returns {import('@sveltejs/kit').Handle}
* @param {...Handle} handlers The chain of `handle` functions
* @returns {Handle}
*/
export function sequence(...handlers) {
const length = handlers.length;
if (!length) return ({ event, resolve }) => resolve(event);

return ({ event, resolve }) => {
return async ({ event, resolve }) => {
const rootSpan = /** @type {NonNullable<RequestEvent['tracing']>['rootSpan']} */ (
event.tracing?.rootSpan
);
const tracer = await get_tracer();
return apply_handle(0, event, {});

/**
* @param {number} i
* @param {import('@sveltejs/kit').RequestEvent} event
* @param {import('@sveltejs/kit').ResolveOptions | undefined} parent_options
* @returns {import('types').MaybePromise<Response>}
* @param {RequestEvent} event
* @param {ResolveOptions | undefined} parent_options
* @returns {MaybePromise<Response>}
*/
function apply_handle(i, event, parent_options) {
const handle = handlers[i];

return handle({
event,
resolve: (event, options) => {
/** @type {import('@sveltejs/kit').ResolveOptions['transformPageChunk']} */
const transformPageChunk = async ({ html, done }) => {
if (options?.transformPageChunk) {
html = (await options.transformPageChunk({ html, done })) ?? '';
}
return record_span({
tracer,
name: 'sveltekit.handle.child',
attributes: {
'sveltekit.handle.child.index': i
},
fn: async (span) => {
const traced_event = { ...event, tracing: { rootSpan, currentSpan: span } };
return await with_event(traced_event, () =>
handle({
event: traced_event,
resolve: (event, options) => {
/** @type {ResolveOptions['transformPageChunk']} */
const transformPageChunk = async ({ html, done }) => {
if (options?.transformPageChunk) {
html = (await options.transformPageChunk({ html, done })) ?? '';
}

if (parent_options?.transformPageChunk) {
html = (await parent_options.transformPageChunk({ html, done })) ?? '';
}
if (parent_options?.transformPageChunk) {
html = (await parent_options.transformPageChunk({ html, done })) ?? '';
}

return html;
};
return html;
};

/** @type {import('@sveltejs/kit').ResolveOptions['filterSerializedResponseHeaders']} */
const filterSerializedResponseHeaders =
parent_options?.filterSerializedResponseHeaders ??
options?.filterSerializedResponseHeaders;
/** @type {ResolveOptions['filterSerializedResponseHeaders']} */
const filterSerializedResponseHeaders =
parent_options?.filterSerializedResponseHeaders ??
options?.filterSerializedResponseHeaders;

/** @type {import('@sveltejs/kit').ResolveOptions['preload']} */
const preload = parent_options?.preload ?? options?.preload;
/** @type {ResolveOptions['preload']} */
const preload = parent_options?.preload ?? options?.preload;

return i < length - 1
? apply_handle(i + 1, event, {
transformPageChunk,
filterSerializedResponseHeaders,
preload
})
: resolve(event, { transformPageChunk, filterSerializedResponseHeaders, preload });
return i < length - 1
? apply_handle(i + 1, event, {
transformPageChunk,
filterSerializedResponseHeaders,
preload
})
: resolve(event, {
transformPageChunk,
filterSerializedResponseHeaders,
preload
});
}
})
);
}
});
}
Expand Down
40 changes: 40 additions & 0 deletions packages/kit/src/exports/public.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
} from '../types/private.js';
import { BuildData, SSRNodeLoader, SSRRoute, ValidatedConfig } from 'types';
import type { SvelteConfig } from '@sveltejs/vite-plugin-svelte';
import { Span } from '@opentelemetry/api';

export { PrerenderOption } from '../types/private.js';

Expand Down Expand Up @@ -401,6 +402,15 @@ export interface KitConfig {
*/
privatePrefix?: string;
};
/** Experimental features. Here be dragons. Breaking changes may occur in minor releases. */
experimental?: {
/**
* Whether to enable serverside OpenTelemetry tracing for SvelteKit operations including handle hooks, load functions, and form actions.
* @default undefined
* @since 2.22.0 // TODO: update this before publishing
*/
tracing?: 'server';
};
/**
* Where to find various files within your project.
*/
Expand Down Expand Up @@ -967,6 +977,16 @@ export interface LoadEvent<
* ```
*/
untrack: <T>(fn: () => T) => T;

/**
* Access to spans for tracing. If tracing is not enabled, this will be `undefined`.
*/
tracing?: {
/** The root span for the request. This span is named `sveltekit.handle.root`. */
rootSpan: Span;
/** The span associated with the current `load` function. */
currentSpan: Span;
};
}

export interface NavigationEvent<
Expand Down Expand Up @@ -1242,6 +1262,16 @@ export interface RequestEvent<
* `true` for `+server.js` calls coming from SvelteKit without the overhead of actually making an HTTP request. This happens when you make same-origin `fetch` requests on the server.
*/
isSubRequest: boolean;

/**
* Access to spans for tracing. If tracing is not enabled, this will be `undefined`.
*/
tracing?: {
/** The root span for the request. This span is named `sveltekit.handle.root`. */
rootSpan: Span;
/** The span associated with the current `handle` hook, `load` function, or server action. */
currentSpan: Span;
};
}

/**
Expand Down Expand Up @@ -1398,6 +1428,16 @@ export interface ServerLoadEvent<
* ```
*/
untrack: <T>(fn: () => T) => T;

/**
* Access to spans for tracing. If tracing is not enabled, this will be `undefined`.
*/
tracing?: {
/** The root span for the request. This span is named `sveltekit.handle.root`. */
rootSpan: Span;
/** The span associated with the current `load` function. */
currentSpan: Span;
};
}

/**
Expand Down
21 changes: 7 additions & 14 deletions packages/kit/src/runtime/client/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,12 @@ import {
} from './constants.js';
import { validate_page_exports } from '../../utils/exports.js';
import { compact } from '../../utils/array.js';
import { INVALIDATED_PARAM, TRAILING_SLASH_PARAM, validate_depends } from '../shared.js';
import {
INVALIDATED_PARAM,
TRAILING_SLASH_PARAM,
validate_depends,
validate_load_response
} from '../shared.js';
import { get_message, get_status } from '../../utils/error.js';
import { writable } from 'svelte/store';
import { page, update, navigating } from './state.svelte.js';
Expand Down Expand Up @@ -763,19 +768,7 @@ async function load_node({ loader, parent, url, params, route, server_data_node
try {
lock_fetch();
data = (await node.universal.load.call(null, load_input)) ?? null;
if (data != null && Object.getPrototypeOf(data) !== Object.prototype) {
throw new Error(
`a load function related to route '${route.id}' returned ${
typeof data !== 'object'
? `a ${typeof data}`
: data instanceof Response
? 'a Response object'
: Array.isArray(data)
? 'an array'
: 'a non-plain object'
}, but must return a plain object at the top level (i.e. \`return {...}\`)`
);
}
validate_load_response(data, `related to route '${route.id}'`);
} finally {
unlock_fetch();
}
Expand Down
1 change: 1 addition & 0 deletions packages/kit/src/runtime/server/data/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ export async function render_data(
event: new_event,
state,
node,
tracer: await options.tracer,
parent: async () => {
/** @type {Record<string, any>} */
const data = {};
Expand Down
Loading
Loading