Skip to content

chore: extract record_traced_span for the span + request-store ceremony #16599

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.

Sign up for GitHub

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

Closed
Nic-Polumeyv wants to merge 4 commits into from
+141 −124
Closed

chore: extract record_traced_span for the span + request-store ceremony #16599

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c559b52
chore: extract record_traced_span for span + store ceremony
Nic-Polumeyv Jul 31, 2026
6d77642
chore: simplify the helper types
Nic-Polumeyv Jul 31, 2026
55641f4
chore: use record_traced_span for remote calls and form posts
Nic-Polumeyv Jul 31, 2026
424b6ab
chore: expression-bodied load callbacks
Nic-Polumeyv Jul 31, 2026
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
15 changes: 6 additions & 9 deletions packages/kit/src/runtime/server/page/actions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,11 @@ import * as devalue from 'devalue';
import { DEV } from 'esm-env';
import { json } from '@sveltejs/kit';
import { HttpError, Redirect, ActionFailure, SvelteKitError } from '@sveltejs/kit/internal';
import { with_request_store, merge_tracing } from '@sveltejs/kit/internal/server';
import { normalize_error } from '../../../utils/error.js';
import { is_form_content_type, negotiate } from '../../../utils/http.js';
import { create_replacer, with_version_header } from '../utils.js';
import { handle_error_and_jsonify } from '../errors.js';
import { record_span } from '../../telemetry/record_span.js';
import { record_traced_span } from '../../telemetry/record_span.js';

/** @param {RequestEvent} event */
export function is_action_json_request(event) {
Expand Down Expand Up @@ -274,18 +273,16 @@ async function call_action(event, event_state, actions) {
);
}

