feat(node)!: Avoid http spans by default for custom OTEL setups (#14678)

With this PR, the default value for the `spans` option in the
`httpIntegration` is changed to `false`, if `skipOpenTelemetrySetup:
true` is configured. This is what you'd expect as a user, you do not
want Sentry to register any OTEL instrumentation and emit any spans in
this scenario.

Closes #14675

Author: mydea

dev-packages/e2e-tests/test-applications/node-otel-custom-sampler/src/instrument.ts

@@ -14,6 +14,7 @@ Sentry.init({
   skipOpenTelemetrySetup: true,
   // By defining _any_ sample rate, tracing integrations will be added by default
   tracesSampleRate: 0,
+  integrations: [Sentry.httpIntegration({ spans: true })],
 });
 
 const provider = new NodeTracerProvider({

dev-packages/e2e-tests/test-applications/node-otel-sdk-node/src/instrument.ts

@@ -14,6 +14,7 @@ const sentryClient = Sentry.init({
   tracesSampleRate: 1,
 
   skipOpenTelemetrySetup: true,
+  integrations: [Sentry.httpIntegration({ spans: true })],
 });
 
 const sdk = new opentelemetry.NodeSDK({

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

@@ -15,9 +15,6 @@ Sentry.init({
   debug: !!process.env.DEBUG,
   tunnel: `http://localhost:3031/`, // proxy server
   // Tracing is completely disabled
-
-  integrations: [Sentry.httpIntegration({ spans: false })],
-
   // Custom OTEL setup
   skipOpenTelemetrySetup: true,
 });

docs/migration/draft-v9-migration-guide.md

@@ -158,3 +158,4 @@
 - Deprecated `addOpenTelemetryInstrumentation`. Use the `openTelemetryInstrumentations` option in `Sentry.init()` or your custom Sentry Client instead.
 - Deprecated `registerEsmLoaderHooks.include` and `registerEsmLoaderHooks.exclude`. Set `onlyIncludeInstrumentedModules: true` instead.
 - `registerEsmLoaderHooks` will only accept `true | false | undefined` in the future. The SDK will default to wrapping modules that are used as part of OpenTelemetry Instrumentation.
+- `httpIntegration({ spans: false })` is configured by default if `skipOpenTelemetrySetup: true` is set. You can still overwrite this if desired.

docs/migration/v8-to-v9.md

@@ -78,6 +78,12 @@ If you need to support older browsers, we recommend transpiling your code using
 
 ## 2. Behavior Changes
 
+### `@sentry/node`
+
+- When `skipOpenTelemetrySetup: true` is configured, `httpIntegration({ spans: false })` will be configured by default. This means that you no longer have to specify this yourself in this scenario. With this change, no spans are emitted once `skipOpenTelemetrySetup: true` is configured, without any further configuration being needed.
+
+### Uncategorized (TODO)
+
 - Next.js withSentryConfig returning Promise
 - `request` on sdk processing metadata will be ignored going forward
 - respect sourcemap generation settings

packages/node/src/integrations/http/index.ts

@@ -8,6 +8,7 @@ import { getClient } from '@sentry/opentelemetry';
 import { generateInstrumentOnce } from '../../otel/instrument';
 import type { NodeClient } from '../../sdk/client';
 import type { HTTPModuleRequestIncomingMessage } from '../../transports/http-module';
+import type { NodeClientOptions } from '../../types';
 import { addOriginToSpan } from '../../utils/addOriginToSpan';
 import { getRequestUrl } from '../../utils/getRequestUrl';
 import { SentryHttpInstrumentation } from './SentryHttpInstrumentation';
@@ -27,6 +28,8 @@ interface HttpOptions {
    * If set to false, do not emit any spans.
    * This will ensure that the default HttpInstrumentation from OpenTelemetry is not setup,
    * only the Sentry-specific instrumentation for request isolation is applied.
+   *
+   * If `skipOpenTelemetrySetup: true` is configured, this defaults to `false`, otherwise it defaults to `true`.
    */
   spans?: boolean;
 
@@ -118,12 +121,21 @@ export const instrumentOtelHttp = generateInstrumentOnce<HttpInstrumentationConf
   return instrumentation;
 });
 
+/** Exported only for tests. */
+export function _shouldInstrumentSpans(options: HttpOptions, clientOptions: Partial<NodeClientOptions> = {}): boolean {
+  // If `spans` is passed in, it takes precedence
+  // Else, we by default emit spans, unless `skipOpenTelemetrySetup` is set to `true`
+  return typeof options.spans === 'boolean' ? options.spans : !clientOptions.skipOpenTelemetrySetup;
+}
+
 /**
  * Instrument the HTTP and HTTPS modules.
  */
 const instrumentHttp = (options: HttpOptions = {}): void => {
+  const instrumentSpans = _shouldInstrumentSpans(options, getClient<NodeClient>()?.getOptions());
+
   // This is the "regular" OTEL instrumentation that emits spans
-  if (options.spans !== false) {
+  if (instrumentSpans) {
     const instrumentationConfig = getConfigWithDefaults(options);
     instrumentOtelHttp(instrumentationConfig);
   }

packages/node/test/integrations/http.test.ts

@@ -0,0 +1,18 @@
+import { _shouldInstrumentSpans } from '../../src/integrations/http';
+
+describe('httpIntegration', () => {
+  describe('_shouldInstrumentSpans', () => {
+    it.each([
+      [{}, {}, true],
+      [{ spans: true }, {}, true],
+      [{ spans: false }, {}, false],
+      [{ spans: true }, { skipOpenTelemetrySetup: true }, true],
+      [{ spans: false }, { skipOpenTelemetrySetup: true }, false],
+      [{}, { skipOpenTelemetrySetup: true }, false],
+      [{}, { skipOpenTelemetrySetup: false }, true],
+    ])('returns the correct value for options=%p and clientOptions=%p', (options, clientOptions, expected) => {
+      const actual = _shouldInstrumentSpans(options, clientOptions);
+      expect(actual).toBe(expected);
+    });
+  });
+});