fix(sveltekit): Handle nested server calls in sentryHandle (#7598)

Previously, we always created a new domain when entering the `sentryHandle` function. This caused "nested" SvelteKit Server calls to create new transaction whenever a new domain was created in the `sentryHandle` function because the active span is bound to the domain. This fix introduces a check if there is an active transaction, in which case we just execute the handler and don't create a new domain, as we are already in this active domain. 

As a result, nested SK server calls become a span of the parent server call instead of a new transaction.

Author: Lms24

packages/sveltekit/src/server/handle.ts

@@ -1,6 +1,6 @@
 /* eslint-disable @sentry-internal/sdk/no-optional-chaining */
 import type { Span } from '@sentry/core';
-import { getActiveTransaction, trace } from '@sentry/core';
+import { getActiveTransaction, getCurrentHub, trace } from '@sentry/core';
 import { captureException } from '@sentry/node';
 import {
   addExceptionMechanism,
@@ -64,32 +64,42 @@ export const transformPageChunk: NonNullable<ResolveOptions['transformPageChunk'
  * // export const handle = sequence(sentryHandle, yourCustomHandle);
  * ```
  */
-export const sentryHandle: Handle = ({ event, resolve }) => {
+export const sentryHandle: Handle = input => {
+  // if there is an active transaction, we know that this handle call is nested and hence
+  // we don't create a new domain for it. If we created one, nested server calls would
+  // create new transactions instead of adding a child span to the currently active span.
+  if (getCurrentHub().getScope().getSpan()) {
+    return instrumentHandle(input);
+  }
   return domain.create().bind(() => {
-    const sentryTraceHeader = event.request.headers.get('sentry-trace');
-    const baggageHeader = event.request.headers.get('baggage');
-    const traceparentData = sentryTraceHeader ? extractTraceparentData(sentryTraceHeader) : undefined;
-    const dynamicSamplingContext = baggageHeaderToDynamicSamplingContext(baggageHeader);
-
-    return trace(
-      {
-        op: 'http.server',
-        name: `${event.request.method} ${event.route.id}`,
-        status: 'ok',
-        ...traceparentData,
-        metadata: {
-          source: 'route',
-          dynamicSamplingContext: traceparentData && !dynamicSamplingContext ? {} : dynamicSamplingContext,
-        },
-      },
-      async (span?: Span) => {
-        const res = await resolve(event, { transformPageChunk });
-        if (span) {
-          span.setHttpStatus(res.status);
-        }
-        return res;
-      },
-      sendErrorToSentry,
-    );
+    return instrumentHandle(input);
   })();
 };
+
+function instrumentHandle({ event, resolve }: Parameters<Handle>[0]): ReturnType<Handle> {
+  const sentryTraceHeader = event.request.headers.get('sentry-trace');
+  const baggageHeader = event.request.headers.get('baggage');
+  const traceparentData = sentryTraceHeader ? extractTraceparentData(sentryTraceHeader) : undefined;
+  const dynamicSamplingContext = baggageHeaderToDynamicSamplingContext(baggageHeader);
+
+  return trace(
+    {
+      op: 'http.server',
+      name: `${event.request.method} ${event.route.id}`,
+      status: 'ok',
+      ...traceparentData,
+      metadata: {
+        source: 'route',
+        dynamicSamplingContext: traceparentData && !dynamicSamplingContext ? {} : dynamicSamplingContext,
+      },
+    },
+    async (span?: Span) => {
+      const res = await resolve(event, { transformPageChunk });
+      if (span) {
+        span.setHttpStatus(res.status);
+      }
+      return res;
+    },
+    sendErrorToSentry,
+  );
+}

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

@@ -129,7 +129,7 @@ describe('handleSentry', () => {
       expect(response).toEqual(mockResponse);
     });
 
-    it('creates a transaction', async () => {
+    it("creates a transaction if there's no active span", async () => {
       let ref: any = undefined;
       client.on('finishTransaction', (transaction: Transaction) => {
         ref = transaction;
@@ -149,6 +149,50 @@ describe('handleSentry', () => {
       expect(ref.metadata.source).toEqual('route');
 
       expect(ref.endTimestamp).toBeDefined();
+      expect(ref.spanRecorder.spans).toHaveLength(1);
+    });
+
+    it('creates a child span for nested server calls (i.e. if there is an active span)', async () => {
+      let ref: any = undefined;
+      let txnCount = 0;
+      client.on('finishTransaction', (transaction: Transaction) => {
+        ref = transaction;
+        ++txnCount;
+      });
+
+      try {
+        await sentryHandle({
+          event: mockEvent(),
+          resolve: async _ => {
+            // simulateing a nested load call:
+            await sentryHandle({
+              event: mockEvent({ route: { id: 'api/users/details/[id]' } }),
+              resolve: resolve(type, isError),
+            });
+            return mockResponse;
+          },
+        });
+      } catch (e) {
+        //
+      }
+
+      expect(txnCount).toEqual(1);
+      expect(ref).toBeDefined();
+
+      expect(ref.name).toEqual('GET /users/[id]');
+      expect(ref.op).toEqual('http.server');
+      expect(ref.status).toEqual(isError ? 'internal_error' : 'ok');
+      expect(ref.metadata.source).toEqual('route');
+
+      expect(ref.endTimestamp).toBeDefined();
+
+      expect(ref.spanRecorder.spans).toHaveLength(2);
+      expect(ref.spanRecorder.spans).toEqual(
+        expect.arrayContaining([
+          expect.objectContaining({ op: 'http.server', description: 'GET /users/[id]' }),
+          expect.objectContaining({ op: 'http.server', description: 'GET api/users/details/[id]' }),
+        ]),
+      );
     });
 
     it('creates a transaction from sentry-trace header', async () => {