From c9a0e08a723ccd70921ad2f47d864ec1acd65acc Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 28 Apr 2025 14:06:29 +0200 Subject: [PATCH 01/15] test(node): Add utility to test esm & cjs instrumentation --- .../node-integration-tests/.eslintrc.js | 3 +- .../node-integration-tests/.gitignore | 1 + .../suites/anr/app-path.mjs | 3 +- .../suites/anr/basic-multiple.mjs | 3 +- .../suites/anr/basic.mjs | 3 +- .../suites/anr/indefinite.mjs | 3 +- .../suites/anr/isolated.mjs | 3 +- .../suites/breadcrumbs/process-thread/app.mjs | 28 ++--- .../suites/child-process/fork.mjs | 5 +- .../suites/child-process/worker.mjs | 5 +- .../filename-with-spaces/instrument.mjs | 2 +- .../contextLines/filename-with-spaces/test.ts | 2 +- .../suites/esm/import-in-the-middle/app.mjs | 10 +- .../suites/esm/modules-integration/app.mjs | 2 +- .../suites/esm/warn-esm/server.mjs | 6 +- .../LocalVariables/deny-inspector.mjs | 7 +- .../LocalVariables/local-variables-caught.mjs | 2 +- .../suites/tracing/amqplib/constants.ts | 15 --- .../amqplib/{init.ts => instrument.mjs} | 0 .../tracing/amqplib/scenario-message.ts | 26 ----- .../suites/tracing/amqplib/scenario.mjs | 75 ++++++++++++ .../suites/tracing/amqplib/test.ts | 46 ++++---- .../suites/tracing/amqplib/utils.ts | 45 ------- .../suites/tracing/connect/instrument.mjs | 9 ++ .../connect/{scenario.js => scenario.mjs} | 16 +-- .../suites/tracing/connect/test.ts | 49 ++++---- .../suites/tracing/dataloader/instrument.mjs | 10 ++ .../suites/tracing/dataloader/scenario.js | 34 ------ .../suites/tracing/dataloader/scenario.mjs | 24 ++++ .../suites/tracing/dataloader/test.ts | 20 +++- .../suites/tracing/hapi/instrument.mjs | 9 ++ .../hapi/{scenario.js => scenario.mjs} | 26 ++--- .../suites/tracing/hapi/test.ts | 110 +++++++++--------- .../requests/http-sampled-esm/instrument.mjs | 2 +- .../requests/http-sampled-esm/scenario.mjs | 2 +- .../tracing/requests/http-sampled-esm/test.ts | 2 +- .../node-integration-tests/utils/runner.ts | 108 ++++++++++++++++- 37 files changed, 417 insertions(+), 299 deletions(-) create mode 100644 dev-packages/node-integration-tests/.gitignore delete mode 100644 dev-packages/node-integration-tests/suites/tracing/amqplib/constants.ts rename dev-packages/node-integration-tests/suites/tracing/amqplib/{init.ts => instrument.mjs} (100%) delete mode 100644 dev-packages/node-integration-tests/suites/tracing/amqplib/scenario-message.ts create mode 100644 dev-packages/node-integration-tests/suites/tracing/amqplib/scenario.mjs delete mode 100644 dev-packages/node-integration-tests/suites/tracing/amqplib/utils.ts create mode 100644 dev-packages/node-integration-tests/suites/tracing/connect/instrument.mjs rename dev-packages/node-integration-tests/suites/tracing/connect/{scenario.js => scenario.mjs} (52%) create mode 100644 dev-packages/node-integration-tests/suites/tracing/dataloader/instrument.mjs delete mode 100644 dev-packages/node-integration-tests/suites/tracing/dataloader/scenario.js create mode 100644 dev-packages/node-integration-tests/suites/tracing/dataloader/scenario.mjs create mode 100644 dev-packages/node-integration-tests/suites/tracing/hapi/instrument.mjs rename dev-packages/node-integration-tests/suites/tracing/hapi/{scenario.js => scenario.mjs} (60%) diff --git a/dev-packages/node-integration-tests/.eslintrc.js b/dev-packages/node-integration-tests/.eslintrc.js index c69f03a3b819..a3501df39470 100644 --- a/dev-packages/node-integration-tests/.eslintrc.js +++ b/dev-packages/node-integration-tests/.eslintrc.js @@ -12,10 +12,11 @@ module.exports = { }, }, { - files: ['suites/**/*.ts'], + files: ['suites/**/*.ts', 'suites/**/*.mjs'], parserOptions: { project: ['tsconfig.test.json'], sourceType: 'module', + ecmaVersion: 'latest', }, rules: { '@typescript-eslint/typedef': 'off', diff --git a/dev-packages/node-integration-tests/.gitignore b/dev-packages/node-integration-tests/.gitignore new file mode 100644 index 000000000000..365cb959a94c --- /dev/null +++ b/dev-packages/node-integration-tests/.gitignore @@ -0,0 +1 @@ +suites/**/tmp_* diff --git a/dev-packages/node-integration-tests/suites/anr/app-path.mjs b/dev-packages/node-integration-tests/suites/anr/app-path.mjs index 97f28d07c59e..5ace9ebbb3e8 100644 --- a/dev-packages/node-integration-tests/suites/anr/app-path.mjs +++ b/dev-packages/node-integration-tests/suites/anr/app-path.mjs @@ -1,10 +1,9 @@ +import * as Sentry from '@sentry/node'; import * as assert from 'assert'; import * as crypto from 'crypto'; import * as path from 'path'; import * as url from 'url'; -import * as Sentry from '@sentry/node'; - global._sentryDebugIds = { [new Error().stack]: 'aaaaaaaa-aaaa-4aaa-aaaa-aaaaaaaaaa' }; const __dirname = path.dirname(url.fileURLToPath(import.meta.url)); diff --git a/dev-packages/node-integration-tests/suites/anr/basic-multiple.mjs b/dev-packages/node-integration-tests/suites/anr/basic-multiple.mjs index 49c28cb21dbf..d24e5d9f20d9 100644 --- a/dev-packages/node-integration-tests/suites/anr/basic-multiple.mjs +++ b/dev-packages/node-integration-tests/suites/anr/basic-multiple.mjs @@ -1,8 +1,7 @@ +import * as Sentry from '@sentry/node'; import * as assert from 'assert'; import * as crypto from 'crypto'; -import * as Sentry from '@sentry/node'; - global._sentryDebugIds = { [new Error().stack]: 'aaaaaaaa-aaaa-4aaa-aaaa-aaaaaaaaaa' }; setTimeout(() => { diff --git a/dev-packages/node-integration-tests/suites/anr/basic.mjs b/dev-packages/node-integration-tests/suites/anr/basic.mjs index 85b5cfb55c35..19b766c1e0e4 100644 --- a/dev-packages/node-integration-tests/suites/anr/basic.mjs +++ b/dev-packages/node-integration-tests/suites/anr/basic.mjs @@ -1,8 +1,7 @@ +import * as Sentry from '@sentry/node'; import * as assert from 'assert'; import * as crypto from 'crypto'; -import * as Sentry from '@sentry/node'; - global._sentryDebugIds = { [new Error().stack]: 'aaaaaaaa-aaaa-4aaa-aaaa-aaaaaaaaaa' }; setTimeout(() => { diff --git a/dev-packages/node-integration-tests/suites/anr/indefinite.mjs b/dev-packages/node-integration-tests/suites/anr/indefinite.mjs index 000c63a12cf3..927e4e5fc4ad 100644 --- a/dev-packages/node-integration-tests/suites/anr/indefinite.mjs +++ b/dev-packages/node-integration-tests/suites/anr/indefinite.mjs @@ -1,8 +1,7 @@ +import * as Sentry from '@sentry/node'; import * as assert from 'assert'; import * as crypto from 'crypto'; -import * as Sentry from '@sentry/node'; - setTimeout(() => { process.exit(); }, 10000); diff --git a/dev-packages/node-integration-tests/suites/anr/isolated.mjs b/dev-packages/node-integration-tests/suites/anr/isolated.mjs index 26ec9eaf4546..c0ca76da4319 100644 --- a/dev-packages/node-integration-tests/suites/anr/isolated.mjs +++ b/dev-packages/node-integration-tests/suites/anr/isolated.mjs @@ -1,8 +1,7 @@ +import * as Sentry from '@sentry/node'; import * as assert from 'assert'; import * as crypto from 'crypto'; -import * as Sentry from '@sentry/node'; - setTimeout(() => { process.exit(); }, 10000); diff --git a/dev-packages/node-integration-tests/suites/breadcrumbs/process-thread/app.mjs b/dev-packages/node-integration-tests/suites/breadcrumbs/process-thread/app.mjs index 298952d58ced..4377723e6f4b 100644 --- a/dev-packages/node-integration-tests/suites/breadcrumbs/process-thread/app.mjs +++ b/dev-packages/node-integration-tests/suites/breadcrumbs/process-thread/app.mjs @@ -1,7 +1,7 @@ +import * as Sentry from '@sentry/node'; +import { loggingTransport } from '@sentry-internal/node-integration-tests'; import { spawn } from 'child_process'; import { join } from 'path'; -import { loggingTransport } from '@sentry-internal/node-integration-tests'; -import * as Sentry from '@sentry/node'; import { Worker } from 'worker_threads'; const __dirname = new URL('.', import.meta.url).pathname; @@ -13,16 +13,18 @@ Sentry.init({ transport: loggingTransport, }); -await new Promise(resolve => { - const child = spawn('sleep', ['a']); - child.on('error', resolve); - child.on('exit', resolve); -}); +(async () => { + await new Promise(resolve => { + const child = spawn('sleep', ['a']); + child.on('error', resolve); + child.on('exit', resolve); + }); -await new Promise(resolve => { - const worker = new Worker(join(__dirname, 'worker.mjs')); - worker.on('error', resolve); - worker.on('exit', resolve); -}); + await new Promise(resolve => { + const worker = new Worker(join(__dirname, 'worker.mjs')); + worker.on('error', resolve); + worker.on('exit', resolve); + }); -throw new Error('This is a test error'); + throw new Error('This is a test error'); +})(); diff --git a/dev-packages/node-integration-tests/suites/child-process/fork.mjs b/dev-packages/node-integration-tests/suites/child-process/fork.mjs index 88503fa887a9..d1678479143c 100644 --- a/dev-packages/node-integration-tests/suites/child-process/fork.mjs +++ b/dev-packages/node-integration-tests/suites/child-process/fork.mjs @@ -1,7 +1,7 @@ +import * as Sentry from '@sentry/node'; +import { loggingTransport } from '@sentry-internal/node-integration-tests'; import { fork } from 'child_process'; import * as path from 'path'; -import { loggingTransport } from '@sentry-internal/node-integration-tests'; -import * as Sentry from '@sentry/node'; const __dirname = new URL('.', import.meta.url).pathname; @@ -12,6 +12,7 @@ Sentry.init({ transport: loggingTransport, }); +// eslint-disable-next-line no-unused-vars const _child = fork(path.join(__dirname, 'child.mjs')); setTimeout(() => { diff --git a/dev-packages/node-integration-tests/suites/child-process/worker.mjs b/dev-packages/node-integration-tests/suites/child-process/worker.mjs index dcca0bcc4105..d8642095ac04 100644 --- a/dev-packages/node-integration-tests/suites/child-process/worker.mjs +++ b/dev-packages/node-integration-tests/suites/child-process/worker.mjs @@ -1,6 +1,6 @@ -import * as path from 'path'; -import { loggingTransport } from '@sentry-internal/node-integration-tests'; import * as Sentry from '@sentry/node'; +import { loggingTransport } from '@sentry-internal/node-integration-tests'; +import * as path from 'path'; import { Worker } from 'worker_threads'; const __dirname = new URL('.', import.meta.url).pathname; @@ -12,6 +12,7 @@ Sentry.init({ transport: loggingTransport, }); +// eslint-disable-next-line no-unused-vars const _worker = new Worker(path.join(__dirname, 'child.mjs')); setTimeout(() => { diff --git a/dev-packages/node-integration-tests/suites/contextLines/filename-with-spaces/instrument.mjs b/dev-packages/node-integration-tests/suites/contextLines/filename-with-spaces/instrument.mjs index 89dcca029527..9ffde125d498 100644 --- a/dev-packages/node-integration-tests/suites/contextLines/filename-with-spaces/instrument.mjs +++ b/dev-packages/node-integration-tests/suites/contextLines/filename-with-spaces/instrument.mjs @@ -1,5 +1,5 @@ -import { loggingTransport } from '@sentry-internal/node-integration-tests'; import * as Sentry from '@sentry/node'; +import { loggingTransport } from '@sentry-internal/node-integration-tests'; Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', diff --git a/dev-packages/node-integration-tests/suites/contextLines/filename-with-spaces/test.ts b/dev-packages/node-integration-tests/suites/contextLines/filename-with-spaces/test.ts index f469a585b9ca..276e8930b1ea 100644 --- a/dev-packages/node-integration-tests/suites/contextLines/filename-with-spaces/test.ts +++ b/dev-packages/node-integration-tests/suites/contextLines/filename-with-spaces/test.ts @@ -8,7 +8,7 @@ describe('ContextLines integration in ESM', () => { const instrumentPath = join(__dirname, 'instrument.mjs'); await createRunner(__dirname, 'scenario with space.mjs') - .withFlags('--import', instrumentPath) + .withInstrument(instrumentPath) .expect({ event: { exception: { diff --git a/dev-packages/node-integration-tests/suites/esm/import-in-the-middle/app.mjs b/dev-packages/node-integration-tests/suites/esm/import-in-the-middle/app.mjs index fbd43f8540dc..ac7d4b329b6d 100644 --- a/dev-packages/node-integration-tests/suites/esm/import-in-the-middle/app.mjs +++ b/dev-packages/node-integration-tests/suites/esm/import-in-the-middle/app.mjs @@ -1,5 +1,5 @@ -import { loggingTransport } from '@sentry-internal/node-integration-tests'; import * as Sentry from '@sentry/node'; +import { loggingTransport } from '@sentry-internal/node-integration-tests'; import * as iitm from 'import-in-the-middle'; new iitm.Hook((_, name) => { @@ -14,6 +14,8 @@ Sentry.init({ transport: loggingTransport, }); -await import('./sub-module.mjs'); -await import('http'); -await import('os'); +(async () => { + await import('./sub-module.mjs'); + await import('http'); + await import('os'); +})(); diff --git a/dev-packages/node-integration-tests/suites/esm/modules-integration/app.mjs b/dev-packages/node-integration-tests/suites/esm/modules-integration/app.mjs index 5b2300d7037c..a131c93e86fc 100644 --- a/dev-packages/node-integration-tests/suites/esm/modules-integration/app.mjs +++ b/dev-packages/node-integration-tests/suites/esm/modules-integration/app.mjs @@ -1,5 +1,5 @@ -import { loggingTransport } from '@sentry-internal/node-integration-tests'; import * as Sentry from '@sentry/node'; +import { loggingTransport } from '@sentry-internal/node-integration-tests'; Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', diff --git a/dev-packages/node-integration-tests/suites/esm/warn-esm/server.mjs b/dev-packages/node-integration-tests/suites/esm/warn-esm/server.mjs index fa486c9ddac8..7d4f1952eee2 100644 --- a/dev-packages/node-integration-tests/suites/esm/warn-esm/server.mjs +++ b/dev-packages/node-integration-tests/suites/esm/warn-esm/server.mjs @@ -1,5 +1,6 @@ -import { loggingTransport } from '@sentry-internal/node-integration-tests'; import * as Sentry from '@sentry/node'; +import { loggingTransport, startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests'; +import express from 'express'; Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', @@ -7,9 +8,6 @@ Sentry.init({ transport: loggingTransport, }); -import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests'; -import express from 'express'; - const app = express(); app.get('/test/success', (req, res) => { diff --git a/dev-packages/node-integration-tests/suites/public-api/LocalVariables/deny-inspector.mjs b/dev-packages/node-integration-tests/suites/public-api/LocalVariables/deny-inspector.mjs index 16546a328a9a..6be67a6221d7 100644 --- a/dev-packages/node-integration-tests/suites/public-api/LocalVariables/deny-inspector.mjs +++ b/dev-packages/node-integration-tests/suites/public-api/LocalVariables/deny-inspector.mjs @@ -1,5 +1,6 @@ import { register } from 'node:module'; +// eslint-disable-next-line no-unused-vars const hookScript = Buffer.from(` `); @@ -16,6 +17,8 @@ export async function resolve(specifier, context, nextResolve) { import.meta.url, ); -const Sentry = await import('@sentry/node'); +(async () => { + const Sentry = await import('@sentry/node'); -Sentry.init({}); + Sentry.init({}); +})(); diff --git a/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-caught.mjs b/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-caught.mjs index 8fec9dbb1dad..c0a26a6aa709 100644 --- a/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-caught.mjs +++ b/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-caught.mjs @@ -1,6 +1,6 @@ /* eslint-disable no-unused-vars */ -import { loggingTransport } from '@sentry-internal/node-integration-tests'; import * as Sentry from '@sentry/node'; +import { loggingTransport } from '@sentry-internal/node-integration-tests'; Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', diff --git a/dev-packages/node-integration-tests/suites/tracing/amqplib/constants.ts b/dev-packages/node-integration-tests/suites/tracing/amqplib/constants.ts deleted file mode 100644 index b7a3e8f79ea2..000000000000 --- a/dev-packages/node-integration-tests/suites/tracing/amqplib/constants.ts +++ /dev/null @@ -1,15 +0,0 @@ -const amqpUsername = 'sentry'; -const amqpPassword = 'sentry'; - -export const AMQP_URL = `amqp://${amqpUsername}:${amqpPassword}@localhost:5672/`; -export const ACKNOWLEDGEMENT = { noAck: false }; - -export const QUEUE_OPTIONS = { - durable: true, // Make the queue durable - exclusive: false, // Not exclusive - autoDelete: false, // Don't auto-delete the queue - arguments: { - 'x-message-ttl': 30000, // Message TTL of 30 seconds - 'x-max-length': 1000, // Maximum queue length of 1000 messages - }, -}; diff --git a/dev-packages/node-integration-tests/suites/tracing/amqplib/init.ts b/dev-packages/node-integration-tests/suites/tracing/amqplib/instrument.mjs similarity index 100% rename from dev-packages/node-integration-tests/suites/tracing/amqplib/init.ts rename to dev-packages/node-integration-tests/suites/tracing/amqplib/instrument.mjs diff --git a/dev-packages/node-integration-tests/suites/tracing/amqplib/scenario-message.ts b/dev-packages/node-integration-tests/suites/tracing/amqplib/scenario-message.ts deleted file mode 100644 index 16e9cf370fe3..000000000000 --- a/dev-packages/node-integration-tests/suites/tracing/amqplib/scenario-message.ts +++ /dev/null @@ -1,26 +0,0 @@ -import './init'; -import * as Sentry from '@sentry/node'; -import { connectToRabbitMQ, consumeMessageFromQueue, createQueue, sendMessageToQueue } from './utils'; - -const queueName = 'queue1'; - -// Stop the process from exiting before the transaction is sent -// eslint-disable-next-line @typescript-eslint/no-empty-function -setInterval(() => {}, 1000); - -// eslint-disable-next-line @typescript-eslint/no-floating-promises -(async () => { - const { connection, channel } = await connectToRabbitMQ(); - await createQueue(queueName, channel); - - const consumeMessagePromise = consumeMessageFromQueue(queueName, channel); - - await Sentry.startSpan({ name: 'root span' }, async () => { - sendMessageToQueue(queueName, channel, JSON.stringify({ foo: 'bar01' })); - }); - - await consumeMessagePromise; - - await channel.close(); - await connection.close(); -})(); diff --git a/dev-packages/node-integration-tests/suites/tracing/amqplib/scenario.mjs b/dev-packages/node-integration-tests/suites/tracing/amqplib/scenario.mjs new file mode 100644 index 000000000000..3baf2732ddb9 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/amqplib/scenario.mjs @@ -0,0 +1,75 @@ +import * as Sentry from '@sentry/node'; +import amqp from 'amqplib'; + +const queueName = 'queue1'; +const amqpUsername = 'sentry'; +const amqpPassword = 'sentry'; + +const AMQP_URL = `amqp://${amqpUsername}:${amqpPassword}@localhost:5672/`; +const ACKNOWLEDGEMENT = { noAck: false }; + +const QUEUE_OPTIONS = { + durable: true, // Make the queue durable + exclusive: false, // Not exclusive + autoDelete: false, // Don't auto-delete the queue + arguments: { + 'x-message-ttl': 30000, // Message TTL of 30 seconds + 'x-max-length': 1000, // Maximum queue length of 1000 messages + }, +}; + +(async () => { + const { connection, channel } = await connectToRabbitMQ(); + await createQueue(queueName, channel); + + const consumeMessagePromise = consumeMessageFromQueue(queueName, channel); + + await Sentry.startSpan({ name: 'root span' }, async () => { + sendMessageToQueue(queueName, channel, JSON.stringify({ foo: 'bar01' })); + }); + + await consumeMessagePromise; + + await channel.close(); + await connection.close(); + + // Stop the process from exiting before the transaction is sent + setInterval(() => {}, 1000); +})(); + +async function connectToRabbitMQ() { + const connection = await amqp.connect(AMQP_URL); + const channel = await connection.createChannel(); + return { connection, channel }; +} + +async function createQueue(queueName, channel) { + await channel.assertQueue(queueName, QUEUE_OPTIONS); +} + +function sendMessageToQueue(queueName, channel, message) { + channel.sendToQueue(queueName, Buffer.from(message)); +} + +async function consumer(queueName, channel) { + return new Promise((resolve, reject) => { + channel + .consume( + queueName, + message => { + if (message) { + channel.ack(message); + resolve(); + } else { + reject(new Error('No message received')); + } + }, + ACKNOWLEDGEMENT, + ) + .catch(reject); + }); +} + +async function consumeMessageFromQueue(queueName, channel) { + await consumer(queueName, channel); +} diff --git a/dev-packages/node-integration-tests/suites/tracing/amqplib/test.ts b/dev-packages/node-integration-tests/suites/tracing/amqplib/test.ts index 250e1e31c40c..e2c6de69f91c 100644 --- a/dev-packages/node-integration-tests/suites/tracing/amqplib/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/amqplib/test.ts @@ -1,6 +1,6 @@ import type { TransactionEvent } from '@sentry/core'; import { afterAll, describe, expect, test } from 'vitest'; -import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; +import { cleanupChildProcesses, createEsmAndCjsTests } from '../../../utils/runner'; const EXPECTED_MESSAGE_SPAN_PRODUCER = expect.objectContaining({ op: 'message', @@ -29,26 +29,28 @@ describe('amqplib auto-instrumentation', () => { cleanupChildProcesses(); }); - test('should be able to send and receive messages', { timeout: 90_000 }, async () => { - await createRunner(__dirname, 'scenario-message.ts') - .withDockerCompose({ - workingDirectory: [__dirname], - readyMatches: ['Time to start RabbitMQ'], - }) - .expect({ - transaction: (transaction: TransactionEvent) => { - expect(transaction.transaction).toEqual('root span'); - expect(transaction.spans?.length).toEqual(1); - expect(transaction.spans![0]).toMatchObject(EXPECTED_MESSAGE_SPAN_PRODUCER); - }, - }) - .expect({ - transaction: (transaction: TransactionEvent) => { - expect(transaction.transaction).toEqual('queue1 process'); - expect(transaction.contexts?.trace).toMatchObject(EXPECTED_MESSAGE_SPAN_CONSUMER); - }, - }) - .start() - .completed(); + createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', createTestRunner => { + test('should be able to send and receive messages', async () => { + await createTestRunner() + .withDockerCompose({ + workingDirectory: [__dirname], + readyMatches: ['Time to start RabbitMQ'], + }) + .expect({ + transaction: (transaction: TransactionEvent) => { + expect(transaction.transaction).toEqual('root span'); + expect(transaction.spans?.length).toEqual(1); + expect(transaction.spans![0]).toMatchObject(EXPECTED_MESSAGE_SPAN_PRODUCER); + }, + }) + .expect({ + transaction: (transaction: TransactionEvent) => { + expect(transaction.transaction).toEqual('queue1 process'); + expect(transaction.contexts?.trace).toMatchObject(EXPECTED_MESSAGE_SPAN_CONSUMER); + }, + }) + .start() + .completed(); + }); }); }); diff --git a/dev-packages/node-integration-tests/suites/tracing/amqplib/utils.ts b/dev-packages/node-integration-tests/suites/tracing/amqplib/utils.ts deleted file mode 100644 index 702c100aa67c..000000000000 --- a/dev-packages/node-integration-tests/suites/tracing/amqplib/utils.ts +++ /dev/null @@ -1,45 +0,0 @@ -import type { Channel, Connection } from 'amqplib'; -import amqp from 'amqplib'; -import { ACKNOWLEDGEMENT, AMQP_URL, QUEUE_OPTIONS } from './constants'; - -export type RabbitMQData = { - connection: Connection; - channel: Channel; -}; - -export async function connectToRabbitMQ(): Promise { - const connection = await amqp.connect(AMQP_URL); - const channel = await connection.createChannel(); - return { connection, channel }; -} - -export async function createQueue(queueName: string, channel: Channel): Promise { - await channel.assertQueue(queueName, QUEUE_OPTIONS); -} - -export function sendMessageToQueue(queueName: string, channel: Channel, message: string): void { - channel.sendToQueue(queueName, Buffer.from(message)); -} - -async function consumer(queueName: string, channel: Channel): Promise { - return new Promise((resolve, reject) => { - channel - .consume( - queueName, - message => { - if (message) { - channel.ack(message); - resolve(); - } else { - reject(new Error('No message received')); - } - }, - ACKNOWLEDGEMENT, - ) - .catch(reject); - }); -} - -export async function consumeMessageFromQueue(queueName: string, channel: Channel): Promise { - await consumer(queueName, channel); -} diff --git a/dev-packages/node-integration-tests/suites/tracing/connect/instrument.mjs b/dev-packages/node-integration-tests/suites/tracing/connect/instrument.mjs new file mode 100644 index 000000000000..46a27dd03b74 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/connect/instrument.mjs @@ -0,0 +1,9 @@ +import * as Sentry from '@sentry/node'; +import { loggingTransport } from '@sentry-internal/node-integration-tests'; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + tracesSampleRate: 1.0, + transport: loggingTransport, +}); diff --git a/dev-packages/node-integration-tests/suites/tracing/connect/scenario.js b/dev-packages/node-integration-tests/suites/tracing/connect/scenario.mjs similarity index 52% rename from dev-packages/node-integration-tests/suites/tracing/connect/scenario.js rename to dev-packages/node-integration-tests/suites/tracing/connect/scenario.mjs index db95fad457b2..39054ba009c5 100644 --- a/dev-packages/node-integration-tests/suites/tracing/connect/scenario.js +++ b/dev-packages/node-integration-tests/suites/tracing/connect/scenario.mjs @@ -1,18 +1,10 @@ -const { loggingTransport, sendPortToRunner } = require('@sentry-internal/node-integration-tests'); -const Sentry = require('@sentry/node'); +import * as Sentry from '@sentry/node'; +import { sendPortToRunner } from '@sentry-internal/node-integration-tests'; +import connect from 'connect'; +import http from 'http'; const port = 5986; -Sentry.init({ - dsn: 'https://public@dsn.ingest.sentry.io/1337', - release: '1.0', - tracesSampleRate: 1.0, - transport: loggingTransport, -}); - -const connect = require('connect'); -const http = require('http'); - const run = async () => { const app = connect(); diff --git a/dev-packages/node-integration-tests/suites/tracing/connect/test.ts b/dev-packages/node-integration-tests/suites/tracing/connect/test.ts index 8b03de6e6a37..0449003e02c6 100644 --- a/dev-packages/node-integration-tests/suites/tracing/connect/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/connect/test.ts @@ -1,5 +1,5 @@ import { afterAll, describe, expect, test } from 'vitest'; -import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; +import { cleanupChildProcesses, createEsmAndCjsTests } from '../../../utils/runner'; describe('connect auto-instrumentation', () => { afterAll(async () => { @@ -36,27 +36,32 @@ describe('connect auto-instrumentation', () => { }, }; - test('CJS - should auto-instrument `connect` package.', async () => { - const runner = createRunner(__dirname, 'scenario.js').expect({ transaction: EXPECTED_TRANSACTION }).start(); - runner.makeRequest('get', '/'); - await runner.completed(); - }); + createEsmAndCjsTests( + __dirname, + 'scenario.mjs', + 'instrument.mjs', + createTestRunner => { + test('should auto-instrument `connect` package.', async () => { + const runner = createTestRunner().expect({ transaction: EXPECTED_TRANSACTION }).start(); + runner.makeRequest('get', '/'); + await runner.completed(); + }); - test('CJS - should capture errors in `connect` middleware.', async () => { - const runner = createRunner(__dirname, 'scenario.js') - .ignore('transaction') - .expect({ event: EXPECTED_EVENT }) - .start(); - runner.makeRequest('get', '/error'); - await runner.completed(); - }); + test('should capture errors in `connect` middleware.', async () => { + const runner = createTestRunner().ignore('transaction').expect({ event: EXPECTED_EVENT }).start(); + runner.makeRequest('get', '/error'); + await runner.completed(); + }); - test('CJS - should report errored transactions.', async () => { - const runner = createRunner(__dirname, 'scenario.js') - .ignore('event') - .expect({ transaction: { transaction: 'GET /error' } }) - .start(); - runner.makeRequest('get', '/error'); - await runner.completed(); - }); + test('should report errored transactions.', async () => { + const runner = createTestRunner() + .ignore('event') + .expect({ transaction: { transaction: 'GET /error' } }) + .start(); + runner.makeRequest('get', '/error'); + await runner.completed(); + }); + }, + { skipEsm: true }, + ); }); diff --git a/dev-packages/node-integration-tests/suites/tracing/dataloader/instrument.mjs b/dev-packages/node-integration-tests/suites/tracing/dataloader/instrument.mjs new file mode 100644 index 000000000000..38687180a665 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/dataloader/instrument.mjs @@ -0,0 +1,10 @@ +import * as Sentry from '@sentry/node'; +import { loggingTransport } from '@sentry-internal/node-integration-tests'; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + tracesSampleRate: 1.0, + transport: loggingTransport, + integrations: [Sentry.dataloaderIntegration()], +}); diff --git a/dev-packages/node-integration-tests/suites/tracing/dataloader/scenario.js b/dev-packages/node-integration-tests/suites/tracing/dataloader/scenario.js deleted file mode 100644 index 7bea9a1372ff..000000000000 --- a/dev-packages/node-integration-tests/suites/tracing/dataloader/scenario.js +++ /dev/null @@ -1,34 +0,0 @@ -const { loggingTransport, startExpressServerAndSendPortToRunner } = require('@sentry-internal/node-integration-tests'); -const Sentry = require('@sentry/node'); - -Sentry.init({ - dsn: 'https://public@dsn.ingest.sentry.io/1337', - release: '1.0', - tracesSampleRate: 1.0, - transport: loggingTransport, - integrations: [Sentry.dataloaderIntegration()], -}); - -const PORT = 8008; - -// Stop the process from exiting before the transaction is sent -setInterval(() => {}, 1000); - -const run = async () => { - const express = require('express'); - const Dataloader = require('dataloader'); - - const app = express(); - const dataloader = new Dataloader(async keys => keys.map((_, idx) => idx), { - cache: false, - }); - - app.get('/', (req, res) => { - const user = dataloader.load('user-1'); - res.send(user); - }); - - startExpressServerAndSendPortToRunner(app, PORT); -}; - -run(); diff --git a/dev-packages/node-integration-tests/suites/tracing/dataloader/scenario.mjs b/dev-packages/node-integration-tests/suites/tracing/dataloader/scenario.mjs new file mode 100644 index 000000000000..c392ca9b4972 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/dataloader/scenario.mjs @@ -0,0 +1,24 @@ +import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests'; +import Dataloader from 'dataloader'; +import express from 'express'; + +const PORT = 8008; + +// Stop the process from exiting before the transaction is sent +setInterval(() => {}, 1000); + +const run = async () => { + const app = express(); + const dataloader = new Dataloader(async keys => keys.map((_, idx) => idx), { + cache: false, + }); + + app.get('/', (req, res) => { + const user = dataloader.load('user-1'); + res.send(user); + }); + + startExpressServerAndSendPortToRunner(app, PORT); +}; + +run(); diff --git a/dev-packages/node-integration-tests/suites/tracing/dataloader/test.ts b/dev-packages/node-integration-tests/suites/tracing/dataloader/test.ts index bd495ea24a7a..599a0f8d53e3 100644 --- a/dev-packages/node-integration-tests/suites/tracing/dataloader/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/dataloader/test.ts @@ -1,5 +1,5 @@ import { afterAll, describe, expect, test } from 'vitest'; -import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; +import { cleanupChildProcesses, createEsmAndCjsTests } from '../../../utils/runner'; describe('dataloader auto-instrumentation', () => { afterAll(async () => { @@ -32,9 +32,17 @@ describe('dataloader auto-instrumentation', () => { ]), }; - test('should auto-instrument `dataloader` package.', async () => { - const runner = createRunner(__dirname, 'scenario.js').expect({ transaction: EXPECTED_TRANSACTION }).start(); - runner.makeRequest('get', '/'); - await runner.completed(); - }); + createEsmAndCjsTests( + __dirname, + 'scenario.mjs', + 'instrument.mjs', + createRunner => { + test('should auto-instrument `dataloader` package.', async () => { + const runner = createRunner().expect({ transaction: EXPECTED_TRANSACTION }).start(); + runner.makeRequest('get', '/'); + await runner.completed(); + }); + }, + { skipEsm: true }, + ); }); diff --git a/dev-packages/node-integration-tests/suites/tracing/hapi/instrument.mjs b/dev-packages/node-integration-tests/suites/tracing/hapi/instrument.mjs new file mode 100644 index 000000000000..46a27dd03b74 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/hapi/instrument.mjs @@ -0,0 +1,9 @@ +import * as Sentry from '@sentry/node'; +import { loggingTransport } from '@sentry-internal/node-integration-tests'; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + tracesSampleRate: 1.0, + transport: loggingTransport, +}); diff --git a/dev-packages/node-integration-tests/suites/tracing/hapi/scenario.js b/dev-packages/node-integration-tests/suites/tracing/hapi/scenario.mjs similarity index 60% rename from dev-packages/node-integration-tests/suites/tracing/hapi/scenario.js rename to dev-packages/node-integration-tests/suites/tracing/hapi/scenario.mjs index f3171eb085e0..c1497d2c5e2f 100644 --- a/dev-packages/node-integration-tests/suites/tracing/hapi/scenario.js +++ b/dev-packages/node-integration-tests/suites/tracing/hapi/scenario.mjs @@ -1,15 +1,7 @@ -const { loggingTransport, sendPortToRunner } = require('@sentry-internal/node-integration-tests'); -const Sentry = require('@sentry/node'); - -Sentry.init({ - dsn: 'https://public@dsn.ingest.sentry.io/1337', - release: '1.0', - tracesSampleRate: 1.0, - transport: loggingTransport, -}); - -const Hapi = require('@hapi/hapi'); -const Boom = require('@hapi/boom'); +import Boom from '@hapi/boom'; +import Hapi from '@hapi/hapi'; +import * as Sentry from '@sentry/node'; +import { sendPortToRunner } from '@sentry-internal/node-integration-tests'; const port = 5999; @@ -22,7 +14,7 @@ const run = async () => { server.route({ method: 'GET', path: '/', - handler: (_request, _h) => { + handler: () => { return 'Hello World!'; }, }); @@ -30,7 +22,7 @@ const run = async () => { server.route({ method: 'GET', path: '/error', - handler: (_request, _h) => { + handler: () => { return new Error('Sentry Test Error'); }, }); @@ -38,7 +30,7 @@ const run = async () => { server.route({ method: 'GET', path: '/error/{id}', - handler: (_request, _h) => { + handler: () => { return new Error('Sentry Test Error'); }, }); @@ -46,7 +38,7 @@ const run = async () => { server.route({ method: 'GET', path: '/boom-error', - handler: (_request, _h) => { + handler: () => { return new Boom.Boom('Sentry Test Error'); }, }); @@ -54,7 +46,7 @@ const run = async () => { server.route({ method: 'GET', path: '/promise-error', - handler: async (_request, _h) => { + handler: async () => { return Promise.reject(new Error('Sentry Test Error')); }, }); diff --git a/dev-packages/node-integration-tests/suites/tracing/hapi/test.ts b/dev-packages/node-integration-tests/suites/tracing/hapi/test.ts index e783c77e2fc5..1a4cbfac933c 100644 --- a/dev-packages/node-integration-tests/suites/tracing/hapi/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/hapi/test.ts @@ -1,5 +1,5 @@ import { afterAll, describe, expect, test } from 'vitest'; -import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; +import { cleanupChildProcesses, createEsmAndCjsTests } from '../../../utils/runner'; describe('hapi auto-instrumentation', () => { afterAll(async () => { @@ -36,62 +36,64 @@ describe('hapi auto-instrumentation', () => { }, }; - test('CJS - should auto-instrument `@hapi/hapi` package.', async () => { - const runner = createRunner(__dirname, 'scenario.js').expect({ transaction: EXPECTED_TRANSACTION }).start(); - runner.makeRequest('get', '/'); - await runner.completed(); - }); + createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', createRunner => { + test('should auto-instrument `@hapi/hapi` package.', async () => { + const runner = createRunner().expect({ transaction: EXPECTED_TRANSACTION }).start(); + runner.makeRequest('get', '/'); + await runner.completed(); + }); - test('CJS - should handle returned plain errors in routes.', async () => { - const runner = createRunner(__dirname, 'scenario.js') - .expect({ - transaction: { - transaction: 'GET /error', - }, - }) - .expect({ event: EXPECTED_ERROR_EVENT }) - .start(); - runner.makeRequest('get', '/error', { expectError: true }); - await runner.completed(); - }); + test('should handle returned plain errors in routes.', async () => { + const runner = createRunner() + .expect({ + transaction: { + transaction: 'GET /error', + }, + }) + .expect({ event: EXPECTED_ERROR_EVENT }) + .start(); + runner.makeRequest('get', '/error', { expectError: true }); + await runner.completed(); + }); - test('CJS - should assign parameterized transactionName to error.', async () => { - const runner = createRunner(__dirname, 'scenario.js') - .expect({ - event: { - ...EXPECTED_ERROR_EVENT, - transaction: 'GET /error/{id}', - }, - }) - .ignore('transaction') - .start(); - runner.makeRequest('get', '/error/123', { expectError: true }); - await runner.completed(); - }); + test('should assign parameterized transactionName to error.', async () => { + const runner = createRunner() + .expect({ + event: { + ...EXPECTED_ERROR_EVENT, + transaction: 'GET /error/{id}', + }, + }) + .ignore('transaction') + .start(); + runner.makeRequest('get', '/error/123', { expectError: true }); + await runner.completed(); + }); - test('CJS - should handle returned Boom errors in routes.', async () => { - const runner = createRunner(__dirname, 'scenario.js') - .expect({ - transaction: { - transaction: 'GET /boom-error', - }, - }) - .expect({ event: EXPECTED_ERROR_EVENT }) - .start(); - runner.makeRequest('get', '/boom-error', { expectError: true }); - await runner.completed(); - }); + test('should handle returned Boom errors in routes.', async () => { + const runner = createRunner() + .expect({ + transaction: { + transaction: 'GET /boom-error', + }, + }) + .expect({ event: EXPECTED_ERROR_EVENT }) + .start(); + runner.makeRequest('get', '/boom-error', { expectError: true }); + await runner.completed(); + }); - test('CJS - should handle promise rejections in routes.', async () => { - const runner = createRunner(__dirname, 'scenario.js') - .expect({ - transaction: { - transaction: 'GET /promise-error', - }, - }) - .expect({ event: EXPECTED_ERROR_EVENT }) - .start(); - runner.makeRequest('get', '/promise-error', { expectError: true }); - await runner.completed(); + test('should handle promise rejections in routes.', async () => { + const runner = createRunner() + .expect({ + transaction: { + transaction: 'GET /promise-error', + }, + }) + .expect({ event: EXPECTED_ERROR_EVENT }) + .start(); + runner.makeRequest('get', '/promise-error', { expectError: true }); + await runner.completed(); + }); }); }); diff --git a/dev-packages/node-integration-tests/suites/tracing/requests/http-sampled-esm/instrument.mjs b/dev-packages/node-integration-tests/suites/tracing/requests/http-sampled-esm/instrument.mjs index d843ca07fce8..518e3f83de83 100644 --- a/dev-packages/node-integration-tests/suites/tracing/requests/http-sampled-esm/instrument.mjs +++ b/dev-packages/node-integration-tests/suites/tracing/requests/http-sampled-esm/instrument.mjs @@ -1,5 +1,5 @@ -import { loggingTransport } from '@sentry-internal/node-integration-tests'; import * as Sentry from '@sentry/node'; +import { loggingTransport } from '@sentry-internal/node-integration-tests'; Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', diff --git a/dev-packages/node-integration-tests/suites/tracing/requests/http-sampled-esm/scenario.mjs b/dev-packages/node-integration-tests/suites/tracing/requests/http-sampled-esm/scenario.mjs index 07e234983b4d..9fafd4b528af 100644 --- a/dev-packages/node-integration-tests/suites/tracing/requests/http-sampled-esm/scenario.mjs +++ b/dev-packages/node-integration-tests/suites/tracing/requests/http-sampled-esm/scenario.mjs @@ -1,5 +1,5 @@ -import * as http from 'http'; import * as Sentry from '@sentry/node'; +import * as http from 'http'; // eslint-disable-next-line @typescript-eslint/no-floating-promises Sentry.startSpan({ name: 'test_span' }, async () => { diff --git a/dev-packages/node-integration-tests/suites/tracing/requests/http-sampled-esm/test.ts b/dev-packages/node-integration-tests/suites/tracing/requests/http-sampled-esm/test.ts index 05c8913a9d2d..8bd7dd5f2502 100644 --- a/dev-packages/node-integration-tests/suites/tracing/requests/http-sampled-esm/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/requests/http-sampled-esm/test.ts @@ -30,7 +30,7 @@ describe('outgoing http in ESM', () => { const instrumentPath = join(__dirname, 'instrument.mjs'); await createRunner(__dirname, 'scenario.mjs') - .withFlags('--import', instrumentPath) + .withInstrument(instrumentPath) .withEnv({ SERVER_URL }) .expect({ transaction: { diff --git a/dev-packages/node-integration-tests/utils/runner.ts b/dev-packages/node-integration-tests/utils/runner.ts index 79e43d056ce6..3cdc203d43dc 100644 --- a/dev-packages/node-integration-tests/utils/runner.ts +++ b/dev-packages/node-integration-tests/utils/runner.ts @@ -14,8 +14,9 @@ import type { import { normalize } from '@sentry/core'; import axios from 'axios'; import { execSync, spawn, spawnSync } from 'child_process'; -import { existsSync } from 'fs'; +import { existsSync, readFileSync, unlinkSync, writeFileSync } from 'fs'; import { join } from 'path'; +import { afterAll, describe } from 'vitest'; import { assertEnvelopeHeader, assertSentryCheckIn, @@ -164,6 +165,67 @@ type StartResult = { ): Promise; }; +export function createEsmAndCjsTests( + cwd: string, + scenarioPath: string, + instrumentPath: string, + callback: (createTestRunner: () => ReturnType, mode: 'esm' | 'cjs') => void, + options?: { skipCjs?: boolean; skipEsm?: boolean }, +): void { + const mjsScenarioPath = join(cwd, scenarioPath); + const mjsInstrumentPath = join(cwd, instrumentPath); + + if (!mjsScenarioPath.endsWith('.mjs')) { + throw new Error(`Scenario path must end with .mjs: ${scenarioPath}`); + } + + if (!existsSync(mjsInstrumentPath)) { + throw new Error(`Instrument file not found: ${mjsInstrumentPath}`); + } + + const cjsScenarioPath = join(cwd, `tmp_${scenarioPath.replace('.mjs', '.cjs')}`); + const cjsInstrumentPath = join(cwd, `tmp_${instrumentPath.replace('.mjs', '.cjs')}`); + + // For the CJS runner, we create some temporary files... + if (!options?.skipCjs) { + convertEsmFileToCjs(mjsScenarioPath, cjsScenarioPath); + convertEsmFileToCjs(mjsInstrumentPath, cjsInstrumentPath); + } + + describe('esm & cjs', () => { + afterAll(() => { + try { + unlinkSync(cjsInstrumentPath); + } catch { + // Ignore errors here + } + try { + unlinkSync(cjsScenarioPath); + } catch { + // Ignore errors here + } + }); + + const scenarios: [mode: 'esm' | 'cjs', getRunner: () => ReturnType][] = []; + if (!options?.skipEsm) { + scenarios.push(['esm', () => createRunner(mjsScenarioPath).withFlags('--import', mjsInstrumentPath)]); + } + if (!options?.skipCjs) { + scenarios.push(['cjs', () => createRunner(cjsScenarioPath).withFlags('--require', cjsInstrumentPath)]); + } + + describe.each(scenarios)('%s', (mode, getRunner) => { + callback(getRunner, mode); + }); + }); +} + +function convertEsmFileToCjs(inputPath: string, outputPath: string): void { + const cjsFileContent = readFileSync(inputPath, 'utf8'); + const cjsFileContentConverted = convertEsmToCjs(cjsFileContent); + writeFileSync(outputPath, cjsFileContentConverted); +} + /** Creates a test runner */ // eslint-disable-next-line @typescript-eslint/explicit-function-return-type export function createRunner(...paths: string[]) { @@ -183,6 +245,7 @@ export function createRunner(...paths: string[]) { let dockerOptions: DockerOptions | undefined; let ensureNoErrorOutput = false; const logs: string[] = []; + const cleanups: VoidFunction[] = []; if (testPath.endsWith('.ts')) { flags.push('-r', 'ts-node/register'); @@ -215,6 +278,14 @@ export function createRunner(...paths: string[]) { flags.push(...args); return this; }, + withInstrument: function (instrumentPath: string) { + flags.push('--import', instrumentPath); + return this; + }, + withCleanup: function (cleanup: VoidFunction) { + cleanups.push(cleanup); + return this; + }, withMockSentryServer: function () { withSentryServer = true; return this; @@ -252,6 +323,8 @@ export function createRunner(...paths: string[]) { let child: ReturnType | undefined; function complete(error?: Error): void { + cleanups.forEach(cleanup => cleanup()); + child?.kill(); done?.(normalize(error)); if (error) { @@ -565,3 +638,36 @@ function expectLog(item: SerializedLogContainer, expected: ExpectedLogContainer) assertSentryLogContainer(item, expected); } } + +/** + * Converts ESM import statements to CommonJS require statements + * @param content The content of an ESM file + * @returns The content with require statements instead of imports + */ +function convertEsmToCjs(content: string): string { + let newContent = content; + + // Handle default imports: import x from 'y' -> const x = require('y') + newContent = newContent.replace( + /import\s+([\w*{}\s,]+)\s+from\s+['"]([^'"]+)['"]/g, + (_, imports: string, module: string) => { + if (imports.includes('* as')) { + // Handle namespace imports: import * as x from 'y' -> const x = require('y') + return `const ${imports.replace('* as', '').trim()} = require('${module}')`; + } else if (imports.includes('{')) { + // Handle named imports: import {x, y} from 'z' -> const {x, y} = require('z') + return `const ${imports} = require('${module}')`; + } else { + // Handle default imports: import x from 'y' -> const x = require('y') + return `const ${imports} = require('${module}')`; + } + }, + ); + + // Handle side-effect imports: import 'x' -> require('x') + newContent = newContent.replace(/import\s+['"]([^'"]+)['"]/g, (_, module) => { + return `require('${module}')`; + }); + + return newContent; +} From 88eee44e6027e79324c4e57c0b3657ff75a1ec60 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 29 Apr 2025 14:44:51 +0200 Subject: [PATCH 02/15] add more tests --- .../suites/express/with-http/instrument.mjs | 9 +++ .../suites/express/with-http/scenario.mjs | 28 +++++++++ .../suites/express/with-http/test.ts | 41 +++++++++++++ .../suites/tracing/ai/scenario.js | 58 ------------------- .../suites/tracing/ai/test.ts | 2 +- 5 files changed, 79 insertions(+), 59 deletions(-) create mode 100644 dev-packages/node-integration-tests/suites/express/with-http/instrument.mjs create mode 100644 dev-packages/node-integration-tests/suites/express/with-http/scenario.mjs create mode 100644 dev-packages/node-integration-tests/suites/express/with-http/test.ts delete mode 100644 dev-packages/node-integration-tests/suites/tracing/ai/scenario.js diff --git a/dev-packages/node-integration-tests/suites/express/with-http/instrument.mjs b/dev-packages/node-integration-tests/suites/express/with-http/instrument.mjs new file mode 100644 index 000000000000..46a27dd03b74 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express/with-http/instrument.mjs @@ -0,0 +1,9 @@ +import * as Sentry from '@sentry/node'; +import { loggingTransport } from '@sentry-internal/node-integration-tests'; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + tracesSampleRate: 1.0, + transport: loggingTransport, +}); diff --git a/dev-packages/node-integration-tests/suites/express/with-http/scenario.mjs b/dev-packages/node-integration-tests/suites/express/with-http/scenario.mjs new file mode 100644 index 000000000000..4ecdd158f785 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express/with-http/scenario.mjs @@ -0,0 +1,28 @@ +import * as Sentry from '@sentry/node'; +import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests'; +import express from 'express'; +import http from 'http'; + +const app = express(); + +app.get('/test', (_req, res) => { + http.get(`http://localhost:${app.port}/test2`, httpRes => { + httpRes.on('data', () => { + setTimeout(() => { + res.send({ response: 'response 1' }); + }, 200); + }); + }); +}); + +app.get('/test2', (_req, res) => { + res.send({ response: 'response 2' }); +}); + +app.get('/test3', (_req, res) => { + res.send({ response: 'response 3' }); +}); + +Sentry.setupExpressErrorHandler(app); + +startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/express/with-http/test.ts b/dev-packages/node-integration-tests/suites/express/with-http/test.ts new file mode 100644 index 000000000000..0687dad8546e --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express/with-http/test.ts @@ -0,0 +1,41 @@ +import { afterAll, describe, test } from 'vitest'; +import { cleanupChildProcesses, createEsmAndCjsTests } from '../../../utils/runner'; + +describe('express with http import xxx', () => { + afterAll(() => { + cleanupChildProcesses(); + }); + + createEsmAndCjsTests( + __dirname, + 'scenario.mjs', + 'instrument.mjs', + createRunner => { + test('it works when importing the http module', async () => { + const runner = createRunner() + .expect({ + transaction: { + transaction: 'GET /test2', + }, + }) + .expect({ + transaction: { + transaction: 'GET /test', + }, + }) + .expect({ + transaction: { + transaction: 'GET /test3', + }, + }) + .start(); + await runner.makeRequest('get', '/test'); + await runner.makeRequest('get', '/test3'); + await runner.completed(); + }); + // TODO: This is failing on ESM because importing http is triggering the http spans twice :( + // We need to fix this! + }, + { skipEsm: true }, + ); +}); diff --git a/dev-packages/node-integration-tests/suites/tracing/ai/scenario.js b/dev-packages/node-integration-tests/suites/tracing/ai/scenario.js deleted file mode 100644 index 780e322c0639..000000000000 --- a/dev-packages/node-integration-tests/suites/tracing/ai/scenario.js +++ /dev/null @@ -1,58 +0,0 @@ -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', - tracesSampleRate: 1.0, - transport: loggingTransport, -}); - -const { generateText } = require('ai'); -const { MockLanguageModelV1 } = require('ai/test'); - -async function run() { - await Sentry.startSpan({ op: 'function', name: 'main' }, async () => { - await generateText({ - model: new MockLanguageModelV1({ - doGenerate: async () => ({ - rawCall: { rawPrompt: null, rawSettings: {} }, - finishReason: 'stop', - usage: { promptTokens: 10, completionTokens: 20 }, - text: 'First span here!', - }), - }), - prompt: 'Where is the first span?', - }); - - // This span should have input and output prompts attached because telemetry is explicitly enabled. - await generateText({ - experimental_telemetry: { isEnabled: true }, - model: new MockLanguageModelV1({ - doGenerate: async () => ({ - rawCall: { rawPrompt: null, rawSettings: {} }, - finishReason: 'stop', - usage: { promptTokens: 10, completionTokens: 20 }, - text: 'Second span here!', - }), - }), - prompt: 'Where is the second span?', - }); - - // This span should not be captured because we've disabled telemetry - await generateText({ - experimental_telemetry: { isEnabled: false }, - model: new MockLanguageModelV1({ - doGenerate: async () => ({ - rawCall: { rawPrompt: null, rawSettings: {} }, - finishReason: 'stop', - usage: { promptTokens: 10, completionTokens: 20 }, - text: 'Third span here!', - }), - }), - prompt: 'Where is the third span?', - }); - }); -} - -run(); diff --git a/dev-packages/node-integration-tests/suites/tracing/ai/test.ts b/dev-packages/node-integration-tests/suites/tracing/ai/test.ts index bb380febab78..4feeb0534330 100644 --- a/dev-packages/node-integration-tests/suites/tracing/ai/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/ai/test.ts @@ -1,6 +1,6 @@ import { join } from 'node:path'; import { afterAll, describe, expect, test } from 'vitest'; -import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; +import { cleanupChildProcesses, createEsmAndCjsTests } from '../../../utils/runner'; // `ai` SDK only support Node 18+ describe('ai', () => { From fb38aed3fc6d118cf93423e69cd27487ee83fedb Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 30 Apr 2025 09:03:06 +0200 Subject: [PATCH 03/15] WIP --- .../suites/express/with-http/test.ts | 6 +- .../suites/tracing/amqplib/test.ts | 4 +- .../suites/tracing/connect/test.ts | 4 +- .../suites/tracing/dataloader/test.ts | 4 +- .../suites/tracing/hapi/test.ts | 4 +- .../node-integration-tests/utils/runner.ts | 60 +++++++++---------- 6 files changed, 39 insertions(+), 43 deletions(-) diff --git a/dev-packages/node-integration-tests/suites/express/with-http/test.ts b/dev-packages/node-integration-tests/suites/express/with-http/test.ts index 0687dad8546e..7583cd04bf50 100644 --- a/dev-packages/node-integration-tests/suites/express/with-http/test.ts +++ b/dev-packages/node-integration-tests/suites/express/with-http/test.ts @@ -1,7 +1,7 @@ -import { afterAll, describe, test } from 'vitest'; +import { afterAll, describe } from 'vitest'; import { cleanupChildProcesses, createEsmAndCjsTests } from '../../../utils/runner'; -describe('express with http import xxx', () => { +describe('express with http import', () => { afterAll(() => { cleanupChildProcesses(); }); @@ -10,7 +10,7 @@ describe('express with http import xxx', () => { __dirname, 'scenario.mjs', 'instrument.mjs', - createRunner => { + (createRunner, test) => { test('it works when importing the http module', async () => { const runner = createRunner() .expect({ diff --git a/dev-packages/node-integration-tests/suites/tracing/amqplib/test.ts b/dev-packages/node-integration-tests/suites/tracing/amqplib/test.ts index e2c6de69f91c..ad84a74b929a 100644 --- a/dev-packages/node-integration-tests/suites/tracing/amqplib/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/amqplib/test.ts @@ -1,5 +1,5 @@ import type { TransactionEvent } from '@sentry/core'; -import { afterAll, describe, expect, test } from 'vitest'; +import { afterAll, describe, expect } from 'vitest'; import { cleanupChildProcesses, createEsmAndCjsTests } from '../../../utils/runner'; const EXPECTED_MESSAGE_SPAN_PRODUCER = expect.objectContaining({ @@ -29,7 +29,7 @@ describe('amqplib auto-instrumentation', () => { cleanupChildProcesses(); }); - createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', createTestRunner => { + createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createTestRunner, test) => { test('should be able to send and receive messages', async () => { await createTestRunner() .withDockerCompose({ diff --git a/dev-packages/node-integration-tests/suites/tracing/connect/test.ts b/dev-packages/node-integration-tests/suites/tracing/connect/test.ts index 0449003e02c6..12f3541daf8b 100644 --- a/dev-packages/node-integration-tests/suites/tracing/connect/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/connect/test.ts @@ -1,4 +1,4 @@ -import { afterAll, describe, expect, test } from 'vitest'; +import { afterAll, describe, expect } from 'vitest'; import { cleanupChildProcesses, createEsmAndCjsTests } from '../../../utils/runner'; describe('connect auto-instrumentation', () => { @@ -40,7 +40,7 @@ describe('connect auto-instrumentation', () => { __dirname, 'scenario.mjs', 'instrument.mjs', - createTestRunner => { + (createTestRunner, test) => { test('should auto-instrument `connect` package.', async () => { const runner = createTestRunner().expect({ transaction: EXPECTED_TRANSACTION }).start(); runner.makeRequest('get', '/'); diff --git a/dev-packages/node-integration-tests/suites/tracing/dataloader/test.ts b/dev-packages/node-integration-tests/suites/tracing/dataloader/test.ts index 599a0f8d53e3..c0e04bdb00aa 100644 --- a/dev-packages/node-integration-tests/suites/tracing/dataloader/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/dataloader/test.ts @@ -1,4 +1,4 @@ -import { afterAll, describe, expect, test } from 'vitest'; +import { afterAll, describe, expect } from 'vitest'; import { cleanupChildProcesses, createEsmAndCjsTests } from '../../../utils/runner'; describe('dataloader auto-instrumentation', () => { @@ -36,7 +36,7 @@ describe('dataloader auto-instrumentation', () => { __dirname, 'scenario.mjs', 'instrument.mjs', - createRunner => { + (createRunner, test) => { test('should auto-instrument `dataloader` package.', async () => { const runner = createRunner().expect({ transaction: EXPECTED_TRANSACTION }).start(); runner.makeRequest('get', '/'); diff --git a/dev-packages/node-integration-tests/suites/tracing/hapi/test.ts b/dev-packages/node-integration-tests/suites/tracing/hapi/test.ts index 1a4cbfac933c..5693f55a6a8c 100644 --- a/dev-packages/node-integration-tests/suites/tracing/hapi/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/hapi/test.ts @@ -1,4 +1,4 @@ -import { afterAll, describe, expect, test } from 'vitest'; +import { afterAll, describe, expect } from 'vitest'; import { cleanupChildProcesses, createEsmAndCjsTests } from '../../../utils/runner'; describe('hapi auto-instrumentation', () => { @@ -36,7 +36,7 @@ describe('hapi auto-instrumentation', () => { }, }; - createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', createRunner => { + createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => { test('should auto-instrument `@hapi/hapi` package.', async () => { const runner = createRunner().expect({ transaction: EXPECTED_TRANSACTION }).start(); runner.makeRequest('get', '/'); diff --git a/dev-packages/node-integration-tests/utils/runner.ts b/dev-packages/node-integration-tests/utils/runner.ts index 3cdc203d43dc..cd943c9de0fb 100644 --- a/dev-packages/node-integration-tests/utils/runner.ts +++ b/dev-packages/node-integration-tests/utils/runner.ts @@ -16,7 +16,7 @@ import axios from 'axios'; import { execSync, spawn, spawnSync } from 'child_process'; import { existsSync, readFileSync, unlinkSync, writeFileSync } from 'fs'; import { join } from 'path'; -import { afterAll, describe } from 'vitest'; +import { afterAll, describe, test } from 'vitest'; import { assertEnvelopeHeader, assertSentryCheckIn, @@ -169,7 +169,11 @@ export function createEsmAndCjsTests( cwd: string, scenarioPath: string, instrumentPath: string, - callback: (createTestRunner: () => ReturnType, mode: 'esm' | 'cjs') => void, + callback: ( + createTestRunner: () => ReturnType, + testFn: typeof test | typeof test.fails, + mode: 'esm' | 'cjs', + ) => void, options?: { skipCjs?: boolean; skipEsm?: boolean }, ): void { const mjsScenarioPath = join(cwd, scenarioPath); @@ -206,16 +210,14 @@ export function createEsmAndCjsTests( } }); - const scenarios: [mode: 'esm' | 'cjs', getRunner: () => ReturnType][] = []; - if (!options?.skipEsm) { - scenarios.push(['esm', () => createRunner(mjsScenarioPath).withFlags('--import', mjsInstrumentPath)]); - } - if (!options?.skipCjs) { - scenarios.push(['cjs', () => createRunner(cjsScenarioPath).withFlags('--require', cjsInstrumentPath)]); - } + describe('esm', () => { + const testFn = options?.skipEsm ? test.fails : test; + callback(() => createRunner(mjsScenarioPath).withFlags('--import', mjsInstrumentPath), testFn, 'esm'); + }); - describe.each(scenarios)('%s', (mode, getRunner) => { - callback(getRunner, mode); + describe('cjs', () => { + const testFn = options?.skipCjs ? test.fails : test; + callback(() => createRunner(cjsScenarioPath).withFlags('--require', cjsInstrumentPath), testFn, 'cjs'); }); }); } @@ -245,7 +247,6 @@ export function createRunner(...paths: string[]) { let dockerOptions: DockerOptions | undefined; let ensureNoErrorOutput = false; const logs: string[] = []; - const cleanups: VoidFunction[] = []; if (testPath.endsWith('.ts')) { flags.push('-r', 'ts-node/register'); @@ -282,10 +283,6 @@ export function createRunner(...paths: string[]) { flags.push('--import', instrumentPath); return this; }, - withCleanup: function (cleanup: VoidFunction) { - cleanups.push(cleanup); - return this; - }, withMockSentryServer: function () { withSentryServer = true; return this; @@ -308,13 +305,10 @@ export function createRunner(...paths: string[]) { ensureNoErrorOutput = true; return this; }, - start: function (done?: (e?: unknown) => void): StartResult { - let resolve: (value: void) => void; - let reject: (reason?: unknown) => void; - const completePromise = new Promise((res, rej) => { - resolve = res; - reject = rej; - }); + start: function (): StartResult { + let isComplete = false; + let completeError: Error | undefined; + const expectedEnvelopeCount = Math.max(expectedEnvelopes.length, (expectedEnvelopeHeaders || []).length); let envelopeCount = 0; @@ -323,15 +317,13 @@ export function createRunner(...paths: string[]) { let child: ReturnType | undefined; function complete(error?: Error): void { - cleanups.forEach(cleanup => cleanup()); + if (isComplete) { + return; + } + isComplete = true; + completeError = error || undefined; child?.kill(); - done?.(normalize(error)); - if (error) { - reject(error); - } else { - resolve(); - } } /** Called after each expect callback to check if we're complete */ @@ -526,8 +518,12 @@ export function createRunner(...paths: string[]) { .catch(e => complete(e)); return { - completed: function (): Promise { - return completePromise; + completed: async function (): Promise { + await waitFor(() => isComplete); + + if (completeError) { + throw completeError; + } }, childHasExited: function (): boolean { return hasExited; From 6d1a73c43ff38e5c29327f804d0d632217c155e3 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 30 Apr 2025 09:43:35 +0200 Subject: [PATCH 04/15] update ai test --- .../suites/tracing/ai/test.ts | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/dev-packages/node-integration-tests/suites/tracing/ai/test.ts b/dev-packages/node-integration-tests/suites/tracing/ai/test.ts index 4feeb0534330..c0a3ccb4a78a 100644 --- a/dev-packages/node-integration-tests/suites/tracing/ai/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/ai/test.ts @@ -1,5 +1,4 @@ -import { join } from 'node:path'; -import { afterAll, describe, expect, test } from 'vitest'; +import { afterAll, describe, expect } from 'vitest'; import { cleanupChildProcesses, createEsmAndCjsTests } from '../../../utils/runner'; // `ai` SDK only support Node 18+ @@ -126,15 +125,9 @@ describe('ai', () => { ]), }; - test('creates ai related spans - cjs', async () => { - await createRunner(__dirname, 'scenario.js').expect({ transaction: EXPECTED_TRANSACTION }).start().completed(); - }); - - test('creates ai related spans - esm', async () => { - await createRunner(__dirname, 'scenario.mjs') - .withFlags('--import', join(__dirname, 'instrument.mjs')) - .expect({ transaction: EXPECTED_TRANSACTION }) - .start() - .completed(); + createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => { + test('creates ai related spans ', async () => { + await createRunner().expect({ transaction: EXPECTED_TRANSACTION }).start().completed(); + }); }); }); From 4e41faf2434d21b2313cee9c4f33d54bd8be723d Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 30 Apr 2025 09:50:11 +0200 Subject: [PATCH 05/15] update timeout --- dev-packages/node-integration-tests/utils/runner.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/dev-packages/node-integration-tests/utils/runner.ts b/dev-packages/node-integration-tests/utils/runner.ts index cd943c9de0fb..b75af3d129bb 100644 --- a/dev-packages/node-integration-tests/utils/runner.ts +++ b/dev-packages/node-integration-tests/utils/runner.ts @@ -41,13 +41,13 @@ export function cleanupChildProcesses(): void { process.on('exit', cleanupChildProcesses); /** Promise only resolves when fn returns true */ -async function waitFor(fn: () => boolean, timeout = 10_000): Promise { +async function waitFor(fn: () => boolean, timeout = 10_000, message = 'Timed out waiting'): Promise { let remaining = timeout; while (fn() === false) { await new Promise(resolve => setTimeout(resolve, 100)); remaining -= 100; if (remaining < 0) { - throw new Error('Timed out waiting for server port'); + throw new Error(message); } } } @@ -519,7 +519,7 @@ export function createRunner(...paths: string[]) { return { completed: async function (): Promise { - await waitFor(() => isComplete); + await waitFor(() => isComplete, 120_000, 'Timed out waiting for test to complete'); if (completeError) { throw completeError; @@ -537,7 +537,7 @@ export function createRunner(...paths: string[]) { options: { headers?: Record; data?: unknown; expectError?: boolean } = {}, ): Promise { try { - await waitFor(() => scenarioServerPort !== undefined); + await waitFor(() => scenarioServerPort !== undefined, 10_000, 'Timed out waiting for server port'); } catch (e) { complete(e as Error); return; From 68ac69e2cf66eb5a50ccea79475321b305591cd8 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 30 Apr 2025 10:04:25 +0200 Subject: [PATCH 06/15] increase anr timeout --- dev-packages/node-integration-tests/suites/anr/test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-packages/node-integration-tests/suites/anr/test.ts b/dev-packages/node-integration-tests/suites/anr/test.ts index 5bee31aa571c..08b6a6571e17 100644 --- a/dev-packages/node-integration-tests/suites/anr/test.ts +++ b/dev-packages/node-integration-tests/suites/anr/test.ts @@ -107,7 +107,7 @@ const ANR_EVENT_WITH_DEBUG_META: Event = { }, }; -describe('should report ANR when event loop blocked', { timeout: 60_000 }, () => { +describe('should report ANR when event loop blocked', { timeout: 90_000 }, () => { afterAll(() => { cleanupChildProcesses(); }); From 616c39192d040ec17a741117d09cf901c2ddec70 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 30 Apr 2025 10:05:34 +0200 Subject: [PATCH 07/15] rename to failsOnEsm --- .../suites/express/with-http/test.ts | 2 +- .../node-integration-tests/suites/tracing/connect/test.ts | 2 +- .../suites/tracing/dataloader/test.ts | 2 +- dev-packages/node-integration-tests/utils/runner.ts | 8 ++++---- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/dev-packages/node-integration-tests/suites/express/with-http/test.ts b/dev-packages/node-integration-tests/suites/express/with-http/test.ts index 7583cd04bf50..ef27c3129020 100644 --- a/dev-packages/node-integration-tests/suites/express/with-http/test.ts +++ b/dev-packages/node-integration-tests/suites/express/with-http/test.ts @@ -36,6 +36,6 @@ describe('express with http import', () => { // TODO: This is failing on ESM because importing http is triggering the http spans twice :( // We need to fix this! }, - { skipEsm: true }, + { failsOnEsm: true }, ); }); diff --git a/dev-packages/node-integration-tests/suites/tracing/connect/test.ts b/dev-packages/node-integration-tests/suites/tracing/connect/test.ts index 12f3541daf8b..0c37c58f8a4c 100644 --- a/dev-packages/node-integration-tests/suites/tracing/connect/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/connect/test.ts @@ -62,6 +62,6 @@ describe('connect auto-instrumentation', () => { await runner.completed(); }); }, - { skipEsm: true }, + { failsOnEsm: true }, ); }); diff --git a/dev-packages/node-integration-tests/suites/tracing/dataloader/test.ts b/dev-packages/node-integration-tests/suites/tracing/dataloader/test.ts index c0e04bdb00aa..1a653dc6496a 100644 --- a/dev-packages/node-integration-tests/suites/tracing/dataloader/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/dataloader/test.ts @@ -43,6 +43,6 @@ describe('dataloader auto-instrumentation', () => { await runner.completed(); }); }, - { skipEsm: true }, + { failsOnEsm: true }, ); }); diff --git a/dev-packages/node-integration-tests/utils/runner.ts b/dev-packages/node-integration-tests/utils/runner.ts index b75af3d129bb..a2b8642af9b0 100644 --- a/dev-packages/node-integration-tests/utils/runner.ts +++ b/dev-packages/node-integration-tests/utils/runner.ts @@ -174,7 +174,7 @@ export function createEsmAndCjsTests( testFn: typeof test | typeof test.fails, mode: 'esm' | 'cjs', ) => void, - options?: { skipCjs?: boolean; skipEsm?: boolean }, + options?: { failsOnCjs?: boolean; failsOnEsm?: boolean }, ): void { const mjsScenarioPath = join(cwd, scenarioPath); const mjsInstrumentPath = join(cwd, instrumentPath); @@ -191,7 +191,7 @@ export function createEsmAndCjsTests( const cjsInstrumentPath = join(cwd, `tmp_${instrumentPath.replace('.mjs', '.cjs')}`); // For the CJS runner, we create some temporary files... - if (!options?.skipCjs) { + if (!options?.failsOnCjs) { convertEsmFileToCjs(mjsScenarioPath, cjsScenarioPath); convertEsmFileToCjs(mjsInstrumentPath, cjsInstrumentPath); } @@ -211,12 +211,12 @@ export function createEsmAndCjsTests( }); describe('esm', () => { - const testFn = options?.skipEsm ? test.fails : test; + const testFn = options?.failsOnEsm ? test.fails : test; callback(() => createRunner(mjsScenarioPath).withFlags('--import', mjsInstrumentPath), testFn, 'esm'); }); describe('cjs', () => { - const testFn = options?.skipCjs ? test.fails : test; + const testFn = options?.failsOnCjs ? test.fails : test; callback(() => createRunner(cjsScenarioPath).withFlags('--require', cjsInstrumentPath), testFn, 'cjs'); }); }); From 68d07337b2bfc6d57f9b03fa191fcf6861a11196 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 30 Apr 2025 10:13:36 +0200 Subject: [PATCH 08/15] genericPool --- .../suites/tracing/genericPool/instrument.mjs | 9 ++++ .../genericPool/{scenario.js => scenario.mjs} | 16 ++---- .../suites/tracing/genericPool/test.ts | 52 ++++++++++--------- 3 files changed, 39 insertions(+), 38 deletions(-) create mode 100644 dev-packages/node-integration-tests/suites/tracing/genericPool/instrument.mjs rename dev-packages/node-integration-tests/suites/tracing/genericPool/{scenario.js => scenario.mjs} (74%) diff --git a/dev-packages/node-integration-tests/suites/tracing/genericPool/instrument.mjs b/dev-packages/node-integration-tests/suites/tracing/genericPool/instrument.mjs new file mode 100644 index 000000000000..46a27dd03b74 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/genericPool/instrument.mjs @@ -0,0 +1,9 @@ +import * as Sentry from '@sentry/node'; +import { loggingTransport } from '@sentry-internal/node-integration-tests'; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + tracesSampleRate: 1.0, + transport: loggingTransport, +}); diff --git a/dev-packages/node-integration-tests/suites/tracing/genericPool/scenario.js b/dev-packages/node-integration-tests/suites/tracing/genericPool/scenario.mjs similarity index 74% rename from dev-packages/node-integration-tests/suites/tracing/genericPool/scenario.js rename to dev-packages/node-integration-tests/suites/tracing/genericPool/scenario.mjs index 74d5f73693f5..47f0fb46a4fd 100644 --- a/dev-packages/node-integration-tests/suites/tracing/genericPool/scenario.js +++ b/dev-packages/node-integration-tests/suites/tracing/genericPool/scenario.mjs @@ -1,19 +1,10 @@ -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', - tracesSampleRate: 1.0, - transport: loggingTransport, -}); +import * as Sentry from '@sentry/node'; +import genericPool from 'generic-pool'; +import mysql from 'mysql'; // Stop the process from exiting before the transaction is sent setInterval(() => {}, 1000); -const mysql = require('mysql'); -const genericPool = require('generic-pool'); - const factory = { create: function () { return mysql.createConnection({ @@ -67,5 +58,4 @@ async function run() { ); } -// eslint-disable-next-line @typescript-eslint/no-floating-promises run(); diff --git a/dev-packages/node-integration-tests/suites/tracing/genericPool/test.ts b/dev-packages/node-integration-tests/suites/tracing/genericPool/test.ts index 42f2a3603f95..dd18c456e958 100644 --- a/dev-packages/node-integration-tests/suites/tracing/genericPool/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/genericPool/test.ts @@ -1,35 +1,37 @@ -import { afterAll, describe, expect, test } from 'vitest'; -import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; +import { afterAll, describe, expect } from 'vitest'; +import { cleanupChildProcesses, createEsmAndCjsTests } from '../../../utils/runner'; describe('genericPool auto instrumentation', () => { afterAll(() => { cleanupChildProcesses(); }); - test('should auto-instrument `genericPool` package when calling pool.require()', async () => { - const EXPECTED_TRANSACTION = { - transaction: 'Test Transaction', - spans: expect.arrayContaining([ - expect.objectContaining({ - description: expect.stringMatching(/^generic-pool\.ac?quire/), - origin: 'auto.db.otel.generic_pool', - data: { - 'sentry.origin': 'auto.db.otel.generic_pool', - }, - status: 'ok', - }), + createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => { + test('should auto-instrument `genericPool` package when calling pool.require()', async () => { + const EXPECTED_TRANSACTION = { + transaction: 'Test Transaction', + spans: expect.arrayContaining([ + expect.objectContaining({ + description: expect.stringMatching(/^generic-pool\.ac?quire/), + origin: 'auto.db.otel.generic_pool', + data: { + 'sentry.origin': 'auto.db.otel.generic_pool', + }, + status: 'ok', + }), - expect.objectContaining({ - description: expect.stringMatching(/^generic-pool\.ac?quire/), - origin: 'auto.db.otel.generic_pool', - data: { - 'sentry.origin': 'auto.db.otel.generic_pool', - }, - status: 'ok', - }), - ]), - }; + expect.objectContaining({ + description: expect.stringMatching(/^generic-pool\.ac?quire/), + origin: 'auto.db.otel.generic_pool', + data: { + 'sentry.origin': 'auto.db.otel.generic_pool', + }, + status: 'ok', + }), + ]), + }; - await createRunner(__dirname, 'scenario.js').expect({ transaction: EXPECTED_TRANSACTION }).start().completed(); + await createRunner().expect({ transaction: EXPECTED_TRANSACTION }).start().completed(); + }); }); }); From d058fa85cca59e3d698012628f4280511cb7f095 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 30 Apr 2025 10:17:13 +0200 Subject: [PATCH 09/15] httpIntegration --- .../{server.js => instrument.mjs} | 21 +---- .../suites/tracing/httpIntegration/server.mjs | 16 ++++ .../suites/tracing/httpIntegration/test.ts | 84 +++++++++---------- 3 files changed, 58 insertions(+), 63 deletions(-) rename dev-packages/node-integration-tests/suites/tracing/httpIntegration/{server.js => instrument.mjs} (66%) create mode 100644 dev-packages/node-integration-tests/suites/tracing/httpIntegration/server.mjs diff --git a/dev-packages/node-integration-tests/suites/tracing/httpIntegration/server.js b/dev-packages/node-integration-tests/suites/tracing/httpIntegration/instrument.mjs similarity index 66% rename from dev-packages/node-integration-tests/suites/tracing/httpIntegration/server.js rename to dev-packages/node-integration-tests/suites/tracing/httpIntegration/instrument.mjs index d10c24db500d..8cf2e8a5248f 100644 --- a/dev-packages/node-integration-tests/suites/tracing/httpIntegration/server.js +++ b/dev-packages/node-integration-tests/suites/tracing/httpIntegration/instrument.mjs @@ -1,5 +1,5 @@ -const { loggingTransport } = require('@sentry-internal/node-integration-tests'); -const Sentry = require('@sentry/node'); +import * as Sentry from '@sentry/node'; +import { loggingTransport } from '@sentry-internal/node-integration-tests'; Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', @@ -39,20 +39,3 @@ Sentry.init({ }), ], }); - -// 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', (_req, res) => { - res.send({ response: 'response 1' }); -}); - -Sentry.setupExpressErrorHandler(app); - -startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/tracing/httpIntegration/server.mjs b/dev-packages/node-integration-tests/suites/tracing/httpIntegration/server.mjs new file mode 100644 index 000000000000..44122f375857 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/httpIntegration/server.mjs @@ -0,0 +1,16 @@ +import * as Sentry from '@sentry/node'; +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', (_req, res) => { + res.send({ response: 'response 1' }); +}); + +Sentry.setupExpressErrorHandler(app); + +startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/tracing/httpIntegration/test.ts b/dev-packages/node-integration-tests/suites/tracing/httpIntegration/test.ts index 6e4ad622ea26..9682f4aa28ac 100644 --- a/dev-packages/node-integration-tests/suites/tracing/httpIntegration/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/httpIntegration/test.ts @@ -1,5 +1,5 @@ import { afterAll, describe, expect, test } from 'vitest'; -import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; +import { cleanupChildProcesses, createEsmAndCjsTests, createRunner } from '../../../utils/runner'; import { createTestServer } from '../../../utils/server'; describe('httpIntegration', () => { @@ -7,52 +7,48 @@ describe('httpIntegration', () => { cleanupChildProcesses(); }); - test('allows to pass instrumentation options to integration', async () => { - // response shape seems different on Node 14, so we skip this there - const nodeMajorVersion = Number(process.versions.node.split('.')[0]); - if (nodeMajorVersion <= 14) { - return; - } - - const runner = 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$/), - 'http.response.status_code': 200, - attr1: 'yes', - attr2: 'yes', - attr3: 'yes', + createEsmAndCjsTests(__dirname, 'server.mjs', 'instrument.mjs', (createRunner, test) => { + test('allows to pass instrumentation options to integration', async () => { + const runner = createRunner() + .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$/), + 'http.response.status_code': 200, + attr1: 'yes', + attr2: 'yes', + attr3: 'yes', + }, + op: 'http.server', + status: 'ok', }, - op: 'http.server', - status: 'ok', - }, - }, - extra: { - requestHookCalled: { - url: expect.stringMatching(/\/test$/), - method: 'GET', - }, - responseHookCalled: { - url: expect.stringMatching(/\/test$/), - method: 'GET', }, - applyCustomAttributesOnSpanCalled: { - reqUrl: expect.stringMatching(/\/test$/), - reqMethod: 'GET', - resUrl: expect.stringMatching(/\/test$/), - resMethod: 'GET', + extra: { + requestHookCalled: { + url: expect.stringMatching(/\/test$/), + method: 'GET', + }, + responseHookCalled: { + url: expect.stringMatching(/\/test$/), + method: 'GET', + }, + applyCustomAttributesOnSpanCalled: { + reqUrl: expect.stringMatching(/\/test$/), + reqMethod: 'GET', + resUrl: expect.stringMatching(/\/test$/), + resMethod: 'GET', + }, }, }, - }, - }) - .start(); - runner.makeRequest('get', '/test'); - await runner.completed(); + }) + .start(); + runner.makeRequest('get', '/test'); + await runner.completed(); + }); }); test('allows to pass experimental config through to integration', async () => { @@ -155,7 +151,7 @@ describe('httpIntegration', () => { expect(breadcrumbs![0]?.data?.url).toEqual(`${SERVER_URL}/pass`); }, }) - .start(closeTestServer); + .start(); runner.makeRequest('get', '/testUrl'); await runner.completed(); closeTestServer(); From 05e2692ce93b0234cbb83537047d72484b1b6b85 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 30 Apr 2025 10:19:58 +0200 Subject: [PATCH 10/15] kafkajs --- .../suites/tracing/kafkajs/instrument.mjs | 9 ++ .../kafkajs/{scenario.js => scenario.mjs} | 16 +--- .../suites/tracing/kafkajs/test.ts | 86 ++++++++++--------- 3 files changed, 54 insertions(+), 57 deletions(-) create mode 100644 dev-packages/node-integration-tests/suites/tracing/kafkajs/instrument.mjs rename dev-packages/node-integration-tests/suites/tracing/kafkajs/{scenario.js => scenario.mjs} (67%) diff --git a/dev-packages/node-integration-tests/suites/tracing/kafkajs/instrument.mjs b/dev-packages/node-integration-tests/suites/tracing/kafkajs/instrument.mjs new file mode 100644 index 000000000000..46a27dd03b74 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/kafkajs/instrument.mjs @@ -0,0 +1,9 @@ +import * as Sentry from '@sentry/node'; +import { loggingTransport } from '@sentry-internal/node-integration-tests'; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + tracesSampleRate: 1.0, + transport: loggingTransport, +}); diff --git a/dev-packages/node-integration-tests/suites/tracing/kafkajs/scenario.js b/dev-packages/node-integration-tests/suites/tracing/kafkajs/scenario.mjs similarity index 67% rename from dev-packages/node-integration-tests/suites/tracing/kafkajs/scenario.js rename to dev-packages/node-integration-tests/suites/tracing/kafkajs/scenario.mjs index d4541aa3a7de..d92414c2818b 100644 --- a/dev-packages/node-integration-tests/suites/tracing/kafkajs/scenario.js +++ b/dev-packages/node-integration-tests/suites/tracing/kafkajs/scenario.mjs @@ -1,17 +1,7 @@ -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', - tracesSampleRate: 1.0, - transport: loggingTransport, -}); - // Stop the process from exiting before the transaction is sent setInterval(() => {}, 1000); -const { Kafka } = require('kafkajs'); +import { Kafka } from 'kafkajs'; async function run() { const kafka = new Kafka({ @@ -54,10 +44,6 @@ async function run() { }, ], }); - - // Wait for the message to be received - await new Promise(resolve => setTimeout(resolve, 5000)); } -// eslint-disable-next-line @typescript-eslint/no-floating-promises run(); diff --git a/dev-packages/node-integration-tests/suites/tracing/kafkajs/test.ts b/dev-packages/node-integration-tests/suites/tracing/kafkajs/test.ts index f1d6e46db743..6e03921c4b73 100644 --- a/dev-packages/node-integration-tests/suites/tracing/kafkajs/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/kafkajs/test.ts @@ -1,54 +1,56 @@ -import { afterAll, describe, expect, test } from 'vitest'; -import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; +import { afterAll, describe, expect } from 'vitest'; +import { cleanupChildProcesses, createEsmAndCjsTests } from '../../../utils/runner'; describe('kafkajs', () => { afterAll(() => { cleanupChildProcesses(); }); - test('traces producers and consumers', { timeout: 60_000 }, async () => { - await createRunner(__dirname, 'scenario.js') - .withDockerCompose({ - workingDirectory: [__dirname], - readyMatches: ['9092'], - }) - .expect({ - transaction: { - transaction: 'test-topic', - contexts: { - trace: expect.objectContaining({ - op: 'message', - status: 'ok', - data: expect.objectContaining({ - 'messaging.system': 'kafka', - 'messaging.destination': 'test-topic', - 'otel.kind': 'PRODUCER', - 'sentry.op': 'message', - 'sentry.origin': 'auto.kafkajs.otel.producer', + createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => { + test('traces producers and consumers', { timeout: 60_000 }, async () => { + await createRunner() + .withDockerCompose({ + workingDirectory: [__dirname], + readyMatches: ['9092'], + }) + .expect({ + transaction: { + transaction: 'test-topic', + contexts: { + trace: expect.objectContaining({ + op: 'message', + status: 'ok', + data: expect.objectContaining({ + 'messaging.system': 'kafka', + 'messaging.destination': 'test-topic', + 'otel.kind': 'PRODUCER', + 'sentry.op': 'message', + 'sentry.origin': 'auto.kafkajs.otel.producer', + }), }), - }), + }, }, - }, - }) - .expect({ - transaction: { - transaction: 'test-topic', - contexts: { - trace: expect.objectContaining({ - op: 'message', - status: 'ok', - data: expect.objectContaining({ - 'messaging.system': 'kafka', - 'messaging.destination': 'test-topic', - 'otel.kind': 'CONSUMER', - 'sentry.op': 'message', - 'sentry.origin': 'auto.kafkajs.otel.consumer', + }) + .expect({ + transaction: { + transaction: 'test-topic', + contexts: { + trace: expect.objectContaining({ + op: 'message', + status: 'ok', + data: expect.objectContaining({ + 'messaging.system': 'kafka', + 'messaging.destination': 'test-topic', + 'otel.kind': 'CONSUMER', + 'sentry.op': 'message', + 'sentry.origin': 'auto.kafkajs.otel.consumer', + }), }), - }), + }, }, - }, - }) - .start() - .completed(); + }) + .start() + .completed(); + }); }); }); From 117553911307cc380ee7b7d211fa3b7c43b3d33c Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 30 Apr 2025 10:45:52 +0200 Subject: [PATCH 11/15] always create files even if cjs is skipped --- dev-packages/node-integration-tests/utils/runner.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/dev-packages/node-integration-tests/utils/runner.ts b/dev-packages/node-integration-tests/utils/runner.ts index a2b8642af9b0..56d94c74206d 100644 --- a/dev-packages/node-integration-tests/utils/runner.ts +++ b/dev-packages/node-integration-tests/utils/runner.ts @@ -191,10 +191,8 @@ export function createEsmAndCjsTests( const cjsInstrumentPath = join(cwd, `tmp_${instrumentPath.replace('.mjs', '.cjs')}`); // For the CJS runner, we create some temporary files... - if (!options?.failsOnCjs) { - convertEsmFileToCjs(mjsScenarioPath, cjsScenarioPath); - convertEsmFileToCjs(mjsInstrumentPath, cjsInstrumentPath); - } + convertEsmFileToCjs(mjsScenarioPath, cjsScenarioPath); + convertEsmFileToCjs(mjsInstrumentPath, cjsInstrumentPath); describe('esm & cjs', () => { afterAll(() => { From fffc2c213bd48da5fd91207d02a22348955e03bc Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 30 Apr 2025 10:47:01 +0200 Subject: [PATCH 12/15] remove one level of nesting of describe calls --- .../node-integration-tests/utils/runner.ts | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/dev-packages/node-integration-tests/utils/runner.ts b/dev-packages/node-integration-tests/utils/runner.ts index 56d94c74206d..cd7145e4d26c 100644 --- a/dev-packages/node-integration-tests/utils/runner.ts +++ b/dev-packages/node-integration-tests/utils/runner.ts @@ -194,7 +194,12 @@ export function createEsmAndCjsTests( convertEsmFileToCjs(mjsScenarioPath, cjsScenarioPath); convertEsmFileToCjs(mjsInstrumentPath, cjsInstrumentPath); - describe('esm & cjs', () => { + describe('esm', () => { + const testFn = options?.failsOnEsm ? test.fails : test; + callback(() => createRunner(mjsScenarioPath).withFlags('--import', mjsInstrumentPath), testFn, 'esm'); + }); + + describe('cjs', () => { afterAll(() => { try { unlinkSync(cjsInstrumentPath); @@ -208,15 +213,8 @@ export function createEsmAndCjsTests( } }); - describe('esm', () => { - const testFn = options?.failsOnEsm ? test.fails : test; - callback(() => createRunner(mjsScenarioPath).withFlags('--import', mjsInstrumentPath), testFn, 'esm'); - }); - - describe('cjs', () => { - const testFn = options?.failsOnCjs ? test.fails : test; - callback(() => createRunner(cjsScenarioPath).withFlags('--require', cjsInstrumentPath), testFn, 'cjs'); - }); + const testFn = options?.failsOnCjs ? test.fails : test; + callback(() => createRunner(cjsScenarioPath).withFlags('--require', cjsInstrumentPath), testFn, 'cjs'); }); } From 3d9ce1496c0e8cbf8bcd11e821cfb78ffa197f58 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 30 Apr 2025 10:46:52 +0200 Subject: [PATCH 13/15] knex --- .../tracing/knex/instrument-withMysql2.mjs | 10 + .../suites/tracing/knex/instrument-withPg.mjs | 10 + ...-withMysql2.js => scenario-withMysql2.mjs} | 15 +- ...io-withPostgres.js => scenario-withPg.mjs} | 15 +- .../suites/tracing/knex/test.ts | 240 +++++++++--------- 5 files changed, 148 insertions(+), 142 deletions(-) create mode 100644 dev-packages/node-integration-tests/suites/tracing/knex/instrument-withMysql2.mjs create mode 100644 dev-packages/node-integration-tests/suites/tracing/knex/instrument-withPg.mjs rename dev-packages/node-integration-tests/suites/tracing/knex/{scenario-withMysql2.js => scenario-withMysql2.mjs} (70%) rename dev-packages/node-integration-tests/suites/tracing/knex/{scenario-withPostgres.js => scenario-withPg.mjs} (70%) diff --git a/dev-packages/node-integration-tests/suites/tracing/knex/instrument-withMysql2.mjs b/dev-packages/node-integration-tests/suites/tracing/knex/instrument-withMysql2.mjs new file mode 100644 index 000000000000..b40a8eaed8dd --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/knex/instrument-withMysql2.mjs @@ -0,0 +1,10 @@ +import * as Sentry from '@sentry/node'; +import { loggingTransport } from '@sentry-internal/node-integration-tests'; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + tracesSampleRate: 1.0, + transport: loggingTransport, + integrations: [Sentry.knexIntegration()], +}); diff --git a/dev-packages/node-integration-tests/suites/tracing/knex/instrument-withPg.mjs b/dev-packages/node-integration-tests/suites/tracing/knex/instrument-withPg.mjs new file mode 100644 index 000000000000..b40a8eaed8dd --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/knex/instrument-withPg.mjs @@ -0,0 +1,10 @@ +import * as Sentry from '@sentry/node'; +import { loggingTransport } from '@sentry-internal/node-integration-tests'; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + tracesSampleRate: 1.0, + transport: loggingTransport, + integrations: [Sentry.knexIntegration()], +}); diff --git a/dev-packages/node-integration-tests/suites/tracing/knex/scenario-withMysql2.js b/dev-packages/node-integration-tests/suites/tracing/knex/scenario-withMysql2.mjs similarity index 70% rename from dev-packages/node-integration-tests/suites/tracing/knex/scenario-withMysql2.js rename to dev-packages/node-integration-tests/suites/tracing/knex/scenario-withMysql2.mjs index 5d57e38d9318..300e90a3df3e 100644 --- a/dev-packages/node-integration-tests/suites/tracing/knex/scenario-withMysql2.js +++ b/dev-packages/node-integration-tests/suites/tracing/knex/scenario-withMysql2.mjs @@ -1,19 +1,9 @@ -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', - tracesSampleRate: 1.0, - transport: loggingTransport, - integrations: [Sentry.knexIntegration()], -}); +import * as Sentry from '@sentry/node'; +import knex from 'knex'; // Stop the process from exiting before the transaction is sent setInterval(() => {}, 1000); -const knex = require('knex').default; - const mysql2Client = knex({ client: 'mysql2', connection: { @@ -49,5 +39,4 @@ async function run() { ); } -// eslint-disable-next-line @typescript-eslint/no-floating-promises run(); diff --git a/dev-packages/node-integration-tests/suites/tracing/knex/scenario-withPostgres.js b/dev-packages/node-integration-tests/suites/tracing/knex/scenario-withPg.mjs similarity index 70% rename from dev-packages/node-integration-tests/suites/tracing/knex/scenario-withPostgres.js rename to dev-packages/node-integration-tests/suites/tracing/knex/scenario-withPg.mjs index a9f2d558a618..a5f4c62c85b9 100644 --- a/dev-packages/node-integration-tests/suites/tracing/knex/scenario-withPostgres.js +++ b/dev-packages/node-integration-tests/suites/tracing/knex/scenario-withPg.mjs @@ -1,19 +1,9 @@ -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', - tracesSampleRate: 1.0, - transport: loggingTransport, - integrations: [Sentry.knexIntegration()], -}); +import * as Sentry from '@sentry/node'; +import knex from 'knex'; // Stop the process from exiting before the transaction is sent setInterval(() => {}, 1000); -const knex = require('knex').default; - const pgClient = knex({ client: 'pg', connection: { @@ -49,5 +39,4 @@ async function run() { ); } -// eslint-disable-next-line @typescript-eslint/no-floating-promises run(); diff --git a/dev-packages/node-integration-tests/suites/tracing/knex/test.ts b/dev-packages/node-integration-tests/suites/tracing/knex/test.ts index 8fd445d51525..f60ea76a1c54 100644 --- a/dev-packages/node-integration-tests/suites/tracing/knex/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/knex/test.ts @@ -1,129 +1,137 @@ -import { describe, expect, test } from 'vitest'; -import { createRunner } from '../../../utils/runner'; +import { describe, expect } from 'vitest'; +import { createEsmAndCjsTests } from '../../../utils/runner'; describe('knex auto instrumentation', () => { // Update this if another knex version is installed const KNEX_VERSION = '2.5.1'; - test('should auto-instrument `knex` package when using `pg` client', { timeout: 60_000 }, async () => { - const EXPECTED_TRANSACTION = { - transaction: 'Test Transaction', - spans: expect.arrayContaining([ - expect.objectContaining({ - data: expect.objectContaining({ - 'knex.version': KNEX_VERSION, - 'db.system': 'postgresql', - 'db.name': 'tests', - 'sentry.origin': 'auto.db.otel.knex', - 'sentry.op': 'db', - 'net.peer.name': 'localhost', - 'net.peer.port': 5445, - }), - status: 'ok', - description: - 'create table "User" ("id" serial primary key, "createdAt" timestamptz(3) not null default CURRENT_TIMESTAMP(3), "email" text not null, "name" text not null)', - origin: 'auto.db.otel.knex', - }), - expect.objectContaining({ - data: expect.objectContaining({ - 'knex.version': KNEX_VERSION, - 'db.system': 'postgresql', - 'db.name': 'tests', - 'sentry.origin': 'auto.db.otel.knex', - 'sentry.op': 'db', - 'net.peer.name': 'localhost', - 'net.peer.port': 5445, - }), - status: 'ok', - // In the knex-otel spans, the placeholders (e.g., `$1`) are replaced by a `?`. - description: 'insert into "User" ("email", "name") values (?, ?)', - origin: 'auto.db.otel.knex', - }), + describe('with `pg` client', () => { + createEsmAndCjsTests(__dirname, 'scenario-withPg.mjs', 'instrument-withPg.mjs', (createRunner, test) => { + test('should auto-instrument `knex` package', { timeout: 60_000 }, async () => { + const EXPECTED_TRANSACTION = { + transaction: 'Test Transaction', + spans: expect.arrayContaining([ + expect.objectContaining({ + data: expect.objectContaining({ + 'knex.version': KNEX_VERSION, + 'db.system': 'postgresql', + 'db.name': 'tests', + 'sentry.origin': 'auto.db.otel.knex', + 'sentry.op': 'db', + 'net.peer.name': 'localhost', + 'net.peer.port': 5445, + }), + status: 'ok', + description: + 'create table "User" ("id" serial primary key, "createdAt" timestamptz(3) not null default CURRENT_TIMESTAMP(3), "email" text not null, "name" text not null)', + origin: 'auto.db.otel.knex', + }), + expect.objectContaining({ + data: expect.objectContaining({ + 'knex.version': KNEX_VERSION, + 'db.system': 'postgresql', + 'db.name': 'tests', + 'sentry.origin': 'auto.db.otel.knex', + 'sentry.op': 'db', + 'net.peer.name': 'localhost', + 'net.peer.port': 5445, + }), + status: 'ok', + // In the knex-otel spans, the placeholders (e.g., `$1`) are replaced by a `?`. + description: 'insert into "User" ("email", "name") values (?, ?)', + origin: 'auto.db.otel.knex', + }), - expect.objectContaining({ - data: expect.objectContaining({ - 'knex.version': KNEX_VERSION, - 'db.operation': 'select', - 'db.sql.table': 'User', - 'db.system': 'postgresql', - 'db.name': 'tests', - 'db.statement': 'select * from "User"', - 'sentry.origin': 'auto.db.otel.knex', - 'sentry.op': 'db', - }), - status: 'ok', - description: 'select * from "User"', - origin: 'auto.db.otel.knex', - }), - ]), - }; + expect.objectContaining({ + data: expect.objectContaining({ + 'knex.version': KNEX_VERSION, + 'db.operation': 'select', + 'db.sql.table': 'User', + 'db.system': 'postgresql', + 'db.name': 'tests', + 'db.statement': 'select * from "User"', + 'sentry.origin': 'auto.db.otel.knex', + 'sentry.op': 'db', + }), + status: 'ok', + description: 'select * from "User"', + origin: 'auto.db.otel.knex', + }), + ]), + }; - await createRunner(__dirname, 'scenario-withPostgres.js') - .withDockerCompose({ workingDirectory: [__dirname], readyMatches: ['port 5432'] }) - .expect({ transaction: EXPECTED_TRANSACTION }) - .start() - .completed(); + await createRunner() + .withDockerCompose({ workingDirectory: [__dirname], readyMatches: ['port 5432'] }) + .expect({ transaction: EXPECTED_TRANSACTION }) + .start() + .completed(); + }); + }); }); - test('should auto-instrument `knex` package when using `mysql2` client', { timeout: 60_000 }, async () => { - const EXPECTED_TRANSACTION = { - transaction: 'Test Transaction', - spans: expect.arrayContaining([ - expect.objectContaining({ - data: expect.objectContaining({ - 'knex.version': KNEX_VERSION, - 'db.system': 'mysql2', - 'db.name': 'tests', - 'db.user': 'root', - 'sentry.origin': 'auto.db.otel.knex', - 'sentry.op': 'db', - 'net.peer.name': 'localhost', - 'net.peer.port': 3307, - }), - status: 'ok', - description: - 'create table `User` (`id` int unsigned not null auto_increment primary key, `createdAt` timestamp(3) not null default CURRENT_TIMESTAMP(3), `email` text not null, `name` text not null)', - origin: 'auto.db.otel.knex', - }), - expect.objectContaining({ - data: expect.objectContaining({ - 'knex.version': KNEX_VERSION, - 'db.system': 'mysql2', - 'db.name': 'tests', - 'db.user': 'root', - 'sentry.origin': 'auto.db.otel.knex', - 'sentry.op': 'db', - 'net.peer.name': 'localhost', - 'net.peer.port': 3307, - }), - status: 'ok', - description: 'insert into `User` (`email`, `name`) values (?, ?)', - origin: 'auto.db.otel.knex', - }), + describe('with `mysql2` client', () => { + createEsmAndCjsTests(__dirname, 'scenario-withMysql2.mjs', 'instrument-withMysql2.mjs', (createRunner, test) => { + test('should auto-instrument `knex` package', { timeout: 60_000 }, async () => { + const EXPECTED_TRANSACTION = { + transaction: 'Test Transaction', + spans: expect.arrayContaining([ + expect.objectContaining({ + data: expect.objectContaining({ + 'knex.version': KNEX_VERSION, + 'db.system': 'mysql2', + 'db.name': 'tests', + 'db.user': 'root', + 'sentry.origin': 'auto.db.otel.knex', + 'sentry.op': 'db', + 'net.peer.name': 'localhost', + 'net.peer.port': 3307, + }), + status: 'ok', + description: + 'create table `User` (`id` int unsigned not null auto_increment primary key, `createdAt` timestamp(3) not null default CURRENT_TIMESTAMP(3), `email` text not null, `name` text not null)', + origin: 'auto.db.otel.knex', + }), + expect.objectContaining({ + data: expect.objectContaining({ + 'knex.version': KNEX_VERSION, + 'db.system': 'mysql2', + 'db.name': 'tests', + 'db.user': 'root', + 'sentry.origin': 'auto.db.otel.knex', + 'sentry.op': 'db', + 'net.peer.name': 'localhost', + 'net.peer.port': 3307, + }), + status: 'ok', + description: 'insert into `User` (`email`, `name`) values (?, ?)', + origin: 'auto.db.otel.knex', + }), - expect.objectContaining({ - data: expect.objectContaining({ - 'knex.version': KNEX_VERSION, - 'db.operation': 'select', - 'db.sql.table': 'User', - 'db.system': 'mysql2', - 'db.name': 'tests', - 'db.statement': 'select * from `User`', - 'db.user': 'root', - 'sentry.origin': 'auto.db.otel.knex', - 'sentry.op': 'db', - }), - status: 'ok', - description: 'select * from `User`', - origin: 'auto.db.otel.knex', - }), - ]), - }; + expect.objectContaining({ + data: expect.objectContaining({ + 'knex.version': KNEX_VERSION, + 'db.operation': 'select', + 'db.sql.table': 'User', + 'db.system': 'mysql2', + 'db.name': 'tests', + 'db.statement': 'select * from `User`', + 'db.user': 'root', + 'sentry.origin': 'auto.db.otel.knex', + 'sentry.op': 'db', + }), + status: 'ok', + description: 'select * from `User`', + origin: 'auto.db.otel.knex', + }), + ]), + }; - await createRunner(__dirname, 'scenario-withMysql2.js') - .withDockerCompose({ workingDirectory: [__dirname], readyMatches: ['port: 3306'] }) - .expect({ transaction: EXPECTED_TRANSACTION }) - .start() - .completed(); + await createRunner() + .withDockerCompose({ workingDirectory: [__dirname], readyMatches: ['port: 3306'] }) + .expect({ transaction: EXPECTED_TRANSACTION }) + .start() + .completed(); + }); + }); }); }); From 410cecf24e54f5cff9c4e3fc4377f10ea74f6151 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 30 Apr 2025 10:51:34 +0200 Subject: [PATCH 14/15] cleanup unneeded comments/vars --- .../node-integration-tests/suites/child-process/fork.js | 3 +-- .../node-integration-tests/suites/child-process/fork.mjs | 3 +-- .../node-integration-tests/suites/child-process/worker.js | 3 +-- .../node-integration-tests/suites/child-process/worker.mjs | 3 +-- .../suites/public-api/LocalVariables/deny-inspector.mjs | 5 ----- 5 files changed, 4 insertions(+), 13 deletions(-) diff --git a/dev-packages/node-integration-tests/suites/child-process/fork.js b/dev-packages/node-integration-tests/suites/child-process/fork.js index c6e5cd3f0b7f..064a21d57657 100644 --- a/dev-packages/node-integration-tests/suites/child-process/fork.js +++ b/dev-packages/node-integration-tests/suites/child-process/fork.js @@ -10,8 +10,7 @@ Sentry.init({ transport: loggingTransport, }); -// eslint-disable-next-line no-unused-vars -const _child = fork(path.join(__dirname, 'child.mjs')); +fork(path.join(__dirname, 'child.mjs')); setTimeout(() => { throw new Error('Exiting main process'); diff --git a/dev-packages/node-integration-tests/suites/child-process/fork.mjs b/dev-packages/node-integration-tests/suites/child-process/fork.mjs index d1678479143c..f28b7f313ec7 100644 --- a/dev-packages/node-integration-tests/suites/child-process/fork.mjs +++ b/dev-packages/node-integration-tests/suites/child-process/fork.mjs @@ -12,8 +12,7 @@ Sentry.init({ transport: loggingTransport, }); -// eslint-disable-next-line no-unused-vars -const _child = fork(path.join(__dirname, 'child.mjs')); +fork(path.join(__dirname, 'child.mjs')); setTimeout(() => { throw new Error('Exiting main process'); diff --git a/dev-packages/node-integration-tests/suites/child-process/worker.js b/dev-packages/node-integration-tests/suites/child-process/worker.js index 99b645d9001c..5468f7f45a9d 100644 --- a/dev-packages/node-integration-tests/suites/child-process/worker.js +++ b/dev-packages/node-integration-tests/suites/child-process/worker.js @@ -10,8 +10,7 @@ Sentry.init({ transport: loggingTransport, }); -// eslint-disable-next-line no-unused-vars -const _worker = new Worker(path.join(__dirname, 'child.js')); +new Worker(path.join(__dirname, 'child.js')); setTimeout(() => { process.exit(); diff --git a/dev-packages/node-integration-tests/suites/child-process/worker.mjs b/dev-packages/node-integration-tests/suites/child-process/worker.mjs index d8642095ac04..d0a749d464e3 100644 --- a/dev-packages/node-integration-tests/suites/child-process/worker.mjs +++ b/dev-packages/node-integration-tests/suites/child-process/worker.mjs @@ -12,8 +12,7 @@ Sentry.init({ transport: loggingTransport, }); -// eslint-disable-next-line no-unused-vars -const _worker = new Worker(path.join(__dirname, 'child.mjs')); +new Worker(path.join(__dirname, 'child.mjs')); setTimeout(() => { process.exit(); diff --git a/dev-packages/node-integration-tests/suites/public-api/LocalVariables/deny-inspector.mjs b/dev-packages/node-integration-tests/suites/public-api/LocalVariables/deny-inspector.mjs index 6be67a6221d7..bbf5655ea903 100644 --- a/dev-packages/node-integration-tests/suites/public-api/LocalVariables/deny-inspector.mjs +++ b/dev-packages/node-integration-tests/suites/public-api/LocalVariables/deny-inspector.mjs @@ -1,10 +1,5 @@ import { register } from 'node:module'; -// eslint-disable-next-line no-unused-vars -const hookScript = Buffer.from(` - - `); - register( new URL(`data:application/javascript, export async function resolve(specifier, context, nextResolve) { From 2804ee5fd66c811bc5fca8c58b7506f07716e3cc Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 30 Apr 2025 10:59:09 +0200 Subject: [PATCH 15/15] move cjs creation to beforeAll --- dev-packages/node-integration-tests/utils/runner.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/dev-packages/node-integration-tests/utils/runner.ts b/dev-packages/node-integration-tests/utils/runner.ts index cd7145e4d26c..fe01b5531bb6 100644 --- a/dev-packages/node-integration-tests/utils/runner.ts +++ b/dev-packages/node-integration-tests/utils/runner.ts @@ -16,7 +16,7 @@ import axios from 'axios'; import { execSync, spawn, spawnSync } from 'child_process'; import { existsSync, readFileSync, unlinkSync, writeFileSync } from 'fs'; import { join } from 'path'; -import { afterAll, describe, test } from 'vitest'; +import { afterAll, beforeAll, describe, test } from 'vitest'; import { assertEnvelopeHeader, assertSentryCheckIn, @@ -190,16 +190,18 @@ export function createEsmAndCjsTests( const cjsScenarioPath = join(cwd, `tmp_${scenarioPath.replace('.mjs', '.cjs')}`); const cjsInstrumentPath = join(cwd, `tmp_${instrumentPath.replace('.mjs', '.cjs')}`); - // For the CJS runner, we create some temporary files... - convertEsmFileToCjs(mjsScenarioPath, cjsScenarioPath); - convertEsmFileToCjs(mjsInstrumentPath, cjsInstrumentPath); - describe('esm', () => { const testFn = options?.failsOnEsm ? test.fails : test; callback(() => createRunner(mjsScenarioPath).withFlags('--import', mjsInstrumentPath), testFn, 'esm'); }); describe('cjs', () => { + beforeAll(() => { + // For the CJS runner, we create some temporary files... + convertEsmFileToCjs(mjsScenarioPath, cjsScenarioPath); + convertEsmFileToCjs(mjsInstrumentPath, cjsInstrumentPath); + }); + afterAll(() => { try { unlinkSync(cjsInstrumentPath);