fix(browser): Only start http.client spans if there is an active pa…

…rent span (#11974) We introduced sending standalone `http.client` spans in #11783. Subsequently, we had to revert this change due to internal timeline problems (😢). However, in the revert PR (#11879) we missed that originally, we checked for an active parent span and only then created an `http.client` span. The result were `http.client` _transactions_ which we also don't want. This patch fixes that by re-introducing the `hasParent` check as it was pre-#11783.
getsentry · May 16, 2024 · 4f3fc0a · 4f3fc0a
1 parent 06f47c3
commit 4f3fc0a
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 33 deletions.
diff --git a/packages/browser/src/tracing/request.ts b/packages/browser/src/tracing/request.ts
@@ -7,6 +7,7 @@ import {
   SEMANTIC_ATTRIBUTE_SENTRY_OP,
   SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
   SentryNonRecordingSpan,
+  getActiveSpan,
   getClient,
   getCurrentScope,
   getDynamicSamplingContextFromClient,
@@ -324,20 +325,23 @@ export function xhrCallback(
   const fullUrl = getFullURL(sentryXhrData.url);
   const host = fullUrl ? parseUrl(fullUrl).host : undefined;
 
-  const span = shouldCreateSpanResult
-    ? startInactiveSpan({
-        name: `${sentryXhrData.method} ${sentryXhrData.url}`,
-        attributes: {
-          type: 'xhr',
-          'http.method': sentryXhrData.method,
-          'http.url': fullUrl,
-          url: sentryXhrData.url,
-          'server.address': host,
-          [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.browser',
-          [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.client',
-        },
-      })
-    : new SentryNonRecordingSpan();
+  const hasParent = !!getActiveSpan();
+
+  const span =
+    shouldCreateSpanResult && hasParent
+      ? startInactiveSpan({
+          name: `${sentryXhrData.method} ${sentryXhrData.url}`,
+          attributes: {
+            type: 'xhr',
+            'http.method': sentryXhrData.method,
+            'http.url': fullUrl,
+            url: sentryXhrData.url,
+            'server.address': host,
+            [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.browser',
+            [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.client',
+          },
+        })
+      : new SentryNonRecordingSpan();
 
   xhr.__sentry_xhr_span_id__ = span.spanContext().spanId;
   spans[xhr.__sentry_xhr_span_id__] = span;
@@ -348,9 +352,10 @@ export function xhrCallback(
     addTracingHeadersToXhrRequest(
       xhr,
       client,
-      // If performance is disabled (TWP), we do not want to use the span as base for the trace headers,
+      // If performance is disabled (TWP) or there's no active root span (pageload/navigation/interaction),
+      // we do not want to use the span as base for the trace headers,
       // which means that the headers will be generated from the scope and the sampling decision is deferred
-      hasTracingEnabled() ? span : undefined,
+      hasTracingEnabled() && hasParent ? span : undefined,
     );
   }
 

diff --git a/packages/core/src/fetch.ts b/packages/core/src/fetch.ts
@@ -17,7 +17,7 @@ import {
 } from './tracing';
 import { SentryNonRecordingSpan } from './tracing/sentryNonRecordingSpan';
 import { hasTracingEnabled } from './utils/hasTracingEnabled';
-import { spanToTraceHeader } from './utils/spanUtils';
+import { getActiveSpan, spanToTraceHeader } from './utils/spanUtils';
 
 type PolymorphicRequestHeaders =
   | Record<string, string | undefined>
@@ -70,20 +70,23 @@ export function instrumentFetchRequest(
   const fullUrl = getFullURL(url);
   const host = fullUrl ? parseUrl(fullUrl).host : undefined;
 
-  const span = shouldCreateSpanResult
-    ? startInactiveSpan({
-        name: `${method} ${url}`,
-        attributes: {
-          url,
-          type: 'fetch',
-          'http.method': method,
-          'http.url': fullUrl,
-          'server.address': host,
-          [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: spanOrigin,
-          [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.client',
-        },
-      })
-    : new SentryNonRecordingSpan();
+  const hasParent = !!getActiveSpan();
+
+  const span =
+    shouldCreateSpanResult && hasParent
+      ? startInactiveSpan({
+          name: `${method} ${url}`,
+          attributes: {
+            url,
+            type: 'fetch',
+            'http.method': method,
+            'http.url': fullUrl,
+            'server.address': host,
+            [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: spanOrigin,
+            [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.client',
+          },
+        })
+      : new SentryNonRecordingSpan();
 
   handlerData.fetchData.__span = span.spanContext().spanId;
   spans[span.spanContext().spanId] = span;
@@ -102,9 +105,10 @@ export function instrumentFetchRequest(
       client,
       scope,
       options,
-      // If performance is disabled (TWP), we do not want to use the span as base for the trace headers,
+      // If performance is disabled (TWP) or there's no active root span (pageload/navigation/interaction),
+      // we do not want to use the span as base for the trace headers,
       // which means that the headers will be generated from the scope and the sampling decision is deferred
-      hasTracingEnabled() ? span : undefined,
+      hasTracingEnabled() && hasParent ? span : undefined,
     );
   }