fix(node): Do not emit fetch spans when tracing is disabled (#13003)

Ensure that TWP works with fetch, but we do not emit a span. Otherwise,
if a user does not use our tracing functionality but adds their own
fetch instrumentation, they get spans twice.

The benefit we have with fetch is that fetch is instrumented via
diagnostics channel, which means that this can be instrumented multiple
times. So if we instrument it, on the plus side this does not interfere
with a user instrumenting it again themselves if they want.

However, with the previous implementation our integration would always
emit spans. If a user is using our own sampler, this is "OK" because it
will sample it out. But if a user does not use our sampler, the spans we
emit there get sent unintentionally.

With this change, we ensure to ignore requests if tracing is disabled.
In order to make TWP work, we instead manually inject the propagator
headers into the fetch request in this scenario.

Part of #12984

Fixes #12969

Author: mydea

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

@@ -1,5 +1,5 @@
 {
-  "name": "node-otel-sdk-trace",
+  "name": "node-otel-sdk-node",
   "version": "1.0.0",
   "private": true,
   "scripts": {

dev-packages/e2e-tests/test-applications/node-otel-sdk-node/start-event-proxy.mjs

@@ -2,5 +2,5 @@ import { startEventProxyServer } from '@sentry-internal/test-utils';
 
 startEventProxyServer({
   port: 3031,
-  proxyServerName: 'node-otel-sdk-trace',
+  proxyServerName: 'node-otel-sdk-node',
 });

dev-packages/e2e-tests/test-applications/node-otel-sdk-node/start-otel-proxy.mjs

@@ -2,5 +2,5 @@ import { startProxyServer } from '@sentry-internal/test-utils';
 
 startProxyServer({
   port: 3032,
-  proxyServerName: 'node-otel-sdk-trace-otel',
+  proxyServerName: 'node-otel-sdk-node-otel',
 });

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

@@ -2,7 +2,7 @@ import { expect, test } from '@playwright/test';
 import { waitForError } from '@sentry-internal/test-utils';
 
 test('Sends correct error event', async ({ baseURL }) => {
-  const errorEventPromise = waitForError('node-otel-sdk-trace', event => {
+  const errorEventPromise = waitForError('node-otel-sdk-node', event => {
     return !event.type && event.exception?.values?.[0]?.value === 'This is an exception with id 123';
   });
 

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

@@ -2,15 +2,15 @@ import { expect, test } from '@playwright/test';
 import { waitForPlainRequest, waitForTransaction } from '@sentry-internal/test-utils';
 
 test('Sends an API route transaction', async ({ baseURL }) => {
-  const pageloadTransactionEventPromise = waitForTransaction('node-otel-sdk-trace', transactionEvent => {
+  const pageloadTransactionEventPromise = waitForTransaction('node-otel-sdk-node', transactionEvent => {
     return (
       transactionEvent?.contexts?.trace?.op === 'http.server' &&
       transactionEvent?.transaction === 'GET /test-transaction'
     );
   });
 
   // Ensure we also send data to the OTLP endpoint
-  const otelPromise = waitForPlainRequest('node-otel-sdk-trace-otel', data => {
+  const otelPromise = waitForPlainRequest('node-otel-sdk-node-otel', data => {
     const json = JSON.parse(data) as any;
 
     return json.resourceSpans.length > 0;
@@ -129,7 +129,7 @@ test('Sends an API route transaction', async ({ baseURL }) => {
 });
 
 test('Sends an API route transaction for an errored route', async ({ baseURL }) => {
-  const transactionEventPromise = waitForTransaction('node-otel-sdk-trace', transactionEvent => {
+  const transactionEventPromise = waitForTransaction('node-otel-sdk-node', transactionEvent => {
     return (
       transactionEvent.contexts?.trace?.op === 'http.server' &&
       transactionEvent.transaction === 'GET /test-exception/:id' &&

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

@@ -0,0 +1 @@
+dist

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

@@ -0,0 +1,2 @@
+@sentry:registry=http://127.0.0.1:4873
+@sentry-internal:registry=http://127.0.0.1:4873

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

@@ -0,0 +1,34 @@
+{
+  "name": "node-otel-without-tracing",
+  "version": "1.0.0",
+  "private": true,
+  "scripts": {
+    "build": "tsc",
+    "start": "node dist/app.js",
+    "test": "playwright test",
+    "clean": "npx rimraf node_modules pnpm-lock.yaml",
+    "test:build": "pnpm install && pnpm build",
+    "test:assert": "pnpm test"
+  },
+  "dependencies": {
+    "@opentelemetry/sdk-trace-node": "1.25.1",
+    "@opentelemetry/exporter-trace-otlp-http": "0.52.1",
+    "@opentelemetry/instrumentation-undici": "0.4.0",
+    "@opentelemetry/instrumentation": "0.52.1",
+    "@sentry/core": "latest || *",
+    "@sentry/node": "latest || *",
+    "@sentry/opentelemetry": "latest || *",
+    "@sentry/types": "latest || *",
+    "@types/express": "4.17.17",
+    "@types/node": "18.15.1",
+    "express": "4.19.2",
+    "typescript": "4.9.5"
+  },
+  "devDependencies": {
+    "@playwright/test": "^1.44.1",
+    "@sentry-internal/test-utils": "link:../../../test-utils"
+  },
+  "volta": {
+    "extends": "../../package.json"
+  }
+}

dev-packages/e2e-tests/test-applications/node-otel-without-tracing/playwright.config.mjs

@@ -0,0 +1,34 @@
+import { getPlaywrightConfig } from '@sentry-internal/test-utils';
+
+const config = getPlaywrightConfig(
+  {
+    startCommand: `pnpm start`,
+  },
+  {
+    webServer: [
+      {
+        command: `node ./start-event-proxy.mjs`,
+        port: 3031,
+        stdout: 'pipe',
+        stderr: 'pipe',
+      },
+      {
+        command: `node ./start-otel-proxy.mjs`,
+        port: 3032,
+        stdout: 'pipe',
+        stderr: 'pipe',
+      },
+      {
+        command: 'pnpm start',
+        port: 3030,
+        stdout: 'pipe',
+        stderr: 'pipe',
+        env: {
+          PORT: 3030,
+        },
+      },
+    ],
+  },
+);
+
+export default config;

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

@@ -0,0 +1,55 @@
+import './instrument';
+
+// Other imports below
+import * as Sentry from '@sentry/node';
+import express from 'express';
+
+const app = express();
+const port = 3030;
+
+app.get('/test-success', function (req, res) {
+  res.send({ version: 'v1' });
+});
+
+app.get('/test-param/:param', function (req, res) {
+  res.send({ paramWas: req.params.param });
+});
+
+app.get('/test-transaction', function (req, res) {
+  Sentry.withActiveSpan(null, async () => {
+    Sentry.startSpan({ name: 'test-transaction', op: 'e2e-test' }, () => {
+      Sentry.startSpan({ name: 'test-span' }, () => undefined);
+    });
+
+    await fetch('http://localhost:3030/test-success');
+
+    await Sentry.flush();
+
+    res.send({});
+  });
+});
+
+app.get('/test-error', async function (req, res) {
+  const exceptionId = Sentry.captureException(new Error('This is an error'));
+
+  await Sentry.flush(2000);
+
+  res.send({ exceptionId });
+});
+
+app.get('/test-exception/:id', function (req, _res) {
+  throw new Error(`This is an exception with id ${req.params.id}`);
+});
+
+Sentry.setupExpressErrorHandler(app);
+
+app.use(function onError(err: unknown, req: any, res: any, next: any) {
+  // The error id is attached to `res.sentry` to be returned
+  // and optionally displayed to the user for support.
+  res.statusCode = 500;
+  res.end(res.sentry + '\n');
+});
+
+app.listen(port, () => {
+  console.log(`Example app listening on port ${port}`);
+});

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

@@ -0,0 +1,41 @@
+const { NodeTracerProvider, BatchSpanProcessor } = require('@opentelemetry/sdk-trace-node');
+const { OTLPTraceExporter } = require('@opentelemetry/exporter-trace-otlp-http');
+const Sentry = require('@sentry/node');
+const { SentrySpanProcessor, SentryPropagator } = require('@sentry/opentelemetry');
+const { UndiciInstrumentation } = require('@opentelemetry/instrumentation-undici');
+const { registerInstrumentations } = require('@opentelemetry/instrumentation');
+
+const sentryClient = Sentry.init({
+  environment: 'qa', // dynamic sampling bias to keep transactions
+  dsn:
+    process.env.E2E_TEST_DSN ||
+    'https://3b6c388182fb435097f41d181be2b2ba@o4504321058471936.ingest.sentry.io/4504321066008576',
+  includeLocalVariables: true,
+  debug: !!process.env.DEBUG,
+  tunnel: `http://localhost:3031/`, // proxy server
+  // Tracing is completely disabled
+
+  // Custom OTEL setup
+  skipOpenTelemetrySetup: true,
+});
+
+// Create and configure NodeTracerProvider
+const provider = new NodeTracerProvider({});
+
+provider.addSpanProcessor(
+  new BatchSpanProcessor(
+    new OTLPTraceExporter({
+      url: 'http://localhost:3032/',
+    }),
+  ),
+);
+
+// Initialize the provider
+provider.register({
+  propagator: new SentryPropagator(),
+  contextManager: new Sentry.SentryContextManager(),
+});
+
+registerInstrumentations({
+  instrumentations: [new UndiciInstrumentation()],
+});

dev-packages/e2e-tests/test-applications/node-otel-without-tracing/start-event-proxy.mjs

@@ -0,0 +1,6 @@
+import { startEventProxyServer } from '@sentry-internal/test-utils';
+
+startEventProxyServer({
+  port: 3031,
+  proxyServerName: 'node-otel-without-tracing',
+});

dev-packages/e2e-tests/test-applications/node-otel-without-tracing/start-otel-proxy.mjs

@@ -0,0 +1,6 @@
+import { startProxyServer } from '@sentry-internal/test-utils';
+
+startProxyServer({
+  port: 3032,
+  proxyServerName: 'node-otel-without-tracing-otel',
+});

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

@@ -0,0 +1,30 @@
+import { expect, test } from '@playwright/test';
+import { waitForError } from '@sentry-internal/test-utils';
+
+test('Sends correct error event', async ({ baseURL }) => {
+  const errorEventPromise = waitForError('node-otel-without-tracing', event => {
+    return !event.type && event.exception?.values?.[0]?.value === 'This is an exception with id 123';
+  });
+
+  await fetch(`${baseURL}/test-exception/123`);
+
+  const errorEvent = await errorEventPromise;
+
+  expect(errorEvent.exception?.values).toHaveLength(1);
+  expect(errorEvent.exception?.values?.[0]?.value).toBe('This is an exception with id 123');
+
+  expect(errorEvent.request).toEqual({
+    method: 'GET',
+    cookies: {},
+    headers: expect.any(Object),
+    url: 'http://localhost:3030/test-exception/123',
+  });
+
+  // This is unparametrized here because we do not have the express instrumentation
+  expect(errorEvent.transaction).toEqual('GET /test-exception/123');
+
+  expect(errorEvent.contexts?.trace).toEqual({
+    trace_id: expect.any(String),
+    span_id: expect.any(String),
+  });
+});