Merge branch 'develop' into abhi-sveltekit-perf-monitor-load-client

Author: AbhiPrasad

packages/nextjs/src/server/wrapServerComponentWithSentry.ts

@@ -1,4 +1,4 @@
-import { captureException, getCurrentHub, startTransaction } from '@sentry/core';
+import { addTracingExtensions, captureException, getCurrentHub, startTransaction } from '@sentry/core';
 import { baggageHeaderToDynamicSamplingContext, extractTraceparentData } from '@sentry/utils';
 import * as domain from 'domain';
 
@@ -11,6 +11,8 @@ export function wrapServerComponentWithSentry<F extends (...args: any[]) => any>
   appDirComponent: F,
   context: ServerComponentContext,
 ): F {
+  addTracingExtensions();
+
   const { componentRoute, componentType } = context;
 
   // Even though users may define server components as async functions, for the client bundles

packages/sveltekit/package.json

@@ -30,6 +30,7 @@
   },
   "devDependencies": {
     "@sveltejs/kit": "^1.11.0",
+    "svelte": "^3.44.0",
     "typescript": "^4.9.3",
     "vite": "4.0.0"
   },

packages/sveltekit/rollup.npm.config.js

@@ -1,14 +1,10 @@
 import { makeBaseNPMConfig, makeNPMConfigVariants } from '../../rollup/index.js';
 
-export default
-  makeNPMConfigVariants(
-    makeBaseNPMConfig({
-      entrypoints: [
-        'src/index.server.ts',
-        'src/index.client.ts',
-        'src/client/index.ts',
-        'src/server/index.ts',
-      ],
-    }),
-  )
-;
+export default makeNPMConfigVariants(
+  makeBaseNPMConfig({
+    entrypoints: ['src/index.server.ts', 'src/index.client.ts', 'src/client/index.ts', 'src/server/index.ts'],
+    packageSpecificConfig: {
+      external: ['$app/stores'],
+    },
+  }),
+);

packages/sveltekit/src/client/router.ts

@@ -0,0 +1,111 @@
+import { getActiveTransaction } from '@sentry/core';
+import { WINDOW } from '@sentry/svelte';
+import type { Span, Transaction, TransactionContext } from '@sentry/types';
+
+import { navigating, page } from '$app/stores';
+
+const DEFAULT_TAGS = {
+  'routing.instrumentation': '@sentry/sveltekit',
+};
+
+/**
+ * Automatically creates pageload and navigation transactions for the client-side SvelteKit router.
+ *
+ * This instrumentation makes use of SvelteKit's `page` and `navigating` stores which can be accessed
+ * anywhere on the client side.
+ *
+ * @param startTransactionFn the function used to start (idle) transactions
+ * @param startTransactionOnPageLoad controls if pageload transactions should be created (defaults to `true`)
+ * @param startTransactionOnLocationChange controls if navigation transactions should be created (defauls to `true`)
+ */
+export function svelteKitRoutingInstrumentation<T extends Transaction>(
+  startTransactionFn: (context: TransactionContext) => T | undefined,
+  startTransactionOnPageLoad: boolean = true,
+  startTransactionOnLocationChange: boolean = true,
+): void {
+  if (startTransactionOnPageLoad) {
+    instrumentPageload(startTransactionFn);
+  }
+
+  if (startTransactionOnLocationChange) {
+    instrumentNavigations(startTransactionFn);
+  }
+}
+
+function instrumentPageload(startTransactionFn: (context: TransactionContext) => Transaction | undefined): void {
+  const initialPath = WINDOW && WINDOW.location && WINDOW.location.pathname;
+
+  const pageloadTransaction = startTransactionFn({
+    name: initialPath,
+    op: 'pageload',
+    description: initialPath,
+    tags: {
+      ...DEFAULT_TAGS,
+    },
+  });
+
+  page.subscribe(page => {
+    if (!page) {
+      return;
+    }
+
+    const routeId = page.route && page.route.id;
+
+    if (pageloadTransaction && routeId) {
+      pageloadTransaction.setName(routeId, 'route');
+    }
+  });
+}
+
+/**
+ * Use the `navigating` store to start a transaction on navigations.
+ */
+function instrumentNavigations(startTransactionFn: (context: TransactionContext) => Transaction | undefined): void {
+  let routingSpan: Span | undefined = undefined;
+  let activeTransaction: Transaction | undefined;
+
+  navigating.subscribe(navigation => {
+    if (!navigation) {
+      // `navigating` emits a 'null' value when the navigation is completed.
+      // So in this case, we can finish the routing span. If the transaction was an IdleTransaction,
+      // it will finish automatically and if it was user-created users also need to finish it.
+      if (routingSpan) {
+        routingSpan.finish();
+        routingSpan = undefined;
+      }
+      return;
+    }
+
+    const routeDestination = navigation.to && navigation.to.route.id;
+    const routeOrigin = navigation.from && navigation.from.route.id;
+
+    if (routeOrigin === routeDestination) {
+      return;
+    }
+
+    activeTransaction = getActiveTransaction();
+
+    if (!activeTransaction) {
+      activeTransaction = startTransactionFn({
+        name: routeDestination || (WINDOW && WINDOW.location && WINDOW.location.pathname),
+        op: 'navigation',
+        metadata: { source: 'route' },
+        tags: {
+          ...DEFAULT_TAGS,
+        },
+      });
+    }
+
+    if (activeTransaction) {
+      if (routingSpan) {
+        // If a routing span is still open from a previous navigation, we finish it.
+        routingSpan.finish();
+      }
+      routingSpan = activeTransaction.startChild({
+        op: 'ui.sveltekit.routing',
+        description: 'SvelteKit Route Change',
+      });
+      activeTransaction.setTag('from', routeOrigin);
+    }
+  });
+}

