diff --git a/dev-packages/e2e-tests/test-applications/nestjs-11/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/nestjs-11/tests/errors.test.ts index 0fa13fea32aa..a24d1010eca4 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-11/tests/errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-11/tests/errors.test.ts @@ -50,13 +50,11 @@ test('Does not send HttpExceptions to Sentry', async ({ baseURL }) => { }); const transactionEventPromise400 = waitForTransaction('nestjs-11', transactionEvent => { - // todo(express-5): parametrize /test-expected-400-exception/:id - return transactionEvent?.transaction === 'GET /test-expected-400-exception/123'; + return transactionEvent?.transaction === 'GET /test-expected-400-exception/:id'; }); const transactionEventPromise500 = waitForTransaction('nestjs-11', transactionEvent => { - // todo(express-5): parametrize /test-expected-500-exception/:id - return transactionEvent?.transaction === 'GET /test-expected-500-exception/123'; + return transactionEvent?.transaction === 'GET /test-expected-500-exception/:id'; }); const response400 = await fetch(`${baseURL}/test-expected-400-exception/123`); @@ -81,13 +79,11 @@ test('Does not send RpcExceptions to Sentry', async ({ baseURL }) => { errorEventOccurred = true; } - // todo(express-5): parametrize /test-expected-rpc-exception/:id - return event?.transaction === 'GET /test-expected-rpc-exception/123'; + return event?.transaction === 'GET /test-expected-rpc-exception/:id'; }); const transactionEventPromise = waitForTransaction('nestjs-11', transactionEvent => { - // todo(express-5): parametrize /test-expected-rpc-exception/:id - return transactionEvent?.transaction === 'GET /test-expected-rpc-exception/123'; + return transactionEvent?.transaction === 'GET /test-expected-rpc-exception/:id'; }); const response = await fetch(`${baseURL}/test-expected-rpc-exception/123`); diff --git a/dev-packages/e2e-tests/test-applications/nestjs-11/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/nestjs-11/tests/transactions.test.ts index 7e0947d53ec1..1209eae1ada9 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-11/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-11/tests/transactions.test.ts @@ -15,7 +15,7 @@ test('Sends an API route transaction', async ({ baseURL }) => { expect(transactionEvent.contexts?.trace).toEqual({ data: { - 'sentry.source': 'url', // todo(express-5): 'route' + 'sentry.source': 'route', 'sentry.origin': 'auto.http.otel.http', 'sentry.op': 'http.server', 'sentry.sample_rate': 1, @@ -37,7 +37,7 @@ test('Sends an API route transaction', async ({ baseURL }) => { 'net.peer.port': expect.any(Number), 'http.status_code': 200, 'http.status_text': 'OK', - // 'http.route': '/test-transaction', // todo(express-5): add this line again + 'http.route': '/test-transaction', }, op: 'http.server', span_id: expect.stringMatching(/[a-f0-9]{16}/), @@ -49,7 +49,6 @@ test('Sends an API route transaction', async ({ baseURL }) => { expect(transactionEvent).toEqual( expect.objectContaining({ spans: expect.arrayContaining([ - /* todo(express-5): add this part again { data: { 'express.name': '/test-transaction', @@ -67,7 +66,7 @@ test('Sends an API route transaction', async ({ baseURL }) => { timestamp: expect.any(Number), trace_id: expect.stringMatching(/[a-f0-9]{32}/), origin: 'auto.http.otel.express', - }, */ + }, { data: { 'sentry.origin': 'manual', @@ -117,7 +116,7 @@ test('Sends an API route transaction', async ({ baseURL }) => { transaction: 'GET /test-transaction', type: 'transaction', transaction_info: { - source: 'url', // todo(express-5): 'route' + source: 'route', }, }), ); @@ -272,8 +271,7 @@ test('API route transaction includes nest pipe span for valid request', async ({ const transactionEventPromise = waitForTransaction('nestjs-11', transactionEvent => { return ( transactionEvent?.contexts?.trace?.op === 'http.server' && - // todo(express-5): parametrize test-pipe-instrumentation/:id - transactionEvent?.transaction === 'GET /test-pipe-instrumentation/123' && + transactionEvent?.transaction === 'GET /test-pipe-instrumentation/:id' && transactionEvent?.request?.url?.includes('/test-pipe-instrumentation/123') ); }); @@ -310,8 +308,7 @@ test('API route transaction includes nest pipe span for invalid request', async const transactionEventPromise = waitForTransaction('nestjs-11', transactionEvent => { return ( transactionEvent?.contexts?.trace?.op === 'http.server' && - // todo(express-5): parametrize test-pipe-instrumentation/:id - transactionEvent?.transaction === 'GET /test-pipe-instrumentation/abc' && + transactionEvent?.transaction === 'GET /test-pipe-instrumentation/:id' && transactionEvent?.request?.url?.includes('/test-pipe-instrumentation/abc') ); }); diff --git a/dev-packages/node-integration-tests/package.json b/dev-packages/node-integration-tests/package.json index 9d89dea67940..bbb7e300ecee 100644 --- a/dev-packages/node-integration-tests/package.json +++ b/dev-packages/node-integration-tests/package.json @@ -16,9 +16,11 @@ "build:types": "tsc -p tsconfig.types.json", "clean": "rimraf -g **/node_modules && run-p clean:script", "clean:script": "node scripts/clean.js", + "express-v5-install": "cd suites/express-v5 && yarn --no-lockfile", "lint": "eslint . --format stylish", "fix": "eslint . --format stylish --fix", "type-check": "tsc", + "pretest": "yarn express-v5-install", "test": "jest --config ./jest.config.js", "test:no-prisma": "jest --config ./jest.config.js", "test:watch": "yarn test --watch" diff --git a/dev-packages/node-integration-tests/suites/express-v5/handle-error-scope-data-loss/server.ts b/dev-packages/node-integration-tests/suites/express-v5/handle-error-scope-data-loss/server.ts new file mode 100644 index 000000000000..079d9834b01c --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/handle-error-scope-data-loss/server.ts @@ -0,0 +1,31 @@ +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, +}); + +import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests'; +import express from 'express'; + +const app = express(); + +Sentry.setTag('global', 'tag'); + +app.get('/test/withScope', () => { + Sentry.withScope(scope => { + scope.setTag('local', 'tag'); + throw new Error('test_error'); + }); +}); + +app.get('/test/isolationScope', () => { + Sentry.getIsolationScope().setTag('isolation-scope', 'tag'); + throw new Error('isolation_test_error'); +}); + +Sentry.setupExpressErrorHandler(app); + +startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/express-v5/handle-error-scope-data-loss/test.ts b/dev-packages/node-integration-tests/suites/express-v5/handle-error-scope-data-loss/test.ts new file mode 100644 index 000000000000..58d4a299174c --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/handle-error-scope-data-loss/test.ts @@ -0,0 +1,85 @@ +import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; + +afterAll(() => { + cleanupChildProcesses(); +}); + +/** + * Why does this test exist? + * + * We recently discovered that errors caught by global handlers will potentially loose scope data from the active scope + * where the error was originally thrown in. The simple example in this test (see subject.ts) demonstrates this behavior + * (in a Node environment but the same behavior applies to the browser; see the test there). + * + * This test nevertheless covers the behavior so that we're aware. + */ +test('withScope scope is NOT applied to thrown error caught by global handler', done => { + createRunner(__dirname, 'server.ts') + .expect({ + event: { + exception: { + values: [ + { + mechanism: { + type: 'middleware', + handled: false, + }, + type: 'Error', + value: 'test_error', + stacktrace: { + frames: expect.arrayContaining([ + expect.objectContaining({ + function: expect.any(String), + lineno: expect.any(Number), + colno: expect.any(Number), + }), + ]), + }, + }, + ], + }, + // 'local' tag is not applied to the event + tags: expect.not.objectContaining({ local: expect.anything() }), + }, + }) + .start(done) + .makeRequest('get', '/test/withScope', { expectError: true }); +}); + +/** + * This test shows that the isolation scope set tags are applied correctly to the error. + */ +test('isolation scope is applied to thrown error caught by global handler', done => { + createRunner(__dirname, 'server.ts') + .expect({ + event: { + exception: { + values: [ + { + mechanism: { + type: 'middleware', + handled: false, + }, + type: 'Error', + value: 'isolation_test_error', + stacktrace: { + frames: expect.arrayContaining([ + expect.objectContaining({ + function: expect.any(String), + lineno: expect.any(Number), + colno: expect.any(Number), + }), + ]), + }, + }, + ], + }, + tags: { + global: 'tag', + 'isolation-scope': 'tag', + }, + }, + }) + .start(done) + .makeRequest('get', '/test/isolationScope', { expectError: true }); +}); diff --git a/dev-packages/node-integration-tests/suites/express-v5/handle-error-tracesSampleRate-0/server.ts b/dev-packages/node-integration-tests/suites/express-v5/handle-error-tracesSampleRate-0/server.ts new file mode 100644 index 000000000000..3f52580dda1d --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/handle-error-tracesSampleRate-0/server.ts @@ -0,0 +1,22 @@ +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, + tracesSampleRate: 1, +}); + +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); diff --git a/dev-packages/node-integration-tests/suites/express-v5/handle-error-tracesSampleRate-0/test.ts b/dev-packages/node-integration-tests/suites/express-v5/handle-error-tracesSampleRate-0/test.ts new file mode 100644 index 000000000000..3ad6a3d2068f --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/handle-error-tracesSampleRate-0/test.ts @@ -0,0 +1,38 @@ +import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; + +afterAll(() => { + cleanupChildProcesses(); +}); + +test('should capture and send Express controller error with txn name if tracesSampleRate is 0', done => { + createRunner(__dirname, 'server.ts') + .ignore('transaction') + .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) + .makeRequest('get', '/test/express/123', { expectError: true }); +}); diff --git a/dev-packages/node-integration-tests/suites/express-v5/handle-error-tracesSampleRate-unset/server.ts b/dev-packages/node-integration-tests/suites/express-v5/handle-error-tracesSampleRate-unset/server.ts new file mode 100644 index 000000000000..38833d7a9bc7 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/handle-error-tracesSampleRate-unset/server.ts @@ -0,0 +1,21 @@ +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, +}); + +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); diff --git a/dev-packages/node-integration-tests/suites/express-v5/handle-error-tracesSampleRate-unset/test.ts b/dev-packages/node-integration-tests/suites/express-v5/handle-error-tracesSampleRate-unset/test.ts new file mode 100644 index 000000000000..b02d74016ad4 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/handle-error-tracesSampleRate-unset/test.ts @@ -0,0 +1,37 @@ +import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; + +afterAll(() => { + cleanupChildProcesses(); +}); + +test('should capture and send Express controller error if tracesSampleRate is not set.', done => { + createRunner(__dirname, 'server.ts') + .ignore('transaction') + .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), + }), + ]), + }, + }, + ], + }, + }, + }) + .start(done) + .makeRequest('get', '/test/express/123', { expectError: true }); +}); diff --git a/dev-packages/node-integration-tests/suites/express-v5/multiple-init/server.ts b/dev-packages/node-integration-tests/suites/express-v5/multiple-init/server.ts new file mode 100644 index 000000000000..39d56710f043 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/multiple-init/server.ts @@ -0,0 +1,74 @@ +import { loggingTransport } from '@sentry-internal/node-integration-tests'; +import * as Sentry from '@sentry/node'; + +Sentry.init({ + // No dsn, means client is disabled + // dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + transport: loggingTransport, +}); + +// We add http integration to ensure request isolation etc. works +const initialClient = Sentry.getClient(); +initialClient?.addIntegration(Sentry.httpIntegration()); + +// Store this so we can update the client later +const initialCurrentScope = Sentry.getCurrentScope(); + +import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests'; +import express from 'express'; + +const app = express(); + +Sentry.setTag('global', 'tag'); + +app.get('/test/no-init', (_req, res) => { + Sentry.addBreadcrumb({ message: 'no init breadcrumb' }); + Sentry.setTag('no-init', 'tag'); + + res.send({}); +}); + +app.get('/test/init', (_req, res) => { + // Call init again, but with DSN + Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + transport: loggingTransport, + }); + // Set this on initial scope, to ensure it can be inherited + initialCurrentScope.setClient(Sentry.getClient()!); + + Sentry.addBreadcrumb({ message: 'init breadcrumb' }); + Sentry.setTag('init', 'tag'); + + res.send({}); +}); + +app.get('/test/error/:id', (req, res) => { + const id = req.params.id; + Sentry.addBreadcrumb({ message: `error breadcrumb ${id}` }); + Sentry.setTag('error', id); + + Sentry.captureException(new Error(`This is an exception ${id}`)); + + setTimeout(() => { + // We flush to ensure we are sending exceptions in a certain order + Sentry.flush(1000).then( + () => { + // We send this so we can wait for this, to know the test is ended & server can be closed + if (id === '3') { + Sentry.captureException(new Error('Final exception was captured')); + } + res.send({}); + }, + () => { + res.send({}); + }, + ); + }, 1); +}); + +Sentry.setupExpressErrorHandler(app); + +startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/express-v5/multiple-init/test.ts b/dev-packages/node-integration-tests/suites/express-v5/multiple-init/test.ts new file mode 100644 index 000000000000..b80669a7c432 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/multiple-init/test.ts @@ -0,0 +1,70 @@ +import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; + +afterAll(() => { + cleanupChildProcesses(); +}); + +test('allows to call init multiple times', done => { + const runner = createRunner(__dirname, 'server.ts') + .expect({ + event: { + exception: { + values: [ + { + value: 'This is an exception 2', + }, + ], + }, + breadcrumbs: [ + { + message: 'error breadcrumb 2', + timestamp: expect.any(Number), + }, + ], + tags: { + global: 'tag', + error: '2', + }, + }, + }) + .expect({ + event: { + exception: { + values: [ + { + value: 'This is an exception 3', + }, + ], + }, + breadcrumbs: [ + { + message: 'error breadcrumb 3', + timestamp: expect.any(Number), + }, + ], + tags: { + global: 'tag', + error: '3', + }, + }, + }) + .expect({ + event: { + exception: { + values: [ + { + value: 'Final exception was captured', + }, + ], + }, + }, + }) + .start(done); + + runner + .makeRequest('get', '/test/no-init') + .then(() => runner.makeRequest('get', '/test/error/1')) + .then(() => runner.makeRequest('get', '/test/init')) + .then(() => runner.makeRequest('get', '/test/error/2')) + .then(() => runner.makeRequest('get', '/test/error/3')); +}); diff --git a/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/common-infix-parameterized/server.ts b/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/common-infix-parameterized/server.ts new file mode 100644 index 000000000000..673c146e9d8c --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/common-infix-parameterized/server.ts @@ -0,0 +1,33 @@ +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', + tracesSampleRate: 1.0, + transport: loggingTransport, +}); + +import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests'; +import cors from 'cors'; +import express from 'express'; + +const app = express(); + +app.use(cors()); + +const APIv1 = express.Router(); + +APIv1.get('/user/:userId', function (_req, res) { + Sentry.captureMessage('Custom Message'); + res.send('Success'); +}); + +const root = express.Router(); + +app.use('/api2/v1', root); +app.use('/api/v1', APIv1); + +Sentry.setupExpressErrorHandler(app); + +startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/common-infix-parameterized/test.ts b/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/common-infix-parameterized/test.ts new file mode 100644 index 000000000000..e7b9edabbfc8 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/common-infix-parameterized/test.ts @@ -0,0 +1,13 @@ +import { cleanupChildProcesses, createRunner } from '../../../../utils/runner'; + +afterAll(() => { + cleanupChildProcesses(); +}); + +test('should construct correct url with common infixes with multiple parameterized routers.', done => { + createRunner(__dirname, 'server.ts') + .ignore('transaction') + .expect({ event: { message: 'Custom Message', transaction: 'GET /api/v1/user/:userId' } }) + .start(done) + .makeRequest('get', '/api/v1/user/3212'); +}); diff --git a/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/common-infix/server.ts b/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/common-infix/server.ts new file mode 100644 index 000000000000..24073af67fa4 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/common-infix/server.ts @@ -0,0 +1,34 @@ +import { loggingTransport } from '@sentry-internal/node-integration-tests'; +import * as Sentry from '@sentry/node'; + +Sentry.init({ + debug: true, + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + tracesSampleRate: 1.0, + transport: loggingTransport, +}); + +import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests'; +import cors from 'cors'; +import express from 'express'; + +const app = express(); + +app.use(cors()); + +const APIv1 = express.Router(); + +APIv1.get('/test', function (_req, res) { + Sentry.captureMessage('Custom Message'); + res.send('Success'); +}); + +const root = express.Router(); + +app.use('/api/v1', root); +app.use('/api2/v1', APIv1); + +Sentry.setupExpressErrorHandler(app); + +startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/common-infix/test.ts b/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/common-infix/test.ts new file mode 100644 index 000000000000..52d6b631bea2 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/common-infix/test.ts @@ -0,0 +1,13 @@ +import { cleanupChildProcesses, createRunner } from '../../../../utils/runner'; + +afterAll(() => { + cleanupChildProcesses(); +}); + +test('should construct correct url with common infixes with multiple routers.', done => { + createRunner(__dirname, 'server.ts') + .ignore('transaction') + .expect({ event: { message: 'Custom Message', transaction: 'GET /api2/v1/test' } }) + .start(done) + .makeRequest('get', '/api2/v1/test'); +}); diff --git a/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/common-prefix-parameterized-reverse/server.ts b/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/common-prefix-parameterized-reverse/server.ts new file mode 100644 index 000000000000..755a32bf4389 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/common-prefix-parameterized-reverse/server.ts @@ -0,0 +1,33 @@ +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', + tracesSampleRate: 1.0, + transport: loggingTransport, +}); + +import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests'; +import cors from 'cors'; +import express from 'express'; + +const app = express(); + +app.use(cors()); + +const APIv1 = express.Router(); + +APIv1.get('/user/:userId', function (_req, res) { + Sentry.captureMessage('Custom Message'); + res.send('Success'); +}); + +const root = express.Router(); + +app.use('/api/v1', APIv1); +app.use('/api', root); + +Sentry.setupExpressErrorHandler(app); + +startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/common-prefix-parameterized-reverse/test.ts b/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/common-prefix-parameterized-reverse/test.ts new file mode 100644 index 000000000000..5fabe5b92df6 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/common-prefix-parameterized-reverse/test.ts @@ -0,0 +1,13 @@ +import { cleanupChildProcesses, createRunner } from '../../../../utils/runner'; + +afterAll(() => { + cleanupChildProcesses(); +}); + +test('should construct correct urls with multiple parameterized routers (use order reversed).', done => { + createRunner(__dirname, 'server.ts') + .ignore('transaction') + .expect({ event: { message: 'Custom Message', transaction: 'GET /api/v1/user/:userId' } }) + .start(done) + .makeRequest('get', '/api/v1/user/1234/'); +}); diff --git a/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/common-prefix-parameterized/server.ts b/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/common-prefix-parameterized/server.ts new file mode 100644 index 000000000000..7db74e8e3dea --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/common-prefix-parameterized/server.ts @@ -0,0 +1,33 @@ +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', + tracesSampleRate: 1.0, + transport: loggingTransport, +}); + +import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests'; +import cors from 'cors'; +import express from 'express'; + +const app = express(); + +app.use(cors()); + +const APIv1 = express.Router(); + +APIv1.get('/user/:userId', function (_req, res) { + Sentry.captureMessage('Custom Message'); + res.send('Success'); +}); + +const root = express.Router(); + +app.use('/api', root); +app.use('/api/v1', APIv1); + +Sentry.setupExpressErrorHandler(app); + +startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/common-prefix-parameterized/test.ts b/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/common-prefix-parameterized/test.ts new file mode 100644 index 000000000000..bab934f54522 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/common-prefix-parameterized/test.ts @@ -0,0 +1,13 @@ +import { cleanupChildProcesses, createRunner } from '../../../../utils/runner'; + +afterAll(() => { + cleanupChildProcesses(); +}); + +test('should construct correct urls with multiple parameterized routers.', done => { + createRunner(__dirname, 'server.ts') + .ignore('transaction') + .expect({ event: { message: 'Custom Message', transaction: 'GET /api/v1/user/:userId' } }) + .start(done) + .makeRequest('get', '/api/v1/user/1234/'); +}); diff --git a/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/common-prefix-same-length-parameterized copy/server.ts b/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/common-prefix-same-length-parameterized copy/server.ts new file mode 100644 index 000000000000..654afa3b8c8d --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/common-prefix-same-length-parameterized copy/server.ts @@ -0,0 +1,33 @@ +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', + tracesSampleRate: 1.0, + transport: loggingTransport, +}); + +import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests'; +import cors from 'cors'; +import express from 'express'; + +const app = express(); + +app.use(cors()); + +const APIv1 = express.Router(); + +APIv1.get('/:userId', function (_req, res) { + Sentry.captureMessage('Custom Message'); + res.send('Success'); +}); + +const root = express.Router(); + +app.use('/api/v1', APIv1); +app.use('/api', root); + +Sentry.setupExpressErrorHandler(app); + +startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/common-prefix-same-length-parameterized copy/test.ts b/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/common-prefix-same-length-parameterized copy/test.ts new file mode 100644 index 000000000000..94d363f4faa4 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/common-prefix-same-length-parameterized copy/test.ts @@ -0,0 +1,13 @@ +import { cleanupChildProcesses, createRunner } from '../../../../utils/runner'; + +afterAll(() => { + cleanupChildProcesses(); +}); + +test('should construct correct url with multiple parameterized routers of the same length (use order reversed).', done => { + createRunner(__dirname, 'server.ts') + .ignore('transaction') + .expect({ event: { message: 'Custom Message', transaction: 'GET /api/v1/:userId' } }) + .start(done) + .makeRequest('get', '/api/v1/1234/'); +}); diff --git a/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/common-prefix-same-length-parameterized/server.ts b/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/common-prefix-same-length-parameterized/server.ts new file mode 100644 index 000000000000..017c810ed842 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/common-prefix-same-length-parameterized/server.ts @@ -0,0 +1,33 @@ +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', + tracesSampleRate: 1.0, + transport: loggingTransport, +}); + +import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests'; +import cors from 'cors'; +import express from 'express'; + +const app = express(); + +app.use(cors()); + +const APIv1 = express.Router(); + +APIv1.get('/:userId', function (_req, res) { + Sentry.captureMessage('Custom Message'); + res.send('Success'); +}); + +const root = express.Router(); + +app.use('/api', root); +app.use('/api/v1', APIv1); + +Sentry.setupExpressErrorHandler(app); + +startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/common-prefix-same-length-parameterized/test.ts b/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/common-prefix-same-length-parameterized/test.ts new file mode 100644 index 000000000000..373b2c102c4c --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/common-prefix-same-length-parameterized/test.ts @@ -0,0 +1,13 @@ +import { cleanupChildProcesses, createRunner } from '../../../../utils/runner'; + +afterAll(() => { + cleanupChildProcesses(); +}); + +test('should construct correct url with multiple parameterized routers of the same length.', done => { + createRunner(__dirname, 'server.ts') + .ignore('transaction') + .expect({ event: { message: 'Custom Message', transaction: 'GET /api/v1/:userId' } }) + .start(done) + .makeRequest('get', '/api/v1/1234/'); +}); diff --git a/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/common-prefix/server.ts b/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/common-prefix/server.ts new file mode 100644 index 000000000000..497cbf2efffb --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/common-prefix/server.ts @@ -0,0 +1,33 @@ +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', + tracesSampleRate: 1.0, + transport: loggingTransport, +}); + +import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests'; +import cors from 'cors'; +import express from 'express'; + +const app = express(); + +app.use(cors()); + +const APIv1 = express.Router(); + +APIv1.get('/test', function (_req, res) { + Sentry.captureMessage('Custom Message'); + res.send('Success'); +}); + +const root = express.Router(); + +app.use('/api', root); +app.use('/api/v1', APIv1); + +Sentry.setupExpressErrorHandler(app); + +startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/common-prefix/test.ts b/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/common-prefix/test.ts new file mode 100644 index 000000000000..ea217bf6bc05 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/common-prefix/test.ts @@ -0,0 +1,13 @@ +import { cleanupChildProcesses, createRunner } from '../../../../utils/runner'; + +afterAll(() => { + cleanupChildProcesses(); +}); + +test('should construct correct urls with multiple routers.', done => { + createRunner(__dirname, 'server.ts') + .ignore('transaction') + .expect({ event: { message: 'Custom Message', transaction: 'GET /api/v1/test' } }) + .start(done) + .makeRequest('get', '/api/v1/test'); +}); diff --git a/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/complex-router/server.ts b/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/complex-router/server.ts new file mode 100644 index 000000000000..b7ffeeba937a --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/complex-router/server.ts @@ -0,0 +1,33 @@ +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', + tracesSampleRate: 1.0, + transport: loggingTransport, +}); + +import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests'; +import express from 'express'; + +const app = express(); + +const APIv1 = express.Router(); + +APIv1.use( + '/users/:userId', + APIv1.get('/posts/:postId', (_req, res) => { + Sentry.captureMessage('Custom Message'); + return res.send('Success'); + }), +); + +const router = express.Router(); + +app.use('/api', router); +app.use('/api/api/v1', APIv1.use('/sub-router', APIv1)); + +Sentry.setupExpressErrorHandler(app); + +startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/complex-router/test.ts b/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/complex-router/test.ts new file mode 100644 index 000000000000..fe065d0dc550 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/complex-router/test.ts @@ -0,0 +1,52 @@ +import { cleanupChildProcesses, createRunner } from '../../../../utils/runner'; + +afterAll(() => { + cleanupChildProcesses(); +}); + +describe('complex-router', () => { + test('should construct correct url with multiple parameterized routers, when param is also contain in middle layer route and express used multiple middlewares with route', done => { + const EXPECTED_TRANSACTION = { + transaction: 'GET /api/api/v1/sub-router/users/:userId/posts/:postId', + transaction_info: { + source: 'route', + }, + }; + + createRunner(__dirname, 'server.ts') + .ignore('event') + .expect({ transaction: EXPECTED_TRANSACTION as any }) + .start(done) + .makeRequest('get', '/api/api/v1/sub-router/users/123/posts/456'); + }); + + test('should construct correct url with multiple parameterized routers, when param is also contain in middle layer route and express used multiple middlewares with route and original url has query params', done => { + const EXPECTED_TRANSACTION = { + transaction: 'GET /api/api/v1/sub-router/users/:userId/posts/:postId', + transaction_info: { + source: 'route', + }, + }; + + createRunner(__dirname, 'server.ts') + .ignore('event') + .expect({ transaction: EXPECTED_TRANSACTION as any }) + .start(done) + .makeRequest('get', '/api/api/v1/sub-router/users/123/posts/456?param=1'); + }); + + test('should construct correct url with multiple parameterized routers, when param is also contain in middle layer route and express used multiple middlewares with route and original url ends with trailing slash and has query params', done => { + const EXPECTED_TRANSACTION = { + transaction: 'GET /api/api/v1/sub-router/users/:userId/posts/:postId', + transaction_info: { + source: 'route', + }, + }; + + createRunner(__dirname, 'server.ts') + .ignore('event') + .expect({ transaction: EXPECTED_TRANSACTION as any }) + .start(done) + .makeRequest('get', '/api/api/v1/sub-router/users/123/posts/456/?param=1'); + }); +}); diff --git a/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/middle-layer-parameterized/server.ts b/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/middle-layer-parameterized/server.ts new file mode 100644 index 000000000000..12a00ce4e1db --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/middle-layer-parameterized/server.ts @@ -0,0 +1,33 @@ +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', + tracesSampleRate: 1.0, + transport: loggingTransport, +}); + +import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests'; +import express from 'express'; + +const app = express(); + +const APIv1 = express.Router(); + +APIv1.use( + '/users/:userId', + APIv1.get('/posts/:postId', (_req, res) => { + Sentry.captureMessage('Custom Message'); + return res.send('Success'); + }), +); + +const root = express.Router(); + +app.use('/api/v1', APIv1); +app.use('/api', root); + +Sentry.setupExpressErrorHandler(app); + +startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/middle-layer-parameterized/test.ts b/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/middle-layer-parameterized/test.ts new file mode 100644 index 000000000000..52a6ce154684 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/multiple-routers/middle-layer-parameterized/test.ts @@ -0,0 +1,23 @@ +import { cleanupChildProcesses, createRunner } from '../../../../utils/runner'; + +afterAll(() => { + cleanupChildProcesses(); +}); + +// Before Node 16, parametrization is not working properly here +describe('middle-layer-parameterized', () => { + test('should construct correct url with multiple parameterized routers, when param is also contain in middle layer route', done => { + const EXPECTED_TRANSACTION = { + transaction: 'GET /api/v1/users/:userId/posts/:postId', + transaction_info: { + source: 'route', + }, + }; + + createRunner(__dirname, 'server.ts') + .ignore('event') + .expect({ transaction: EXPECTED_TRANSACTION as any }) + .start(done) + .makeRequest('get', '/api/v1/users/123/posts/456'); + }); +}); diff --git a/dev-packages/node-integration-tests/suites/express-v5/package.json b/dev-packages/node-integration-tests/suites/express-v5/package.json new file mode 100644 index 000000000000..b3855635c556 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/package.json @@ -0,0 +1,6 @@ +{ + "name": "express-v5", + "dependencies": { + "express": "^5.0.0" + } +} diff --git a/dev-packages/node-integration-tests/suites/express-v5/requestUser/server.js b/dev-packages/node-integration-tests/suites/express-v5/requestUser/server.js new file mode 100644 index 000000000000..d93d22905506 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/requestUser/server.js @@ -0,0 +1,49 @@ +const { loggingTransport } = require('@sentry-internal/node-integration-tests'); +const Sentry = require('@sentry/node'); + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + transport: loggingTransport, + debug: true, +}); + +// express must be required after Sentry is initialized +const express = require('express'); +const cors = require('cors'); +const { startExpressServerAndSendPortToRunner } = require('@sentry-internal/node-integration-tests'); + +const app = express(); + +app.use(cors()); + +app.use((req, _res, next) => { + // We simulate this, which would in other cases be done by some middleware + req.user = { + id: '1', + email: 'test@sentry.io', + }; + + next(); +}); + +app.get('/test1', (_req, _res) => { + throw new Error('error_1'); +}); + +app.use((_req, _res, next) => { + Sentry.setUser({ + id: '2', + email: 'test2@sentry.io', + }); + + next(); +}); + +app.get('/test2', (_req, _res) => { + throw new Error('error_2'); +}); + +Sentry.setupExpressErrorHandler(app); + +startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/express-v5/requestUser/test.ts b/dev-packages/node-integration-tests/suites/express-v5/requestUser/test.ts new file mode 100644 index 000000000000..2a9fc58a7c18 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/requestUser/test.ts @@ -0,0 +1,42 @@ +import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; + +describe('express user handling', () => { + afterAll(() => { + cleanupChildProcesses(); + }); + + test('ignores user from request', done => { + expect.assertions(2); + + createRunner(__dirname, 'server.js') + .expect({ + event: event => { + expect(event.user).toBeUndefined(); + expect(event.exception?.values?.[0]?.value).toBe('error_1'); + }, + }) + .start(done) + .makeRequest('get', '/test1', { expectError: true }); + }); + + test('using setUser in middleware works', done => { + createRunner(__dirname, 'server.js') + .expect({ + event: { + user: { + id: '2', + email: 'test2@sentry.io', + }, + exception: { + values: [ + { + value: 'error_2', + }, + ], + }, + }, + }) + .start(done) + .makeRequest('get', '/test2', { expectError: true }); + }); +}); diff --git a/dev-packages/node-integration-tests/suites/express-v5/sentry-trace/baggage-header-assign/test.ts b/dev-packages/node-integration-tests/suites/express-v5/sentry-trace/baggage-header-assign/test.ts new file mode 100644 index 000000000000..513cf6146d0f --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/sentry-trace/baggage-header-assign/test.ts @@ -0,0 +1,147 @@ +import { parseBaggageHeader } from '@sentry/core'; +import { cleanupChildProcesses, createRunner } from '../../../../utils/runner'; +import type { TestAPIResponse } from '../server'; + +afterAll(() => { + cleanupChildProcesses(); +}); + +test('Should overwrite baggage if the incoming request already has Sentry baggage data but no sentry-trace', async () => { + const runner = createRunner(__dirname, '..', 'server.ts').start(); + + const response = await runner.makeRequest('get', '/test/express', { + headers: { + baggage: 'sentry-release=2.0.0,sentry-environment=myEnv', + }, + }); + + expect(response).toBeDefined(); + expect(response).not.toMatchObject({ + test_data: { + host: 'somewhere.not.sentry', + baggage: 'sentry-release=2.0.0,sentry-environment=myEnv', + }, + }); +}); + +test('Should propagate sentry trace baggage data from an incoming to an outgoing request.', async () => { + const runner = createRunner(__dirname, '..', 'server.ts').start(); + + const response = await runner.makeRequest('get', '/test/express', { + headers: { + 'sentry-trace': '12312012123120121231201212312012-1121201211212012-1', + baggage: 'sentry-release=2.0.0,sentry-environment=myEnv,dogs=great,sentry-sample_rand=0.42', + }, + }); + + expect(response).toBeDefined(); + expect(response).toMatchObject({ + test_data: { + host: 'somewhere.not.sentry', + baggage: 'sentry-release=2.0.0,sentry-environment=myEnv,sentry-sample_rand=0.42', + }, + }); +}); + +test('Should not propagate baggage data from an incoming to an outgoing request if sentry-trace is faulty.', async () => { + const runner = createRunner(__dirname, '..', 'server.ts').start(); + + const response = await runner.makeRequest('get', '/test/express', { + headers: { + 'sentry-trace': '', + baggage: 'sentry-release=2.0.0,sentry-environment=myEnv,dogs=great', + }, + }); + + expect(response).toBeDefined(); + expect(response).not.toMatchObject({ + test_data: { + host: 'somewhere.not.sentry', + baggage: 'sentry-release=2.0.0,sentry-environment=myEnv', + }, + }); +}); + +test('Should not propagate baggage if sentry-trace header is present in incoming request but no baggage header', async () => { + const runner = createRunner(__dirname, '..', 'server.ts').start(); + + const response = await runner.makeRequest('get', '/test/express', { + headers: { + 'sentry-trace': '12312012123120121231201212312012-1121201211212012-1', + }, + }); + + expect(response).toBeDefined(); + expect(response).toMatchObject({ + test_data: { + host: 'somewhere.not.sentry', + }, + }); +}); + +test('Should not propagate baggage and ignore original 3rd party baggage entries if sentry-trace header is present', async () => { + const runner = createRunner(__dirname, '..', 'server.ts').start(); + + const response = await runner.makeRequest('get', '/test/express', { + headers: { + 'sentry-trace': '12312012123120121231201212312012-1121201211212012-1', + baggage: 'foo=bar', + }, + }); + + expect(response).toBeDefined(); + expect(response).toMatchObject({ + test_data: { + host: 'somewhere.not.sentry', + }, + }); +}); + +test('Should populate and propagate sentry baggage if sentry-trace header does not exist', async () => { + const runner = createRunner(__dirname, '..', 'server.ts').start(); + + const response = await runner.makeRequest('get', '/test/express'); + + expect(response).toBeDefined(); + + const parsedBaggage = parseBaggageHeader(response?.test_data.baggage); + + expect(response?.test_data.host).toBe('somewhere.not.sentry'); + expect(parsedBaggage).toStrictEqual({ + 'sentry-environment': 'prod', + 'sentry-release': '1.0', + 'sentry-public_key': 'public', + // TraceId changes, hence we only expect that the string contains the traceid key + 'sentry-trace_id': expect.stringMatching(/[\S]*/), + 'sentry-sample_rand': expect.stringMatching(/[\S]*/), + 'sentry-sample_rate': '1', + 'sentry-sampled': 'true', + 'sentry-transaction': 'GET /test/express', + }); +}); + +test('Should populate Sentry and ignore 3rd party content if sentry-trace header does not exist', async () => { + const runner = createRunner(__dirname, '..', 'server.ts').start(); + + const response = await runner.makeRequest('get', '/test/express', { + headers: { + baggage: 'foo=bar,bar=baz', + }, + }); + + expect(response).toBeDefined(); + expect(response?.test_data.host).toBe('somewhere.not.sentry'); + + const parsedBaggage = parseBaggageHeader(response?.test_data.baggage); + expect(parsedBaggage).toStrictEqual({ + 'sentry-environment': 'prod', + 'sentry-release': '1.0', + 'sentry-public_key': 'public', + // TraceId changes, hence we only expect that the string contains the traceid key + 'sentry-trace_id': expect.stringMatching(/[\S]*/), + 'sentry-sample_rand': expect.stringMatching(/[\S]*/), + 'sentry-sample_rate': '1', + 'sentry-sampled': 'true', + 'sentry-transaction': 'GET /test/express', + }); +}); diff --git a/dev-packages/node-integration-tests/suites/express-v5/sentry-trace/baggage-header-out/server.ts b/dev-packages/node-integration-tests/suites/express-v5/sentry-trace/baggage-header-out/server.ts new file mode 100644 index 000000000000..07c21c8d21ea --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/sentry-trace/baggage-header-out/server.ts @@ -0,0 +1,38 @@ +import { loggingTransport, startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests'; +import * as Sentry from '@sentry/node'; + +export type TestAPIResponse = { test_data: { host: string; 'sentry-trace': string; baggage: string } }; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + environment: 'prod', + tracePropagationTargets: [/^(?!.*express).*$/], + tracesSampleRate: 1.0, + transport: loggingTransport, +}); + +import http from 'http'; +import cors from 'cors'; +import express from 'express'; + +const app = express(); + +Sentry.setUser({ id: 'user123' }); + +app.use(cors()); + +app.get('/test/express', (_req, res) => { + const span = Sentry.getActiveSpan(); + const traceId = span?.spanContext().traceId; + const headers = http.get('http://somewhere.not.sentry/').getHeaders(); + if (traceId) { + headers['baggage'] = (headers['baggage'] as string).replace(traceId, '__SENTRY_TRACE_ID__'); + } + // Responding with the headers outgoing request headers back to the assertions. + res.send({ test_data: headers }); +}); + +Sentry.setupExpressErrorHandler(app); + +startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/express-v5/sentry-trace/baggage-header-out/test.ts b/dev-packages/node-integration-tests/suites/express-v5/sentry-trace/baggage-header-out/test.ts new file mode 100644 index 000000000000..72b6a7139f35 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/sentry-trace/baggage-header-out/test.ts @@ -0,0 +1,35 @@ +import { cleanupChildProcesses, createRunner } from '../../../../utils/runner'; +import type { TestAPIResponse } from './server'; + +afterAll(() => { + cleanupChildProcesses(); +}); + +test('should attach a baggage header to an outgoing request.', async () => { + const runner = createRunner(__dirname, 'server.ts').start(); + + const response = await runner.makeRequest('get', '/test/express'); + + expect(response).toBeDefined(); + + const baggage = response?.test_data.baggage?.split(','); + + [ + 'sentry-environment=prod', + 'sentry-public_key=public', + 'sentry-release=1.0', + 'sentry-sample_rate=1', + 'sentry-sampled=true', + 'sentry-trace_id=__SENTRY_TRACE_ID__', + 'sentry-transaction=GET%20%2Ftest%2Fexpress', + expect.stringMatching(/sentry-sample_rand=0\.[0-9]+/), + ].forEach(item => { + expect(baggage).toContainEqual(item); + }); + + expect(response).toMatchObject({ + test_data: { + host: 'somewhere.not.sentry', + }, + }); +}); diff --git a/dev-packages/node-integration-tests/suites/express-v5/sentry-trace/baggage-other-vendors-with-sentry-entries/server.ts b/dev-packages/node-integration-tests/suites/express-v5/sentry-trace/baggage-other-vendors-with-sentry-entries/server.ts new file mode 100644 index 000000000000..260fb34af5c2 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/sentry-trace/baggage-other-vendors-with-sentry-entries/server.ts @@ -0,0 +1,43 @@ +import { loggingTransport } from '@sentry-internal/node-integration-tests'; +import * as Sentry from '@sentry/node'; + +export type TestAPIResponse = { test_data: { host: string; 'sentry-trace': string; baggage: string } }; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + environment: 'prod', + // disable requests to /express + tracePropagationTargets: [/^(?!.*express).*$/], + tracesSampleRate: 1.0, + transport: loggingTransport, +}); + +import * as http from 'http'; +import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests'; +import cors from 'cors'; +import express from 'express'; + +const app = express(); + +app.use(cors()); + +app.get('/test/express', (_req, res) => { + // simulate setting a "third party" baggage header which the Sentry SDK should merge with Sentry DSC entries + const headers = http + .get({ + hostname: 'somewhere.not.sentry', + headers: { + baggage: + 'other=vendor,foo=bar,third=party,sentry-release=9.9.9,sentry-environment=staging,sentry-sample_rate=0.54,last=item', + }, + }) + .getHeaders(); + + // Responding with the headers outgoing request headers back to the assertions. + res.send({ test_data: headers }); +}); + +Sentry.setupExpressErrorHandler(app); + +startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/express-v5/sentry-trace/baggage-other-vendors-with-sentry-entries/test.ts b/dev-packages/node-integration-tests/suites/express-v5/sentry-trace/baggage-other-vendors-with-sentry-entries/test.ts new file mode 100644 index 000000000000..ebf2a15bedf4 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/sentry-trace/baggage-other-vendors-with-sentry-entries/test.ts @@ -0,0 +1,69 @@ +import { cleanupChildProcesses, createRunner } from '../../../../utils/runner'; +import type { TestAPIResponse } from '../server'; + +afterAll(() => { + cleanupChildProcesses(); +}); + +test('should ignore sentry-values in `baggage` header of a third party vendor and overwrite them with incoming DSC', async () => { + const runner = createRunner(__dirname, 'server.ts').start(); + + const response = await runner.makeRequest('get', '/test/express', { + headers: { + 'sentry-trace': '12312012123120121231201212312012-1121201211212012-1', + baggage: 'sentry-release=2.1.0,sentry-environment=myEnv', + }, + }); + + expect(response).toBeDefined(); + + const baggage = response?.test_data.baggage?.split(',').sort(); + + expect(response).toMatchObject({ + test_data: { + host: 'somewhere.not.sentry', + }, + }); + + expect(baggage).toEqual([ + 'foo=bar', + 'last=item', + 'other=vendor', + 'sentry-environment=myEnv', + 'sentry-release=2.1.0', + expect.stringMatching(/sentry-sample_rand=[0-9]+/), + 'sentry-sample_rate=0.54', + 'third=party', + ]); +}); + +test('should ignore sentry-values in `baggage` header of a third party vendor and overwrite them with new DSC', async () => { + const runner = createRunner(__dirname, 'server.ts').start(); + + const response = await runner.makeRequest('get', '/test/express'); + + expect(response).toBeDefined(); + + const baggage = response?.test_data.baggage?.split(',').sort(); + + expect(response).toMatchObject({ + test_data: { + host: 'somewhere.not.sentry', + }, + }); + + expect(baggage).toEqual([ + 'foo=bar', + 'last=item', + 'other=vendor', + 'sentry-environment=prod', + 'sentry-public_key=public', + 'sentry-release=1.0', + expect.stringMatching(/sentry-sample_rand=[0-9]+/), + 'sentry-sample_rate=1', + 'sentry-sampled=true', + expect.stringMatching(/sentry-trace_id=[0-9a-f]{32}/), + 'sentry-transaction=GET%20%2Ftest%2Fexpress', + 'third=party', + ]); +}); diff --git a/dev-packages/node-integration-tests/suites/express-v5/sentry-trace/baggage-other-vendors/server.ts b/dev-packages/node-integration-tests/suites/express-v5/sentry-trace/baggage-other-vendors/server.ts new file mode 100644 index 000000000000..1c00fbd72bde --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/sentry-trace/baggage-other-vendors/server.ts @@ -0,0 +1,37 @@ +import { loggingTransport } from '@sentry-internal/node-integration-tests'; +import * as Sentry from '@sentry/node'; + +export type TestAPIResponse = { test_data: { host: string; 'sentry-trace': string; baggage: string } }; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + environment: 'prod', + // disable requests to /express + tracePropagationTargets: [/^(?!.*express).*$/], + tracesSampleRate: 1.0, + transport: loggingTransport, +}); + +import http from 'http'; +import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests'; +import cors from 'cors'; +import express from 'express'; + +const app = express(); + +app.use(cors()); + +app.get('/test/express', (_req, res) => { + // simulate setting a "third party" baggage header which the Sentry SDK should merge with Sentry DSC entries + const headers = http + .get({ hostname: 'somewhere.not.sentry', headers: { baggage: 'other=vendor,foo=bar,third=party' } }) + .getHeaders(); + + // Responding with the headers outgoing request headers back to the assertions. + res.send({ test_data: headers }); +}); + +Sentry.setupExpressErrorHandler(app); + +startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/express-v5/sentry-trace/baggage-other-vendors/test.ts b/dev-packages/node-integration-tests/suites/express-v5/sentry-trace/baggage-other-vendors/test.ts new file mode 100644 index 000000000000..0beecb54a905 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/sentry-trace/baggage-other-vendors/test.ts @@ -0,0 +1,25 @@ +import { cleanupChildProcesses, createRunner } from '../../../../utils/runner'; +import type { TestAPIResponse } from './server'; + +afterAll(() => { + cleanupChildProcesses(); +}); + +test('should merge `baggage` header of a third party vendor with the Sentry DSC baggage items', async () => { + const runner = createRunner(__dirname, 'server.ts').start(); + + const response = await runner.makeRequest('get', '/test/express', { + headers: { + 'sentry-trace': '12312012123120121231201212312012-1121201211212012-1', + baggage: 'sentry-release=2.0.0,sentry-environment=myEnv,sentry-sample_rand=0.42', + }, + }); + + expect(response).toBeDefined(); + expect(response).toMatchObject({ + test_data: { + host: 'somewhere.not.sentry', + baggage: 'other=vendor,foo=bar,third=party,sentry-release=2.0.0,sentry-environment=myEnv,sentry-sample_rand=0.42', + }, + }); +}); diff --git a/dev-packages/node-integration-tests/suites/express-v5/sentry-trace/baggage-transaction-name/server.ts b/dev-packages/node-integration-tests/suites/express-v5/sentry-trace/baggage-transaction-name/server.ts new file mode 100644 index 000000000000..80bb7b38a39a --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/sentry-trace/baggage-transaction-name/server.ts @@ -0,0 +1,37 @@ +import { loggingTransport } from '@sentry-internal/node-integration-tests'; +import * as Sentry from '@sentry/node'; + +export type TestAPIResponse = { test_data: { host: string; 'sentry-trace': string; baggage: string } }; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + environment: 'prod', + // disable requests to /express + tracePropagationTargets: [/^(?!.*express).*$/], + tracesSampleRate: 1.0, + // TODO: We're rethinking the mechanism for including Pii data in DSC, hence commenting out sendDefaultPii for now + // sendDefaultPii: true, + transport: loggingTransport, +}); + +import http from 'http'; +import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests'; +import cors from 'cors'; +import express from 'express'; + +const app = express(); + +Sentry.setUser({ id: 'user123' }); + +app.use(cors()); + +app.get('/test/express', (_req, res) => { + const headers = http.get('http://somewhere.not.sentry/').getHeaders(); + // Responding with the headers outgoing request headers back to the assertions. + res.send({ test_data: headers }); +}); + +Sentry.setupExpressErrorHandler(app); + +startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/express-v5/sentry-trace/baggage-transaction-name/test.ts b/dev-packages/node-integration-tests/suites/express-v5/sentry-trace/baggage-transaction-name/test.ts new file mode 100644 index 000000000000..1001d0839aea --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/sentry-trace/baggage-transaction-name/test.ts @@ -0,0 +1,20 @@ +import { cleanupChildProcesses, createRunner } from '../../../../utils/runner'; +import type { TestAPIResponse } from '../server'; + +afterAll(() => { + cleanupChildProcesses(); +}); + +test('Includes transaction in baggage if the transaction name is parameterized', async () => { + const runner = createRunner(__dirname, 'server.ts').start(); + + const response = await runner.makeRequest('get', '/test/express'); + + expect(response).toBeDefined(); + expect(response).toMatchObject({ + test_data: { + host: 'somewhere.not.sentry', + baggage: expect.stringContaining('sentry-transaction=GET%20%2Ftest%2Fexpress'), + }, + }); +}); diff --git a/dev-packages/node-integration-tests/suites/express-v5/sentry-trace/server.ts b/dev-packages/node-integration-tests/suites/express-v5/sentry-trace/server.ts new file mode 100644 index 000000000000..6ebc2d4cac95 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/sentry-trace/server.ts @@ -0,0 +1,33 @@ +import { loggingTransport } from '@sentry-internal/node-integration-tests'; +import * as Sentry from '@sentry/node'; + +export type TestAPIResponse = { test_data: { host: string; 'sentry-trace': string; baggage: string } }; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + environment: 'prod', + tracePropagationTargets: [/^(?!.*express).*$/], + tracesSampleRate: 1.0, + transport: loggingTransport, +}); + +import http from 'http'; +import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests'; +import cors from 'cors'; +import express from 'express'; + +const app = express(); + +app.use(cors()); + +app.get('/test/express', (_req, res) => { + const headers = http.get('http://somewhere.not.sentry/').getHeaders(); + + // Responding with the headers outgoing request headers back to the assertions. + res.send({ test_data: headers }); +}); + +Sentry.setupExpressErrorHandler(app); + +startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/express-v5/sentry-trace/trace-header-assign/server.ts b/dev-packages/node-integration-tests/suites/express-v5/sentry-trace/trace-header-assign/server.ts new file mode 100644 index 000000000000..1cc4a0dcc639 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/sentry-trace/trace-header-assign/server.ts @@ -0,0 +1,32 @@ +import { loggingTransport } from '@sentry-internal/node-integration-tests'; +import * as Sentry from '@sentry/node'; + +export type TestAPIResponse = { test_data: { host: string; 'sentry-trace': string; baggage: string } }; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + environment: 'prod', + tracesSampleRate: 1.0, + transport: loggingTransport, +}); + +import http from 'http'; +import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests'; +import cors from 'cors'; +import express from 'express'; + +const app = express(); + +app.use(cors()); + +app.get('/test/express', (_req, res) => { + const headers = http.get('http://somewhere.not.sentry/').getHeaders(); + + // Responding with the headers outgoing request headers back to the assertions. + res.send({ test_data: headers }); +}); + +Sentry.setupExpressErrorHandler(app); + +startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/express-v5/sentry-trace/trace-header-assign/test.ts b/dev-packages/node-integration-tests/suites/express-v5/sentry-trace/trace-header-assign/test.ts new file mode 100644 index 000000000000..40bbb03f8d50 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/sentry-trace/trace-header-assign/test.ts @@ -0,0 +1,27 @@ +import { TRACEPARENT_REGEXP } from '@sentry/core'; +import { cleanupChildProcesses, createRunner } from '../../../../utils/runner'; +import type { TestAPIResponse } from '../server'; + +afterAll(() => { + cleanupChildProcesses(); +}); + +test('Should assign `sentry-trace` header which sets parent trace id of an outgoing request.', async () => { + const runner = createRunner(__dirname, 'server.ts').start(); + + const response = await runner.makeRequest('get', '/test/express', { + headers: { + 'sentry-trace': '12312012123120121231201212312012-1121201211212012-0', + }, + }); + + expect(response).toBeDefined(); + expect(response).toMatchObject({ + test_data: { + host: 'somewhere.not.sentry', + 'sentry-trace': expect.stringContaining('12312012123120121231201212312012-'), + }, + }); + + expect(TRACEPARENT_REGEXP.test(response?.test_data['sentry-trace'] || '')).toBe(true); +}); diff --git a/dev-packages/node-integration-tests/suites/express-v5/sentry-trace/trace-header-out/test.ts b/dev-packages/node-integration-tests/suites/express-v5/sentry-trace/trace-header-out/test.ts new file mode 100644 index 000000000000..db46bb491904 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/sentry-trace/trace-header-out/test.ts @@ -0,0 +1,23 @@ +import { TRACEPARENT_REGEXP } from '@sentry/core'; +import { cleanupChildProcesses, createRunner } from '../../../../utils/runner'; +import type { TestAPIResponse } from '../server'; + +afterAll(() => { + cleanupChildProcesses(); +}); + +test('should attach a `sentry-trace` header to an outgoing request.', async () => { + const runner = createRunner(__dirname, '..', 'server.ts').start(); + + const response = await runner.makeRequest('get', '/test/express'); + + expect(response).toBeDefined(); + expect(response).toMatchObject({ + test_data: { + host: 'somewhere.not.sentry', + 'sentry-trace': expect.any(String), + }, + }); + + expect(TRACEPARENT_REGEXP.test(response?.test_data['sentry-trace'] || '')).toBe(true); +}); diff --git a/dev-packages/node-integration-tests/suites/express-v5/setupExpressErrorHandler/server.js b/dev-packages/node-integration-tests/suites/express-v5/setupExpressErrorHandler/server.js new file mode 100644 index 000000000000..0e73923cf88a --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/setupExpressErrorHandler/server.js @@ -0,0 +1,33 @@ +const { loggingTransport } = require('@sentry-internal/node-integration-tests'); +const Sentry = require('@sentry/node'); + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + transport: loggingTransport, +}); + +// express must be required after Sentry is initialized +const express = require('express'); +const cors = require('cors'); +const { startExpressServerAndSendPortToRunner } = require('@sentry-internal/node-integration-tests'); + +const app = express(); + +app.use(cors()); + +app.get('/test1', (_req, _res) => { + throw new Error('error_1'); +}); + +app.get('/test2', (_req, _res) => { + throw new Error('error_2'); +}); + +Sentry.setupExpressErrorHandler(app, { + shouldHandleError: error => { + return error.message === 'error_2'; + }, +}); + +startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/express-v5/setupExpressErrorHandler/test.ts b/dev-packages/node-integration-tests/suites/express-v5/setupExpressErrorHandler/test.ts new file mode 100644 index 000000000000..ffc702d63057 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/setupExpressErrorHandler/test.ts @@ -0,0 +1,30 @@ +import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; + +describe('express setupExpressErrorHandler', () => { + afterAll(() => { + cleanupChildProcesses(); + }); + + describe('CJS', () => { + test('allows to pass options to setupExpressErrorHandler', done => { + const runner = createRunner(__dirname, 'server.js') + .expect({ + event: { + exception: { + values: [ + { + value: 'error_2', + }, + ], + }, + }, + }) + .start(done); + + // this error is filtered & ignored + runner.makeRequest('get', '/test1', { expectError: true }); + // this error is actually captured + runner.makeRequest('get', '/test2', { expectError: true }); + }); + }); +}); diff --git a/dev-packages/node-integration-tests/suites/express-v5/span-isolationScope/server.ts b/dev-packages/node-integration-tests/suites/express-v5/span-isolationScope/server.ts new file mode 100644 index 000000000000..99a9c53e932e --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/span-isolationScope/server.ts @@ -0,0 +1,29 @@ +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', + tracesSampleRate: 1.0, + transport: loggingTransport, +}); + +import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests'; +import express from 'express'; + +const app = express(); + +Sentry.setTag('global', 'tag'); + +app.get('/test/isolationScope', (_req, res) => { + // eslint-disable-next-line no-console + console.log('This is a test log.'); + Sentry.addBreadcrumb({ message: 'manual breadcrumb' }); + Sentry.setTag('isolation-scope', 'tag'); + + res.send({}); +}); + +Sentry.setupExpressErrorHandler(app); + +startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/express-v5/span-isolationScope/test.ts b/dev-packages/node-integration-tests/suites/express-v5/span-isolationScope/test.ts new file mode 100644 index 000000000000..2e2b6945526e --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/span-isolationScope/test.ts @@ -0,0 +1,38 @@ +import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; + +afterAll(() => { + cleanupChildProcesses(); +}); + +test('correctly applies isolation scope to span', done => { + createRunner(__dirname, 'server.ts') + .expect({ + transaction: { + transaction: 'GET /test/isolationScope', + breadcrumbs: [ + { + category: 'console', + level: 'log', + message: expect.stringMatching(/\{"port":(\d+)\}/), + timestamp: expect.any(Number), + }, + { + category: 'console', + level: 'log', + message: 'This is a test log.', + timestamp: expect.any(Number), + }, + { + message: 'manual breadcrumb', + timestamp: expect.any(Number), + }, + ], + tags: { + global: 'tag', + 'isolation-scope': 'tag', + }, + }, + }) + .start(done) + .makeRequest('get', '/test/isolationScope'); +}); diff --git a/dev-packages/node-integration-tests/suites/express-v5/tracing/server.js b/dev-packages/node-integration-tests/suites/express-v5/tracing/server.js new file mode 100644 index 000000000000..f9b4ae24b339 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/tracing/server.js @@ -0,0 +1,48 @@ +const { loggingTransport } = require('@sentry-internal/node-integration-tests'); +const Sentry = require('@sentry/node'); + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + // disable attaching headers to /test/* endpoints + tracePropagationTargets: [/^(?!.*test).*$/], + tracesSampleRate: 1.0, + transport: loggingTransport, +}); + +// express must be required after Sentry is initialized +const express = require('express'); +const cors = require('cors'); +const bodyParser = require('body-parser'); +const { startExpressServerAndSendPortToRunner } = require('@sentry-internal/node-integration-tests'); + +const app = express(); + +app.use(cors()); +app.use(bodyParser.json()); +app.use(bodyParser.text()); +app.use(bodyParser.raw()); + +app.get('/test/express', (_req, res) => { + res.send({ response: 'response 1' }); +}); + +app.get(/\/test\/regex/, (_req, res) => { + res.send({ response: 'response 2' }); +}); + +app.get(['/test/array1', /\/test\/array[2-9]/], (_req, res) => { + res.send({ response: 'response 3' }); +}); + +app.get(['/test/arr/:id', /\/test\/arr[0-9]*\/required(path)?(\/optionalPath)?\/(lastParam)?/], (_req, res) => { + res.send({ response: 'response 4' }); +}); + +app.post('/test-post', function (req, res) { + res.send({ status: 'ok', body: req.body }); +}); + +Sentry.setupExpressErrorHandler(app); + +startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/express-v5/tracing/test.ts b/dev-packages/node-integration-tests/suites/express-v5/tracing/test.ts new file mode 100644 index 000000000000..6f87fdd89f76 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/tracing/test.ts @@ -0,0 +1,240 @@ +import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; + +describe('express tracing', () => { + afterAll(() => { + cleanupChildProcesses(); + }); + + describe('CJS', () => { + test('should create and send transactions for Express routes and spans for middlewares.', done => { + createRunner(__dirname, 'server.js') + .expect({ + transaction: { + contexts: { + trace: { + span_id: expect.stringMatching(/[a-f0-9]{16}/), + trace_id: expect.stringMatching(/[a-f0-9]{32}/), + data: { + url: expect.stringMatching(/\/test\/express$/), + 'http.response.status_code': 200, + }, + op: 'http.server', + status: 'ok', + }, + }, + spans: expect.arrayContaining([ + expect.objectContaining({ + data: expect.objectContaining({ + 'express.name': 'corsMiddleware', + 'express.type': 'middleware', + }), + description: 'corsMiddleware', + op: 'middleware.express', + origin: 'auto.http.otel.express', + }), + expect.objectContaining({ + data: expect.objectContaining({ + 'express.name': '/test/express', + 'express.type': 'request_handler', + }), + description: '/test/express', + op: 'request_handler.express', + origin: 'auto.http.otel.express', + }), + ]), + }, + }) + .start(done) + .makeRequest('get', '/test/express'); + }); + + test('should set a correct transaction name for routes specified in RegEx', done => { + createRunner(__dirname, 'server.js') + .expect({ + transaction: { + transaction: 'GET /\\/test\\/regex/', + transaction_info: { + source: 'route', + }, + contexts: { + trace: { + trace_id: expect.stringMatching(/[a-f0-9]{32}/), + span_id: expect.stringMatching(/[a-f0-9]{16}/), + data: { + url: expect.stringMatching(/\/test\/regex$/), + 'http.response.status_code': 200, + }, + op: 'http.server', + status: 'ok', + }, + }, + }, + }) + .start(done) + .makeRequest('get', '/test/regex'); + }); + + test.each([['array1'], ['array5']])( + 'should set a correct transaction name for routes consisting of arrays of routes for %p', + ((segment: string, done: () => void) => { + createRunner(__dirname, 'server.js') + .expect({ + transaction: { + transaction: 'GET /test/array1,/\\/test\\/array[2-9]/', + transaction_info: { + source: 'route', + }, + contexts: { + trace: { + trace_id: expect.stringMatching(/[a-f0-9]{32}/), + span_id: expect.stringMatching(/[a-f0-9]{16}/), + data: { + url: expect.stringMatching(`/test/${segment}$`), + 'http.response.status_code': 200, + }, + op: 'http.server', + status: 'ok', + }, + }, + }, + }) + .start(done) + .makeRequest('get', `/test/${segment}`); + }) as any, + ); + + test.each([ + ['arr/545'], + ['arr/required'], + ['arr/required'], + ['arr/requiredPath'], + ['arr/required/lastParam'], + ['arr55/required/lastParam'], + ])('should handle more complex regexes in route arrays correctly for %p', ((segment: string, done: () => void) => { + createRunner(__dirname, 'server.js') + .expect({ + transaction: { + transaction: 'GET /test/arr/:id,/\\/test\\/arr[0-9]*\\/required(path)?(\\/optionalPath)?\\/(lastParam)?/', + transaction_info: { + source: 'route', + }, + contexts: { + trace: { + trace_id: expect.stringMatching(/[a-f0-9]{32}/), + span_id: expect.stringMatching(/[a-f0-9]{16}/), + data: { + url: expect.stringMatching(`/test/${segment}$`), + 'http.response.status_code': 200, + }, + op: 'http.server', + status: 'ok', + }, + }, + }, + }) + .start(done) + .makeRequest('get', `/test/${segment}`); + }) as any); + + describe('request data', () => { + test('correctly captures JSON request data', done => { + const runner = createRunner(__dirname, 'server.js') + .expect({ + transaction: { + transaction: 'POST /test-post', + request: { + url: expect.stringMatching(/^http:\/\/localhost:(\d+)\/test-post$/), + method: 'POST', + headers: { + 'user-agent': expect.stringContaining(''), + 'content-type': 'application/json', + }, + data: JSON.stringify({ + foo: 'bar', + other: 1, + }), + }, + }, + }) + .start(done); + + runner.makeRequest('post', '/test-post', { data: { foo: 'bar', other: 1 } }); + }); + + test('correctly captures plain text request data', done => { + const runner = createRunner(__dirname, 'server.js') + .expect({ + transaction: { + transaction: 'POST /test-post', + request: { + url: expect.stringMatching(/^http:\/\/localhost:(\d+)\/test-post$/), + method: 'POST', + headers: { + 'user-agent': expect.stringContaining(''), + 'content-type': 'text/plain', + }, + data: 'some plain text', + }, + }, + }) + .start(done); + + runner.makeRequest('post', '/test-post', { + headers: { 'Content-Type': 'text/plain' }, + data: 'some plain text', + }); + }); + + test('correctly captures text buffer request data', done => { + const runner = createRunner(__dirname, 'server.js') + .expect({ + transaction: { + transaction: 'POST /test-post', + request: { + url: expect.stringMatching(/^http:\/\/localhost:(\d+)\/test-post$/), + method: 'POST', + headers: { + 'user-agent': expect.stringContaining(''), + 'content-type': 'application/octet-stream', + }, + data: 'some plain text in buffer', + }, + }, + }) + .start(done); + + runner.makeRequest('post', '/test-post', { + headers: { 'Content-Type': 'application/octet-stream' }, + data: Buffer.from('some plain text in buffer'), + }); + }); + + test('correctly captures non-text buffer request data', done => { + const runner = createRunner(__dirname, 'server.js') + .expect({ + transaction: { + transaction: 'POST /test-post', + request: { + url: expect.stringMatching(/^http:\/\/localhost:(\d+)\/test-post$/), + method: 'POST', + headers: { + 'user-agent': expect.stringContaining(''), + 'content-type': 'application/octet-stream', + }, + // This is some non-ascii string representation + data: expect.any(String), + }, + }, + }) + .start(done); + + const body = new Uint8Array([1, 2, 3, 4, 5]).buffer; + + runner.makeRequest('post', '/test-post', { + headers: { 'Content-Type': 'application/octet-stream' }, + data: body, + }); + }); + }); + }); +}); diff --git a/dev-packages/node-integration-tests/suites/express-v5/tracing/tracesSampler/scenario-normalizedRequest.js b/dev-packages/node-integration-tests/suites/express-v5/tracing/tracesSampler/scenario-normalizedRequest.js new file mode 100644 index 000000000000..da31780f2c5f --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/tracing/tracesSampler/scenario-normalizedRequest.js @@ -0,0 +1,34 @@ +const { loggingTransport } = require('@sentry-internal/node-integration-tests'); +const Sentry = require('@sentry/node'); + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + transport: loggingTransport, + tracesSampler: samplingContext => { + // The sampling decision is based on whether the data in `normalizedRequest` is available --> this is what we want to test for + return ( + samplingContext.normalizedRequest.url.includes('/test-normalized-request?query=123') && + samplingContext.normalizedRequest.method && + samplingContext.normalizedRequest.query_string === 'query=123' && + !!samplingContext.normalizedRequest.headers + ); + }, +}); + +// express must be required after Sentry is initialized +const express = require('express'); +const cors = require('cors'); +const { startExpressServerAndSendPortToRunner } = require('@sentry-internal/node-integration-tests'); + +const app = express(); + +app.use(cors()); + +app.get('/test-normalized-request', (_req, res) => { + res.send('Success'); +}); + +Sentry.setupExpressErrorHandler(app); + +startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/express-v5/tracing/tracesSampler/server.js b/dev-packages/node-integration-tests/suites/express-v5/tracing/tracesSampler/server.js new file mode 100644 index 000000000000..b60ea07b636f --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/tracing/tracesSampler/server.js @@ -0,0 +1,39 @@ +const { loggingTransport } = require('@sentry-internal/node-integration-tests'); +const Sentry = require('@sentry/node'); + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + transport: loggingTransport, + tracesSampler: samplingContext => { + // The name we get here is inferred at span creation time + // At this point, we sadly do not have a http.route attribute yet, + // so we infer the name from the unparameterized route instead + return ( + samplingContext.name === 'GET /test/123' && + samplingContext.attributes['sentry.op'] === 'http.server' && + samplingContext.attributes['http.method'] === 'GET' + ); + }, +}); + +// express must be required after Sentry is initialized +const express = require('express'); +const cors = require('cors'); +const { startExpressServerAndSendPortToRunner } = require('@sentry-internal/node-integration-tests'); + +const app = express(); + +app.use(cors()); + +app.get('/test/:id', (_req, res) => { + res.send('Success'); +}); + +app.get('/test2', (_req, res) => { + res.send('Success'); +}); + +Sentry.setupExpressErrorHandler(app); + +startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/express-v5/tracing/tracesSampler/test.ts b/dev-packages/node-integration-tests/suites/express-v5/tracing/tracesSampler/test.ts new file mode 100644 index 000000000000..07cc8d094d8f --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/tracing/tracesSampler/test.ts @@ -0,0 +1,44 @@ +import { cleanupChildProcesses, createRunner } from '../../../../utils/runner'; + +describe('express tracesSampler', () => { + afterAll(() => { + cleanupChildProcesses(); + }); + + describe('CJS', () => { + test('correctly samples & passes data to tracesSampler', done => { + const runner = createRunner(__dirname, 'server.js') + .expect({ + transaction: { + transaction: 'GET /test/:id', + }, + }) + .start(done); + + // This is not sampled + runner.makeRequest('get', '/test2?q=1'); + // This is sampled + runner.makeRequest('get', '/test/123?q=1'); + }); + }); +}); + +describe('express tracesSampler includes normalizedRequest data', () => { + afterAll(() => { + cleanupChildProcesses(); + }); + + describe('CJS', () => { + test('correctly samples & passes data to tracesSampler', done => { + const runner = createRunner(__dirname, 'scenario-normalizedRequest.js') + .expect({ + transaction: { + transaction: 'GET /test-normalized-request', + }, + }) + .start(done); + + runner.makeRequest('get', '/test-normalized-request?query=123'); + }); + }); +}); diff --git a/dev-packages/node-integration-tests/suites/express-v5/tracing/updateName/server.js b/dev-packages/node-integration-tests/suites/express-v5/tracing/updateName/server.js new file mode 100644 index 000000000000..c98e17276d92 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/tracing/updateName/server.js @@ -0,0 +1,58 @@ +const { loggingTransport } = require('@sentry-internal/node-integration-tests'); +const Sentry = require('@sentry/node'); + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + // disable attaching headers to /test/* endpoints + tracePropagationTargets: [/^(?!.*test).*$/], + tracesSampleRate: 1.0, + transport: loggingTransport, +}); + +// express must be required after Sentry is initialized +const express = require('express'); +const cors = require('cors'); +const bodyParser = require('body-parser'); +const { startExpressServerAndSendPortToRunner } = require('@sentry-internal/node-integration-tests'); + +const app = express(); + +app.use(cors()); +app.use(bodyParser.json()); +app.use(bodyParser.text()); +app.use(bodyParser.raw()); + +app.get('/test/:id/span-updateName', (_req, res) => { + const span = Sentry.getActiveSpan(); + const rootSpan = Sentry.getRootSpan(span); + rootSpan.updateName('new-name'); + res.send({ response: 'response 1' }); +}); + +app.get('/test/:id/span-updateName-source', (_req, res) => { + const span = Sentry.getActiveSpan(); + const rootSpan = Sentry.getRootSpan(span); + rootSpan.updateName('new-name'); + rootSpan.setAttribute(Sentry.SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'custom'); + res.send({ response: 'response 2' }); +}); + +app.get('/test/:id/updateSpanName', (_req, res) => { + const span = Sentry.getActiveSpan(); + const rootSpan = Sentry.getRootSpan(span); + Sentry.updateSpanName(rootSpan, 'new-name'); + res.send({ response: 'response 3' }); +}); + +app.get('/test/:id/updateSpanNameAndSource', (_req, res) => { + const span = Sentry.getActiveSpan(); + const rootSpan = Sentry.getRootSpan(span); + Sentry.updateSpanName(rootSpan, 'new-name'); + rootSpan.setAttribute(Sentry.SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'component'); + res.send({ response: 'response 4' }); +}); + +Sentry.setupExpressErrorHandler(app); + +startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/express-v5/tracing/updateName/test.ts b/dev-packages/node-integration-tests/suites/express-v5/tracing/updateName/test.ts new file mode 100644 index 000000000000..c6345713fd7e --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/tracing/updateName/test.ts @@ -0,0 +1,94 @@ +import { SEMANTIC_ATTRIBUTE_SENTRY_CUSTOM_SPAN_NAME } from '@sentry/core'; +import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/node'; +import { cleanupChildProcesses, createRunner } from '../../../../utils/runner'; + +describe('express tracing', () => { + afterAll(() => { + cleanupChildProcesses(); + }); + + describe('CJS', () => { + // This test documents the unfortunate behaviour of using `span.updateName` on the server-side. + // For http.server root spans (which is the root span on the server 99% of the time), Otel's http instrumentation + // calls `span.updateName` and overwrites whatever the name was set to before (by us or by users). + test("calling just `span.updateName` doesn't update the final name in express (missing source)", done => { + createRunner(__dirname, 'server.js') + .expect({ + transaction: { + transaction: 'GET /test/:id/span-updateName', + transaction_info: { + source: 'route', + }, + }, + }) + .start(done) + .makeRequest('get', '/test/123/span-updateName'); + }); + + // Also calling `updateName` AND setting a source doesn't change anything - Otel has no concept of source, this is sentry-internal. + // Therefore, only the source is updated but the name is still overwritten by Otel. + test("calling `span.updateName` and setting attribute source doesn't update the final name in express but it updates the source", done => { + createRunner(__dirname, 'server.js') + .expect({ + transaction: { + transaction: 'GET /test/:id/span-updateName-source', + transaction_info: { + source: 'custom', + }, + }, + }) + .start(done) + .makeRequest('get', '/test/123/span-updateName-source'); + }); + + // This test documents the correct way to update the span name (and implicitly the source) in Node: + test('calling `Sentry.updateSpanName` updates the final name and source in express', done => { + createRunner(__dirname, 'server.js') + .expect({ + transaction: txnEvent => { + expect(txnEvent).toMatchObject({ + transaction: 'new-name', + transaction_info: { + source: 'custom', + }, + contexts: { + trace: { + op: 'http.server', + data: { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'custom' }, + }, + }, + }); + // ensure we delete the internal attribute once we're done with it + expect(txnEvent.contexts?.trace?.data?.[SEMANTIC_ATTRIBUTE_SENTRY_CUSTOM_SPAN_NAME]).toBeUndefined(); + }, + }) + .start(done) + .makeRequest('get', '/test/123/updateSpanName'); + }); + }); + + // This test documents the correct way to update the span name (and implicitly the source) in Node: + test('calling `Sentry.updateSpanName` and setting source subsequently updates the final name and sets correct source', done => { + createRunner(__dirname, 'server.js') + .expect({ + transaction: txnEvent => { + expect(txnEvent).toMatchObject({ + transaction: 'new-name', + transaction_info: { + source: 'component', + }, + contexts: { + trace: { + op: 'http.server', + data: { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'component' }, + }, + }, + }); + // ensure we delete the internal attribute once we're done with it + expect(txnEvent.contexts?.trace?.data?.[SEMANTIC_ATTRIBUTE_SENTRY_CUSTOM_SPAN_NAME]).toBeUndefined(); + }, + }) + .start(done) + .makeRequest('get', '/test/123/updateSpanNameAndSource'); + }); +}); diff --git a/dev-packages/node-integration-tests/suites/express-v5/tracing/withError/server.js b/dev-packages/node-integration-tests/suites/express-v5/tracing/withError/server.js new file mode 100644 index 000000000000..d9ccc80fb7ad --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/tracing/withError/server.js @@ -0,0 +1,30 @@ +const { loggingTransport } = require('@sentry-internal/node-integration-tests'); +const Sentry = require('@sentry/node'); + +Sentry.init({ + debug: true, + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + // disable attaching headers to /test/* endpoints + tracePropagationTargets: [/^(?!.*test).*$/], + tracesSampleRate: 1.0, + transport: loggingTransport, +}); + +// express must be required after Sentry is initialized +const express = require('express'); +const cors = require('cors'); +const { startExpressServerAndSendPortToRunner } = require('@sentry-internal/node-integration-tests'); + +const app = express(); + +app.use(cors()); + +app.get('/test/:id1/:id2', (_req, res) => { + Sentry.captureException(new Error('error_1')); + res.send('Success'); +}); + +Sentry.setupExpressErrorHandler(app); + +startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/express-v5/tracing/withError/test.ts b/dev-packages/node-integration-tests/suites/express-v5/tracing/withError/test.ts new file mode 100644 index 000000000000..4dd004ad2239 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/tracing/withError/test.ts @@ -0,0 +1,28 @@ +import { cleanupChildProcesses, createRunner } from '../../../../utils/runner'; + +describe('express tracing experimental', () => { + afterAll(() => { + cleanupChildProcesses(); + }); + + describe('CJS', () => { + test('should apply the scope transactionName to error events', done => { + createRunner(__dirname, 'server.js') + .ignore('transaction') + .expect({ + event: { + exception: { + values: [ + { + value: 'error_1', + }, + ], + }, + transaction: 'GET /test/:id1/:id2', + }, + }) + .start(done) + .makeRequest('get', '/test/123/abc?q=1'); + }); + }); +}); diff --git a/dev-packages/node-integration-tests/suites/express-v5/without-tracing/server.ts b/dev-packages/node-integration-tests/suites/express-v5/without-tracing/server.ts new file mode 100644 index 000000000000..5b96e8b1a2a3 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/without-tracing/server.ts @@ -0,0 +1,40 @@ +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, +}); + +import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests'; +import bodyParser from 'body-parser'; +import express from 'express'; + +const app = express(); + +app.use(bodyParser.json()); +app.use(bodyParser.text()); +app.use(bodyParser.raw()); + +Sentry.setTag('global', 'tag'); + +app.get('/test/isolationScope/:id', (req, res) => { + const id = req.params.id; + Sentry.setTag('isolation-scope', 'tag'); + Sentry.setTag(`isolation-scope-${id}`, id); + + Sentry.captureException(new Error('This is an exception')); + + res.send({}); +}); + +app.post('/test-post', function (req, res) { + Sentry.captureException(new Error('This is an exception')); + + res.send({ status: 'ok', body: req.body }); +}); + +Sentry.setupExpressErrorHandler(app); + +startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/express-v5/without-tracing/test.ts b/dev-packages/node-integration-tests/suites/express-v5/without-tracing/test.ts new file mode 100644 index 000000000000..fdd63ad4aa4b --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express-v5/without-tracing/test.ts @@ -0,0 +1,132 @@ +import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; + +afterAll(() => { + cleanupChildProcesses(); +}); + +describe('express without tracing', () => { + test('correctly applies isolation scope even without tracing', done => { + const runner = createRunner(__dirname, 'server.ts') + .expect({ + event: { + transaction: 'GET /test/isolationScope/1', + tags: { + global: 'tag', + 'isolation-scope': 'tag', + 'isolation-scope-1': '1', + }, + // Request is correctly set + request: { + url: expect.stringMatching(/^http:\/\/localhost:(\d+)\/test\/isolationScope\/1$/), + method: 'GET', + headers: { + 'user-agent': expect.stringContaining(''), + }, + }, + }, + }) + .start(done); + + runner.makeRequest('get', '/test/isolationScope/1'); + }); + + describe('request data', () => { + test('correctly captures JSON request data', done => { + const runner = createRunner(__dirname, 'server.ts') + .expect({ + event: { + transaction: 'POST /test-post', + request: { + url: expect.stringMatching(/^http:\/\/localhost:(\d+)\/test-post$/), + method: 'POST', + headers: { + 'user-agent': expect.stringContaining(''), + 'content-type': 'application/json', + }, + data: JSON.stringify({ + foo: 'bar', + other: 1, + }), + }, + }, + }) + .start(done); + + runner.makeRequest('post', '/test-post', { data: { foo: 'bar', other: 1 } }); + }); + + test('correctly captures plain text request data', done => { + const runner = createRunner(__dirname, 'server.ts') + .expect({ + event: { + transaction: 'POST /test-post', + request: { + url: expect.stringMatching(/^http:\/\/localhost:(\d+)\/test-post$/), + method: 'POST', + headers: { + 'user-agent': expect.stringContaining(''), + 'content-type': 'text/plain', + }, + data: 'some plain text', + }, + }, + }) + .start(done); + + runner.makeRequest('post', '/test-post', { + headers: { + 'Content-Type': 'text/plain', + }, + data: 'some plain text', + }); + }); + + test('correctly captures text buffer request data', done => { + const runner = createRunner(__dirname, 'server.ts') + .expect({ + event: { + transaction: 'POST /test-post', + request: { + url: expect.stringMatching(/^http:\/\/localhost:(\d+)\/test-post$/), + method: 'POST', + headers: { + 'user-agent': expect.stringContaining(''), + 'content-type': 'application/octet-stream', + }, + data: 'some plain text in buffer', + }, + }, + }) + .start(done); + + runner.makeRequest('post', '/test-post', { + headers: { 'Content-Type': 'application/octet-stream' }, + data: Buffer.from('some plain text in buffer'), + }); + }); + + test('correctly captures non-text buffer request data', done => { + const runner = createRunner(__dirname, 'server.ts') + .expect({ + event: { + transaction: 'POST /test-post', + request: { + url: expect.stringMatching(/^http:\/\/localhost:(\d+)\/test-post$/), + method: 'POST', + headers: { + 'user-agent': expect.stringContaining(''), + 'content-type': 'application/octet-stream', + }, + // This is some non-ascii string representation + data: expect.any(String), + }, + }, + }) + .start(done); + + const body = new Uint8Array([1, 2, 3, 4, 5]).buffer; + + runner.makeRequest('post', '/test-post', { headers: { 'Content-Type': 'application/octet-stream' }, data: body }); + }); + }); +}); diff --git a/packages/node/src/integrations/tracing/express-v5/enums/AttributeNames.ts b/packages/node/src/integrations/tracing/express-v5/enums/AttributeNames.ts new file mode 100644 index 000000000000..f6a83e31b073 --- /dev/null +++ b/packages/node/src/integrations/tracing/express-v5/enums/AttributeNames.ts @@ -0,0 +1,19 @@ +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +export enum AttributeNames { + EXPRESS_TYPE = 'express.type', + EXPRESS_NAME = 'express.name', +} diff --git a/packages/node/src/integrations/tracing/express-v5/enums/ExpressLayerType.ts b/packages/node/src/integrations/tracing/express-v5/enums/ExpressLayerType.ts new file mode 100644 index 000000000000..5cfc47c555d9 --- /dev/null +++ b/packages/node/src/integrations/tracing/express-v5/enums/ExpressLayerType.ts @@ -0,0 +1,20 @@ +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +export enum ExpressLayerType { + ROUTER = 'router', + MIDDLEWARE = 'middleware', + REQUEST_HANDLER = 'request_handler', +} diff --git a/packages/node/src/integrations/tracing/express-v5/instrumentation.ts b/packages/node/src/integrations/tracing/express-v5/instrumentation.ts new file mode 100644 index 000000000000..bf2acb26c67d --- /dev/null +++ b/packages/node/src/integrations/tracing/express-v5/instrumentation.ts @@ -0,0 +1,324 @@ +/* eslint-disable @typescript-eslint/no-explicit-any */ +/* eslint-disable @typescript-eslint/no-unsafe-member-access */ +/* eslint-disable @typescript-eslint/member-ordering */ +/* eslint-disable guard-for-in */ +/* eslint-disable @typescript-eslint/ban-types */ +/* eslint-disable prefer-rest-params */ +/* eslint-disable @typescript-eslint/no-this-alias */ +/* eslint-disable jsdoc/require-jsdoc */ +/* eslint-disable @typescript-eslint/explicit-function-return-type */ +/* eslint-disable @typescript-eslint/explicit-member-accessibility */ + +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import type { Attributes } from '@opentelemetry/api'; +import { SpanStatusCode, context, diag, trace } from '@opentelemetry/api'; +import { RPCType, getRPCMetadata } from '@opentelemetry/core'; +import { + InstrumentationBase, + InstrumentationNodeModuleDefinition, + isWrapped, + safeExecuteInTheMiddle, +} from '@opentelemetry/instrumentation'; +import { SEMATTRS_HTTP_ROUTE } from '@opentelemetry/semantic-conventions'; +import type * as express from 'express'; +import { AttributeNames } from './enums/AttributeNames'; +import { ExpressLayerType } from './enums/ExpressLayerType'; +import type { ExpressLayer, ExpressRouter, PatchedRequest } from './internal-types'; +import { _LAYERS_STORE_PROPERTY, kLayerPatched } from './internal-types'; +import type { ExpressInstrumentationConfig, ExpressRequestInfo } from './types'; +import { asErrorAndMessage, getLayerMetadata, getLayerPath, isLayerIgnored, storeLayerPath } from './utils'; + +export const PACKAGE_VERSION = '0.1.0'; +export const PACKAGE_NAME = '@sentry/instrumentation-express-v5'; + +/** Express instrumentation for OpenTelemetry */ +export class ExpressInstrumentationV5 extends InstrumentationBase { + constructor(config: ExpressInstrumentationConfig = {}) { + super(PACKAGE_NAME, PACKAGE_VERSION, config); + } + + init() { + return [ + new InstrumentationNodeModuleDefinition( + 'express', + ['>=5.0.0'], + moduleExports => this._setup(moduleExports), + moduleExports => this._tearDown(moduleExports), + ), + ]; + } + + private _setup(moduleExports: any) { + const routerProto = moduleExports.Router.prototype; + // patch express.Router.route + if (isWrapped(routerProto.route)) { + this._unwrap(routerProto, 'route'); + } + this._wrap(routerProto, 'route', this._getRoutePatch()); + // patch express.Router.use + if (isWrapped(routerProto.use)) { + this._unwrap(routerProto, 'use'); + } + this._wrap(routerProto, 'use', this._getRouterUsePatch() as any); + // patch express.Application.use + if (isWrapped(moduleExports.application.use)) { + this._unwrap(moduleExports.application, 'use'); + } + this._wrap(moduleExports.application, 'use', this._getAppUsePatch() as any); + return moduleExports; + } + + private _tearDown(moduleExports: any) { + if (moduleExports === undefined) return; + const routerProto = moduleExports.Router.prototype; + this._unwrap(routerProto, 'route'); + this._unwrap(routerProto, 'use'); + this._unwrap(moduleExports.application, 'use'); + } + + /** + * Get the patch for Router.route function + */ + private _getRoutePatch() { + const instrumentation = this; + return function (original: express.Router['route']) { + return function route_trace(this: ExpressRouter, ...args: Parameters) { + const route = original.apply(this, args); + const layer = this.stack[this.stack.length - 1] as ExpressLayer; + instrumentation._applyPatch(layer, getLayerPath(args)); + return route; + }; + }; + } + + /** + * Get the patch for Router.use function + */ + private _getRouterUsePatch() { + const instrumentation = this; + return function (original: express.Router['use']) { + return function use(this: express.Application, ...args: Parameters) { + const route = original.apply(this, args); + const layer = this.stack[this.stack.length - 1] as ExpressLayer; + instrumentation._applyPatch(layer, getLayerPath(args)); + return route; + }; + }; + } + + /** + * Get the patch for Application.use function + */ + private _getAppUsePatch() { + const instrumentation = this; + return function (original: express.Application['use']) { + return function use( + // In express 5.x the router is stored in `router` whereas in 4.x it's stored in `_router` + this: { _router?: ExpressRouter; router?: ExpressRouter }, + ...args: Parameters + ) { + // if we access app.router in express 4.x we trigger an assertion error + // This property existed in v3, was removed in v4 and then re-added in v5 + const router = this.router; + const route = original.apply(this, args); + if (router) { + const layer = router.stack[router.stack.length - 1] as ExpressLayer; + instrumentation._applyPatch(layer, getLayerPath(args)); + } + return route; + }; + }; + } + + /** Patch each express layer to create span and propagate context */ + private _applyPatch(this: ExpressInstrumentationV5, layer: ExpressLayer, layerPath?: string) { + const instrumentation = this; + // avoid patching multiple times the same layer + if (layer[kLayerPatched] === true) return; + layer[kLayerPatched] = true; + + this._wrap(layer, 'handle', original => { + // TODO: instrument error handlers + if (original.length === 4) return original; + + const patched = function (this: ExpressLayer, req: PatchedRequest, res: express.Response) { + storeLayerPath(req, layerPath); + const route = (req[_LAYERS_STORE_PROPERTY] as string[]) + .filter(path => path !== '/' && path !== '/*') + .join('') + // remove duplicate slashes to normalize route + .replace(/\/{2,}/g, '/'); + + const attributes: Attributes = { + // eslint-disable-next-line deprecation/deprecation + [SEMATTRS_HTTP_ROUTE]: route.length > 0 ? route : '/', + }; + const metadata = getLayerMetadata(route, layer, layerPath); + const type = metadata.attributes[AttributeNames.EXPRESS_TYPE] as ExpressLayerType; + + const rpcMetadata = getRPCMetadata(context.active()); + if (rpcMetadata?.type === RPCType.HTTP) { + rpcMetadata.route = route || '/'; + } + + // verify against the config if the layer should be ignored + if (isLayerIgnored(metadata.name, type, instrumentation.getConfig())) { + if (type === ExpressLayerType.MIDDLEWARE) { + (req[_LAYERS_STORE_PROPERTY] as string[]).pop(); + } + return original.apply(this, arguments); + } + + if (trace.getSpan(context.active()) === undefined) { + return original.apply(this, arguments); + } + + const spanName = instrumentation._getSpanName( + { + request: req, + layerType: type, + route, + }, + metadata.name, + ); + const span = instrumentation.tracer.startSpan(spanName, { + attributes: Object.assign(attributes, metadata.attributes), + }); + + const { requestHook } = instrumentation.getConfig(); + if (requestHook) { + safeExecuteInTheMiddle( + () => + requestHook(span, { + request: req, + layerType: type, + route, + }), + e => { + if (e) { + diag.error('express instrumentation: request hook failed', e); + } + }, + true, + ); + } + + let spanHasEnded = false; + if (metadata.attributes[AttributeNames.EXPRESS_TYPE] !== ExpressLayerType.MIDDLEWARE) { + span.end(); + spanHasEnded = true; + } + // listener for response.on('finish') + const onResponseFinish = () => { + if (spanHasEnded === false) { + spanHasEnded = true; + span.end(); + } + }; + + // verify we have a callback + const args = Array.from(arguments); + const callbackIdx = args.findIndex(arg => typeof arg === 'function'); + if (callbackIdx >= 0) { + arguments[callbackIdx] = function () { + // express considers anything but an empty value, "route" or "router" + // passed to its callback to be an error + const maybeError = arguments[0]; + const isError = ![undefined, null, 'route', 'router'].includes(maybeError); + if (!spanHasEnded && isError) { + const [error, message] = asErrorAndMessage(maybeError); + span.recordException(error); + span.setStatus({ + code: SpanStatusCode.ERROR, + message, + }); + } + + if (spanHasEnded === false) { + spanHasEnded = true; + req.res?.removeListener('finish', onResponseFinish); + span.end(); + } + if (!(req.route && isError)) { + (req[_LAYERS_STORE_PROPERTY] as string[]).pop(); + } + const callback = args[callbackIdx] as Function; + return callback.apply(this, arguments); + }; + } + + try { + return original.apply(this, arguments); + } catch (anyError) { + const [error, message] = asErrorAndMessage(anyError); + span.recordException(error); + span.setStatus({ + code: SpanStatusCode.ERROR, + message, + }); + throw anyError; + } finally { + /** + * At this point if the callback wasn't called, that means either the + * layer is asynchronous (so it will call the callback later on) or that + * the layer directly end the http response, so we'll hook into the "finish" + * event to handle the later case. + */ + if (!spanHasEnded) { + res.once('finish', onResponseFinish); + } + } + }; + + // `handle` isn't just a regular function in some cases. It also contains + // some properties holding metadata and state so we need to proxy them + // through through patched function + // ref: https://github.com/open-telemetry/opentelemetry-js-contrib/issues/1950 + // Also some apps/libs do their own patching before OTEL and have these properties + // in the proptotype. So we use a `for...in` loop to get own properties and also + // any enumerable prop in the prototype chain + // ref: https://github.com/open-telemetry/opentelemetry-js-contrib/issues/2271 + for (const key in original) { + Object.defineProperty(patched, key, { + get() { + return original[key]; + }, + set(value) { + original[key] = value; + }, + }); + } + return patched; + }); + } + + _getSpanName(info: ExpressRequestInfo, defaultName: string) { + const { spanNameHook } = this.getConfig(); + + if (!(spanNameHook instanceof Function)) { + return defaultName; + } + + try { + return spanNameHook(info, defaultName) ?? defaultName; + } catch (err) { + diag.error('express instrumentation: error calling span name rewrite hook', err); + return defaultName; + } + } +} diff --git a/packages/node/src/integrations/tracing/express-v5/internal-types.ts b/packages/node/src/integrations/tracing/express-v5/internal-types.ts new file mode 100644 index 000000000000..482dc0b6b4ea --- /dev/null +++ b/packages/node/src/integrations/tracing/express-v5/internal-types.ts @@ -0,0 +1,63 @@ +/* eslint-disable @typescript-eslint/no-explicit-any */ +/* eslint-disable @typescript-eslint/ban-types */ + +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import type { Request } from 'express'; + +/** + * This symbol is used to mark express layer as being already instrumented + * since its possible to use a given layer multiple times (ex: middlewares) + */ +export const kLayerPatched: unique symbol = Symbol('express-layer-patched'); + +/** + * This const define where on the `request` object the Instrumentation will mount the + * current stack of express layer. + * + * It is necessary because express doesn't store the different layers + * (ie: middleware, router etc) that it called to get to the current layer. + * Given that, the only way to know the route of a given layer is to + * store the path of where each previous layer has been mounted. + * + * ex: bodyParser > auth middleware > /users router > get /:id + * in this case the stack would be: ["/users", "/:id"] + * + * ex2: bodyParser > /api router > /v1 router > /users router > get /:id + * stack: ["/api", "/v1", "/users", ":id"] + * + */ +export const _LAYERS_STORE_PROPERTY = '__ot_middlewares'; + +export type PatchedRequest = { + [_LAYERS_STORE_PROPERTY]?: string[]; +} & Request; +export type PathParams = string | RegExp | Array; + +// https://github.com/expressjs/express/blob/main/lib/router/index.js#L53 +export type ExpressRouter = { + stack: ExpressLayer[]; +}; + +// https://github.com/expressjs/express/blob/main/lib/router/layer.js#L33 +export type ExpressLayer = { + handle: Function & Record; + [kLayerPatched]?: boolean; + name: string; + path: string; + route?: ExpressLayer; +}; diff --git a/packages/node/src/integrations/tracing/express-v5/types.ts b/packages/node/src/integrations/tracing/express-v5/types.ts new file mode 100644 index 000000000000..0623cac1cbc5 --- /dev/null +++ b/packages/node/src/integrations/tracing/express-v5/types.ts @@ -0,0 +1,65 @@ +/* eslint-disable @typescript-eslint/no-explicit-any */ + +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import type { Span } from '@opentelemetry/api'; +import type { InstrumentationConfig } from '@opentelemetry/instrumentation'; +import type { ExpressLayerType } from './enums/ExpressLayerType'; + +export type LayerPathSegment = string | RegExp | number; + +export type IgnoreMatcher = string | RegExp | ((name: string) => boolean); + +export type ExpressRequestInfo = { + /** An express request object */ + request: T; + route: string; + layerType: ExpressLayerType; +}; + +export type SpanNameHook = ( + info: ExpressRequestInfo, + /** + * If no decision is taken based on RequestInfo, the default name + * supplied by the instrumentation can be used instead. + */ + defaultName: string, +) => string; + +/** + * Function that can be used to add custom attributes to the current span or the root span on + * a Express request + * @param span - The Express middleware layer span. + * @param info - An instance of ExpressRequestInfo that contains info about the request such as the route, and the layer type. + */ +export interface ExpressRequestCustomAttributeFunction { + (span: Span, info: ExpressRequestInfo): void; +} + +/** + * Options available for the Express Instrumentation (see [documentation](https://github.com/open-telemetry/opentelemetry-js-contrib/tree/main/plugins/node/opentelemetry-instrumentation-express#express-instrumentation-options)) + */ +export interface ExpressInstrumentationConfig extends InstrumentationConfig { + /** Ignore specific based on their name */ + ignoreLayers?: IgnoreMatcher[]; + /** Ignore specific layers based on their type */ + ignoreLayersType?: ExpressLayerType[]; + spanNameHook?: SpanNameHook; + + /** Function for adding custom attributes on Express request */ + requestHook?: ExpressRequestCustomAttributeFunction; +} diff --git a/packages/node/src/integrations/tracing/express-v5/utils.ts b/packages/node/src/integrations/tracing/express-v5/utils.ts new file mode 100644 index 000000000000..45ef61ed7eb6 --- /dev/null +++ b/packages/node/src/integrations/tracing/express-v5/utils.ts @@ -0,0 +1,191 @@ +/* eslint-disable @typescript-eslint/no-non-null-assertion */ +/* eslint-disable @typescript-eslint/explicit-function-return-type */ +/* eslint-disable @typescript-eslint/no-unsafe-member-access */ + +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import type { Attributes } from '@opentelemetry/api'; +import { AttributeNames } from './enums/AttributeNames'; +import { ExpressLayerType } from './enums/ExpressLayerType'; +import type { ExpressLayer, PatchedRequest } from './internal-types'; +import { _LAYERS_STORE_PROPERTY } from './internal-types'; +import type { ExpressInstrumentationConfig, IgnoreMatcher, LayerPathSegment } from './types'; + +/** + * Store layers path in the request to be able to construct route later + * @param request The request where + * @param [value] the value to push into the array + */ +export const storeLayerPath = (request: PatchedRequest, value?: string): void => { + if (Array.isArray(request[_LAYERS_STORE_PROPERTY]) === false) { + Object.defineProperty(request, _LAYERS_STORE_PROPERTY, { + enumerable: false, + value: [], + }); + } + if (value === undefined) return; + (request[_LAYERS_STORE_PROPERTY] as string[]).push(value); +}; + +/** + * Recursively search the router path from layer stack + * @param path The path to reconstruct + * @param layer The layer to reconstruct from + * @returns The reconstructed path + */ +export const getRouterPath = (path: string, layer: ExpressLayer): string => { + const stackLayer = layer.handle?.stack?.[0]; + + if (stackLayer?.route?.path) { + return `${path}${stackLayer.route.path}`; + } + + if (stackLayer?.handle?.stack) { + return getRouterPath(path, stackLayer); + } + + return path; +}; + +/** + * Parse express layer context to retrieve a name and attributes. + * @param route The route of the layer + * @param layer Express layer + * @param [layerPath] if present, the path on which the layer has been mounted + */ +export const getLayerMetadata = ( + route: string, + layer: ExpressLayer, + layerPath?: string, +): { + attributes: Attributes; + name: string; +} => { + if (layer.name === 'router') { + const maybeRouterPath = getRouterPath('', layer); + const extractedRouterPath = maybeRouterPath ? maybeRouterPath : layerPath || route || '/'; + + return { + attributes: { + [AttributeNames.EXPRESS_NAME]: extractedRouterPath, + [AttributeNames.EXPRESS_TYPE]: ExpressLayerType.ROUTER, + }, + name: `router - ${extractedRouterPath}`, + }; + } else if (layer.name === 'bound dispatch' || layer.name === 'handle') { + return { + attributes: { + [AttributeNames.EXPRESS_NAME]: (route || layerPath) ?? 'request handler', + [AttributeNames.EXPRESS_TYPE]: ExpressLayerType.REQUEST_HANDLER, + }, + name: `request handler${layer.path ? ` - ${route || layerPath}` : ''}`, + }; + } else { + return { + attributes: { + [AttributeNames.EXPRESS_NAME]: layer.name, + [AttributeNames.EXPRESS_TYPE]: ExpressLayerType.MIDDLEWARE, + }, + name: `middleware - ${layer.name}`, + }; + } +}; + +/** + * Check whether the given obj match pattern + * @param constant e.g URL of request + * @param obj obj to inspect + * @param pattern Match pattern + */ +const satisfiesPattern = (constant: string, pattern: IgnoreMatcher): boolean => { + if (typeof pattern === 'string') { + return pattern === constant; + } else if (pattern instanceof RegExp) { + return pattern.test(constant); + } else if (typeof pattern === 'function') { + return pattern(constant); + } else { + throw new TypeError('Pattern is in unsupported datatype'); + } +}; + +/** + * Check whether the given request is ignored by configuration + * It will not re-throw exceptions from `list` provided by the client + * @param constant e.g URL of request + * @param [list] List of ignore patterns + * @param [onException] callback for doing something when an exception has + * occurred + */ +export const isLayerIgnored = ( + name: string, + type: ExpressLayerType, + config?: ExpressInstrumentationConfig, +): boolean => { + if (Array.isArray(config?.ignoreLayersType) && config?.ignoreLayersType?.includes(type)) { + return true; + } + if (Array.isArray(config?.ignoreLayers) === false) return false; + try { + for (const pattern of config!.ignoreLayers!) { + if (satisfiesPattern(name, pattern)) { + return true; + } + } + } catch (e) { + /* catch block */ + } + + return false; +}; + +/** + * Converts a user-provided error value into an error and error message pair + * + * @param error - User-provided error value + * @returns Both an Error or string representation of the value and an error message + */ +export const asErrorAndMessage = (error: unknown): [error: string | Error, message: string] => + error instanceof Error ? [error, error.message] : [String(error), String(error)]; + +/** + * Extracts the layer path from the route arguments + * + * @param args - Arguments of the route + * @returns The layer path + */ +export const getLayerPath = (args: [LayerPathSegment | LayerPathSegment[], ...unknown[]]): string | undefined => { + const firstArg = args[0]; + + if (Array.isArray(firstArg)) { + return firstArg.map(arg => extractLayerPathSegment(arg) || '').join(','); + } + + return extractLayerPathSegment(firstArg); +}; + +const extractLayerPathSegment = (arg: LayerPathSegment) => { + if (typeof arg === 'string') { + return arg; + } + + if (arg instanceof RegExp || typeof arg === 'number') { + return arg.toString(); + } + + return; +}; diff --git a/packages/node/src/integrations/tracing/express.ts b/packages/node/src/integrations/tracing/express.ts index 13f50cff0202..cf6f6b233cdf 100644 --- a/packages/node/src/integrations/tracing/express.ts +++ b/packages/node/src/integrations/tracing/express.ts @@ -1,4 +1,6 @@ import type * as http from 'node:http'; +import type { Span } from '@opentelemetry/api'; +import type { ExpressRequestInfo } from '@opentelemetry/instrumentation-express'; import { ExpressInstrumentation } from '@opentelemetry/instrumentation-express'; import type { IntegrationFn } from '@sentry/core'; import { @@ -14,44 +16,58 @@ import { DEBUG_BUILD } from '../../debug-build'; import { generateInstrumentOnce } from '../../otel/instrument'; import { addOriginToSpan } from '../../utils/addOriginToSpan'; import { ensureIsWrapped } from '../../utils/ensureIsWrapped'; +import { ExpressInstrumentationV5 } from './express-v5/instrumentation'; const INTEGRATION_NAME = 'Express'; +const INTEGRATION_NAME_V5 = 'Express-V5'; + +function requestHook(span: Span): void { + addOriginToSpan(span, 'auto.http.otel.express'); + + const attributes = spanToJSON(span).data; + // this is one of: middleware, request_handler, router + const type = attributes['express.type']; + + if (type) { + span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OP, `${type}.express`); + } + + // Also update the name, we don't need to "middleware - " prefix + const name = attributes['express.name']; + if (typeof name === 'string') { + span.updateName(name); + } +} + +function spanNameHook(info: ExpressRequestInfo, defaultName: string): string { + if (getIsolationScope() === getDefaultIsolationScope()) { + DEBUG_BUILD && 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 }; + const method = req.method ? req.method.toUpperCase() : 'GET'; + getIsolationScope().setTransactionName(`${method} ${info.route}`); + } + return defaultName; +} export const instrumentExpress = generateInstrumentOnce( INTEGRATION_NAME, () => new ExpressInstrumentation({ - requestHook(span) { - addOriginToSpan(span, 'auto.http.otel.express'); - - const attributes = spanToJSON(span).data; - // this is one of: middleware, request_handler, router - const type = attributes['express.type']; - - if (type) { - span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OP, `${type}.express`); - } - - // Also update the name, we don't need to "middleware - " prefix - const name = attributes['express.name']; - if (typeof name === 'string') { - span.updateName(name); - } - }, - spanNameHook(info, defaultName) { - if (getIsolationScope() === getDefaultIsolationScope()) { - DEBUG_BUILD && - 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 }; - const method = req.method ? req.method.toUpperCase() : 'GET'; - getIsolationScope().setTransactionName(`${method} ${info.route}`); - } - return defaultName; - }, + requestHook: span => requestHook(span), + spanNameHook: (info, defaultName) => spanNameHook(info, defaultName), + }), +); + +export const instrumentExpressV5 = generateInstrumentOnce( + INTEGRATION_NAME_V5, + () => + new ExpressInstrumentationV5({ + requestHook: span => requestHook(span), + spanNameHook: (info, defaultName) => spanNameHook(info, defaultName), }), ); @@ -60,6 +76,7 @@ const _expressIntegration = (() => { name: INTEGRATION_NAME, setupOnce() { instrumentExpress(); + instrumentExpressV5(); }, }; }) satisfies IntegrationFn; diff --git a/packages/node/src/integrations/tracing/index.ts b/packages/node/src/integrations/tracing/index.ts index 7d06689f250d..2873a2643617 100644 --- a/packages/node/src/integrations/tracing/index.ts +++ b/packages/node/src/integrations/tracing/index.ts @@ -3,7 +3,7 @@ import { instrumentOtelHttp } from '../http'; import { amqplibIntegration, instrumentAmqplib } from './amqplib'; import { connectIntegration, instrumentConnect } from './connect'; -import { expressIntegration, instrumentExpress } from './express'; +import { expressIntegration, instrumentExpress, instrumentExpressV5 } from './express'; import { fastifyIntegration, instrumentFastify } from './fastify'; import { genericPoolIntegration, instrumentGenericPool } from './genericPool'; import { graphqlIntegration, instrumentGraphql } from './graphql'; @@ -58,6 +58,7 @@ export function getOpenTelemetryInstrumentationToPreload(): (((options?: any) => return [ instrumentOtelHttp, instrumentExpress, + instrumentExpressV5, instrumentConnect, instrumentFastify, instrumentHapi,