feat(core): Ensure startSpan() can handle spans that require parent (#10386)

This introduces a new `onlyIfParent` option to the start-span APIs,
which, if set, will ensure to not create a span if there is no parent.
I've added this also for otel to demonstrate this works there as well.

This PR also uses this option in a few places, esp. for http.client
spans, to ensure we do not capture these if there is no ongoing
transaction.

This replaces
https://github.com/getsentry/sentry-javascript/pull/10375/files with a
more generic solution, and includes the test from
#10376 (which passes
here as well).

Author: mydea

dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-no-active-span/init.js

@@ -0,0 +1,10 @@
+import * as Sentry from '@sentry/browser';
+
+window.Sentry = Sentry;
+
+Sentry.init({
+  dsn: 'https://[email protected]/1337',
+  // disable pageload transaction
+  integrations: [Sentry.BrowserTracing({ tracingOrigins: ['http://example.com'], startTransactionOnPageLoad: false })],
+  tracesSampleRate: 1,
+});

dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-no-active-span/subject.js

@@ -0,0 +1 @@
+fetch('http://example.com/0').then(fetch('http://example.com/1').then(fetch('http://example.com/2')));

dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-no-active-span/test.ts

@@ -0,0 +1,35 @@
+import { expect } from '@playwright/test';
+
+import { sentryTest } from '../../../../utils/fixtures';
+import { envelopeUrlRegex, shouldSkipTracingTest } from '../../../../utils/helpers';
+
+sentryTest(
+  'there should be no span created for fetch requests with no active span',
+  async ({ getLocalTestPath, page }) => {
+    if (shouldSkipTracingTest()) {
+      sentryTest.skip();
+    }
+
+    const url = await getLocalTestPath({ testDir: __dirname });
+
+    let requestCount = 0;
+    page.on('request', request => {
+      expect(envelopeUrlRegex.test(request.url())).toBe(false);
+      requestCount++;
+    });
+
+    await page.goto(url);
+
+    // Here are the requests that should exist:
+    // 1. HTML page
+    // 2. Init JS bundle
+    // 3. Subject JS bundle
+    // 4 [OPTIONAl] CDN JS bundle
+    // and then 3 fetch requests
+    if (process.env.PW_BUNDLE && process.env.PW_BUNDLE.startsWith('bundle_')) {
+      expect(requestCount).toBe(7);
+    } else {
+      expect(requestCount).toBe(6);
+    }
+  },
+);

dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-no-active-span/init.js

@@ -0,0 +1,10 @@
+import * as Sentry from '@sentry/browser';
+
+window.Sentry = Sentry;
+
+Sentry.init({
+  dsn: 'https://[email protected]/1337',
+  // disable pageload transaction
+  integrations: [Sentry.BrowserTracing({ tracingOrigins: ['http://example.com'], startTransactionOnPageLoad: false })],
+  tracesSampleRate: 1,
+});

dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-no-active-span/subject.js

@@ -0,0 +1,11 @@
+const xhr_1 = new XMLHttpRequest();
+xhr_1.open('GET', 'http://example.com/0');
+xhr_1.send();
+
+const xhr_2 = new XMLHttpRequest();
+xhr_2.open('GET', 'http://example.com/1');
+xhr_2.send();
+
+const xhr_3 = new XMLHttpRequest();
+xhr_3.open('GET', 'http://example.com/2');
+xhr_3.send();

dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-no-active-span/test.ts

@@ -0,0 +1,35 @@
+import { expect } from '@playwright/test';
+
+import { sentryTest } from '../../../../utils/fixtures';
+import { envelopeUrlRegex, shouldSkipTracingTest } from '../../../../utils/helpers';
+
+sentryTest(
+  'there should be no span created for xhr requests with no active span',
+  async ({ getLocalTestPath, page }) => {
+    if (shouldSkipTracingTest()) {
+      sentryTest.skip();
+    }
+
+    const url = await getLocalTestPath({ testDir: __dirname });
+
+    let requestCount = 0;
+    page.on('request', request => {
+      expect(envelopeUrlRegex.test(request.url())).toBe(false);
+      requestCount++;
+    });
+
+    await page.goto(url);
+
+    // Here are the requests that should exist:
+    // 1. HTML page
+    // 2. Init JS bundle
+    // 3. Subject JS bundle
+    // 4 [OPTIONAl] CDN JS bundle
+    // and then 3 fetch requests
+    if (process.env.PW_BUNDLE && process.env.PW_BUNDLE.startsWith('bundle_')) {
+      expect(requestCount).toBe(7);
+    } else {
+      expect(requestCount).toBe(6);
+    }
+  },
+);

dev-packages/browser-integration-tests/utils/helpers.ts

@@ -1,7 +1,7 @@
 import type { Page, Request } from '@playwright/test';
 import type { EnvelopeItemType, Event, EventEnvelopeHeaders } from '@sentry/types';
 
