Skip to content

ref(sveltekit): Handle SvelteKit-generated spans in sentryHandle #17423

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 4 commits into
base: develop
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
136 changes: 105 additions & 31 deletions packages/sveltekit/src/server-common/handle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,13 @@ import {
getDefaultIsolationScope,
getIsolationScope,
getTraceMetaTags,
SEMANTIC_ATTRIBUTE_SENTRY_OP,
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
setHttpStatus,
spanToJSON,
startSpan,
updateSpanName,
winterCGRequestToRequestData,
withIsolationScope,
} from '@sentry/core';
Expand Down Expand Up @@ -88,11 +91,33 @@ export function isFetchProxyRequired(version: string): boolean {
return true;
}

interface BackwardsForwardsCompatibleEvent {
/**
* For now taken from: https://github.com/sveltejs/kit/pull/13899
* Access to spans for tracing. If tracing is not enabled or the function is being run in the browser, these spans will do nothing.
* @since 2.31.0
*/
tracing?: {
/** Whether tracing is enabled. */
enabled: boolean;
current: Span;
root: Span;
};
}

async function instrumentHandle(
{ event, resolve }: Parameters<Handle>[0],
{
event,
resolve,
}: {
event: Parameters<Handle>[0]['event'] & BackwardsForwardsCompatibleEvent;
resolve: Parameters<Handle>[0]['resolve'];
},
options: SentryHandleOptions,
): Promise<Response> {
if (!event.route?.id && !options.handleUnknownRoutes) {
const routeId = event.route?.id;

if (!routeId && !options.handleUnknownRoutes) {
return resolve(event);
}

Expand All @@ -108,42 +133,79 @@ async function instrumentHandle(
}
}

const routeName = `${event.request.method} ${event.route?.id || event.url.pathname}`;
const routeName = `${event.request.method} ${routeId || event.url.pathname}`;

if (getIsolationScope() !== getDefaultIsolationScope()) {
getIsolationScope().setTransactionName(routeName);
} else {
DEBUG_BUILD && debug.warn('Isolation scope is default isolation scope - skipping setting transactionName');
}

// We only start a span if SvelteKit's native tracing is not enabled. Two reasons:
// - Used Kit version doesn't yet support tracing
// - Users didn't enable tracing
const kitTracingEnabled = event.tracing?.enabled;

try {
const resolveResult = await startSpan(
{
op: 'http.server',
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.sveltekit',
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: event.route?.id ? 'route' : 'url',
'http.method': event.request.method,
},
name: routeName,
},
async (span?: Span) => {
getCurrentScope().setSDKProcessingMetadata({
// We specifically avoid cloning the request here to avoid double read errors.
// We only read request headers so we're not consuming the body anyway.
// Note to future readers: This sounds counter-intuitive but please read
// https://github.com/getsentry/sentry-javascript/issues/14583
normalizedRequest: winterCGRequestToRequestData(event.request),
});
const res = await resolve(event, {
transformPageChunk: addSentryCodeToPage({ injectFetchProxyScript: options.injectFetchProxyScript ?? true }),
});
if (span) {
setHttpStatus(span, res.status);
const resolveWithSentry: (sentrySpan?: Span) => Promise<Response> = async (sentrySpan?: Span) => {
getCurrentScope().setSDKProcessingMetadata({
// We specifically avoid cloning the request here to avoid double read errors.
// We only read request headers so we're not consuming the body anyway.
// Note to future readers: This sounds counter-intuitive but please read
// https://github.com/getsentry/sentry-javascript/issues/14583
normalizedRequest: winterCGRequestToRequestData(event.request),
});

const res = await resolve(event, {
transformPageChunk: addSentryCodeToPage({
injectFetchProxyScript: options.injectFetchProxyScript ?? true,
}),
});

const kitRootSpan = event.tracing?.root;

if (sentrySpan) {
setHttpStatus(sentrySpan, res.status);
} else if (kitRootSpan) {
// Update the root span emitted from SvelteKit to resemble a `http.server` span
// We're doing this here instead of an event processor to ensure we update the
// span name as early as possible (for dynamic sampling, et al.)
// Other spans are enhanced in the `processKitSpans` function.
const spanJson = spanToJSON(kitRootSpan);
const kitRootSpanAttributes = spanJson.data;
const originalName = spanJson.description;

const routeName = kitRootSpanAttributes['http.route'];
if (routeName && typeof routeName === 'string') {
updateSpanName(kitRootSpan, routeName);
}
return res;
},
);

kitRootSpan.setAttributes({
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.server',
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.sveltejs.kit',
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: routeId ? 'route' : 'url',
'sveltekit.tracing.original_name': originalName,
});
}

return res;
};

const resolveResult = kitTracingEnabled
? await resolveWithSentry()
: await startSpan(
{
op: 'http.server',
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.sveltekit',
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: routeId ? 'route' : 'url',
'http.method': event.request.method,
},
name: routeName,
},
resolveWithSentry,
);

return resolveResult;
} catch (e: unknown) {
sendErrorToSentry(e, 'handle');
Expand Down Expand Up @@ -176,9 +238,12 @@ export function sentryHandle(handlerOptions?: SentryHandleOptions): Handle {
};

const sentryRequestHandler: Handle = input => {
const backwardsForwardsCompatibleEvent = input.event as typeof input.event & BackwardsForwardsCompatibleEvent;

// Escape hatch to suppress request isolation and trace continuation (see initCloudflareSentryHandle)
const skipIsolation =
'_sentrySkipRequestIsolation' in input.event.locals && input.event.locals._sentrySkipRequestIsolation;
'_sentrySkipRequestIsolation' in backwardsForwardsCompatibleEvent.locals &&
backwardsForwardsCompatibleEvent.locals._sentrySkipRequestIsolation;

// In case of a same-origin `fetch` call within a server`load` function,
// SvelteKit will actually just re-enter the `handle` function and set `isSubRequest`
Expand All @@ -187,7 +252,9 @@ export function sentryHandle(handlerOptions?: SentryHandleOptions): Handle {
// currently active span instead of a new root span to correctly reflect this
// behavior.
if (skipIsolation || input.event.isSubRequest) {
return instrumentHandle(input, options);
return instrumentHandle(input, {
...options,
});
}

return withIsolationScope(isolationScope => {
Expand All @@ -200,6 +267,13 @@ export function sentryHandle(handlerOptions?: SentryHandleOptions): Handle {
// https://github.com/getsentry/sentry-javascript/issues/14583
normalizedRequest: winterCGRequestToRequestData(input.event.request),
});

if (backwardsForwardsCompatibleEvent.tracing?.enabled) {
// if sveltekit tracing is enabled (since 2.31.0), trace continuation is handled by
// kit before our hook is executed. No noeed to call `continueTrace` from our end
return instrumentHandle(input, options);
}

return continueTrace(getTracePropagationData(input.event), () => instrumentHandle(input, options));
});
};
Expand Down
72 changes: 72 additions & 0 deletions packages/sveltekit/src/server-common/processKitSpans.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
import type { Integration, SpanOrigin } from '@sentry/core';
import { type SpanJSON, SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core';

/**
* A small integration that preprocesses spans so that SvelteKit-generated spans
* (via Kit's tracing feature since 2.31.0) get the correct Sentry attributes
* and data.
*/
export function svelteKitSpansIntegration(): Integration {
return {
name: 'SvelteKitSpansEnhancment',
preprocessEvent(event) {
if (event.type === 'transaction') {
event.spans?.forEach(_enhanceKitSpan);
}
},
};
}

/**
* Adds sentry-specific attributes and data to a span emitted by SvelteKit's native tracing (since 2.31.0)
* @exported for testing
*/
export function _enhanceKitSpan(span: SpanJSON): void {
let op: string | undefined = undefined;
let origin: SpanOrigin | undefined = undefined;

const spanName = span.description;

switch (spanName) {
case 'sveltekit.resolve':
op = 'http.sveltekit.resolve';
origin = 'auto.http.sveltekit';
break;
case 'sveltekit.load':
op = 'function.sveltekit.load';
origin = 'auto.function.sveltekit.load';
break;
case 'sveltekit.form_action':
op = 'function.sveltekit.form_action';
origin = 'auto.function.sveltekit.action';
break;
case 'sveltekit.remote.call':
op = 'function.sveltekit.remote';
origin = 'auto.rpc.sveltekit.remote';
break;
case 'sveltekit.handle.root':
// We don't want to overwrite the root handle span at this point since
// we already enhance the root span in our `sentryHandle` hook.
break;
default: {
if (spanName?.startsWith('sveltekit.handle.sequenced.')) {
op = 'function.sveltekit.handle';
origin = 'auto.function.sveltekit.handle';
}
break;
}
}

const previousOp = span.op || span.data[SEMANTIC_ATTRIBUTE_SENTRY_OP];
const previousOrigin = span.origin || span.data[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN];

if (!previousOp && op) {
span.op = op;
span.data[SEMANTIC_ATTRIBUTE_SENTRY_OP] = op;
}

if (!previousOrigin && origin) {
span.origin = origin;
span.data[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN] = origin;
}
}
7 changes: 6 additions & 1 deletion packages/sveltekit/src/server/sdk.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { applySdkMetadata } from '@sentry/core';
import type { NodeClient, NodeOptions } from '@sentry/node';
import { getDefaultIntegrations as getDefaultNodeIntegrations, init as initNodeSdk } from '@sentry/node';
import { svelteKitSpansIntegration } from '../server-common/processKitSpans';
import { rewriteFramesIntegration } from '../server-common/rewriteFramesIntegration';

/**
Expand All @@ -9,7 +10,11 @@ import { rewriteFramesIntegration } from '../server-common/rewriteFramesIntegrat
*/
export function init(options: NodeOptions): NodeClient | undefined {
const opts = {
defaultIntegrations: [...getDefaultNodeIntegrations(options), rewriteFramesIntegration()],
defaultIntegrations: [
...getDefaultNodeIntegrations(options),
rewriteFramesIntegration(),
svelteKitSpansIntegration(),
],
...options,
};

Expand Down
7 changes: 6 additions & 1 deletion packages/sveltekit/src/worker/cloudflare.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
} from '@sentry/cloudflare';
import { addNonEnumerableProperty } from '@sentry/core';
import type { Handle } from '@sveltejs/kit';
import { svelteKitSpansIntegration } from '../server-common/processKitSpans';
import { rewriteFramesIntegration } from '../server-common/rewriteFramesIntegration';

/**
Expand All @@ -16,7 +17,11 @@ import { rewriteFramesIntegration } from '../server-common/rewriteFramesIntegrat
*/
export function initCloudflareSentryHandle(options: CloudflareOptions): Handle {
const opts: CloudflareOptions = {
defaultIntegrations: [...getDefaultCloudflareIntegrations(options), rewriteFramesIntegration()],
defaultIntegrations: [
...getDefaultCloudflareIntegrations(options),
rewriteFramesIntegration(),
svelteKitSpansIntegration(),
],
...options,
};

Expand Down
28 changes: 24 additions & 4 deletions packages/sveltekit/test/server-common/handle.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ describe('sentryHandle', () => {
[Type.Async, true, undefined],
[Type.Async, false, mockResponse],
])('%s resolve with error %s', (type, isError, mockResponse) => {
it('should return a response', async () => {
it('returns a response', async () => {
let response: any = undefined;
try {
response = await sentryHandle()({ event: mockEvent(), resolve: resolve(type, isError) });
Expand All @@ -123,7 +123,7 @@ describe('sentryHandle', () => {
expect(response).toEqual(mockResponse);
});

it("creates a transaction if there's no active span", async () => {
it("starts a span if there's no active span", async () => {
let _span: Span | undefined = undefined;
client.on('spanEnd', span => {
if (span === getRootSpan(span)) {
Expand All @@ -150,7 +150,27 @@ describe('sentryHandle', () => {
expect(spans).toHaveLength(1);
});

it('creates a child span for nested server calls (i.e. if there is an active span)', async () => {
it("doesn't start a span if sveltekit tracing is enabled", async () => {
let _span: Span | undefined = undefined;
client.on('spanEnd', span => {
if (span === getRootSpan(span)) {
_span = span;
}
});

try {
await sentryHandle()({
event: mockEvent({ tracing: { enabled: true } }),
resolve: resolve(type, isError),
});
} catch {
//
}

expect(_span).toBeUndefined();
});

it('starts a child span for nested server calls (i.e. if there is an active span)', async () => {
let _span: Span | undefined = undefined;
let txnCount = 0;
client.on('spanEnd', span => {
Expand Down Expand Up @@ -197,7 +217,7 @@ describe('sentryHandle', () => {
);
});

it("creates a transaction from sentry-trace header but doesn't populate a new DSC", async () => {
it("starts a span from sentry-trace header but doesn't populate a new DSC", async () => {
const event = mockEvent({
request: {
headers: {
Expand Down
Loading
Loading