feat(tracing): Add tracing without performance to browser and client …

…Sveltekit (#8458) Adds tracing without performance support to 1. fetch requests in browser 2. xhr requests in browser 3. Sveltekit fetch monkeypatching (pulled into this PR because it also uses `addTracingHeadersToFetchRequest`
getsentry · Jul 6, 2023 · d439fc5 · d439fc5
1 parent 1d8c81f
commit d439fc5
Show file tree

Hide file tree

Showing 4 changed files with 137 additions and 99 deletions.
diff --git a/packages/sveltekit/src/client/load.ts b/packages/sveltekit/src/client/load.ts
@@ -129,6 +129,8 @@ function instrumentSvelteKitFetch(originalFetch: SvelteKitFetch): SvelteKitFetch
     return originalFetch;
   }
 
+  const options = client.getOptions();
+
   const browserTracingIntegration = client.getIntegrationById('BrowserTracing') as BrowserTracing | undefined;
   const breadcrumbsIntegration = client.getIntegrationById('Breadcrumbs') as Breadcrumbs | undefined;
 
@@ -147,7 +149,10 @@ function instrumentSvelteKitFetch(originalFetch: SvelteKitFetch): SvelteKitFetch
   const shouldAttachHeaders: (url: string) => boolean = url => {
     return (
       !!shouldTraceFetch &&
-      stringMatchesSomePattern(url, browserTracingOptions.tracePropagationTargets || ['localhost', /^\//])
+      stringMatchesSomePattern(
+        url,
+        options.tracePropagationTargets || browserTracingOptions.tracePropagationTargets || ['localhost', /^\//],
+      )
     );
   };
 
@@ -177,20 +182,15 @@ function instrumentSvelteKitFetch(originalFetch: SvelteKitFetch): SvelteKitFetch
       };
 
       const patchedInit: RequestInit = { ...init };
-      const activeSpan = getCurrentHub().getScope().getSpan();
-      const activeTransaction = activeSpan && activeSpan.transaction;
-
-      const createSpan = activeTransaction && shouldCreateSpan(rawUrl);
-      const attachHeaders = createSpan && activeTransaction && shouldAttachHeaders(rawUrl);
-
-      // only attach headers if we should create a span
-      if (attachHeaders) {
-        const dsc = activeTransaction.getDynamicSamplingContext();
+      const hub = getCurrentHub();
+      const scope = hub.getScope();
+      const client = hub.getClient();
 
+      if (client && shouldAttachHeaders(rawUrl)) {
         const headers = addTracingHeadersToFetchRequest(
           input as string | Request,
-          dsc,
-          activeSpan,
+          client,
+          scope,
           patchedInit as {
             headers:
               | {
@@ -207,7 +207,7 @@ function instrumentSvelteKitFetch(originalFetch: SvelteKitFetch): SvelteKitFetch
 
       const patchedFetchArgs = [input, patchedInit];
 
-      if (createSpan) {
+      if (shouldCreateSpan(rawUrl)) {
         fetchPromise = trace(
           {
             name: `${method} ${requestData.url}`, // this will become the description of the span

diff --git a/packages/sveltekit/test/client/load.test.ts b/packages/sveltekit/test/client/load.test.ts
@@ -61,6 +61,7 @@ const mockedGetIntegrationById = vi.fn(id => {
 const mockedGetClient = vi.fn(() => {
   return {
     getIntegrationById: mockedGetIntegrationById,
+    getOptions: () => ({}),
   };
 });
 
@@ -77,6 +78,11 @@ vi.mock('@sentry/core', async () => {
         getClient: mockedGetClient,
         getScope: () => {
           return {
+            getPropagationContext: () => ({
+              traceId: '1234567890abcdef1234567890abcdef',
+              spanId: '1234567890abcdef',
+              sampled: false,
+            }),
             getSpan: () => {
               return {
                 transaction: {
@@ -371,7 +377,7 @@ describe('wrapLoadWithSentry', () => {
         mockedBrowserTracing.options.traceFetch = true;
       });
 
-      it("doesn't create a span nor propagate headers, if `shouldCreateSpanForRequest` returns false", async () => {
+      it("doesn't create a span if `shouldCreateSpanForRequest` returns false", async () => {
         mockedBrowserTracing.options.shouldCreateSpanForRequest = () => false;
 
         const wrappedLoad = wrapLoadWithSentry(load);
@@ -391,10 +397,6 @@ describe('wrapLoadWithSentry', () => {
           expect.any(Function),
         );
 
-        expect(mockedSveltekitFetch).toHaveBeenCalledWith(
-          ...[originalFetchArgs[0], originalFetchArgs.length === 2 ? originalFetchArgs[1] : {}],
-        );
-
         mockedBrowserTracing.options.shouldCreateSpanForRequest = () => true;
       });
 

diff --git a/packages/tracing-internal/src/browser/request.ts b/packages/tracing-internal/src/browser/request.ts
@@ -1,11 +1,12 @@
 /* eslint-disable max-lines */
-import { getCurrentHub, hasTracingEnabled } from '@sentry/core';
-import type { DynamicSamplingContext, Span } from '@sentry/types';
+import { getCurrentHub, getDynamicSamplingContextFromClient, hasTracingEnabled } from '@sentry/core';
+import type { Client, Scope, Span } from '@sentry/types';
 import {
   addInstrumentationHandler,
   BAGGAGE_HEADER_NAME,
   browserPerformanceTimeOrigin,
   dynamicSamplingContextToSentryBaggageHeader,
+  generateSentryTraceHeader,
   isInstanceOf,
   SENTRY_XHR_DATA_KEY,
   stringMatchesSomePattern,
@@ -219,12 +220,14 @@ export function fetchCallback(
   shouldCreateSpan: (url: string) => boolean,
   shouldAttachHeaders: (url: string) => boolean,
   spans: Record<string, Span>,
-): Span | void {
-  if (!hasTracingEnabled() || !(handlerData.fetchData && shouldCreateSpan(handlerData.fetchData.url))) {
-    return;
+): Span | undefined {
+  if (!hasTracingEnabled() || !handlerData.fetchData) {
+    return undefined;
   }
 
-  if (handlerData.endTimestamp) {
+  const shouldCreateSpanResult = shouldCreateSpan(handlerData.fetchData.url);
+
+  if (handlerData.endTimestamp && shouldCreateSpanResult) {
     const spanId = handlerData.fetchData.__span;
     if (!spanId) return;
 
@@ -251,27 +254,35 @@ export function fetchCallback(
       // eslint-disable-next-line @typescript-eslint/no-dynamic-delete
       delete spans[spanId];
     }
-    return;
+    return undefined;
   }
 
-  const currentSpan = getCurrentHub().getScope().getSpan();
-  const activeTransaction = currentSpan && currentSpan.transaction;
-
-  if (currentSpan && activeTransaction) {
-    const { method, url } = handlerData.fetchData;
-    const span = currentSpan.startChild({
-      data: {
-        url,
-        type: 'fetch',
-        'http.method': method,
-      },
-      description: `${method} ${url}`,
-      op: 'http.client',
-    });
-
+  const hub = getCurrentHub();
+  const scope = hub.getScope();
+  const client = hub.getClient();
+  const parentSpan = scope.getSpan();
+
+  const { method, url } = handlerData.fetchData;
+
+  const span =
+    shouldCreateSpanResult && parentSpan
+      ? parentSpan.startChild({
+          data: {
+            url,
+            type: 'fetch',
+            'http.method': method,
+          },
+          description: `${method} ${url}`,
+          op: 'http.client',
+        })
+      : undefined;
+
+  if (span) {
     handlerData.fetchData.__span = span.spanId;
     spans[span.spanId] = span;
+  }
 
+  if (shouldAttachHeaders(handlerData.fetchData.url) && client) {
     const request: string | Request = handlerData.args[0];
 
     // In case the user hasn't set the second argument of a fetch call we default it to `{}`.
@@ -280,35 +291,42 @@ export function fetchCallback(
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
     const options: { [key: string]: any } = handlerData.args[1];
 
-    if (shouldAttachHeaders(handlerData.fetchData.url)) {
-      options.headers = addTracingHeadersToFetchRequest(
-        request,
-        activeTransaction.getDynamicSamplingContext(),
-        span,
-        options,
-      );
-    }
-    return span;
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access
+    options.headers = addTracingHeadersToFetchRequest(request, client, scope, options);
   }
+
+  return span;
 }
 
 /**
  * Adds sentry-trace and baggage headers to the various forms of fetch headers
  */
 export function addTracingHeadersToFetchRequest(
   request: string | unknown, // unknown is actually type Request but we can't export DOM types from this package,
-  dynamicSamplingContext: Partial<DynamicSamplingContext>,
-  span: Span,
+  client: Client,
+  scope: Scope,
   options: {
     headers?:
       | {
           [key: string]: string[] | string | undefined;
         }
       | PolymorphicRequestHeaders;
   },
-): PolymorphicRequestHeaders {
+): PolymorphicRequestHeaders | undefined {
+  const span = scope.getSpan();
+
+  const transaction = span && span.transaction;
+
+  const { traceId, sampled, dsc } = scope.getPropagationContext();
+
+  const sentryTraceHeader = span ? span.toTraceparent() : generateSentryTraceHeader(traceId, undefined, sampled);
+  const dynamicSamplingContext = transaction
+    ? transaction.getDynamicSamplingContext()
+    : dsc
+    ? dsc
+    : getDynamicSamplingContextFromClient(traceId, client, scope);
+
   const sentryBaggageHeader = dynamicSamplingContextToSentryBaggageHeader(dynamicSamplingContext);
-  const sentryTraceHeader = span.toTraceparent();
 
   const headers =
     typeof Request !== 'undefined' && isInstanceOf(request, Request) ? (request as Request).headers : options.headers;
@@ -364,25 +382,24 @@ export function addTracingHeadersToFetchRequest(
  *
  * @returns Span if a span was created, otherwise void.
  */
+// eslint-disable-next-line complexity
 export function xhrCallback(
   handlerData: XHRData,
   shouldCreateSpan: (url: string) => boolean,
   shouldAttachHeaders: (url: string) => boolean,
   spans: Record<string, Span>,
-): Span | void {
+): Span | undefined {
   const xhr = handlerData.xhr;
   const sentryXhrData = xhr && xhr[SENTRY_XHR_DATA_KEY];
 
-  if (
-    !hasTracingEnabled() ||
-    (xhr && xhr.__sentry_own_request__) ||
-    !(xhr && sentryXhrData && shouldCreateSpan(sentryXhrData.url))
-  ) {
-    return;
+  if (!hasTracingEnabled() || (xhr && xhr.__sentry_own_request__) || !xhr || !sentryXhrData) {
+    return undefined;
   }
 
+  const shouldCreateSpanResult = shouldCreateSpan(sentryXhrData.url);
+
   // check first if the request has finished and is tracked by an existing span which should now end
-  if (handlerData.endTimestamp) {
+  if (handlerData.endTimestamp && shouldCreateSpanResult) {
     const spanId = xhr.__sentry_xhr_span_id__;
     if (!spanId) return;
 
@@ -394,45 +411,68 @@ export function xhrCallback(
       // eslint-disable-next-line @typescript-eslint/no-dynamic-delete
       delete spans[spanId];
     }
-    return;
+    return undefined;
   }
 
-  const currentSpan = getCurrentHub().getScope().getSpan();
-  const activeTransaction = currentSpan && currentSpan.transaction;
-
-  if (currentSpan && activeTransaction) {
-    const span = currentSpan.startChild({
-      data: {
-        ...sentryXhrData.data,
-        type: 'xhr',
-        'http.method': sentryXhrData.method,
-        url: sentryXhrData.url,
-      },
-      description: `${sentryXhrData.method} ${sentryXhrData.url}`,
-      op: 'http.client',
-    });
-
+  const hub = getCurrentHub();
+  const scope = hub.getScope();
+  const parentSpan = scope.getSpan();
+
+  const span =
+    shouldCreateSpanResult && parentSpan
+      ? parentSpan.startChild({
+          data: {
+            ...sentryXhrData.data,
+            type: 'xhr',
+            'http.method': sentryXhrData.method,
+            url: sentryXhrData.url,
+          },
+          description: `${sentryXhrData.method} ${sentryXhrData.url}`,
+          op: 'http.client',
+        })
+      : undefined;
+
+  if (span) {
     xhr.__sentry_xhr_span_id__ = span.spanId;
     spans[xhr.__sentry_xhr_span_id__] = span;
+  }
 
-    if (xhr.setRequestHeader && shouldAttachHeaders(sentryXhrData.url)) {
-      try {
-        xhr.setRequestHeader('sentry-trace', span.toTraceparent());
+  if (xhr.setRequestHeader && shouldAttachHeaders(sentryXhrData.url)) {
+    if (span) {
+      const transaction = span && span.transaction;
+      const dynamicSamplingContext = transaction && transaction.getDynamicSamplingContext();
+      const sentryBaggageHeader = dynamicSamplingContextToSentryBaggageHeader(dynamicSamplingContext);
+      setHeaderOnXhr(xhr, span.toTraceparent(), sentryBaggageHeader);
+    } else {
+      const client = hub.getClient();
+      const { traceId, sampled, dsc } = scope.getPropagationContext();
+      const sentryTraceHeader = generateSentryTraceHeader(traceId, undefined, sampled);
+      const dynamicSamplingContext =
+        dsc || (client ? getDynamicSamplingContextFromClient(traceId, client, scope) : undefined);
+      const sentryBaggageHeader = dynamicSamplingContextToSentryBaggageHeader(dynamicSamplingContext);
+      setHeaderOnXhr(xhr, sentryTraceHeader, sentryBaggageHeader);
+    }
+  }
 
-        const dynamicSamplingContext = activeTransaction.getDynamicSamplingContext();
-        const sentryBaggageHeader = dynamicSamplingContextToSentryBaggageHeader(dynamicSamplingContext);
+  return span;
+}
 
-        if (sentryBaggageHeader) {
-          // From MDN: "If this method is called several times with the same header, the values are merged into one single request header."
-          // We can therefore simply set a baggage header without checking what was there before
-          // https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/setRequestHeader
-          xhr.setRequestHeader(BAGGAGE_HEADER_NAME, sentryBaggageHeader);
-        }
-      } catch (_) {
-        // Error: InvalidStateError: Failed to execute 'setRequestHeader' on 'XMLHttpRequest': The object's state must be OPENED.
-      }
+function setHeaderOnXhr(
+  xhr: NonNullable<XHRData['xhr']>,
+  sentryTraceHeader: string,
+  sentryBaggageHeader: string | undefined,
+): void {
+  try {
+    // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+    xhr.setRequestHeader!('sentry-trace', sentryTraceHeader);
+    if (sentryBaggageHeader) {
+      // From MDN: "If this method is called several times with the same header, the values are merged into one single request header."
+      // We can therefore simply set a baggage header without checking what was there before
+      // https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/setRequestHeader
+      // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+      xhr.setRequestHeader!(BAGGAGE_HEADER_NAME, sentryBaggageHeader);
     }
-
-    return span;
+  } catch (_) {
+    // Error: InvalidStateError: Failed to execute 'setRequestHeader' on 'XMLHttpRequest': The object's state must be OPENED.
   }
 }
diff --git a/packages/tracing-internal/test/browser/request.test.ts b/packages/tracing-internal/test/browser/request.test.ts
@@ -98,9 +98,7 @@ describe('callbacks', () => {
       // each case is [shouldCreateSpanReturnValue, shouldAttachHeadersReturnValue, expectedSpan, expectedHeaderKeys]
       [true, true, expect.objectContaining(fetchSpan), ['sentry-trace', 'baggage']],
       [true, false, expect.objectContaining(fetchSpan), []],
-      // If there's no span then there's no parent span id to stick into a header, so no headers, even if there's a
-      // `tracingOrigins` match
-      [false, true, undefined, []],
+      [false, true, undefined, ['sentry-trace', 'baggage']],
       [false, false, undefined, []],
     ])(
       'span creation/header attachment interaction - shouldCreateSpan: %s, shouldAttachHeaders: %s',
@@ -284,9 +282,7 @@ describe('callbacks', () => {
       // each case is [shouldCreateSpanReturnValue, shouldAttachHeadersReturnValue, expectedSpan, expectedHeaderKeys]
       [true, true, expect.objectContaining(xhrSpan), ['sentry-trace', 'baggage']],
       [true, false, expect.objectContaining(xhrSpan), []],
-      // If there's no span then there's no parent span id to stick into a header, so no headers, even if there's a
-      // `tracingOrigins` match
-      [false, true, undefined, []],
+      [false, true, undefined, ['sentry-trace', 'baggage']],
       [false, false, undefined, []],
     ])(
       'span creation/header attachment interaction - shouldCreateSpan: %s, shouldAttachHeaders: %s',