-const envelopeUrlRegex = /\.sentry\.io\/api\/\d+\/envelope\//;
+export const envelopeUrlRegex = /\.sentry\.io\/api\/\d+\/envelope\//;
 
 export const envelopeParser = (request: Request | null): unknown[] => {
   // https://develop.sentry.dev/sdk/envelopes/

packages/core/src/tracing/trace.ts

@@ -80,7 +80,9 @@ export function startSpan<T>(context: StartSpanOptions, callback: (span: Span |
     // eslint-disable-next-line deprecation/deprecation
     const parentSpan = scope.getSpan();
 
-    const activeSpan = createChildSpanOrTransaction(hub, parentSpan, ctx);
+    const shouldSkipSpan = context.onlyIfParent && !parentSpan;
+    const activeSpan = shouldSkipSpan ? undefined : createChildSpanOrTransaction(hub, parentSpan, ctx);
+
     // eslint-disable-next-line deprecation/deprecation
     scope.setSpan(activeSpan);
 
@@ -128,7 +130,9 @@ export function startSpanManual<T>(
     // eslint-disable-next-line deprecation/deprecation
     const parentSpan = scope.getSpan();
 
-    const activeSpan = createChildSpanOrTransaction(hub, parentSpan, ctx);
+    const shouldSkipSpan = context.onlyIfParent && !parentSpan;
+    const activeSpan = shouldSkipSpan ? undefined : createChildSpanOrTransaction(hub, parentSpan, ctx);
+
     // eslint-disable-next-line deprecation/deprecation
     scope.setSpan(activeSpan);
 
@@ -174,6 +178,12 @@ export function startInactiveSpan(context: StartSpanOptions): Span | undefined {
       context.scope.getSpan()
     : getActiveSpan();
 
+  const shouldSkipSpan = context.onlyIfParent && !parentSpan;
+
+  if (shouldSkipSpan) {
+    return undefined;
+  }
+
   if (parentSpan) {
     // eslint-disable-next-line deprecation/deprecation
     return parentSpan.startChild(ctx);

packages/core/test/lib/tracing/trace.test.ts

@@ -335,6 +335,28 @@ describe('startSpan', () => {
       });
     });
   });
+
+  describe('onlyIfParent', () => {
+    it('does not create a span if there is no parent', () => {
+      const span = startSpan({ name: 'test span', onlyIfParent: true }, span => {
+        return span;
+      });
+
+      expect(span).toBeUndefined();
+    });
+
+    it('creates a span if there is a parent', () => {
+      const span = startSpan({ name: 'parent span' }, () => {
+        const span = startSpan({ name: 'test span', onlyIfParent: true }, span => {
+          return span;
+        });
+
+        return span;
+      });
+
+      expect(span).toBeDefined();
+    });
+  });
 });
 
 describe('startSpanManual', () => {
@@ -415,6 +437,28 @@ describe('startSpanManual', () => {
       });
     });
   });
+
+  describe('onlyIfParent', () => {
+    it('does not create a span if there is no parent', () => {
+      const span = startSpanManual({ name: 'test span', onlyIfParent: true }, span => {
+        return span;
+      });
+
+      expect(span).toBeUndefined();
+    });
+
+    it('creates a span if there is a parent', () => {
+      const span = startSpan({ name: 'parent span' }, () => {
+        const span = startSpanManual({ name: 'test span', onlyIfParent: true }, span => {
+          return span;
+        });
+
+        return span;
+      });
+
+      expect(span).toBeDefined();
+    });
+  });
 });
 
 describe('startInactiveSpan', () => {
@@ -479,6 +523,24 @@ describe('startInactiveSpan', () => {
       span?.end();
     });
   });
+
+  describe('onlyIfParent', () => {
+    it('does not create a span if there is no parent', () => {
+      const span = startInactiveSpan({ name: 'test span', onlyIfParent: true });
+
+      expect(span).toBeUndefined();
+    });
+
+    it('creates a span if there is a parent', () => {
+      const span = startSpan({ name: 'parent span' }, () => {
+        const span = startInactiveSpan({ name: 'test span', onlyIfParent: true });
+
+        return span;
+      });
+
+      expect(span).toBeDefined();
+    });
+  });
 });
 
 describe('continueTrace', () => {

packages/opentelemetry/src/trace.ts

@@ -1,5 +1,7 @@
 import type { Span, Tracer } from '@opentelemetry/api';
+import { context } from '@opentelemetry/api';
 import { SpanStatusCode, trace } from '@opentelemetry/api';
+import { suppressTracing } from '@opentelemetry/core';
 import { SDK_VERSION, handleCallbackErrors } from '@sentry/core';
 import type { Client } from '@sentry/types';
 
@@ -22,7 +24,11 @@ export function startSpan<T>(spanContext: OpenTelemetrySpanContext, callback: (s
 
   const { name } = spanContext;
 
-  return tracer.startActiveSpan(name, spanContext, span => {
+  const activeCtx = context.active();
+  const shouldSkipSpan = spanContext.onlyIfParent && !trace.getSpan(activeCtx);
+  const ctx = shouldSkipSpan ? suppressTracing(activeCtx) : activeCtx;
+
+  return tracer.startActiveSpan(name, spanContext, ctx, span => {
     _applySentryAttributesToSpan(span, spanContext);
 
     return handleCallbackErrors(
@@ -49,7 +55,11 @@ export function startSpanManual<T>(spanContext: OpenTelemetrySpanContext, callba
 
   const { name } = spanContext;
 
-  return tracer.startActiveSpan(name, spanContext, span => {
+  const activeCtx = context.active();
+  const shouldSkipSpan = spanContext.onlyIfParent && !trace.getSpan(activeCtx);
+  const ctx = shouldSkipSpan ? suppressTracing(activeCtx) : activeCtx;
+
+  return tracer.startActiveSpan(name, spanContext, ctx, span => {
     _applySentryAttributesToSpan(span, spanContext);
 
     return handleCallbackErrors(
@@ -81,7 +91,11 @@ export function startInactiveSpan(spanContext: OpenTelemetrySpanContext): Span {
 
   const { name } = spanContext;
 
-  const span = tracer.startSpan(name, spanContext);
+  const activeCtx = context.active();
+  const shouldSkipSpan = spanContext.onlyIfParent && !trace.getSpan(activeCtx);
+  const ctx = shouldSkipSpan ? suppressTracing(activeCtx) : activeCtx;
+
+  const span = tracer.startSpan(name, spanContext, ctx);
 
   _applySentryAttributesToSpan(span, spanContext);