fix(node): Ensure isolation scope is correctly cloned for non-recording spans

Author: Lms24

dev-packages/node-integration-tests/suites/express/handle-error-no-tracesSampleRate/server.ts

@@ -0,0 +1,23 @@
+import { loggingTransport } from '@sentry-internal/node-integration-tests';
+import * as Sentry from '@sentry/node';
+
+Sentry.init({
+  dsn: 'https://public@dsn.ingest.sentry.io/1337',
+  release: '1.0',
+  transport: loggingTransport,
+  debug: true,
+  tracesSampleRate: 0,
+});
+
+import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests';
+import express from 'express';
+
+const app = express();
+
+app.get('/test/express/:id', req => {
+  throw new Error(`test_error with id ${req.params.id}`);
+});
+
+Sentry.setupExpressErrorHandler(app);
+
+startExpressServerAndSendPortToRunner(app);

dev-packages/node-integration-tests/suites/express/handle-error-no-tracesSampleRate/test.ts

@@ -0,0 +1,39 @@
+import { cleanupChildProcesses, createRunner } from '../../../utils/runner';
+
+afterAll(() => {
+  cleanupChildProcesses();
+});
+
+test('should capture and send Express controller error.', done => {
+  const runner = createRunner(__dirname, 'server.ts')
+    .ignore('session', 'sessions')
+    .expect({
+      event: {
+        exception: {
+          values: [
+            {
+              mechanism: {
+                type: 'middleware',
+                handled: false,
+              },
+              type: 'Error',
+              value: 'test_error with id 123',
+              stacktrace: {
+                frames: expect.arrayContaining([
+                  expect.objectContaining({
+                    function: expect.any(String),
+                    lineno: expect.any(Number),
+                    colno: expect.any(Number),
+                  }),
+                ]),
+              },
+            },
+          ],
+        },
+        transaction: 'GET /test/express/:id',
+      },
+    })
+    .start(done);
+
+  expect(() => runner.makeRequest('get', '/test/express/123')).rejects.toThrow();
+});

dev-packages/node-integration-tests/suites/express/handle-error/server.ts renamed to dev-packages/node-integration-tests/suites/express/handle-error-tracesSampleRate-0/server.ts

@@ -5,15 +5,16 @@ Sentry.init({
   dsn: 'https://public@dsn.ingest.sentry.io/1337',
   release: '1.0',
   transport: loggingTransport,
+  debug: true,
 });
 
 import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests';
 import express from 'express';
 
 const app = express();
 
-app.get('/test/express', () => {
-  throw new Error('test_error');
+app.get('/test/express/:id', req => {
+  throw new Error(`test_error with id ${req.params.id}`);
 });
 
 Sentry.setupExpressErrorHandler(app);

dev-packages/node-integration-tests/suites/express/handle-error/test.ts renamed to dev-packages/node-integration-tests/suites/express/handle-error-tracesSampleRate-0/test.ts

@@ -17,7 +17,7 @@ test('should capture and send Express controller error.', done => {
                 handled: false,
               },
               type: 'Error',
-              value: 'test_error',
+              value: 'test_error with id 123',
               stacktrace: {
                 frames: expect.arrayContaining([
                   expect.objectContaining({
@@ -34,5 +34,5 @@ test('should capture and send Express controller error.', done => {
     })
     .start(done);
 
-  expect(() => runner.makeRequest('get', '/test/express')).rejects.toThrow();
+  expect(() => runner.makeRequest('get', '/test/express/123')).rejects.toThrow();
 });

packages/node/src/integrations/http.ts

@@ -93,7 +93,7 @@ const _httpIntegration = ((options: HttpOptions = {}) => {
           requestHook: (span, req) => {
             addOriginToSpan(span, 'auto.http.otel.http');
 
-            if (getSpanKind(span) !== SpanKind.SERVER) {
+            if (span.isRecording() && getSpanKind(span) !== SpanKind.SERVER) {
               return;
             }
 

packages/node/src/integrations/tracing/express.ts

@@ -1,10 +1,11 @@
 import type * as http from 'http';
 import { registerInstrumentations } from '@opentelemetry/instrumentation';
 import { ExpressInstrumentation } from '@opentelemetry/instrumentation-express';
-import { defineIntegration } from '@sentry/core';
+import { defineIntegration, getDefaultIsolationScope } from '@sentry/core';
 import { captureException, getClient, getIsolationScope } from '@sentry/core';
 import type { IntegrationFn } from '@sentry/types';
 
+import { logger } from '@sentry/utils';
 import type { NodeClient } from '../../sdk/client';
 import { addOriginToSpan } from '../../utils/addOriginToSpan';
 
@@ -19,6 +20,10 @@ const _expressIntegration = (() => {
               addOriginToSpan(span, 'auto.http.otel.express');
             },
             spanNameHook(info, defaultName) {
+              if (getIsolationScope() === getDefaultIsolationScope()) {
+                logger.warn('Isolation scope is still default isolation scope - skipping setting transactionName');
+                return defaultName;
+              }
               if (info.layerType === 'request_handler') {
                 // type cast b/c Otel unfortunately types info.request as any :(
                 const req = info.request as { method?: string };