Skip to content

Commit bf91961

Browse files
elliott-with-the-longest-name-on-githubelliott-with-the-longest-name-on-github
committed
feat: Add tracing to load, server actions, and handle/resolve 
1 parent cc0eef5 commit bf91961

File tree

4 files changed

+235
-119
lines changed

4 files changed

+235
-119
lines changed

‎packages/kit/src/runtime/client/client.js

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@ import { get_message, get_status } from '../../utils/error.js';
4141
import { writable } from 'svelte/store';
4242
import { page, update, navigating } from './state.svelte.js';
4343
import { add_data_suffix, add_resolution_suffix } from '../pathname.js';
44+
import { record_span } from '../server/telemetry/record-span.js';
45+
import { get_tracer } from '../server/telemetry/get-tracer.js';
4446

4547
export { load_css };
4648

@@ -753,10 +755,27 @@ async function load_node({ loader, parent, url, params, route, server_data_node
753755
}
754756
};
755757

758+
async function traced_load() {
759+
const tracer = get_tracer({ is_enabled: true }); // TODO: Make this configurable
760+
761+
return record_span({
762+
name: 'sveltekit.load.universal',
763+
tracer,
764+
attributes: {
765+
'sveltekit.load.node_id': 'client-load', // TODO: can we actually get an id here?
766+
'sveltekit.load.type': 'universal',
767+
'sveltekit.load.environment': 'client',
768+
'sveltekit.route.id': route.id || 'unknown'
769+
},
770+
fn: async () => (await node.universal?.load?.call(null, load_input)) ?? null
771+
});
772+
}
773+
756774
if (DEV) {
757775
try {
758776
lock_fetch();
759-
data = (await node.universal.load.call(null, load_input)) ?? null;
777+
data = await traced_load();
778+
760779
if (data != null && Object.getPrototypeOf(data) !== Object.prototype) {
761780
throw new Error(
762781
`a load function related to route '${route.id}' returned ${
@@ -774,7 +793,7 @@ async function load_node({ loader, parent, url, params, route, server_data_node
774793
unlock_fetch();
775794
}
776795
} else {
777-
data = (await node.universal.load.call(null, load_input)) ?? null;
796+
data = await traced_load();
778797
}
779798
}
780799

‎packages/kit/src/runtime/server/page/actions.js

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import { is_form_content_type, negotiate } from '../../../utils/http.js';
66
import { HttpError, Redirect, ActionFailure, SvelteKitError } from '../../control.js';
77
import { handle_error_and_jsonify } from '../utils.js';
88
import { with_event } from '../../app/server/event.js';
9+
import { record_span } from '../telemetry/record-span.js';
10+
import { get_tracer } from '../telemetry/get-tracer.js';
911

1012
/** @param {import('@sveltejs/kit').RequestEvent} event */
1113
export function is_action_json_request(event) {
@@ -247,7 +249,30 @@ async function call_action(event, actions) {
247249
);
248250
}
249251

250-
return with_event(event, () => action(event));
252+
const tracer = get_tracer({ is_enabled: true }); // TODO: Make this configurable
253+
254+
return record_span({
255+
name: 'sveltekit.action',
256+
tracer,
257+
attributes: {
258+
'sveltekit.action.name': name,
259+
'sveltekit.route.id': event.route.id || 'unknown'
260+
},
261+
fn: async (action_span) => {
262+
const result = await with_event(event, () => action(event));
263+
if (result instanceof ActionFailure) {
264+
action_span.setAttributes({
265+
'sveltekit.action.result.type': 'failure',
266+
'sveltekit.action.result.status': result.status
267+
});
268+
} else {
269+
action_span.setAttributes({
270+
'sveltekit.action.result.type': 'success'
271+
});
272+
}
273+
return result;
274+
}
275+
});
251276
}
252277

253278
/** @param {any} data */

‎packages/kit/src/runtime/server/page/load_data.js

Lines changed: 125 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ import { disable_search, make_trackable } from '../../../utils/url.js';
33
import { validate_depends } from '../../shared.js';
44
import { b64_encode } from '../../utils.js';
55
import { with_event } from '../../app/server/event.js';
6+
import { record_span } from '../telemetry/record-span.js';
7+
import { get_tracer } from '../telemetry/get-tracer.js';
68

79
/**
810
* Calls the user's server `load` function.
@@ -68,94 +70,109 @@ export async function load_server_data({ event, state, node, parent }) {
6870

6971
let done = false;
7072

71-
const result = await with_event(event, () =>
72-
load.call(null, {
73-
...event,
74-
fetch: (info, init) => {
75-
const url = new URL(info instanceof Request ? info.url : info, event.url);
73+
const tracer = get_tracer({ is_enabled: true }); // TODO: Make this configurable
7674

77-
if (DEV && done && !uses.dependencies.has(url.href)) {
78-
console.warn(
79-
`${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`
80-
);
81-
}
82-
83-
// Note: server fetches are not added to uses.depends due to security concerns
84-
return event.fetch(info, init);
85-
},
86-
/** @param {string[]} deps */
87-
depends: (...deps) => {
88-
for (const dep of deps) {
89-
const { href } = new URL(dep, event.url);
90-
91-
if (DEV) {
92-
validate_depends(node.server_id || 'missing route ID', dep);
93-
94-
if (done && !uses.dependencies.has(href)) {
75+
const result = await record_span({
76+
name: 'sveltekit.load.server',
77+
tracer,
78+
attributes: {
79+
'sveltekit.load.node_id': node.server_id || 'unknown',
80+
'sveltekit.load.type': 'server',
81+
'sveltekit.route.id': event.route.id || 'unknown'
82+
},
83+
fn: async () => {
84+
const result = await with_event(event, () =>
85+
load.call(null, {
86+
...event,
87+
fetch: (info, init) => {
88+
const url = new URL(info instanceof Request ? info.url : info, event.url);
89+
90+
if (DEV && done && !uses.dependencies.has(url.href)) {
9591
console.warn(
96-
`${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`
92+
`${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`
9793
);
9894
}
99-
}
100-
101-
uses.dependencies.add(href);
102-
}
103-
},
104-
params: new Proxy(event.params, {
105-
get: (target, key) => {
106-
if (DEV && done && typeof key === 'string' && !uses.params.has(key)) {
107-
console.warn(
108-
`${node.server_id}: Accessing \`params.${String(
109-
key
110-
)}\` in a promise handler after \`load(...)\` has returned will not cause the function to re-run when the param changes`
111-
);
112-
}
11395

114-
if (is_tracking) {
115-
uses.params.add(key);
116-
}
117-
return target[/** @type {string} */ (key)];
118-
}
119-
}),
120-
parent: async () => {
121-
if (DEV && done && !uses.parent) {
122-
console.warn(
123-
`${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`
124-
);
125-
}
96+
// Note: server fetches are not added to uses.depends due to security concerns
97+
return event.fetch(info, init);
98+
},
99+
/** @param {string[]} deps */
100+
depends: (...deps) => {
101+
for (const dep of deps) {
102+
const { href } = new URL(dep, event.url);
103+
104+
if (DEV) {
105+
validate_depends(node.server_id || 'missing route ID', dep);
106+
107+
if (done && !uses.dependencies.has(href)) {
108+
console.warn(
109+
`${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`
110+
);
111+
}
112+
}
113+
114+
uses.dependencies.add(href);
115+
}
116+
},
117+
params: new Proxy(event.params, {
118+
get: (target, key) => {
119+
if (DEV && done && typeof key === 'string' && !uses.params.has(key)) {
120+
console.warn(
121+
`${node.server_id}: Accessing \`params.${String(
122+
key
123+
)}\` in a promise handler after \`load(...)\` has returned will not cause the function to re-run when the param changes`
124+
);
125+
}
126+
127+
if (is_tracking) {
128+
uses.params.add(key);
129+
}
130+
return target[/** @type {string} */ (key)];
131+
}
132+
}),
133+
parent: async () => {
134+
if (DEV && done && !uses.parent) {
135+
console.warn(
136+
`${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`
137+
);
138+
}
126139

127-
if (is_tracking) {
128-
uses.parent = true;
129-
}
130-
return parent();
131-
},
132-
route: new Proxy(event.route, {
133-
get: (target, key) => {
134-
if (DEV && done && typeof key === 'string' && !uses.route) {
135-
console.warn(
136-
`${node.server_id}: Accessing \`route.${String(
137-
key
138-
)}\` in a promise handler after \`load(...)\` has returned will not cause the function to re-run when the route changes`
139-
);
140+
if (is_tracking) {
141+
uses.parent = true;
142+
}
143+
return parent();
144+
},
145+
route: new Proxy(event.route, {
146+
get: (target, key) => {
147+
if (DEV && done && typeof key === 'string' && !uses.route) {
148+
console.warn(
149+
`${node.server_id}: Accessing \`route.${String(
150+
key
151+
)}\` in a promise handler after \`load(...)\` has returned will not cause the function to re-run when the route changes`
152+
);
153+
}
154+
155+
if (is_tracking) {
156+
uses.route = true;
157+
}
158+
return target[/** @type {'id'} */ (key)];
159+
}
160+
}),
161+
url,
162+
untrack(fn) {
163+
is_tracking = false;
164+
try {
165+
return fn();
166+
} finally {
167+
is_tracking = true;
168+
}
140169
}
170+
})
171+
);
141172

142-
if (is_tracking) {
143-
uses.route = true;
144-
}
145-
return target[/** @type {'id'} */ (key)];
146-
}
147-
}),
148-
url,
149-
untrack(fn) {
150-
is_tracking = false;
151-
try {
152-
return fn();
153-
} finally {
154-
is_tracking = true;
155-
}
156-
}
157-
})
158-
);
173+
return result;
174+
}
175+
});
159176