return record_span({
return record_traced_span({
name: 'sveltekit.form_action',
attributes: {
'sveltekit.form_action.name': name,
'http.route': event.route.id || 'unknown'
},
fn: async (current) => {
const traced_event = merge_tracing(event, current);

const result = await with_request_store({ event: traced_event, state: event_state }, () =>
action(traced_event)
);
event,
state: event_state,
fn: async (traced_event, current) => {
const result = await action(traced_event);

if (result instanceof ActionFailure) {
current.setAttributes({
Expand Down
193 changes: 93 additions & 100 deletions packages/kit/src/runtime/server/page/load_data.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ import { DEV } from 'esm-env';
import { noop } from '../../../utils/functions.js';
import { disable_search, make_trackable } from '../../../utils/url.js';
import { validate_depends, validate_load_response } from '../../shared.js';
import { with_request_store, merge_tracing } from '@sveltejs/kit/internal/server';
import { record_span } from '../../telemetry/record_span.js';
import { record_traced_span } from '../../telemetry/record_span.js';
import { base64_encode } from '../../utils.js';
import { NULL_BODY_STATUS } from '../constants.js';
import { get_node_type } from '../utils.js';
Expand Down Expand Up @@ -73,107 +72,103 @@ export async function load_server_data({ event, event_state, state, node, parent

let done = false;

const result = await record_span({
const result = await record_traced_span({
name: 'sveltekit.load',
attributes: {
'sveltekit.load.node_id': node.server_id || 'unknown',
'sveltekit.load.node_type': get_node_type(node.server_id),
'sveltekit.load.environment': 'server',
'http.route': event.route.id || 'unknown'
},
fn: async (current) => {
const traced_event = merge_tracing(event, current);
const result = await with_request_store({ event: traced_event, state: event_state }, () =>
load.call(null, {
...traced_event,
fetch: (info, init) => {
const url = new URL(info instanceof Request ? info.url : info, event.url);

if (DEV && done && !uses.dependencies.has(url.href)) {
console.warn(
`${node.server_id}: Calling \`event.fetch(...)\` in a promise handler after \`load(...)\` has returned will not cause the function to re-run when the dependency is invalidated`
);
}
event,
state: event_state,
fn: async (traced_event) =>
load.call(null, {
...traced_event,
fetch: (info, init) => {
const url = new URL(info instanceof Request ? info.url : info, event.url);

if (DEV && done && !uses.dependencies.has(url.href)) {
console.warn(
`${node.server_id}: Calling \`event.fetch(...)\` in a promise handler after \`load(...)\` has returned will not cause the function to re-run when the dependency is invalidated`
);
}

// Note: server fetches are not added to uses.depends due to security concerns
return event.fetch(info, init);
},
/** @param {string[]} deps */
depends: (...deps) => {
for (const dep of deps) {
const { href } = new URL(dep, event.url);

if (DEV) {
validate_depends(node.server_id || 'missing route ID', dep);

if (done && !uses.dependencies.has(href)) {
console.warn(
`${node.server_id}: Calling \`depends(...)\` in a promise handler after \`load(...)\` has returned will not cause the function to re-run when the dependency is invalidated`
);
}
}
// Note: server fetches are not added to uses.depends due to security concerns
return event.fetch(info, init);
},
/** @param {string[]} deps */
depends: (...deps) => {
for (const dep of deps) {
const { href } = new URL(dep, event.url);

uses.dependencies.add(href);
}
},
params: new Proxy(event.params, {
get: (target, key) => {
if (DEV && done && typeof key === 'string' && !uses.params.has(key)) {
if (DEV) {
validate_depends(node.server_id || 'missing route ID', dep);

if (done && !uses.dependencies.has(href)) {
console.warn(
`${node.server_id}: Accessing \`params.${String(
key
)}\` in a promise handler after \`load(...)\` has returned will not cause the function to re-run when the param changes`
`${node.server_id}: Calling \`depends(...)\` in a promise handler after \`load(...)\` has returned will not cause the function to re-run when the dependency is invalidated`
);
}

if (is_tracking) {
uses.params.add(key);
}
return target[/** @type {string} */ (key)];
}
}),
parent: async () => {
if (DEV && done && !uses.parent) {

uses.dependencies.add(href);
}
},
params: new Proxy(event.params, {
get: (target, key) => {
if (DEV && done && typeof key === 'string' && !uses.params.has(key)) {
console.warn(
`${node.server_id}: Calling \`parent(...)\` in a promise handler after \`load(...)\` has returned will not cause the function to re-run when parent data changes`
`${node.server_id}: Accessing \`params.${String(
key
)}\` in a promise handler after \`load(...)\` has returned will not cause the function to re-run when the param changes`
);
}

if (is_tracking) {
uses.parent = true;
uses.params.add(key);
}
return parent();
},
route: new Proxy(event.route, {
get: (target, key) => {
if (DEV && done && typeof key === 'string' && !uses.route) {
console.warn(
`${node.server_id}: Accessing \`route.${String(
key
)}\` in a promise handler after \`load(...)\` has returned will not cause the function to re-run when the route changes`
);
}
return target[/** @type {string} */ (key)];
}
}),
parent: async () => {
if (DEV && done && !uses.parent) {
console.warn(
`${node.server_id}: Calling \`parent(...)\` in a promise handler after \`load(...)\` has returned will not cause the function to re-run when parent data changes`
);
}

if (is_tracking) {
uses.route = true;
}
return target[/** @type {'id'} */ (key)];
if (is_tracking) {
uses.parent = true;
}
return parent();
},
route: new Proxy(event.route, {
get: (target, key) => {
if (DEV && done && typeof key === 'string' && !uses.route) {
console.warn(
`${node.server_id}: Accessing \`route.${String(
key
)}\` in a promise handler after \`load(...)\` has returned will not cause the function to re-run when the route changes`
);
}
}),
url,
untrack(fn) {
is_tracking = false;
try {
return fn();
} finally {
is_tracking = true;

if (is_tracking) {
uses.route = true;
}
return target[/** @type {'id'} */ (key)];
}
})
);

return result;
}
}),
url,
untrack(fn) {
is_tracking = false;
try {
return fn();
} finally {
is_tracking = true;
}
}
})
});

if (DEV) {
Expand Down Expand Up @@ -224,33 +219,31 @@ export async function load_data({
return server_data_node?.data ?? null;
}

const result = await record_span({
const child_state = { ...event_state, is_in_universal_load: true };

const result = await record_traced_span({
name: 'sveltekit.load',
attributes: {
'sveltekit.load.node_id': node.universal_id || 'unknown',
'sveltekit.load.node_type': get_node_type(node.universal_id),
'sveltekit.load.environment': 'server',
'http.route': event.route.id || 'unknown'
},
fn: async (current) => {
const traced_event = merge_tracing(event, current);
const child_state = { ...event_state, is_in_universal_load: true };

return await with_request_store({ event: traced_event, state: child_state }, () =>
load.call(null, {
url: event.url,
params: event.params,
data: server_data_node?.data ?? null,
route: event.route,
fetch: create_universal_fetch(event, state, fetched, csr, resolve_opts),
setHeaders: event.setHeaders,
depends: noop,
parent,
untrack: (fn) => fn(),
tracing: traced_event.tracing
})
);
}
event,
state: child_state,
fn: async (traced_event) =>
load.call(null, {
url: event.url,
params: event.params,
data: server_data_node?.data ?? null,
route: event.route,
fetch: create_universal_fetch(event, state, fetched, csr, resolve_opts),
setHeaders: event.setHeaders,
depends: noop,
parent,
untrack: (fn) => fn(),
tracing: traced_event.tracing
})
});

if (DEV) {
Expand Down
30 changes: 16 additions & 14 deletions packages/kit/src/runtime/server/remote-functions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@

import { json, error } from '@sveltejs/kit';
import { Redirect, SvelteKitError } from '@sveltejs/kit/internal';
import { with_request_store, merge_tracing } from '@sveltejs/kit/internal/server';
import { with_request_store } from '@sveltejs/kit/internal/server';
import { app_dir, base } from '$app/paths/internal/server';
import { is_form_content_type } from '../../utils/http.js';
import { create_remote_key, parse_remote_arg, split_remote_key, stringify } from '../shared.js';
import { handle_error_and_jsonify } from './errors.js';
import { normalize_error } from '../../utils/error.js';
import { check_incorrect_fail_use } from './page/actions.js';
import { DEV } from 'esm-env';
import { record_span } from '../telemetry/record_span.js';
import { record_traced_span } from '../telemetry/record_span.js';
import { deserialize_binary_form } from '../form-utils.js';
import { with_version_header } from './utils.js';

Expand All @@ -24,15 +24,20 @@ const KEEP_ALIVE_INTERVAL = 30_000;

/** @type {typeof handle_remote_call_internal} */
export async function handle_remote_call(event, state, options, manifest, id) {
return record_span({
return record_traced_span({
name: 'sveltekit.remote.call',
attributes: {
'sveltekit.remote.call.id': id
},
fn: async (current) => {
const traced_event = merge_tracing(event, current);
const response = await with_request_store({ event: traced_event, state }, () =>
handle_remote_call_internal(traced_event, state, options, manifest, id)
event,
state,
fn: async (traced_event) => {
const response = await handle_remote_call_internal(
traced_event,
state,
options,
manifest,
id
);
return with_version_header(response);
}
Expand Down Expand Up @@ -515,17 +520,14 @@ function create_requested_map(refreshes) {

/** @type {typeof handle_remote_form_post_internal} */
export async function handle_remote_form_post(event, state, manifest, id) {
return record_span({
return record_traced_span({
name: 'sveltekit.remote.form.post',
attributes: {
'sveltekit.remote.form.post.id': id
},
fn: (current) => {
const traced_event = merge_tracing(event, current);
return with_request_store({ event: traced_event, state }, () =>
handle_remote_form_post_internal(traced_event, state, manifest, id)
);
}
event,
state,
fn: (traced_event) => handle_remote_form_post_internal(traced_event, state, manifest, id)
});
}

Expand Down
27 changes: 26 additions & 1 deletion packages/kit/src/runtime/telemetry/record_span.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
/** @import { RecordSpan } from 'types' */
/** @import { RequestEvent } from '@sveltejs/kit' */
/** @import { Span } from '@opentelemetry/api' */
/** @import { RecordSpan, RequestState } from 'types' */
import { HttpError, Redirect } from '@sveltejs/kit/internal';
import { merge_tracing, with_request_store } from '@sveltejs/kit/internal/server';
import { noop_span } from './noop.js';
import { otel } from './otel.js';

Expand Down Expand Up @@ -63,3 +66,25 @@ export async function record_span({ name, attributes, fn }) {
}
});
}

/**
* @template T
* @param {{
* name: string;
* attributes: Record<string, any>;
* event: RequestEvent;
* state: RequestState;
* fn: (traced_event: RequestEvent, current: Span) => Promise<T>;
* }} options
* @returns {Promise<T>}
*/
export function record_traced_span({ name, attributes, event, state, fn }) {
return record_span({
name,
attributes,
fn: (current) => {
const traced_event = merge_tracing(event, current);
return with_request_store({ event: traced_event, state }, () => fn(traced_event, current));
}
});
}
Loading