packages/sveltekit/src/client/sdk.ts

@@ -1,17 +1,18 @@
-import { defaultRequestInstrumentationOptions } from '@sentry-internal/tracing';
 import { hasTracingEnabled } from '@sentry/core';
 import type { BrowserOptions } from '@sentry/svelte';
 import { BrowserTracing, configureScope, init as initSvelteSdk } from '@sentry/svelte';
 import { addOrUpdateIntegration } from '@sentry/utils';
 
 import { applySdkMetadata } from '../common/metadata';
+import { svelteKitRoutingInstrumentation } from './router';
 
 // Treeshakable guard to remove all code related to tracing
 declare const __SENTRY_TRACING__: boolean;
 
 /**
+ * Initialize the client side of the Sentry SvelteKit SDK.
  *
- * @param options
+ * @param options Configuration options for the SDK.
  */
 export function init(options: BrowserOptions): void {
   applySdkMetadata(options, ['sveltekit', 'svelte']);
@@ -33,14 +34,11 @@ function addClientIntegrations(options: BrowserOptions): void {
   if (typeof __SENTRY_TRACING__ === 'undefined' || __SENTRY_TRACING__) {
     if (hasTracingEnabled(options)) {
       const defaultBrowserTracingIntegration = new BrowserTracing({
-        tracePropagationTargets: [...defaultRequestInstrumentationOptions.tracePropagationTargets],
-        // TODO: Add SvelteKit router instrumentations
-        // routingInstrumentation: sveltekitRoutingInstrumentation,
+        routingInstrumentation: svelteKitRoutingInstrumentation,
       });
 
       integrations = addOrUpdateIntegration(defaultBrowserTracingIntegration, integrations, {
-        // TODO: Add SvelteKit router instrumentations
-        // options.routingInstrumentation: sveltekitRoutingInstrumentation,
+        'options.routingInstrumentation': svelteKitRoutingInstrumentation,
       });
     }
   }

packages/sveltekit/src/server/handle.ts

@@ -1,11 +1,11 @@
 /* eslint-disable @sentry-internal/sdk/no-optional-chaining */
-import { captureException, getCurrentHub, startTransaction } from '@sentry/node';
-import type { Transaction } from '@sentry/types';
+import type { Span } from '@sentry/core';
+import { trace } from '@sentry/core';
+import { captureException } from '@sentry/node';
 import {
   addExceptionMechanism,
   baggageHeaderToDynamicSamplingContext,
   extractTraceparentData,
-  isThenable,
   objectify,
 } from '@sentry/utils';
 import type { Handle } from '@sveltejs/kit';
@@ -51,53 +51,30 @@ function sendErrorToSentry(e: unknown): unknown {
  */
 export const sentryHandle: Handle = ({ event, resolve }) => {
   return domain.create().bind(() => {
-    let maybePromiseResult;
-
     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);
 
-    // transaction could be undefined if hub extensions were not added.
-    const transaction: Transaction | undefined = startTransaction({
-      op: 'http.server',
-      name: `${event.request.method} ${event.route.id}`,
-      status: 'ok',
-      ...traceparentData,
-      metadata: {
-        source: 'route',
-        dynamicSamplingContext: traceparentData && !dynamicSamplingContext ? {} : dynamicSamplingContext,
-      },
-    });
-
-    getCurrentHub().getScope()?.setSpan(transaction);
-
-    try {
-      maybePromiseResult = resolve(event);
-    } catch (e) {
-      transaction?.setStatus('internal_error');
-      const sentryError = sendErrorToSentry(e);
-      transaction?.finish();
-      throw sentryError;
-    }
-
-    if (isThenable(maybePromiseResult)) {
-      Promise.resolve(maybePromiseResult).then(
-        response => {
-          transaction?.setHttpStatus(response.status);
-          transaction?.finish();
-        },
-        e => {
-          transaction?.setStatus('internal_error');
-          sendErrorToSentry(e);
-          transaction?.finish();
+    return trace(
+      {
+        op: 'http.server',
+        name: `${event.request.method} ${event.route.id}`,
+        status: 'ok',
+        ...traceparentData,
+        metadata: {
+          source: 'route',
+          dynamicSamplingContext: traceparentData && !dynamicSamplingContext ? {} : dynamicSamplingContext,
         },
-      );
-    } else {
-      transaction?.setHttpStatus(maybePromiseResult.status);
-      transaction?.finish();
-    }
-
-    return maybePromiseResult;
+      },
+      async (span?: Span) => {
+        const res = await resolve(event);
+        if (span) {
+          span.setHttpStatus(res.status);
+        }
+        return res;
+      },
+      sendErrorToSentry,
+    );
   })();
 };

packages/sveltekit/src/server/load.ts

@@ -1,6 +1,14 @@
+/* eslint-disable @sentry-internal/sdk/no-optional-chaining */
+import { trace } from '@sentry/core';
 import { captureException } from '@sentry/node';
-import { addExceptionMechanism, isThenable, objectify } from '@sentry/utils';
+import {
+  addExceptionMechanism,
+  baggageHeaderToDynamicSamplingContext,
+  extractTraceparentData,
+  objectify,
+} from '@sentry/utils';
 import type { HttpError, ServerLoad } from '@sveltejs/kit';
+import * as domain from 'domain';
 
 function isHttpError(err: unknown): err is HttpError {
   return typeof err === 'object' && err !== null && 'status' in err && 'body' in err;
@@ -44,21 +52,30 @@ function sendErrorToSentry(e: unknown): unknown {
 export function wrapLoadWithSentry(origLoad: ServerLoad): ServerLoad {
   return new Proxy(origLoad, {
     apply: (wrappingTarget, thisArg, args: Parameters<ServerLoad>) => {
-      let maybePromiseResult;
+      return domain.create().bind(() => {
+        const [event] = args;
 
-      try {
-        maybePromiseResult = wrappingTarget.apply(thisArg, args);
-      } catch (e) {
-        throw sendErrorToSentry(e);
-      }
+        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);
 
-      if (isThenable(maybePromiseResult)) {
-        Promise.resolve(maybePromiseResult).then(null, e => {
-          sendErrorToSentry(e);
-        });
-      }
-
-      return maybePromiseResult;
+        const routeId = event.route.id;
+        return trace(
+          {
+            op: 'function.sveltekit.load',
+            name: routeId ? routeId : event.url.pathname,
+            status: 'ok',
+            ...traceparentData,
+            metadata: {
+              source: routeId ? 'route' : 'url',
+              dynamicSamplingContext: traceparentData && !dynamicSamplingContext ? {} : dynamicSamplingContext,
+            },
+          },
+          () => wrappingTarget.apply(thisArg, args),
+          sendErrorToSentry,
+        );
+      })();
     },
   });
 }