fix(opentelemetry): Ensure Sentry spans don't leak when tracing is disabled

dev-packages/e2e-tests/test-applications/node-otel-without-tracing/package.json

@@ -11,6 +11,7 @@
     "test:assert": "pnpm test"
   },
   "dependencies": {
+    "@opentelemetry/api": "1.9.0",
     "@opentelemetry/sdk-trace-node": "2.2.0",
     "@opentelemetry/exporter-trace-otlp-http": "0.208.0",
     "@opentelemetry/instrumentation-undici": "0.13.2",

dev-packages/e2e-tests/test-applications/node-otel-without-tracing/src/app.ts

@@ -2,6 +2,7 @@ import './instrument';
 
 // Other imports below
 import * as Sentry from '@sentry/node';
+import { trace, type Span } from '@opentelemetry/api';
 import express from 'express';
 
 const app = express();
@@ -29,6 +30,23 @@ app.get('/test-transaction', function (req, res) {
   });
 });
 
+app.get('/test-only-if-parent', function (req, res) {
+  // Remove the HTTP span from the context to simulate no parent span
+  Sentry.withActiveSpan(null, () => {
+    // This should NOT create a span because onlyIfParent is true and there's no parent
+    Sentry.startSpan({ name: 'test-only-if-parent', onlyIfParent: true }, () => {
+      // This custom OTel span SHOULD be created and exported
+      // This tests that custom OTel spans aren't suppressed when onlyIfParent triggers
+      const customTracer = trace.getTracer('custom-tracer');
+      customTracer.startActiveSpan('custom-span-with-only-if-parent', (span: Span) => {
+        span.end();
+      });
+    });
+
+    res.send({});
+  });
+});
+
 app.get('/test-error', async function (req, res) {
   const exceptionId = Sentry.captureException(new Error('This is an error'));
 

dev-packages/e2e-tests/test-applications/node-otel-without-tracing/tests/transactions.test.ts

@@ -34,10 +34,10 @@ test('Sends an API route transaction to OTLP', async ({ baseURL }) => {
   const scopeSpans = json.resourceSpans?.[0]?.scopeSpans;
   expect(scopeSpans).toBeDefined();
 
-  // Http server span & undici client spans are emitted,
-  // as well as the spans emitted via `Sentry.startSpan()`
-  // But our default node-fetch spans are not emitted
-  expect(scopeSpans.length).toEqual(3);
+  // Http server span & undici client spans are emitted.
+  // Sentry.startSpan() spans are NOT emitted (non-recording when tracing is disabled).
+  // Our default node-fetch spans are also not emitted.
+  expect(scopeSpans.length).toEqual(2);
 
   const httpScopes = scopeSpans?.filter(scopeSpan => scopeSpan.scope.name === '@opentelemetry/instrumentation-http');
   const undiciScopes = scopeSpans?.filter(
@@ -47,43 +47,8 @@ test('Sends an API route transaction to OTLP', async ({ baseURL }) => {
 
   expect(httpScopes.length).toBe(1);
 
-  expect(startSpanScopes.length).toBe(1);
-  expect(startSpanScopes[0].spans.length).toBe(2);
-  expect(startSpanScopes[0].spans).toEqual([
-    {
-      traceId: expect.any(String),
-      spanId: expect.any(String),
-      parentSpanId: expect.any(String),
-      name: 'test-span',
-      kind: 1,
-      startTimeUnixNano: expect.any(String),
-      endTimeUnixNano: expect.any(String),
-      attributes: [],
-      droppedAttributesCount: 0,
-      events: [],
-      droppedEventsCount: 0,
-      status: { code: 0 },
-      links: [],
-      droppedLinksCount: 0,
-      flags: expect.any(Number),
-    },
-    {
-      traceId: expect.any(String),
-      spanId: expect.any(String),
-      name: 'test-transaction',
-      kind: 1,
-      startTimeUnixNano: expect.any(String),
-      endTimeUnixNano: expect.any(String),
-      attributes: [{ key: 'sentry.op', value: { stringValue: 'e2e-test' } }],
-      droppedAttributesCount: 0,
-      events: [],
-      droppedEventsCount: 0,
-      status: { code: 0 },
-      links: [],
-      droppedLinksCount: 0,
-      flags: expect.any(Number),
-    },
-  ]);
+  // Sentry spans should not be exported when tracing is disabled
+  expect(startSpanScopes.length).toBe(0);
 
   // Undici spans are emitted correctly
   expect(undiciScopes.length).toBe(1);
@@ -164,3 +129,58 @@ test('Sends an API route transaction to OTLP', async ({ baseURL }) => {
     },
   ]);
 });
+
+test('Custom OTel spans work with onlyIfParent when no parent exists', async ({ baseURL }) => {
+  waitForTransaction('node-otel-without-tracing', transactionEvent => {
+    throw new Error('THIS SHOULD NEVER HAPPEN!');
+  });
+
+  // Ensure we send data to the OTLP endpoint
+  const otelPromise = waitForPlainRequest('node-otel-without-tracing-otel', data => {
+    const json = JSON.parse(data) as any;
+
+    const scopeSpans = json.resourceSpans?.[0]?.scopeSpans;
+
+    // Look for the custom span from our custom-tracer
+    const customScope = scopeSpans?.find(scopeSpan => scopeSpan.scope.name === 'custom-tracer');
+
+    return customScope && customScope.spans.some(span => span.name === 'custom-span-with-only-if-parent');
+  });
+
+  fetch(`${baseURL}/test-only-if-parent`);
+
+  const otelData = await otelPromise;
+
+  expect(otelData).toBeDefined();
+
+  const json = JSON.parse(otelData);
+  expect(json.resourceSpans.length).toBe(1);
+
+  const scopeSpans = json.resourceSpans?.[0]?.scopeSpans;
+  expect(scopeSpans).toBeDefined();
+
+  // Should have HTTP instrumentation span but NO Sentry span
+  const httpScopes = scopeSpans?.filter(scopeSpan => scopeSpan.scope.name === '@opentelemetry/instrumentation-http');
+  const sentryScopes = scopeSpans?.filter(scopeSpan => scopeSpan.scope.name === '@sentry/node');
+  const customScopes = scopeSpans?.filter(scopeSpan => scopeSpan.scope.name === 'custom-tracer');
+
+  // HTTP span exists (from the incoming request)
+  expect(httpScopes.length).toBe(1);
+
+  // Sentry span should NOT exist (onlyIfParent + no parent = suppressed)
+  expect(sentryScopes.length).toBe(0);
+
+  // Custom OTel span SHOULD exist (this is what we're testing - the fix ensures this works)
+  expect(customScopes.length).toBe(1);
+  expect(customScopes[0].spans.length).toBe(1);
+  expect(customScopes[0].spans[0]).toMatchObject({
+    name: 'custom-span-with-only-if-parent',
+    kind: 1,
+    status: { code: 0 },
+  });
+
+  // Verify the custom span is recording (not suppressed)
+  const customSpan = customScopes[0].spans[0];
+  expect(customSpan.spanId).not.toBe('0000000000000000');
+  expect(customSpan.traceId).not.toBe('00000000000000000000000000000000');
+});

packages/opentelemetry/src/trace.ts

@@ -1,6 +1,6 @@
 import type { Context, Span, SpanContext, SpanOptions, Tracer } from '@opentelemetry/api';
 import { context, SpanStatusCode, trace, TraceFlags } from '@opentelemetry/api';
-import { suppressTracing } from '@opentelemetry/core';
+import { isTracingSuppressed, suppressTracing } from '@opentelemetry/core';
 import type {
   Client,
   continueTrace as baseContinueTrace,
@@ -17,6 +17,7 @@ import {
   getRootSpan,
   getTraceContextFromScope,
   handleCallbackErrors,
+  hasSpansEnabled,
   SDK_VERSION,
   SEMANTIC_ATTRIBUTE_SENTRY_OP,
   spanToJSON,
@@ -29,16 +30,12 @@ import { getSamplingDecision } from './utils/getSamplingDecision';
 import { makeTraceState } from './utils/makeTraceState';
 
 /**
- * Wraps a function with a transaction/span and finishes the span after the function is done.
- * The created span is the active span and will be used as parent by other spans created inside the function
- * and can be accessed via `Sentry.getActiveSpan()`, as long as the function is executed while the scope is active.
- *
- * If you want to create a span that is not set as active, use {@link startInactiveSpan}.
- *
- * You'll always get a span passed to the callback,
- * it may just be a non-recording span if the span is not sampled or if tracing is disabled.
+ * Internal helper for starting spans and manual spans. See {@link startSpan} and {@link startSpanManual} for the public APIs.
+ * @param options - The span context options
+ * @param callback - The callback to execute with the span
+ * @param autoEnd - Whether to automatically end the span after the callback completes
  */
-export function startSpan<T>(options: OpenTelemetrySpanContext, callback: (span: Span) => T): T {
+function _startSpan<T>(options: OpenTelemetrySpanContext, callback: (span: Span) => T, autoEnd: boolean): T {
   const tracer = getTracer();
 
   const { name, parentSpan: customParentSpan } = options;
@@ -53,6 +50,37 @@ export function startSpan<T>(options: OpenTelemetrySpanContext, callback: (span:
 
     const spanOptions = getSpanOptions(options);
 
+    // If spans are not enabled, ensure we suppress tracing for the span creation
+    // but preserve the original context for the callback execution
+    // This ensures that we don't create spans when tracing is disabled which
+    // would otherwise be a problem for users that don't enable tracing but use
+    // custom OpenTelemetry setups.
+    if (!hasSpansEnabled()) {
+      const suppressedCtx = isTracingSuppressed(ctx) ? ctx : suppressTracing(ctx);
+
+      return context.with(suppressedCtx, () => {
+        return tracer.startActiveSpan(name, spanOptions, suppressedCtx, span => {
+          // Restore the original unsuppressed context for the callback execution
+          // so that custom OpenTelemetry spans maintain the correct context.
+          // We use activeCtx (not ctx) because ctx may be suppressed when onlyIfParent is true
+          // and no parent span exists. Using activeCtx ensures custom OTel spans are never
+          // inadvertently suppressed.
+          return context.with(activeCtx, () => {
+            return handleCallbackErrors(
+              () => callback(span),
+              () => {
+                // Only set the span status to ERROR when there wasn't any status set before, in order to avoid stomping useful span statuses
+                if (spanToJSON(span).status === undefined) {
+                  span.setStatus({ code: SpanStatusCode.ERROR });
+                }
+              },
+              autoEnd ? () => span.end() : undefined,
+            );
+          });
+        });
+      });
+    }
+
     return tracer.startActiveSpan(name, spanOptions, ctx, span => {
       return handleCallbackErrors(
         () => callback(span),
@@ -62,15 +90,29 @@ export function startSpan<T>(options: OpenTelemetrySpanContext, callback: (span:
             span.setStatus({ code: SpanStatusCode.ERROR });
           }
         },
-        () => span.end(),
+        autoEnd ? () => span.end() : undefined,
       );
     });
   });
 }
 
+/**
+ * Wraps a function with a transaction/span and finishes the span after the function is done.
+ * The created span is the active span and will be used as parent by other spans created inside the function
+ * and can be accessed via `Sentry.getActiveSpan()`, as long as the function is executed while the scope is active.
+ *
+ * If you want to create a span that is not set as active, use {@link startInactiveSpan}.
+ *
+ * You'll always get a span passed to the callback,
+ * it may just be a non-recording span if the span is not sampled or if tracing is disabled.
+ */
+export function startSpan<T>(options: OpenTelemetrySpanContext, callback: (span: Span) => T): T {
+  return _startSpan(options, callback, true);
+}
+
 /**
  * Similar to `Sentry.startSpan`. Wraps a function with a span, but does not finish the span
- * after the function is done automatically. You'll have to call `span.end()` manually.
+ * after the function is done automatically. You'll have to call `span.end()` or the `finish` function passed to the callback manually.
  *
  * The created span is the active span and will be used as parent by other spans created inside the function
  * and can be accessed via `Sentry.getActiveSpan()`, as long as the function is executed while the scope is active.
@@ -82,32 +124,7 @@ export function startSpanManual<T>(
   options: OpenTelemetrySpanContext,
   callback: (span: Span, finish: () => void) => T,
 ): T {
-  const tracer = getTracer();
-
-  const { name, parentSpan: customParentSpan } = options;
-
-  // If `options.parentSpan` is defined, we want to wrap the callback in `withActiveSpan`
-  const wrapper = getActiveSpanWrapper<T>(customParentSpan);
-
-  return wrapper(() => {
-    const activeCtx = getContext(options.scope, options.forceTransaction);
-    const shouldSkipSpan = options.onlyIfParent && !trace.getSpan(activeCtx);
-    const ctx = shouldSkipSpan ? suppressTracing(activeCtx) : activeCtx;
-
-    const spanOptions = getSpanOptions(options);
-
-    return tracer.startActiveSpan(name, spanOptions, ctx, span => {
-      return handleCallbackErrors(
-        () => callback(span, () => span.end()),
-        () => {
-          // Only set the span status to ERROR when there wasn't any status set before, in order to avoid stomping useful span statuses
-          if (spanToJSON(span).status === undefined) {
-            span.setStatus({ code: SpanStatusCode.ERROR });
-          }
-        },
-      );
-    });
-  });
+  return _startSpan(options, span => callback(span, () => span.end()), false);
 }
 
 /**
@@ -130,13 +147,15 @@ export function startInactiveSpan(options: OpenTelemetrySpanContext): Span {
   return wrapper(() => {
     const activeCtx = getContext(options.scope, options.forceTransaction);
     const shouldSkipSpan = options.onlyIfParent && !trace.getSpan(activeCtx);
-    const ctx = shouldSkipSpan ? suppressTracing(activeCtx) : activeCtx;
+    let ctx = shouldSkipSpan ? suppressTracing(activeCtx) : activeCtx;
 
     const spanOptions = getSpanOptions(options);
 
-    const span = tracer.startSpan(name, spanOptions, ctx);
+    if (!hasSpansEnabled()) {
+      ctx = isTracingSuppressed(ctx) ? ctx : suppressTracing(ctx);
+    }
 
-    return span;
+    return tracer.startSpan(name, spanOptions, ctx);
   });
 }
 

packages/opentelemetry/test/trace.test.ts

@@ -1388,6 +1388,55 @@ describe('trace (tracing disabled)', () => {
   });
 });
 
+describe('trace (spans disabled)', () => {
+  beforeEach(() => {
+    // Initialize SDK without any tracing configuration (no tracesSampleRate or tracesSampler)
+    mockSdkInit({ tracesSampleRate: undefined, tracesSampler: undefined });
+  });
+
+  afterEach(async () => {
+    await cleanupOtel();
+  });
+
+  it('startSpan creates non-recording spans when hasSpansEnabled() === false', () => {
+    const val = startSpan({ name: 'outer' }, outerSpan => {
+      expect(outerSpan).toBeDefined();
+      expect(outerSpan.isRecording()).toBe(false);
+
+      // Nested spans should also be non-recording
+      return startSpan({ name: 'inner' }, innerSpan => {
+        expect(innerSpan).toBeDefined();
+        expect(innerSpan.isRecording()).toBe(false);
+        return 'test value';
+      });
+    });
+
+    expect(val).toEqual('test value');
+  });
+
+  it('startSpanManual creates non-recording spans when hasSpansEnabled() === false', () => {
+    const val = startSpanManual({ name: 'outer' }, outerSpan => {
+      expect(outerSpan).toBeDefined();
+      expect(outerSpan.isRecording()).toBe(false);
+
+      return startSpanManual({ name: 'inner' }, innerSpan => {
+        expect(innerSpan).toBeDefined();
+        expect(innerSpan.isRecording()).toBe(false);
+        return 'test value';
+      });
+    });
+
+    expect(val).toEqual('test value');
+  });
+
+  it('startInactiveSpan returns non-recording spans when hasSpansEnabled() === false', () => {
+    const span = startInactiveSpan({ name: 'test' });
+
+    expect(span).toBeDefined();
+    expect(span.isRecording()).toBe(false);
+  });
+});
+
 describe('trace (sampling)', () => {
   afterEach(async () => {
     await cleanupOtel();