160177
if (__SVELTEKIT_DEV__) {
161178
validate_load_response(result, node.server_id);
@@ -201,16 +218,34 @@ export async function load_data({
201218
return server_data_node?.data ?? null;
202219
}
203220

204-
const result = await node.universal.load.call(null, {
205-
url: event.url,
206-
params: event.params,
207-
data: server_data_node?.data ?? null,
208-
route: event.route,
209-
fetch: create_universal_fetch(event, state, fetched, csr, resolve_opts),
210-
setHeaders: event.setHeaders,
211-
depends: () => {},
212-
parent,
213-
untrack: (fn) => fn()
221+
const { load } = node.universal;
222+
223+
const tracer = get_tracer({ is_enabled: true }); // TODO: Make this configurable
224+
225+
const result = await record_span({
226+
name: 'sveltekit.load.universal',
227+
tracer,
228+
attributes: {
229+
'sveltekit.load.node_id': node.universal_id || 'unknown',
230+
'sveltekit.load.type': 'universal',
231+
'sveltekit.load.environment': 'client',
232+
'sveltekit.route.id': event.route.id || 'unknown'
233+
},
234+
fn: async () => {
235+
const result = await load.call(null, {
236+
url: event.url,
237+
params: event.params,
238+
data: server_data_node?.data ?? null,
239+
route: event.route,
240+
fetch: create_universal_fetch(event, state, fetched, csr, resolve_opts),
241+
setHeaders: event.setHeaders,
242+
depends: () => {},
243+
parent,
244+
untrack: (fn) => fn()
245+
});
246+
247+
return result;
248+
}
214249
});
215250

216251
if (__SVELTEKIT_DEV__) {

0 commit comments

Comments
 (0)