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

Author: Lms24

packages/sveltekit/src/server-common/handle.ts

@@ -89,10 +89,18 @@ export function isFetchProxyRequired(version: string): boolean {
 }
 
 async function instrumentHandle(
-  { event, resolve }: Parameters<Handle>[0],
-  options: SentryHandleOptions,
+  {
+    event,
+    resolve,
+  }: {
+    event: Parameters<Handle>[0]['event'];
+    resolve: Parameters<Handle>[0]['resolve'];
+  },
+  options: SentryHandleOptions & { svelteKitTracingEnabled: boolean },
 ): Promise<Response> {
-  if (!event.route?.id && !options.handleUnknownRoutes) {
+  const routeId = event.route?.id;
+
+  if (!routeId && !options.handleUnknownRoutes) {
     return resolve(event);
   }
 
@@ -108,42 +116,53 @@ 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 shouldStartSpan = !options.svelteKitTracingEnabled;
+
   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);
-        }
-        return res;
-      },
-    );
+    const resolveWithSentry: (span?: Span) => Promise<Response> = 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);
+      }
+      return res;
+    };
+
+    const resolveResult = shouldStartSpan
+      ? 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,
+        )
+      : await resolveWithSentry();
     return resolveResult;
   } catch (e: unknown) {
     sendErrorToSentry(e, 'handle');
@@ -153,6 +172,19 @@ async function instrumentHandle(
   }
 }
 
+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.30.0
+   */
+  tracing?: {
+    /** Whether tracing is enabled. */
+    enabled: boolean;
+    // omitting other properties for now, since we don't use them.
+  };
+}
+
 /**
  * A SvelteKit handle function that wraps the request for Sentry error and
  * performance monitoring.
@@ -176,18 +208,26 @@ 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;
+
+    const svelteKitTracingEnabled = !!backwardsForwardsCompatibleEvent.tracing?.enabled;
 
     // 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`
     // to `true` so that no additional network call is made.
     // We want the `http.server` span of that nested call to be a child span of the
     // currently active span instead of a new root span to correctly reflect this
     // behavior.
-    if (skipIsolation || input.event.isSubRequest) {
-      return instrumentHandle(input, options);
+    if (skipIsolation || input.event.isSubRequest || svelteKitTracingEnabled) {
+      return instrumentHandle(input, {
+        ...options,
+        svelteKitTracingEnabled,
+      });
     }
 
     return withIsolationScope(isolationScope => {
@@ -200,7 +240,12 @@ export function sentryHandle(handlerOptions?: SentryHandleOptions): Handle {
         // https://github.com/getsentry/sentry-javascript/issues/14583
         normalizedRequest: winterCGRequestToRequestData(input.event.request),
       });
-      return continueTrace(getTracePropagationData(input.event), () => instrumentHandle(input, options));
+      return continueTrace(getTracePropagationData(input.event), () =>
+        instrumentHandle(input, {
+          ...options,
+          svelteKitTracingEnabled,
+        }),
+      );
     });
   };
 

packages/sveltekit/test/server-common/handle.test.ts

@@ -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) });
@@ -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)) {
@@ -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 => {
@@ -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: {