From 78923ac2b18879e082e22bd83e3f5f8eedfa033d Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Fri, 3 Jul 2026 14:12:25 +0200 Subject: [PATCH] test(node): Migrate http client span tests to createCjsTests Move the nine `tracing/http-client-spans/*` suites onto the shared `createCjsTests` runner helper, splitting each inline `scenario.ts` into an `instrument.mjs` (`Sentry.init`) + `scenario.mjs` pair. Each suite keeps its own instrument config (tracesSampleRate, traceLifecycle, fetch/http integration options, debug) and runs in CJS as before. The `http-otel-double-instrumentation` suite now shares one `scenario.mjs` between its two instruments (`instrument.mjs` with debug, and `instrument-mitigation.mjs` with `httpIntegration({ spans: false })`), each of which registers the OTel `HttpInstrumentation` to reproduce the double-instrumentation case. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../{scenario.ts => instrument.mjs} | 10 -- .../fetch-basic-streamed/scenario.mjs | 11 ++ .../fetch-basic-streamed/test.ts | 55 +++--- .../instrument.mjs} | 5 - .../fetch-basic/{scenario.ts => scenario.mjs} | 8 - .../http-client-spans/fetch-basic/test.ts | 98 ++++++----- .../fetch-error/instrument.mjs | 9 + .../fetch-error/{scenario.ts => scenario.mjs} | 19 +- .../http-client-spans/fetch-error/test.ts | 46 +++-- .../{scenario.ts => instrument.mjs} | 6 - .../fetch-forward-request-hook/scenario.mjs | 7 + .../fetch-forward-request-hook/test.ts | 114 ++++++------ .../{scenario.ts => instrument.mjs} | 7 +- .../scenario.mjs | 6 + .../fetch-headers-to-span-attributes/test.ts | 68 ++++---- .../fetch-strip-query/instrument.mjs | 9 + .../fetch-strip-query/scenario.mjs | 6 + .../fetch-strip-query/test.ts | 104 +++++------ .../http-basic/instrument.mjs | 9 + .../http-basic/{scenario.ts => scenario.mjs} | 15 +- .../http-client-spans/http-basic/test.ts | 98 ++++++----- ...itigation.ts => instrument-mitigation.mjs} | 17 -- .../{scenario.ts => instrument.mjs} | 17 -- .../scenario.mjs | 17 ++ .../http-otel-double-instrumentation/test.ts | 162 ++++++++++-------- .../http-strip-query/instrument.mjs | 9 + .../{scenario.ts => scenario.mjs} | 15 +- .../http-strip-query/test.ts | 106 ++++++------ 28 files changed, 560 insertions(+), 493 deletions(-) rename dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-basic-streamed/{scenario.ts => instrument.mjs} (58%) create mode 100644 dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-basic-streamed/scenario.mjs rename dev-packages/node-integration-tests/suites/tracing/http-client-spans/{fetch-strip-query/scenario.ts => fetch-basic/instrument.mjs} (55%) rename dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-basic/{scenario.ts => scenario.mjs} (55%) create mode 100644 dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-error/instrument.mjs rename dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-error/{scenario.ts => scenario.mjs} (63%) rename dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-forward-request-hook/{scenario.ts => instrument.mjs} (73%) create mode 100644 dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-forward-request-hook/scenario.mjs rename dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-headers-to-span-attributes/{scenario.ts => instrument.mjs} (66%) create mode 100644 dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-headers-to-span-attributes/scenario.mjs create mode 100644 dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-strip-query/instrument.mjs create mode 100644 dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-strip-query/scenario.mjs create mode 100644 dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-basic/instrument.mjs rename dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-basic/{scenario.ts => scenario.mjs} (63%) rename dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-otel-double-instrumentation/{scenario-mitigation.ts => instrument-mitigation.mjs} (66%) rename dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-otel-double-instrumentation/{scenario.ts => instrument.mjs} (61%) create mode 100644 dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-otel-double-instrumentation/scenario.mjs create mode 100644 dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-strip-query/instrument.mjs rename dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-strip-query/{scenario.ts => scenario.mjs} (61%) diff --git a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-basic-streamed/scenario.ts b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-basic-streamed/instrument.mjs similarity index 58% rename from dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-basic-streamed/scenario.ts rename to dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-basic-streamed/instrument.mjs index 3fe49e76fb35..b52ee5c5fdf1 100644 --- a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-basic-streamed/scenario.ts +++ b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-basic-streamed/instrument.mjs @@ -8,13 +8,3 @@ Sentry.init({ traceLifecycle: 'stream', transport: loggingTransport, }); - -async function run(): Promise { - await Sentry.startSpan({ name: 'test_transaction' }, async () => { - await fetch(`${process.env.SERVER_URL}/api/v0`); - }); - - await Sentry.flush(); -} - -void run(); diff --git a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-basic-streamed/scenario.mjs b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-basic-streamed/scenario.mjs new file mode 100644 index 000000000000..43ddd220de68 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-basic-streamed/scenario.mjs @@ -0,0 +1,11 @@ +import * as Sentry from '@sentry/node'; + +async function run() { + await Sentry.startSpan({ name: 'test_transaction' }, async () => { + await fetch(`${process.env.SERVER_URL}/api/v0`); + }); + + await Sentry.flush(); +} + +void run(); diff --git a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-basic-streamed/test.ts b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-basic-streamed/test.ts index c943957c8ae6..b9b47e0601ab 100644 --- a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-basic-streamed/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-basic-streamed/test.ts @@ -1,29 +1,38 @@ import { createTestServer } from '@sentry-internal/test-utils'; -import { expect, test } from 'vitest'; -import { createRunner } from '../../../../utils/runner'; +import { afterAll, describe, expect } from 'vitest'; +import { cleanupChildProcesses, createCjsTests } from '../../../../utils/runner'; -test('infers sentry.op for streamed outgoing fetch spans', async () => { - expect.assertions(2); +describe('streamed outgoing fetch spans', () => { + afterAll(() => { + cleanupChildProcesses(); + }); - const [SERVER_URL, closeTestServer] = await createTestServer() - .get('/api/v0', () => { - expect(true).toBe(true); - }) - .start(); + createCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => { + test('infers sentry.op for streamed outgoing fetch spans', async () => { + expect.assertions(2); - await createRunner(__dirname, 'scenario.ts') - .withEnv({ SERVER_URL }) - .expect({ - span: container => { - const httpClientSpan = container.items.find( - item => - item.attributes?.['sentry.op']?.type === 'string' && item.attributes['sentry.op'].value === 'http.client', - ); + const [SERVER_URL, closeTestServer] = await createTestServer() + .get('/api/v0', () => { + expect(true).toBe(true); + }) + .start(); - expect(httpClientSpan).toBeDefined(); - }, - }) - .start() - .completed(); - closeTestServer(); + await createRunner() + .withEnv({ SERVER_URL }) + .expect({ + span: container => { + const httpClientSpan = container.items.find( + item => + item.attributes?.['sentry.op']?.type === 'string' && + item.attributes['sentry.op'].value === 'http.client', + ); + + expect(httpClientSpan).toBeDefined(); + }, + }) + .start() + .completed(); + closeTestServer(); + }); + }); }); diff --git a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-strip-query/scenario.ts b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-basic/instrument.mjs similarity index 55% rename from dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-strip-query/scenario.ts rename to dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-basic/instrument.mjs index f2a440a34cde..46a27dd03b74 100644 --- a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-strip-query/scenario.ts +++ b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-basic/instrument.mjs @@ -7,8 +7,3 @@ Sentry.init({ tracesSampleRate: 1.0, transport: loggingTransport, }); - -// eslint-disable-next-line @typescript-eslint/no-floating-promises -Sentry.startSpan({ name: 'test_transaction' }, async () => { - await fetch(`${process.env.SERVER_URL}/api/v0/users?id=1#fragment`); -}); diff --git a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-basic/scenario.ts b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-basic/scenario.mjs similarity index 55% rename from dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-basic/scenario.ts rename to dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-basic/scenario.mjs index c3b82bc82729..b0946f4495d8 100644 --- a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-basic/scenario.ts +++ b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-basic/scenario.mjs @@ -1,12 +1,4 @@ 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, -}); // eslint-disable-next-line @typescript-eslint/no-floating-promises Sentry.startSpan({ name: 'test_transaction' }, async () => { diff --git a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-basic/test.ts b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-basic/test.ts index 1cc06ba6f21e..aaaae6345b2f 100644 --- a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-basic/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-basic/test.ts @@ -1,50 +1,58 @@ import { createTestServer } from '@sentry-internal/test-utils'; -import { expect, test } from 'vitest'; -import { createRunner } from '../../../../utils/runner'; +import { afterAll, describe, expect } from 'vitest'; +import { cleanupChildProcesses, createCjsTests } from '../../../../utils/runner'; -test('captures spans for outgoing fetch requests', async () => { - expect.assertions(3); +describe('outgoing fetch spans', () => { + afterAll(() => { + cleanupChildProcesses(); + }); - const [SERVER_URL, closeTestServer] = await createTestServer() - .get('/api/v0', () => { - // Just ensure we're called - expect(true).toBe(true); - }) - .get( - '/api/v1', - () => { - // Just ensure we're called - expect(true).toBe(true); - }, - 404, - ) - .start(); + createCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => { + test('captures spans for outgoing fetch requests', async () => { + expect.assertions(3); - await createRunner(__dirname, 'scenario.ts') - .withEnv({ SERVER_URL }) - .expect({ - transaction: { - transaction: 'test_transaction', - spans: expect.arrayContaining([ - expect.objectContaining({ - description: expect.stringMatching(/GET .*\/api\/v0/), - op: 'http.client', - origin: 'auto.http.otel.node_fetch', - status: 'ok', - }), - expect.objectContaining({ - description: expect.stringMatching(/GET .*\/api\/v1/), - op: 'http.client', - origin: 'auto.http.otel.node_fetch', - status: 'not_found', - data: expect.objectContaining({ - 'http.response.status_code': 404, - }), - }), - ]), - }, - }) - .start() - .completed(); - closeTestServer(); + const [SERVER_URL, closeTestServer] = await createTestServer() + .get('/api/v0', () => { + // Just ensure we're called + expect(true).toBe(true); + }) + .get( + '/api/v1', + () => { + // Just ensure we're called + expect(true).toBe(true); + }, + 404, + ) + .start(); + + await createRunner() + .withEnv({ SERVER_URL }) + .expect({ + transaction: { + transaction: 'test_transaction', + spans: expect.arrayContaining([ + expect.objectContaining({ + description: expect.stringMatching(/GET .*\/api\/v0/), + op: 'http.client', + origin: 'auto.http.otel.node_fetch', + status: 'ok', + }), + expect.objectContaining({ + description: expect.stringMatching(/GET .*\/api\/v1/), + op: 'http.client', + origin: 'auto.http.otel.node_fetch', + status: 'not_found', + data: expect.objectContaining({ + 'http.response.status_code': 404, + }), + }), + ]), + }, + }) + .start() + .completed(); + closeTestServer(); + }); + }); }); diff --git a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-error/instrument.mjs b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-error/instrument.mjs new file mode 100644 index 000000000000..46a27dd03b74 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-error/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/http-client-spans/fetch-error/scenario.ts b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-error/scenario.mjs similarity index 63% rename from dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-error/scenario.ts rename to dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-error/scenario.mjs index f324c3258bea..e0dcb5137651 100644 --- a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-error/scenario.ts +++ b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-error/scenario.mjs @@ -1,27 +1,22 @@ import { createServer } from 'http'; -import { loggingTransport } from '@sentry-internal/node-integration-tests'; import * as Sentry from '@sentry/node'; -Sentry.init({ - dsn: 'https://public@dsn.ingest.sentry.io/1337', - release: '1.0', - tracesSampleRate: 1.0, - transport: loggingTransport, -}); - // Bind and immediately release a port so we have an address that reliably refuses the connection. // A refused outgoing request fires the `undici:request:error` channel, exercising the error path. -function getRefusedPort(): Promise { +/** + * @returns {Promise} + */ +function getRefusedPort() { return new Promise(resolve => { const server = createServer(); server.listen(0, () => { - const { port } = server.address() as { port: number }; - server.close(() => resolve(port)); + const address = /** @type {{ port: number }} */ (server.address()); + server.close(() => resolve(address.port)); }); }); } -async function run(): Promise { +async function run() { const port = await getRefusedPort(); await Sentry.startSpan({ name: 'test_transaction' }, async () => { diff --git a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-error/test.ts b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-error/test.ts index 9afe7a471591..2e468ba52d31 100644 --- a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-error/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-error/test.ts @@ -1,21 +1,29 @@ -import { expect, test } from 'vitest'; -import { createRunner } from '../../../../utils/runner'; +import { afterAll, describe, expect } from 'vitest'; +import { cleanupChildProcesses, createCjsTests } from '../../../../utils/runner'; -test('captures an errored span for a failed outgoing fetch request', async () => { - await createRunner(__dirname, 'scenario.ts') - .expect({ - transaction: { - transaction: 'test_transaction', - spans: expect.arrayContaining([ - expect.objectContaining({ - description: expect.stringMatching(/GET http:\/\/localhost:\d+\//), - op: 'http.client', - origin: 'auto.http.otel.node_fetch', - status: 'internal_error', - }), - ]), - }, - }) - .start() - .completed(); +describe('outgoing fetch spans - error', () => { + afterAll(() => { + cleanupChildProcesses(); + }); + + createCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => { + test('captures an errored span for a failed outgoing fetch request', async () => { + await createRunner() + .expect({ + transaction: { + transaction: 'test_transaction', + spans: expect.arrayContaining([ + expect.objectContaining({ + description: expect.stringMatching(/GET http:\/\/localhost:\d+\//), + op: 'http.client', + origin: 'auto.http.otel.node_fetch', + status: 'internal_error', + }), + ]), + }, + }) + .start() + .completed(); + }); + }); }); diff --git a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-forward-request-hook/scenario.ts b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-forward-request-hook/instrument.mjs similarity index 73% rename from dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-forward-request-hook/scenario.ts rename to dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-forward-request-hook/instrument.mjs index 0843830321c4..8cb369af50dc 100644 --- a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-forward-request-hook/scenario.ts +++ b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-forward-request-hook/instrument.mjs @@ -18,9 +18,3 @@ Sentry.init({ }), ], }); - -// eslint-disable-next-line @typescript-eslint/no-floating-promises -Sentry.startSpan({ name: 'test_transaction' }, async () => { - await fetch(`${process.env.SERVER_URL}/api/v0`); - await fetch(`${process.env.SERVER_URL}/api/v1`); -}); diff --git a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-forward-request-hook/scenario.mjs b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-forward-request-hook/scenario.mjs new file mode 100644 index 000000000000..b0946f4495d8 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-forward-request-hook/scenario.mjs @@ -0,0 +1,7 @@ +import * as Sentry from '@sentry/node'; + +// eslint-disable-next-line @typescript-eslint/no-floating-promises +Sentry.startSpan({ name: 'test_transaction' }, async () => { + await fetch(`${process.env.SERVER_URL}/api/v0`); + await fetch(`${process.env.SERVER_URL}/api/v1`); +}); diff --git a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-forward-request-hook/test.ts b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-forward-request-hook/test.ts index 6092e212df08..7976ef4cd862 100644 --- a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-forward-request-hook/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-forward-request-hook/test.ts @@ -1,58 +1,66 @@ import { createTestServer } from '@sentry-internal/test-utils'; -import { expect, test } from 'vitest'; -import { createRunner } from '../../../../utils/runner'; +import { afterAll, describe, expect } from 'vitest'; +import { cleanupChildProcesses, createCjsTests } from '../../../../utils/runner'; -test('adds requestHook and responseHook attributes to spans of outgoing fetch requests', async () => { - expect.assertions(3); +describe('outgoing fetch spans - request/response hooks', () => { + afterAll(() => { + cleanupChildProcesses(); + }); - const [SERVER_URL, closeTestServer] = await createTestServer() - .get('/api/v0', () => { - // Just ensure we're called - expect(true).toBe(true); - }) - .get( - '/api/v1', - () => { - // Just ensure we're called - expect(true).toBe(true); - }, - 404, - ) - .start(); + createCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => { + test('adds requestHook and responseHook attributes to spans of outgoing fetch requests', async () => { + expect.assertions(3); - await createRunner(__dirname, 'scenario.ts') - .withEnv({ SERVER_URL }) - .expect({ - transaction: { - transaction: 'test_transaction', - spans: [ - expect.objectContaining({ - description: expect.stringMatching(/GET .*\/api\/v0/), - op: 'http.client', - origin: 'auto.http.otel.node_fetch', - status: 'ok', - data: expect.objectContaining({ - 'sentry.request.hook': '/api/v0', - 'sentry.response.hook.path': '/api/v0', - 'sentry.response.hook.status_code': 200, - }), - }), - expect.objectContaining({ - description: expect.stringMatching(/GET .*\/api\/v1/), - op: 'http.client', - origin: 'auto.http.otel.node_fetch', - status: 'not_found', - data: expect.objectContaining({ - 'sentry.request.hook': '/api/v1', - 'sentry.response.hook.path': '/api/v1', - 'sentry.response.hook.status_code': 404, - 'http.response.status_code': 404, - }), - }), - ], - }, - }) - .start() - .completed(); - closeTestServer(); + const [SERVER_URL, closeTestServer] = await createTestServer() + .get('/api/v0', () => { + // Just ensure we're called + expect(true).toBe(true); + }) + .get( + '/api/v1', + () => { + // Just ensure we're called + expect(true).toBe(true); + }, + 404, + ) + .start(); + + await createRunner() + .withEnv({ SERVER_URL }) + .expect({ + transaction: { + transaction: 'test_transaction', + spans: [ + expect.objectContaining({ + description: expect.stringMatching(/GET .*\/api\/v0/), + op: 'http.client', + origin: 'auto.http.otel.node_fetch', + status: 'ok', + data: expect.objectContaining({ + 'sentry.request.hook': '/api/v0', + 'sentry.response.hook.path': '/api/v0', + 'sentry.response.hook.status_code': 200, + }), + }), + expect.objectContaining({ + description: expect.stringMatching(/GET .*\/api\/v1/), + op: 'http.client', + origin: 'auto.http.otel.node_fetch', + status: 'not_found', + data: expect.objectContaining({ + 'sentry.request.hook': '/api/v1', + 'sentry.response.hook.path': '/api/v1', + 'sentry.response.hook.status_code': 404, + 'http.response.status_code': 404, + }), + }), + ], + }, + }) + .start() + .completed(); + closeTestServer(); + }); + }); }); diff --git a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-headers-to-span-attributes/scenario.ts b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-headers-to-span-attributes/instrument.mjs similarity index 66% rename from dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-headers-to-span-attributes/scenario.ts rename to dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-headers-to-span-attributes/instrument.mjs index ee95ba49aa1b..468b19ccd294 100644 --- a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-headers-to-span-attributes/scenario.ts +++ b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-headers-to-span-attributes/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', @@ -15,8 +15,3 @@ Sentry.init({ }), ], }); - -// eslint-disable-next-line @typescript-eslint/no-floating-promises -Sentry.startSpan({ name: 'test_transaction' }, async () => { - await fetch(`${process.env.SERVER_URL}/api/v0`, { headers: { 'x-test-header': 'test-value' } }); -}); diff --git a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-headers-to-span-attributes/scenario.mjs b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-headers-to-span-attributes/scenario.mjs new file mode 100644 index 000000000000..0edf81a9a50a --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-headers-to-span-attributes/scenario.mjs @@ -0,0 +1,6 @@ +import * as Sentry from '@sentry/node'; + +// eslint-disable-next-line @typescript-eslint/no-floating-promises +Sentry.startSpan({ name: 'test_transaction' }, async () => { + await fetch(`${process.env.SERVER_URL}/api/v0`, { headers: { 'x-test-header': 'test-value' } }); +}); diff --git a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-headers-to-span-attributes/test.ts b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-headers-to-span-attributes/test.ts index eb0ee30352ef..20ac432ca85b 100644 --- a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-headers-to-span-attributes/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-headers-to-span-attributes/test.ts @@ -1,35 +1,43 @@ import { createTestServer } from '@sentry-internal/test-utils'; -import { expect, test } from 'vitest'; -import { createRunner } from '../../../../utils/runner'; +import { afterAll, describe, expect } from 'vitest'; +import { cleanupChildProcesses, createCjsTests } from '../../../../utils/runner'; -test('maps configured request & response headers to span attributes', async () => { - expect.assertions(2); +describe('outgoing fetch spans - headers to span attributes', () => { + afterAll(() => { + cleanupChildProcesses(); + }); - const [SERVER_URL, closeTestServer] = await createTestServer() - .get('/api/v0', headers => { - expect(headers['x-test-header']).toBe('test-value'); - }) - .start(); + createCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => { + test('maps configured request & response headers to span attributes', async () => { + expect.assertions(2); - await createRunner(__dirname, 'scenario.ts') - .withEnv({ SERVER_URL }) - .expect({ - transaction: { - transaction: 'test_transaction', - spans: expect.arrayContaining([ - expect.objectContaining({ - description: expect.stringMatching(/GET .*\/api\/v0/), - op: 'http.client', - origin: 'auto.http.otel.node_fetch', - data: expect.objectContaining({ - 'http.request.header.x-test-header': ['test-value'], - 'http.response.header.x-powered-by': ['Express'], - }), - }), - ]), - }, - }) - .start() - .completed(); - closeTestServer(); + const [SERVER_URL, closeTestServer] = await createTestServer() + .get('/api/v0', headers => { + expect(headers['x-test-header']).toBe('test-value'); + }) + .start(); + + await createRunner() + .withEnv({ SERVER_URL }) + .expect({ + transaction: { + transaction: 'test_transaction', + spans: expect.arrayContaining([ + expect.objectContaining({ + description: expect.stringMatching(/GET .*\/api\/v0/), + op: 'http.client', + origin: 'auto.http.otel.node_fetch', + data: expect.objectContaining({ + 'http.request.header.x-test-header': ['test-value'], + 'http.response.header.x-powered-by': ['Express'], + }), + }), + ]), + }, + }) + .start() + .completed(); + closeTestServer(); + }); + }); }); diff --git a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-strip-query/instrument.mjs b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-strip-query/instrument.mjs new file mode 100644 index 000000000000..46a27dd03b74 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-strip-query/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/http-client-spans/fetch-strip-query/scenario.mjs b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-strip-query/scenario.mjs new file mode 100644 index 000000000000..afdb91e394d8 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-strip-query/scenario.mjs @@ -0,0 +1,6 @@ +import * as Sentry from '@sentry/node'; + +// eslint-disable-next-line @typescript-eslint/no-floating-promises +Sentry.startSpan({ name: 'test_transaction' }, async () => { + await fetch(`${process.env.SERVER_URL}/api/v0/users?id=1#fragment`); +}); diff --git a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-strip-query/test.ts b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-strip-query/test.ts index 2020cfdd09b0..0ab411783aa2 100644 --- a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-strip-query/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-strip-query/test.ts @@ -1,54 +1,62 @@ import { createTestServer } from '@sentry-internal/test-utils'; -import { expect, test } from 'vitest'; -import { createRunner } from '../../../../utils/runner'; +import { afterAll, describe, expect } from 'vitest'; +import { cleanupChildProcesses, createCjsTests } from '../../../../utils/runner'; -test('strips and handles query params in spans of outgoing fetch requests', async () => { - expect.assertions(4); +describe('outgoing fetch spans - strip query', () => { + afterAll(() => { + cleanupChildProcesses(); + }); - const [SERVER_URL, closeTestServer] = await createTestServer() - .get('/api/v0/users', () => { - // Just ensure we're called - expect(true).toBe(true); - }) - .start(); + createCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => { + test('strips and handles query params in spans of outgoing fetch requests', async () => { + expect.assertions(4); - await createRunner(__dirname, 'scenario.ts') - .withEnv({ SERVER_URL }) - .expect({ - transaction: txn => { - expect(txn.transaction).toEqual('test_transaction'); - expect(txn.spans).toHaveLength(1); - expect(txn.spans?.[0]).toMatchObject({ - data: { - url: `${SERVER_URL}/api/v0/users`, - 'url.full': `${SERVER_URL}/api/v0/users?id=1`, - 'url.path': '/api/v0/users', - 'url.query': '?id=1', - 'url.scheme': 'http', - 'http.query': 'id=1', - 'http.request.method': 'GET', - 'http.request.method_original': 'GET', - 'http.response.status_code': 200, - 'network.peer.address': '::1', - 'network.peer.port': expect.any(Number), - 'otel.kind': 'CLIENT', - 'server.port': expect.any(Number), - 'user_agent.original': 'node', - 'sentry.op': 'http.client', - 'sentry.origin': 'auto.http.otel.node_fetch', - 'server.address': 'localhost', + const [SERVER_URL, closeTestServer] = await createTestServer() + .get('/api/v0/users', () => { + // Just ensure we're called + expect(true).toBe(true); + }) + .start(); + + await createRunner() + .withEnv({ SERVER_URL }) + .expect({ + transaction: txn => { + expect(txn.transaction).toEqual('test_transaction'); + expect(txn.spans).toHaveLength(1); + expect(txn.spans?.[0]).toMatchObject({ + data: { + url: `${SERVER_URL}/api/v0/users`, + 'url.full': `${SERVER_URL}/api/v0/users?id=1`, + 'url.path': '/api/v0/users', + 'url.query': '?id=1', + 'url.scheme': 'http', + 'http.query': 'id=1', + 'http.request.method': 'GET', + 'http.request.method_original': 'GET', + 'http.response.status_code': 200, + 'network.peer.address': '::1', + 'network.peer.port': expect.any(Number), + 'otel.kind': 'CLIENT', + 'server.port': expect.any(Number), + 'user_agent.original': 'node', + 'sentry.op': 'http.client', + 'sentry.origin': 'auto.http.otel.node_fetch', + 'server.address': 'localhost', + }, + description: `GET ${SERVER_URL}/api/v0/users`, + op: 'http.client', + origin: 'auto.http.otel.node_fetch', + status: 'ok', + parent_span_id: txn.contexts?.trace?.span_id, + span_id: expect.stringMatching(/[a-f\d]{16}/), + trace_id: txn.contexts?.trace?.trace_id, + }); }, - description: `GET ${SERVER_URL}/api/v0/users`, - op: 'http.client', - origin: 'auto.http.otel.node_fetch', - status: 'ok', - parent_span_id: txn.contexts?.trace?.span_id, - span_id: expect.stringMatching(/[a-f\d]{16}/), - trace_id: txn.contexts?.trace?.trace_id, - }); - }, - }) - .start() - .completed(); - closeTestServer(); + }) + .start() + .completed(); + closeTestServer(); + }); + }); }); diff --git a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-basic/instrument.mjs b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-basic/instrument.mjs new file mode 100644 index 000000000000..46a27dd03b74 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-basic/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/http-client-spans/http-basic/scenario.ts b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-basic/scenario.mjs similarity index 63% rename from dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-basic/scenario.ts rename to dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-basic/scenario.mjs index 8e8aa1781ffd..f9e24fec6c1b 100644 --- a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-basic/scenario.ts +++ b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-basic/scenario.mjs @@ -1,14 +1,5 @@ -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, -}); - import * as http from 'http'; +import * as Sentry from '@sentry/node'; // eslint-disable-next-line @typescript-eslint/no-floating-promises Sentry.startSpan({ name: 'test_transaction' }, async () => { @@ -16,8 +7,8 @@ Sentry.startSpan({ name: 'test_transaction' }, async () => { await makeHttpRequest(`${process.env.SERVER_URL}/api/v1`); }); -function makeHttpRequest(url: string): Promise { - return new Promise(resolve => { +function makeHttpRequest(url) { + return new Promise(resolve => { http .request(url, httpRes => { httpRes.on('data', () => { diff --git a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-basic/test.ts b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-basic/test.ts index 1fea661d33e5..8d10084ce45e 100644 --- a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-basic/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-basic/test.ts @@ -1,50 +1,58 @@ import { createTestServer } from '@sentry-internal/test-utils'; -import { expect, test } from 'vitest'; -import { createRunner } from '../../../../utils/runner'; +import { afterAll, describe, expect } from 'vitest'; +import { cleanupChildProcesses, createCjsTests } from '../../../../utils/runner'; -test('captures spans for outgoing http requests', async () => { - expect.assertions(3); +describe('outgoing http spans', () => { + afterAll(() => { + cleanupChildProcesses(); + }); - const [SERVER_URL, closeTestServer] = await createTestServer() - .get('/api/v0', () => { - // Just ensure we're called - expect(true).toBe(true); - }) - .get( - '/api/v1', - () => { - // Just ensure we're called - expect(true).toBe(true); - }, - 404, - ) - .start(); + createCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => { + test('captures spans for outgoing http requests', async () => { + expect.assertions(3); - await createRunner(__dirname, 'scenario.ts') - .withEnv({ SERVER_URL }) - .expect({ - transaction: { - transaction: 'test_transaction', - spans: expect.arrayContaining([ - expect.objectContaining({ - description: expect.stringMatching(/GET .*\/api\/v0/), - op: 'http.client', - origin: 'auto.http.client', - status: 'ok', - }), - expect.objectContaining({ - description: expect.stringMatching(/GET .*\/api\/v1/), - op: 'http.client', - origin: 'auto.http.client', - status: 'not_found', - data: expect.objectContaining({ - 'http.response.status_code': 404, - }), - }), - ]), - }, - }) - .start() - .completed(); - closeTestServer(); + const [SERVER_URL, closeTestServer] = await createTestServer() + .get('/api/v0', () => { + // Just ensure we're called + expect(true).toBe(true); + }) + .get( + '/api/v1', + () => { + // Just ensure we're called + expect(true).toBe(true); + }, + 404, + ) + .start(); + + await createRunner() + .withEnv({ SERVER_URL }) + .expect({ + transaction: { + transaction: 'test_transaction', + spans: expect.arrayContaining([ + expect.objectContaining({ + description: expect.stringMatching(/GET .*\/api\/v0/), + op: 'http.client', + origin: 'auto.http.client', + status: 'ok', + }), + expect.objectContaining({ + description: expect.stringMatching(/GET .*\/api\/v1/), + op: 'http.client', + origin: 'auto.http.client', + status: 'not_found', + data: expect.objectContaining({ + 'http.response.status_code': 404, + }), + }), + ]), + }, + }) + .start() + .completed(); + closeTestServer(); + }); + }); }); diff --git a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-otel-double-instrumentation/scenario-mitigation.ts b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-otel-double-instrumentation/instrument-mitigation.mjs similarity index 66% rename from dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-otel-double-instrumentation/scenario-mitigation.ts rename to dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-otel-double-instrumentation/instrument-mitigation.mjs index decdf15a413e..5677e7eeb7ea 100644 --- a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-otel-double-instrumentation/scenario-mitigation.ts +++ b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-otel-double-instrumentation/instrument-mitigation.mjs @@ -21,20 +21,3 @@ import { HttpInstrumentation } from '@opentelemetry/instrumentation-http'; registerInstrumentations({ instrumentations: [new HttpInstrumentation()], }); - -import * as http from 'http'; - -void Sentry.startSpan({ name: 'test_transaction' }, async () => { - await makeHttpRequest(`${process.env.SERVER_URL}/api/v0`); -}); - -function makeHttpRequest(url: string): Promise { - return new Promise(resolve => { - http - .request(url, httpRes => { - httpRes.on('data', () => {}); - httpRes.on('end', resolve); - }) - .end(); - }); -} diff --git a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-otel-double-instrumentation/scenario.ts b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-otel-double-instrumentation/instrument.mjs similarity index 61% rename from dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-otel-double-instrumentation/scenario.ts rename to dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-otel-double-instrumentation/instrument.mjs index 69edce226157..019164db1338 100644 --- a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-otel-double-instrumentation/scenario.ts +++ b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-otel-double-instrumentation/instrument.mjs @@ -17,20 +17,3 @@ import { HttpInstrumentation } from '@opentelemetry/instrumentation-http'; registerInstrumentations({ instrumentations: [new HttpInstrumentation()], }); - -import * as http from 'http'; - -void Sentry.startSpan({ name: 'test_transaction' }, async () => { - await makeHttpRequest(`${process.env.SERVER_URL}/api/v0`); -}); - -function makeHttpRequest(url: string): Promise { - return new Promise(resolve => { - http - .request(url, httpRes => { - httpRes.on('data', () => {}); - httpRes.on('end', resolve); - }) - .end(); - }); -} diff --git a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-otel-double-instrumentation/scenario.mjs b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-otel-double-instrumentation/scenario.mjs new file mode 100644 index 000000000000..1a483175c29e --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-otel-double-instrumentation/scenario.mjs @@ -0,0 +1,17 @@ +import * as http from 'http'; +import * as Sentry from '@sentry/node'; + +void Sentry.startSpan({ name: 'test_transaction' }, async () => { + await makeHttpRequest(`${process.env.SERVER_URL}/api/v0`); +}); + +function makeHttpRequest(url) { + return new Promise(resolve => { + http + .request(url, httpRes => { + httpRes.on('data', () => {}); + httpRes.on('end', resolve); + }) + .end(); + }); +} diff --git a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-otel-double-instrumentation/test.ts b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-otel-double-instrumentation/test.ts index 495b6a8a17f6..8a38c4443b97 100644 --- a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-otel-double-instrumentation/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-otel-double-instrumentation/test.ts @@ -1,92 +1,102 @@ import { createTestServer } from '@sentry-internal/test-utils'; -import { expect, test } from 'vitest'; -import { createRunner } from '../../../../utils/runner'; +import { afterAll, describe, expect } from 'vitest'; +import { cleanupChildProcesses, createCjsTests } from '../../../../utils/runner'; -test('registers double spans when OTel HttpInstrumentation is also active — documents known issue', async () => { - const [SERVER_URL, closeTestServer] = await createTestServer() - .get('/api/v0', () => {}) - .start(); +describe('http OTel double instrumentation', () => { + afterAll(() => { + cleanupChildProcesses(); + }); - const runner = createRunner(__dirname, 'scenario.ts') - .withEnv({ SERVER_URL }) - .expect({ - transaction: txn => { - expect(txn.transaction).toBe('test_transaction'); + createCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => { + test('registers double spans when OTel HttpInstrumentation is also active — documents known issue', async () => { + const [SERVER_URL, closeTestServer] = await createTestServer() + .get('/api/v0', () => {}) + .start(); - const httpClientSpans = (txn.spans ?? []).filter(s => s.op === 'http.client'); + const runner = createRunner() + .withEnv({ SERVER_URL }) + .expect({ + transaction: txn => { + expect(txn.transaction).toBe('test_transaction'); - // PROBLEM: two http.client spans are produced for a single - // outgoing request when @opentelemetry/instrumentation-http runs - // alongside Sentry. - // - // - OTel's HttpInstrumentation monkey-patches http.request and - // creates an OTel span; Sentry's SentrySpanProcessor converts that - // to a Sentry span. - // - On Node >=22.12 Sentry's SentryHttpInstrumentation additionally - // subscribes to the http.client.request.created diagnostic channel. - // The channel fires inside OTel's already-patched http.request, - // triggering Sentry to create a *second* http.client span as a - // child of the first. - // - On Node <22.12 both instrumentations monkey-patch http.request, - // so both wrappers fire and each creates its own span. - // - // MITIGATION: pass `spans: false` to httpIntegration() so Sentry - // defers all outgoing span creation to OTel's HttpInstrumentation - // (whose spans Sentry already captures via SentrySpanProcessor). - // - // See the 'mitigation' scenario alongside this test. - expect(httpClientSpans).toHaveLength(2); + const httpClientSpans = (txn.spans ?? []).filter(s => s.op === 'http.client'); - // The outer span comes from OTel HttpInstrumentation (no Sentry - // origin). The inner span is the one Sentry's own handler creates; - // it is a *child* of the outer span with origin 'auto.http.client'. - const sentrySpan = httpClientSpans.find(s => s.data?.['sentry.origin'] === 'auto.http.client'); - const otelSpan = httpClientSpans.find(s => s.data?.['sentry.origin'] !== 'auto.http.client'); + // PROBLEM: two http.client spans are produced for a single + // outgoing request when @opentelemetry/instrumentation-http runs + // alongside Sentry. + // + // - OTel's HttpInstrumentation monkey-patches http.request and + // creates an OTel span; Sentry's SentrySpanProcessor converts that + // to a Sentry span. + // - On Node >=22.12 Sentry's SentryHttpInstrumentation additionally + // subscribes to the http.client.request.created diagnostic channel. + // The channel fires inside OTel's already-patched http.request, + // triggering Sentry to create a *second* http.client span as a + // child of the first. + // - On Node <22.12 both instrumentations monkey-patch http.request, + // so both wrappers fire and each creates its own span. + // + // MITIGATION: pass `spans: false` to httpIntegration() so Sentry + // defers all outgoing span creation to OTel's HttpInstrumentation + // (whose spans Sentry already captures via SentrySpanProcessor). + // + // See the 'mitigation' scenario alongside this test. + expect(httpClientSpans).toHaveLength(2); - expect(sentrySpan).toBeDefined(); - expect(otelSpan).toBeDefined(); + // The outer span comes from OTel HttpInstrumentation (no Sentry + // origin). The inner span is the one Sentry's own handler creates; + // it is a *child* of the outer span with origin 'auto.http.client'. + const sentrySpan = httpClientSpans.find(s => s.data?.['sentry.origin'] === 'auto.http.client'); + const otelSpan = httpClientSpans.find(s => s.data?.['sentry.origin'] !== 'auto.http.client'); - // the sentry-created span is nested inside the otel-created span. - expect(sentrySpan!.parent_span_id).toBe(otelSpan!.span_id); - }, - }) - .start(); + expect(sentrySpan).toBeDefined(); + expect(otelSpan).toBeDefined(); - await runner.completed(); + // the sentry-created span is nested inside the otel-created span. + expect(sentrySpan!.parent_span_id).toBe(otelSpan!.span_id); + }, + }) + .start(); - // The double-wrap warning should have been logged to stderr (via debug.warn) - // since scenario.ts initialises Sentry with debug: true. - const logs = runner.getLogs(); - expect(logs.some(l => l.includes('Double-wrapped http.client detected'))).toBe(true); + await runner.completed(); - closeTestServer(); -}); + // The double-wrap warning should have been logged to stderr (via debug.warn) + // since instrument.mjs initialises Sentry with debug: true. + const logs = runner.getLogs(); + expect(logs.some(l => l.includes('Double-wrapped http.client detected'))).toBe(true); + + closeTestServer(); + }); + }); -test('mitigation: spans: false on httpIntegration prevents double-instrumentation', async () => { - const [SERVER_URL, closeTestServer] = await createTestServer() - .get('/api/v0', () => {}) - .start(); + createCjsTests(__dirname, 'scenario.mjs', 'instrument-mitigation.mjs', (createRunner, test) => { + test('mitigation: spans: false on httpIntegration prevents double-instrumentation', async () => { + const [SERVER_URL, closeTestServer] = await createTestServer() + .get('/api/v0', () => {}) + .start(); - await createRunner(__dirname, 'scenario-mitigation.ts') - .withEnv({ SERVER_URL }) - .expect({ - transaction: txn => { - expect(txn.transaction).toBe('test_transaction'); + await createRunner() + .withEnv({ SERVER_URL }) + .expect({ + transaction: txn => { + expect(txn.transaction).toBe('test_transaction'); - const httpClientSpans = (txn.spans ?? []).filter(s => s.op === 'http.client'); - // With spans: false in httpIntegration(), Sentry does not create its - // own span. OTel's HttpInstrumentation still creates one, which - // flows through SentrySpanProcessor, so there is exactly one - // http.client span. - expect(httpClientSpans).toHaveLength(1); - expect(httpClientSpans[0]).toMatchObject({ - description: expect.stringMatching(/GET .*\/api\/v0/), - status: 'ok', - }); - }, - }) - .start() - .completed(); + const httpClientSpans = (txn.spans ?? []).filter(s => s.op === 'http.client'); + // With spans: false in httpIntegration(), Sentry does not create its + // own span. OTel's HttpInstrumentation still creates one, which + // flows through SentrySpanProcessor, so there is exactly one + // http.client span. + expect(httpClientSpans).toHaveLength(1); + expect(httpClientSpans[0]).toMatchObject({ + description: expect.stringMatching(/GET .*\/api\/v0/), + status: 'ok', + }); + }, + }) + .start() + .completed(); - closeTestServer(); + closeTestServer(); + }); + }); }); diff --git a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-strip-query/instrument.mjs b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-strip-query/instrument.mjs new file mode 100644 index 000000000000..46a27dd03b74 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-strip-query/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/http-client-spans/http-strip-query/scenario.ts b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-strip-query/scenario.mjs similarity index 61% rename from dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-strip-query/scenario.ts rename to dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-strip-query/scenario.mjs index 6e978ce014b9..5acb4ec7f4bf 100644 --- a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-strip-query/scenario.ts +++ b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-strip-query/scenario.mjs @@ -1,22 +1,13 @@ -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, -}); - import * as http from 'http'; +import * as Sentry from '@sentry/node'; // eslint-disable-next-line @typescript-eslint/no-floating-promises Sentry.startSpan({ name: 'test_transaction' }, async () => { await makeHttpRequest(`${process.env.SERVER_URL}/api/v0/users?id=1#fragment`); }); -function makeHttpRequest(url: string): Promise { - return new Promise(resolve => { +function makeHttpRequest(url) { + return new Promise(resolve => { http .request(url, httpRes => { httpRes.on('data', () => { diff --git a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-strip-query/test.ts b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-strip-query/test.ts index 60add149deab..6153a01c7e7f 100644 --- a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-strip-query/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-strip-query/test.ts @@ -1,56 +1,64 @@ import { createTestServer } from '@sentry-internal/test-utils'; -import { expect, test } from 'vitest'; -import { createRunner } from '../../../../utils/runner'; +import { afterAll, describe, expect } from 'vitest'; +import { cleanupChildProcesses, createCjsTests } from '../../../../utils/runner'; -test('strips and handles query params in spans of outgoing http requests', async () => { - expect.assertions(4); +describe('outgoing http spans - strip query', () => { + afterAll(() => { + cleanupChildProcesses(); + }); - const [SERVER_URL, closeTestServer] = await createTestServer() - .get('/api/v0/users', () => { - // Just ensure we're called - expect(true).toBe(true); - }) - .start(); + createCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => { + test('strips and handles query params in spans of outgoing http requests', async () => { + expect.assertions(4); - await createRunner(__dirname, 'scenario.ts') - .withEnv({ SERVER_URL }) - .expect({ - transaction: txn => { - expect(txn.transaction).toEqual('test_transaction'); - expect(txn.spans).toHaveLength(1); - expect(txn.spans?.[0]).toMatchObject({ - data: { - url: `${SERVER_URL}/api/v0/users`, - 'http.url': `${SERVER_URL}/api/v0/users?id=1`, - 'http.target': '/api/v0/users?id=1', - 'http.flavor': '1.1', - 'http.host': expect.stringMatching(/localhost:\d+$/), - 'http.method': 'GET', - 'http.query': 'id=1', - 'http.response.status_code': 200, - 'http.response_content_length_uncompressed': 0, - 'http.status_code': 200, - 'http.status_text': 'OK', - 'net.peer.ip': '::1', - 'net.peer.name': 'localhost', - 'net.peer.port': expect.any(Number), - 'net.transport': 'ip_tcp', - 'otel.kind': 'CLIENT', - 'sentry.op': 'http.client', - 'sentry.origin': 'auto.http.client', + const [SERVER_URL, closeTestServer] = await createTestServer() + .get('/api/v0/users', () => { + // Just ensure we're called + expect(true).toBe(true); + }) + .start(); + + await createRunner() + .withEnv({ SERVER_URL }) + .expect({ + transaction: txn => { + expect(txn.transaction).toEqual('test_transaction'); + expect(txn.spans).toHaveLength(1); + expect(txn.spans?.[0]).toMatchObject({ + data: { + url: `${SERVER_URL}/api/v0/users`, + 'http.url': `${SERVER_URL}/api/v0/users?id=1`, + 'http.target': '/api/v0/users?id=1', + 'http.flavor': '1.1', + 'http.host': expect.stringMatching(/localhost:\d+$/), + 'http.method': 'GET', + 'http.query': 'id=1', + 'http.response.status_code': 200, + 'http.response_content_length_uncompressed': 0, + 'http.status_code': 200, + 'http.status_text': 'OK', + 'net.peer.ip': '::1', + 'net.peer.name': 'localhost', + 'net.peer.port': expect.any(Number), + 'net.transport': 'ip_tcp', + 'otel.kind': 'CLIENT', + 'sentry.op': 'http.client', + 'sentry.origin': 'auto.http.client', + }, + description: `GET ${SERVER_URL}/api/v0/users`, + op: 'http.client', + origin: 'auto.http.client', + status: 'ok', + parent_span_id: txn.contexts?.trace?.span_id, + span_id: expect.stringMatching(/[a-f\d]{16}/), + trace_id: txn.contexts?.trace?.trace_id, + }); }, - description: `GET ${SERVER_URL}/api/v0/users`, - op: 'http.client', - origin: 'auto.http.client', - status: 'ok', - parent_span_id: txn.contexts?.trace?.span_id, - span_id: expect.stringMatching(/[a-f\d]{16}/), - trace_id: txn.contexts?.trace?.trace_id, - }); - }, - }) - .start() - .completed(); + }) + .start() + .completed(); - closeTestServer(); + closeTestServer(); + }); + }); });