From 65b117c7f1d4af352850abcbe2bdfe5f596a3bf7 Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Tue, 8 Sep 2026 10:34:36 +0200 Subject: [PATCH] test(node): Port express tracing suites to span streaming Rewrite the transaction assertions against span v2 containers. Status-code filtering is a no-op under streaming, so its scenario is removed and the 4xx routes are asserted on the main scenario instead. The isolationScope scenario switches from tags to scope attributes, and the tracesSampler matches on url.path since the initial name is just the method. Refs #24136 Co-Authored-By: Claude Fable 5.1 --- .../express/ignore-layers-type/instrument.mjs | 1 - .../suites/express/ignore-layers-type/test.ts | 29 +- .../express/multiple-routers/instrument.mjs | 1 - .../suites/express/multiple-routers/test.ts | 50 +- .../span-isolationScope/instrument.mjs | 1 - .../express/span-isolationScope/scenario.mjs | 8 +- .../express/span-isolationScope/test.ts | 35 +- .../tracing/instrument-filterStatusCode.mjs | 15 - .../suites/express/tracing/instrument.mjs | 2 +- .../tracing/scenario-filterStatusCode.mjs | 34 -- .../suites/express/tracing/test.ts | 466 ++++++++---------- .../instrument-normalized-request.mjs | 1 - .../tracing/tracesSampler/instrument.mjs | 10 +- .../express/tracing/tracesSampler/test.ts | 10 +- .../express/tracing/updateName/instrument.mjs | 1 - .../suites/express/tracing/updateName/test.ts | 55 +-- .../express/with-http/base/instrument.mjs | 1 - .../suites/express/with-http/base/test.ts | 18 +- 18 files changed, 306 insertions(+), 432 deletions(-) delete mode 100644 dev-packages/node-integration-tests/suites/express/tracing/instrument-filterStatusCode.mjs delete mode 100644 dev-packages/node-integration-tests/suites/express/tracing/scenario-filterStatusCode.mjs diff --git a/dev-packages/node-integration-tests/suites/express/ignore-layers-type/instrument.mjs b/dev-packages/node-integration-tests/suites/express/ignore-layers-type/instrument.mjs index d16ee8ded955..8bce5fc7a3d3 100644 --- a/dev-packages/node-integration-tests/suites/express/ignore-layers-type/instrument.mjs +++ b/dev-packages/node-integration-tests/suites/express/ignore-layers-type/instrument.mjs @@ -2,7 +2,6 @@ import * as Sentry from '@sentry/node'; import { loggingTransport } from '@sentry-internal/node-integration-tests'; Sentry.init({ - traceLifecycle: 'static', dsn: 'https://public@dsn.ingest.sentry.io/1337', release: '1.0', tracesSampleRate: 1.0, diff --git a/dev-packages/node-integration-tests/suites/express/ignore-layers-type/test.ts b/dev-packages/node-integration-tests/suites/express/ignore-layers-type/test.ts index f4dc8213e197..a4fd1c6731b9 100644 --- a/dev-packages/node-integration-tests/suites/express/ignore-layers-type/test.ts +++ b/dev-packages/node-integration-tests/suites/express/ignore-layers-type/test.ts @@ -1,5 +1,4 @@ import { afterAll, describe, expect } from 'vitest'; -import { assertSentryTransaction } from '../../../utils/assertions'; import { cleanupChildProcesses, createEsmAndCjsTests } from '../../../utils/runner'; describe('express ignoreLayersType', () => { @@ -11,28 +10,26 @@ describe('express ignoreLayersType', () => { test('suppresses spans for layer types listed in ignoreLayersType', async () => { const runner = createRunner() .expect({ - transaction: transaction => { - assertSentryTransaction(transaction, { - transaction: 'GET /test/express', - contexts: { - trace: { - op: 'http.server', - status: 'ok', - }, - }, + span: container => { + expect(container.items.find(item => item.is_segment)).toMatchObject({ + name: 'GET /test/express', + status: 'ok', + attributes: expect.objectContaining({ + 'sentry.op': { type: 'string', value: 'http.server' }, + }), }); - expect(transaction.spans).toContainEqual( + expect(container.items).toContainEqual( expect.objectContaining({ - data: expect.objectContaining({ - 'express.type': 'request_handler', + attributes: expect.objectContaining({ + 'express.type': { type: 'string', value: 'request_handler' }, }), }), ); // The cors() middleware span is suppressed by ignoreLayersType: ['middleware']. - expect(transaction.spans).not.toContainEqual( + expect(container.items).not.toContainEqual( expect.objectContaining({ - data: expect.objectContaining({ - 'express.type': 'middleware', + attributes: expect.objectContaining({ + 'express.type': { type: 'string', value: 'middleware' }, }), }), ); diff --git a/dev-packages/node-integration-tests/suites/express/multiple-routers/instrument.mjs b/dev-packages/node-integration-tests/suites/express/multiple-routers/instrument.mjs index 170ad6f6a702..46a27dd03b74 100644 --- a/dev-packages/node-integration-tests/suites/express/multiple-routers/instrument.mjs +++ b/dev-packages/node-integration-tests/suites/express/multiple-routers/instrument.mjs @@ -2,7 +2,6 @@ import * as Sentry from '@sentry/node'; import { loggingTransport } from '@sentry-internal/node-integration-tests'; Sentry.init({ - traceLifecycle: 'static', dsn: 'https://public@dsn.ingest.sentry.io/1337', release: '1.0', tracesSampleRate: 1.0, diff --git a/dev-packages/node-integration-tests/suites/express/multiple-routers/test.ts b/dev-packages/node-integration-tests/suites/express/multiple-routers/test.ts index 4be3172ef2c2..062609aaf48f 100644 --- a/dev-packages/node-integration-tests/suites/express/multiple-routers/test.ts +++ b/dev-packages/node-integration-tests/suites/express/multiple-routers/test.ts @@ -1,4 +1,4 @@ -import { afterAll, describe } from 'vitest'; +import { afterAll, describe, expect } from 'vitest'; import { cleanupChildProcesses, createCjsTests } from '../../../utils/runner'; describe('express multiple routers', () => { @@ -9,7 +9,7 @@ describe('express multiple routers', () => { createCjsTests(__dirname, 'scenario-common-infix.mjs', 'instrument.mjs', (createRunner, test) => { test('should construct correct url with common infixes with multiple routers.', async () => { const runner = createRunner() - .ignore('transaction') + .ignore('span') .expect({ event: { message: 'Custom Message', transaction: 'GET /api2/v1/test' } }) .start(); runner.makeRequest('get', '/api2/v1/test'); @@ -20,7 +20,7 @@ describe('express multiple routers', () => { createCjsTests(__dirname, 'scenario-common-infix-parameterized.mjs', 'instrument.mjs', (createRunner, test) => { test('should construct correct url with common infixes with multiple parameterized routers.', async () => { const runner = createRunner() - .ignore('transaction') + .ignore('span') .expect({ event: { message: 'Custom Message', transaction: 'GET /api/v1/user/:userId' } }) .start(); runner.makeRequest('get', '/api/v1/user/3212'); @@ -31,7 +31,7 @@ describe('express multiple routers', () => { createCjsTests(__dirname, 'scenario-common-prefix.mjs', 'instrument.mjs', (createRunner, test) => { test('should construct correct urls with multiple routers.', async () => { const runner = createRunner() - .ignore('transaction') + .ignore('span') .expect({ event: { message: 'Custom Message', transaction: 'GET /api/v1/test' } }) .start(); runner.makeRequest('get', '/api/v1/test'); @@ -40,7 +40,7 @@ describe('express multiple routers', () => { test('should construct correct urls with multiple parameterized routers.', async () => { const runner = createRunner() - .ignore('transaction') + .ignore('span') .expect({ event: { message: 'Custom Message', transaction: 'GET /api/v1/user/:userId' } }) .start(); runner.makeRequest('get', '/api/v1/user/1234/'); @@ -51,7 +51,7 @@ describe('express multiple routers', () => { createCjsTests(__dirname, 'scenario-common-prefix-reverse.mjs', 'instrument.mjs', (createRunner, test) => { test('should construct correct urls with multiple parameterized routers (use order reversed).', async () => { const runner = createRunner() - .ignore('transaction') + .ignore('span') .expect({ event: { message: 'Custom Message', transaction: 'GET /api/v1/user/:userId' } }) .start(); runner.makeRequest('get', '/api/v1/user/1234/'); @@ -62,7 +62,7 @@ describe('express multiple routers', () => { createCjsTests(__dirname, 'scenario-common-prefix-same-length.mjs', 'instrument.mjs', (createRunner, test) => { test('should construct correct url with multiple parameterized routers of the same length.', async () => { const runner = createRunner() - .ignore('transaction') + .ignore('span') .expect({ event: { message: 'Custom Message', transaction: 'GET /api/v1/:userId' } }) .start(); runner.makeRequest('get', '/api/v1/1234/'); @@ -76,11 +76,10 @@ describe('express multiple routers', () => { const runner = createRunner() .ignore('event') .expect({ - transaction: { - transaction: 'GET /api/api/v1/sub-router/users/:userId/posts/:postId', - transaction_info: { - source: 'route', - }, + span: container => { + const serverSpan = container.items.find(item => item.is_segment); + expect(serverSpan?.name).toBe('GET /api/api/v1/sub-router/users/:userId/posts/:postId'); + expect(serverSpan?.attributes['sentry.segment.name.source']).toEqual({ type: 'string', value: 'route' }); }, }) .start(); @@ -92,11 +91,10 @@ describe('express multiple routers', () => { const runner = createRunner() .ignore('event') .expect({ - transaction: { - transaction: 'GET /api/api/v1/sub-router/users/:userId/posts/:postId', - transaction_info: { - source: 'route', - }, + span: container => { + const serverSpan = container.items.find(item => item.is_segment); + expect(serverSpan?.name).toBe('GET /api/api/v1/sub-router/users/:userId/posts/:postId'); + expect(serverSpan?.attributes['sentry.segment.name.source']).toEqual({ type: 'string', value: 'route' }); }, }) .start(); @@ -108,11 +106,10 @@ describe('express multiple routers', () => { const runner = createRunner() .ignore('event') .expect({ - transaction: { - transaction: 'GET /api/api/v1/sub-router/users/:userId/posts/:postId', - transaction_info: { - source: 'route', - }, + span: container => { + const serverSpan = container.items.find(item => item.is_segment); + expect(serverSpan?.name).toBe('GET /api/api/v1/sub-router/users/:userId/posts/:postId'); + expect(serverSpan?.attributes['sentry.segment.name.source']).toEqual({ type: 'string', value: 'route' }); }, }) .start(); @@ -129,11 +126,10 @@ describe('express multiple routers', () => { const runner = createRunner() .ignore('event') .expect({ - transaction: { - transaction: 'GET /api/v1/users/:userId/posts/:postId', - transaction_info: { - source: 'route', - }, + span: container => { + const serverSpan = container.items.find(item => item.is_segment); + expect(serverSpan?.name).toBe('GET /api/v1/users/:userId/posts/:postId'); + expect(serverSpan?.attributes['sentry.segment.name.source']).toEqual({ type: 'string', value: 'route' }); }, }) .start(); diff --git a/dev-packages/node-integration-tests/suites/express/span-isolationScope/instrument.mjs b/dev-packages/node-integration-tests/suites/express/span-isolationScope/instrument.mjs index 170ad6f6a702..46a27dd03b74 100644 --- a/dev-packages/node-integration-tests/suites/express/span-isolationScope/instrument.mjs +++ b/dev-packages/node-integration-tests/suites/express/span-isolationScope/instrument.mjs @@ -2,7 +2,6 @@ import * as Sentry from '@sentry/node'; import { loggingTransport } from '@sentry-internal/node-integration-tests'; Sentry.init({ - traceLifecycle: 'static', dsn: 'https://public@dsn.ingest.sentry.io/1337', release: '1.0', tracesSampleRate: 1.0, diff --git a/dev-packages/node-integration-tests/suites/express/span-isolationScope/scenario.mjs b/dev-packages/node-integration-tests/suites/express/span-isolationScope/scenario.mjs index 328945cd6714..7545550ebc54 100644 --- a/dev-packages/node-integration-tests/suites/express/span-isolationScope/scenario.mjs +++ b/dev-packages/node-integration-tests/suites/express/span-isolationScope/scenario.mjs @@ -4,13 +4,11 @@ import express from 'express'; const app = express(); -Sentry.setTag('global', 'tag'); +Sentry.setAttribute('global', 'attribute'); app.get('/test/isolationScope', (_req, res) => { - // eslint-disable-next-line no-console - console.log('This is a test log.'); - Sentry.addBreadcrumb({ message: 'manual breadcrumb' }); - Sentry.setTag('isolation-scope', 'tag'); + Sentry.setAttribute('isolation-scope', 'attribute'); + Sentry.setUser({ id: 'user-1' }); res.send({}); }); diff --git a/dev-packages/node-integration-tests/suites/express/span-isolationScope/test.ts b/dev-packages/node-integration-tests/suites/express/span-isolationScope/test.ts index b4d03f1cad1e..4eb018fd7da2 100644 --- a/dev-packages/node-integration-tests/suites/express/span-isolationScope/test.ts +++ b/dev-packages/node-integration-tests/suites/express/span-isolationScope/test.ts @@ -10,30 +10,17 @@ describe('express span isolationScope', () => { test('correctly applies isolation scope to span', async () => { const runner = createRunner() .expect({ - transaction: { - transaction: 'GET /test/isolationScope', - breadcrumbs: [ - { - category: 'console', - level: 'log', - message: expect.stringMatching(/\{"port":(\d+)\}/), - timestamp: expect.any(Number), - }, - { - category: 'console', - level: 'log', - message: 'This is a test log.', - timestamp: expect.any(Number), - }, - { - message: 'manual breadcrumb', - timestamp: expect.any(Number), - }, - ], - tags: { - global: 'tag', - 'isolation-scope': 'tag', - }, + span: container => { + const serverSpan = container.items.find(item => item.is_segment); + + expect(serverSpan).toMatchObject({ + name: 'GET /test/isolationScope', + attributes: expect.objectContaining({ + global: { type: 'string', value: 'attribute' }, + 'isolation-scope': { type: 'string', value: 'attribute' }, + 'user.id': { type: 'string', value: 'user-1' }, + }), + }); }, }) .start(); diff --git a/dev-packages/node-integration-tests/suites/express/tracing/instrument-filterStatusCode.mjs b/dev-packages/node-integration-tests/suites/express/tracing/instrument-filterStatusCode.mjs deleted file mode 100644 index 6638b7e92cb8..000000000000 --- a/dev-packages/node-integration-tests/suites/express/tracing/instrument-filterStatusCode.mjs +++ /dev/null @@ -1,15 +0,0 @@ -import * as Sentry from '@sentry/node'; -import { loggingTransport } from '@sentry-internal/node-integration-tests'; - -Sentry.init({ - traceLifecycle: 'static', - dsn: 'https://public@dsn.ingest.sentry.io/1337', - release: '1.0', - tracesSampleRate: 1.0, - transport: loggingTransport, - integrations: [ - Sentry.httpIntegration({ - ignoreStatusCodes: [499, [300, 399]], - }), - ], -}); diff --git a/dev-packages/node-integration-tests/suites/express/tracing/instrument.mjs b/dev-packages/node-integration-tests/suites/express/tracing/instrument.mjs index 2a81e8aef848..ab16bd3acefb 100644 --- a/dev-packages/node-integration-tests/suites/express/tracing/instrument.mjs +++ b/dev-packages/node-integration-tests/suites/express/tracing/instrument.mjs @@ -2,7 +2,7 @@ import * as Sentry from '@sentry/node'; import { loggingTransport } from '@sentry-internal/node-integration-tests'; Sentry.init({ - traceLifecycle: process.env.STREAMED === 'true' ? 'stream' : 'static', + traceLifecycle: 'stream', dsn: 'https://public@dsn.ingest.sentry.io/1337', release: '1.0', // disable attaching headers to /test/* endpoints diff --git a/dev-packages/node-integration-tests/suites/express/tracing/scenario-filterStatusCode.mjs b/dev-packages/node-integration-tests/suites/express/tracing/scenario-filterStatusCode.mjs deleted file mode 100644 index 17dadc5d8e57..000000000000 --- a/dev-packages/node-integration-tests/suites/express/tracing/scenario-filterStatusCode.mjs +++ /dev/null @@ -1,34 +0,0 @@ -import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests'; -import express from 'express'; - -const app = express(); - -app.get('/', (_req, res) => { - res.send({ response: 'response 0' }); -}); - -app.get('/401', (_req, res) => { - res.status(401).send({ response: 'response 401' }); -}); - -app.get('/402', (_req, res) => { - res.status(402).send({ response: 'response 402' }); -}); - -app.get('/403', (_req, res) => { - res.status(403).send({ response: 'response 403' }); -}); - -app.get('/499', (_req, res) => { - res.status(499).send({ response: 'response 499' }); -}); - -app.get('/300', (_req, res) => { - res.status(300).send({ response: 'response 300' }); -}); - -app.get('/399', (_req, res) => { - res.status(399).send({ response: 'response 399' }); -}); - -startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/express/tracing/test.ts b/dev-packages/node-integration-tests/suites/express/tracing/test.ts index 8f86f80bbd04..a7857a4755cc 100644 --- a/dev-packages/node-integration-tests/suites/express/tracing/test.ts +++ b/dev-packages/node-integration-tests/suites/express/tracing/test.ts @@ -1,49 +1,62 @@ +import type { SerializedStreamedSpan, SerializedStreamedSpanContainer } from '@sentry/core'; import { afterAll, describe, expect } from 'vitest'; -import { assertSentryTransaction } from '../../../utils/assertions'; import { cleanupChildProcesses, createEsmAndCjsTests } from '../../../utils/runner'; +function findSegmentSpan(container: SerializedStreamedSpanContainer): SerializedStreamedSpan | undefined { + return container.items.find(item => item.is_segment); +} + +function findExpressSpan(container: SerializedStreamedSpanContainer, type: string): SerializedStreamedSpan | undefined { + return container.items.find(item => item.attributes['express.type']?.value === type); +} + describe('express tracing', () => { afterAll(() => { cleanupChildProcesses(); }); createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => { - test('should create and send transactions for Express routes and spans for middlewares.', async () => { + test('should create and send segment spans for Express routes and spans for middlewares.', async () => { const runner = createRunner() .expect({ - transaction: { - contexts: { - trace: { - span_id: expect.stringMatching(/[a-f\d]{16}/), - trace_id: expect.stringMatching(/[a-f\d]{32}/), - data: { - 'url.full': expect.stringMatching(/\/test\/express$/), - 'http.response.status_code': 200, - }, - op: 'http.server', - status: 'ok', - }, - }, - spans: expect.arrayContaining([ + span: container => { + const serverSpan = findSegmentSpan(container); + + expect(serverSpan).toMatchObject({ + name: 'GET /test/express', + span_id: expect.stringMatching(/[a-f\d]{16}/), + trace_id: expect.stringMatching(/[a-f\d]{32}/), + status: 'ok', + attributes: expect.objectContaining({ + 'sentry.op': { type: 'string', value: 'http.server' }, + 'url.full': { type: 'string', value: expect.stringMatching(/\/test\/express$/) }, + 'http.response.status_code': { type: 'integer', value: 200 }, + }), + }); + + expect(container.items).toContainEqual( expect.objectContaining({ - data: expect.objectContaining({ - 'express.name': 'corsMiddleware', - 'express.type': 'middleware', + name: 'corsMiddleware', + attributes: expect.objectContaining({ + 'express.name': { type: 'string', value: 'corsMiddleware' }, + 'express.type': { type: 'string', value: 'middleware' }, + 'sentry.op': { type: 'string', value: 'middleware' }, + 'sentry.origin': { type: 'string', value: 'auto.http.express' }, }), - description: 'corsMiddleware', - op: 'middleware', - origin: 'auto.http.express', }), + ); + + expect(container.items).toContainEqual( expect.objectContaining({ - data: expect.objectContaining({ - 'express.name': '/test/express', - 'express.type': 'request_handler', + name: '/test/express', + attributes: expect.objectContaining({ + 'express.name': { type: 'string', value: '/test/express' }, + 'express.type': { type: 'string', value: 'request_handler' }, + 'sentry.op': { type: 'string', value: 'handler' }, + 'sentry.origin': { type: 'string', value: 'auto.http.express' }, }), - description: '/test/express', - op: 'handler', - origin: 'auto.http.express', }), - ]), + ); }, }) .start(); @@ -51,21 +64,17 @@ describe('express tracing', () => { await runner.completed(); }); - test('names router and request handler spans after their route when span streaming is enabled', async () => { + test('names router and request handler spans after their route', async () => { const runner = createRunner() - .withEnv({ STREAMED: 'true' }) .expect({ span: container => { - const spanFor = (type: string): (typeof container.items)[number] | undefined => - container.items.find(item => item.attributes['express.type']?.value === type); - - const handlerSpan = spanFor('request_handler'); + const handlerSpan = findExpressSpan(container, 'request_handler'); expect(handlerSpan?.name).toBe('/test/router/user/:id'); // The name has to stay in step with the attribute it comes from. expect(handlerSpan?.attributes['http.route']?.value).toBe('/test/router/user/:id'); expect(handlerSpan?.attributes['sentry.op']?.value).toBe('handler'); - const routerSpan = spanFor('router'); + const routerSpan = findExpressSpan(container, 'router'); expect(routerSpan?.name).toBe('/test/router/user'); // Spans of other layer types keep their names. @@ -82,26 +91,22 @@ describe('express tracing', () => { await runner.completed(); }); - test('should set a correct transaction name for routes specified in RegEx', async () => { + test('should set a correct segment name for routes specified in RegEx', async () => { const runner = createRunner() .expect({ - transaction: { - transaction: 'GET /\\/test\\/regex/', - transaction_info: { - source: 'route', - }, - contexts: { - trace: { - trace_id: expect.stringMatching(/[a-f\d]{32}/), - span_id: expect.stringMatching(/[a-f\d]{16}/), - data: { - 'url.full': expect.stringMatching(/\/test\/regex$/), - 'http.response.status_code': 200, - }, - op: 'http.server', - status: 'ok', - }, - }, + span: container => { + expect(findSegmentSpan(container)).toMatchObject({ + name: 'GET /\\/test\\/regex/', + span_id: expect.stringMatching(/[a-f\d]{16}/), + trace_id: expect.stringMatching(/[a-f\d]{32}/), + status: 'ok', + attributes: expect.objectContaining({ + 'sentry.op': { type: 'string', value: 'http.server' }, + 'sentry.segment.name.source': { type: 'string', value: 'route' }, + 'url.full': { type: 'string', value: expect.stringMatching(/\/test\/regex$/) }, + 'http.response.status_code': { type: 'integer', value: 200 }, + }), + }); }, }) .start(); @@ -112,12 +117,11 @@ describe('express tracing', () => { test('nests a sub-router route handler span under the router span', async () => { const runner = createRunner() .expect({ - transaction: transaction => { - expect(transaction.transaction).toBe('GET /test/router/user/:id'); + span: container => { + expect(findSegmentSpan(container)?.name).toBe('GET /test/router/user/:id'); - const spans = transaction.spans || []; - const routerSpan = spans.find(span => span.data?.['express.type'] === 'router'); - const handlerSpan = spans.find(span => span.data?.['express.type'] === 'request_handler'); + const routerSpan = findExpressSpan(container, 'router'); + const handlerSpan = findExpressSpan(container, 'request_handler'); expect(routerSpan).toBeDefined(); expect(handlerSpan).toBeDefined(); @@ -126,7 +130,7 @@ describe('express tracing', () => { expect(handlerSpan?.parent_span_id).toBe(routerSpan?.span_id); // The handler delays its response by ~100ms (see scenario). - const routerDurationMs = ((routerSpan?.timestamp ?? 0) - (routerSpan?.start_timestamp ?? 0)) * 1000; + const routerDurationMs = ((routerSpan?.end_timestamp ?? 0) - (routerSpan?.start_timestamp ?? 0)) * 1000; // The router span stays open until the response finishes, so it spans the // whole sub-stack it dispatched (~the 100ms handler delay). @@ -141,13 +145,12 @@ describe('express tracing', () => { test('keeps the parameter in a route mounted under a parameterized sub-router path', async () => { const runner = createRunner() .expect({ - transaction: { + span: container => { + const serverSpan = findSegmentSpan(container); // The `:version` parameter must be preserved — using the concrete value // (`/test/version/v1/user`) would explode route cardinality. - transaction: 'GET /test/version/:version/user', - transaction_info: { - source: 'route', - }, + expect(serverSpan?.name).toBe('GET /test/version/:version/user'); + expect(serverSpan?.attributes['sentry.segment.name.source']).toEqual({ type: 'string', value: 'route' }); }, }) .start(); @@ -158,23 +161,21 @@ describe('express tracing', () => { test('handles root page correctly', async () => { const runner = createRunner() .expect({ - transaction: { - transaction: 'GET /', - contexts: { - trace: { - span_id: expect.stringMatching(/[a-f\d]{16}/), - trace_id: expect.stringMatching(/[a-f\d]{32}/), - data: { - 'http.response.status_code': 200, - 'http.request.method': 'GET', - 'url.full': expect.stringMatching(/\/$/), - 'http.route': '/', - 'url.path': '/', - }, - op: 'http.server', - status: 'ok', - }, - }, + span: container => { + expect(findSegmentSpan(container)).toMatchObject({ + name: 'GET /', + span_id: expect.stringMatching(/[a-f\d]{16}/), + trace_id: expect.stringMatching(/[a-f\d]{32}/), + status: 'ok', + attributes: expect.objectContaining({ + 'sentry.op': { type: 'string', value: 'http.server' }, + 'http.response.status_code': { type: 'integer', value: 200 }, + 'http.request.method': { type: 'string', value: 'GET' }, + 'url.full': { type: 'string', value: expect.stringMatching(/\/$/) }, + 'http.route': { type: 'string', value: '/' }, + 'url.path': { type: 'string', value: '/' }, + }), + }); }, }) .start(); @@ -182,42 +183,73 @@ describe('express tracing', () => { await runner.completed(); }); - test.each(['/401', '/402', '/403', '/does-not-exist'])('ignores %s route by default', async (url: string) => { - const runner = createRunner() - .expect({ - // No transaction is sent for the 401, 402, 403, 404 routes - transaction: { - transaction: 'GET /', - }, - }) - .start(); - runner.makeRequest('get', url, { expectError: true }); - runner.makeRequest('get', '/'); - await runner.completed(); - }); + // With span streaming, child spans are sent as they end, before the response status code is + // known, so `ignoreStatusCodes` has no effect and these routes are captured like any other. + test.each([ + { status_code: 401, url: '/401', status_message: 'unauthenticated', name: 'GET /401', source: 'route' }, + { status_code: 402, url: '/402', status_message: 'invalid_argument', name: 'GET /402', source: 'route' }, + { status_code: 403, url: '/403', status_message: 'permission_denied', name: 'GET /403', source: 'route' }, + // Without a matching route the name must not carry the URL path. + { status_code: 404, url: '/does-not-exist', status_message: 'not_found', name: 'GET', source: 'url' }, + ])( + 'handles %s route correctly', + async ({ + status_code, + url, + status_message, + name, + source, + }: { + status_code: number; + url: string; + status_message: string; + name: string; + source: string; + }) => { + const runner = createRunner() + .expect({ + span: container => { + expect(findSegmentSpan(container)).toMatchObject({ + name, + span_id: expect.stringMatching(/[a-f\d]{16}/), + trace_id: expect.stringMatching(/[a-f\d]{32}/), + status: 'error', + attributes: expect.objectContaining({ + 'sentry.op': { type: 'string', value: 'http.server' }, + 'sentry.segment.name.source': { type: 'string', value: source }, + 'sentry.status.message': { type: 'string', value: status_message }, + 'http.response.status_code': { type: 'integer', value: status_code }, + 'http.request.method': { type: 'string', value: 'GET' }, + 'url.full': { type: 'string', value: expect.stringMatching(url) }, + 'url.path': { type: 'string', value: url }, + }), + }); + }, + }) + .start(); + runner.makeRequest('get', url, { expectError: true }); + await runner.completed(); + }, + ); test.each([['array1'], ['array5']])( - 'should set a correct transaction name for routes consisting of arrays of routes for %p', + 'should set a correct segment name for routes consisting of arrays of routes for %p', async (segment: string) => { - const runner = await createRunner() + const runner = createRunner() .expect({ - transaction: { - transaction: 'GET /test/array1,/\\/test\\/array[2-9]/', - transaction_info: { - source: 'route', - }, - contexts: { - trace: { - trace_id: expect.stringMatching(/[a-f\d]{32}/), - span_id: expect.stringMatching(/[a-f\d]{16}/), - data: { - 'url.full': expect.stringMatching(`/test/${segment}$`), - 'http.response.status_code': 200, - }, - op: 'http.server', - status: 'ok', - }, - }, + span: container => { + expect(findSegmentSpan(container)).toMatchObject({ + name: 'GET /test/array1,/\\/test\\/array[2-9]/', + span_id: expect.stringMatching(/[a-f\d]{16}/), + trace_id: expect.stringMatching(/[a-f\d]{32}/), + status: 'ok', + attributes: expect.objectContaining({ + 'sentry.op': { type: 'string', value: 'http.server' }, + 'sentry.segment.name.source': { type: 'string', value: 'route' }, + 'url.full': { type: 'string', value: expect.stringMatching(`/test/${segment}$`) }, + 'http.response.status_code': { type: 'integer', value: 200 }, + }), + }); }, }) .start(); @@ -236,25 +268,21 @@ describe('express tracing', () => { ['arr/requiredPath/optionalPath/'], ['arr/requiredPath/optionalPath/lastParam'], ])('should handle more complex regexes in route arrays correctly for %p', async (segment: string) => { - const runner = await createRunner() + const runner = createRunner() .expect({ - transaction: { - transaction: 'GET /test/arr/:id,/\\/test\\/arr\\d*\\/required(path)?(\\/optionalPath)?\\/(lastParam)?/', - transaction_info: { - source: 'route', - }, - contexts: { - trace: { - trace_id: expect.stringMatching(/[a-f\d]{32}/), - span_id: expect.stringMatching(/[a-f\d]{16}/), - data: { - 'url.full': expect.stringMatching(`/test/${segment}$`), - 'http.response.status_code': 200, - }, - op: 'http.server', - status: 'ok', - }, - }, + span: container => { + expect(findSegmentSpan(container)).toMatchObject({ + name: 'GET /test/arr/:id,/\\/test\\/arr\\d*\\/required(path)?(\\/optionalPath)?\\/(lastParam)?/', + span_id: expect.stringMatching(/[a-f\d]{16}/), + trace_id: expect.stringMatching(/[a-f\d]{32}/), + status: 'ok', + attributes: expect.objectContaining({ + 'sentry.op': { type: 'string', value: 'http.server' }, + 'sentry.segment.name.source': { type: 'string', value: 'route' }, + 'url.full': { type: 'string', value: expect.stringMatching(`/test/${segment}$`) }, + 'http.response.status_code': { type: 'integer', value: 200 }, + }), + }); }, }) .start(); @@ -266,20 +294,17 @@ describe('express tracing', () => { test('correctly captures JSON request data', async () => { const runner = createRunner() .expect({ - transaction: { - transaction: 'POST /test-post', - request: { - url: expect.stringMatching(/^http:\/\/localhost:(\d+)\/test-post$/), - method: 'POST', - headers: { - 'user-agent': expect.stringContaining(''), - 'content-type': 'application/json', - }, - data: JSON.stringify({ - foo: 'bar', - other: 1, + span: container => { + expect(findSegmentSpan(container)).toMatchObject({ + name: 'POST /test-post', + attributes: expect.objectContaining({ + 'url.full': { type: 'string', value: expect.stringMatching(/^http:\/\/localhost:(\d+)\/test-post$/) }, + 'http.request.method': { type: 'string', value: 'POST' }, + 'http.request.header.user_agent': { type: 'string', value: expect.stringContaining('') }, + 'http.request.header.content_type': { type: 'string', value: 'application/json' }, + 'http.request.body.data': { type: 'string', value: JSON.stringify({ foo: 'bar', other: 1 }) }, }), - }, + }); }, }) .start(); @@ -296,17 +321,17 @@ describe('express tracing', () => { test('correctly captures plain text request data', async () => { const runner = createRunner() .expect({ - transaction: { - transaction: 'POST /test-post', - request: { - url: expect.stringMatching(/^http:\/\/localhost:(\d+)\/test-post$/), - method: 'POST', - headers: { - 'user-agent': expect.stringContaining(''), - 'content-type': 'text/plain', - }, - data: 'some plain text', - }, + span: container => { + expect(findSegmentSpan(container)).toMatchObject({ + name: 'POST /test-post', + attributes: expect.objectContaining({ + 'url.full': { type: 'string', value: expect.stringMatching(/^http:\/\/localhost:(\d+)\/test-post$/) }, + 'http.request.method': { type: 'string', value: 'POST' }, + 'http.request.header.user_agent': { type: 'string', value: expect.stringContaining('') }, + 'http.request.header.content_type': { type: 'string', value: 'text/plain' }, + 'http.request.body.data': { type: 'string', value: 'some plain text' }, + }), + }); }, }) .start(); @@ -321,17 +346,17 @@ describe('express tracing', () => { test('correctly captures text buffer request data', async () => { const runner = createRunner() .expect({ - transaction: { - transaction: 'POST /test-post', - request: { - url: expect.stringMatching(/^http:\/\/localhost:(\d+)\/test-post$/), - method: 'POST', - headers: { - 'user-agent': expect.stringContaining(''), - 'content-type': 'application/octet-stream', - }, - data: 'some plain text in buffer', - }, + span: container => { + expect(findSegmentSpan(container)).toMatchObject({ + name: 'POST /test-post', + attributes: expect.objectContaining({ + 'url.full': { type: 'string', value: expect.stringMatching(/^http:\/\/localhost:(\d+)\/test-post$/) }, + 'http.request.method': { type: 'string', value: 'POST' }, + 'http.request.header.user_agent': { type: 'string', value: expect.stringContaining('') }, + 'http.request.header.content_type': { type: 'string', value: 'application/octet-stream' }, + 'http.request.body.data': { type: 'string', value: 'some plain text in buffer' }, + }), + }); }, }) .start(); @@ -346,18 +371,18 @@ describe('express tracing', () => { test('correctly captures non-text buffer request data', async () => { const runner = createRunner() .expect({ - transaction: { - transaction: 'POST /test-post', - request: { - url: expect.stringMatching(/^http:\/\/localhost:(\d+)\/test-post$/), - method: 'POST', - headers: { - 'user-agent': expect.stringContaining(''), - 'content-type': 'application/octet-stream', - }, - // This is some non-ascii string representation - data: expect.any(String), - }, + span: container => { + expect(findSegmentSpan(container)).toMatchObject({ + name: 'POST /test-post', + attributes: expect.objectContaining({ + 'url.full': { type: 'string', value: expect.stringMatching(/^http:\/\/localhost:(\d+)\/test-post$/) }, + 'http.request.method': { type: 'string', value: 'POST' }, + 'http.request.header.user_agent': { type: 'string', value: expect.stringContaining('') }, + 'http.request.header.content_type': { type: 'string', value: 'application/octet-stream' }, + // This is some non-ascii string representation + 'http.request.body.data': { type: 'string', value: expect.any(String) }, + }), + }); }, }) .start(); @@ -374,20 +399,22 @@ describe('express tracing', () => { test('correctly ignores request data', async () => { const runner = createRunner() .expect({ - transaction: e => { - assertSentryTransaction(e, { - transaction: 'POST /test-post-ignore-body', - request: { - url: expect.stringMatching(/^http:\/\/localhost:(\d+)\/test-post-ignore-body$/), - method: 'POST', - headers: { - 'user-agent': expect.stringContaining(''), - 'content-type': 'application/octet-stream', + span: container => { + const serverSpan = findSegmentSpan(container); + expect(serverSpan).toMatchObject({ + name: 'POST /test-post-ignore-body', + attributes: expect.objectContaining({ + 'url.full': { + type: 'string', + value: expect.stringMatching(/^http:\/\/localhost:(\d+)\/test-post-ignore-body$/), }, - }, + 'http.request.method': { type: 'string', value: 'POST' }, + 'http.request.header.user_agent': { type: 'string', value: expect.stringContaining('') }, + 'http.request.header.content_type': { type: 'string', value: 'application/octet-stream' }, + }), }); // Ensure the request body has been ignored - expect(e).have.property('request').that.does.not.have.property('data'); + expect(serverSpan?.attributes['http.request.body.data']).toBeUndefined(); }, }) .start(); @@ -400,63 +427,4 @@ describe('express tracing', () => { }); }); }); - - describe('filter status codes', () => { - createEsmAndCjsTests( - __dirname, - 'scenario-filterStatusCode.mjs', - 'instrument-filterStatusCode.mjs', - (createRunner, test) => { - // We opt-out of the default [401, 404] filtering in order to test how these spans are handled - test.each([ - { status_code: 401, url: '/401', status: 'unauthenticated' }, - { status_code: 402, url: '/402', status: 'invalid_argument' }, - { status_code: 403, url: '/403', status: 'permission_denied' }, - { status_code: 404, url: '/does-not-exist', status: 'not_found' }, - ])( - 'handles %s route correctly', - async ({ status_code, url, status }: { status_code: number; url: string; status: string }) => { - const runner = createRunner() - .expect({ - transaction: { - transaction: `GET ${url}`, - contexts: { - trace: { - span_id: expect.stringMatching(/[a-f\d]{16}/), - trace_id: expect.stringMatching(/[a-f\d]{32}/), - data: { - 'http.response.status_code': status_code, - 'http.request.method': 'GET', - 'url.full': expect.stringMatching(url), - 'url.path': url, - }, - op: 'http.server', - status, - }, - }, - }, - }) - .start(); - runner.makeRequest('get', url, { expectError: true }); - await runner.completed(); - }, - ); - - test('filters defined status codes', async () => { - const runner = createRunner() - .expect({ - transaction: { - transaction: 'GET /', - }, - }) - .start(); - await runner.makeRequest('get', '/499', { expectError: true }); - await runner.makeRequest('get', '/300', { expectError: true }); - await runner.makeRequest('get', '/399', { expectError: true }); - await runner.makeRequest('get', '/'); - await runner.completed(); - }); - }, - ); - }); }); diff --git a/dev-packages/node-integration-tests/suites/express/tracing/tracesSampler/instrument-normalized-request.mjs b/dev-packages/node-integration-tests/suites/express/tracing/tracesSampler/instrument-normalized-request.mjs index 018ac31c6a24..72786de129ff 100644 --- a/dev-packages/node-integration-tests/suites/express/tracing/tracesSampler/instrument-normalized-request.mjs +++ b/dev-packages/node-integration-tests/suites/express/tracing/tracesSampler/instrument-normalized-request.mjs @@ -2,7 +2,6 @@ import * as Sentry from '@sentry/node'; import { loggingTransport } from '@sentry-internal/node-integration-tests'; Sentry.init({ - traceLifecycle: 'static', dsn: 'https://public@dsn.ingest.sentry.io/1337', release: '1.0', transport: loggingTransport, diff --git a/dev-packages/node-integration-tests/suites/express/tracing/tracesSampler/instrument.mjs b/dev-packages/node-integration-tests/suites/express/tracing/tracesSampler/instrument.mjs index 15b20267dd3c..c6d38a0a1720 100644 --- a/dev-packages/node-integration-tests/suites/express/tracing/tracesSampler/instrument.mjs +++ b/dev-packages/node-integration-tests/suites/express/tracing/tracesSampler/instrument.mjs @@ -2,16 +2,16 @@ import * as Sentry from '@sentry/node'; import { loggingTransport } from '@sentry-internal/node-integration-tests'; Sentry.init({ - traceLifecycle: 'static', dsn: 'https://public@dsn.ingest.sentry.io/1337', release: '1.0', transport: loggingTransport, tracesSampler: samplingContext => { - // The name we get here is inferred at span creation time - // At this point, we sadly do not have a http.route attribute yet, - // so we infer the name from the unparameterized route instead + // The name we get here is inferred at span creation time. At this point, we sadly do not have a + // http.route attribute yet, and with span streaming the name is just the request method, so we + // match on the unparameterized URL path attribute instead. return ( - samplingContext.name === 'GET /test/123' && + samplingContext.name === 'GET' && + samplingContext.attributes['url.path'] === '/test/123' && samplingContext.attributes['sentry.op'] === 'http.server' && samplingContext.attributes['http.request.method'] === 'GET' ); diff --git a/dev-packages/node-integration-tests/suites/express/tracing/tracesSampler/test.ts b/dev-packages/node-integration-tests/suites/express/tracing/tracesSampler/test.ts index 92374c5e8903..2c9d6b5f2aa3 100644 --- a/dev-packages/node-integration-tests/suites/express/tracing/tracesSampler/test.ts +++ b/dev-packages/node-integration-tests/suites/express/tracing/tracesSampler/test.ts @@ -1,4 +1,4 @@ -import { afterAll, describe } from 'vitest'; +import { afterAll, describe, expect } from 'vitest'; import { cleanupChildProcesses, createCjsTests } from '../../../../utils/runner'; describe('express tracesSampler', () => { @@ -10,8 +10,8 @@ describe('express tracesSampler', () => { test('correctly samples & passes data to tracesSampler', async () => { const runner = createRunner() .expect({ - transaction: { - transaction: 'GET /test/:id', + span: container => { + expect(container.items.find(item => item.is_segment)?.name).toBe('GET /test/:id'); }, }) .start(); @@ -33,8 +33,8 @@ describe('express tracesSampler', () => { test('correctly samples & passes normalizedRequest data to tracesSampler', async () => { const runner = createRunner() .expect({ - transaction: { - transaction: 'GET /test-normalized-request', + span: container => { + expect(container.items.find(item => item.is_segment)?.name).toBe('GET /test-normalized-request'); }, }) .start(); diff --git a/dev-packages/node-integration-tests/suites/express/tracing/updateName/instrument.mjs b/dev-packages/node-integration-tests/suites/express/tracing/updateName/instrument.mjs index 43bf3e3a5442..5cade6bb7ba1 100644 --- a/dev-packages/node-integration-tests/suites/express/tracing/updateName/instrument.mjs +++ b/dev-packages/node-integration-tests/suites/express/tracing/updateName/instrument.mjs @@ -2,7 +2,6 @@ import * as Sentry from '@sentry/node'; import { loggingTransport } from '@sentry-internal/node-integration-tests'; Sentry.init({ - traceLifecycle: 'static', dsn: 'https://public@dsn.ingest.sentry.io/1337', release: '1.0', // disable attaching headers to /test/* endpoints diff --git a/dev-packages/node-integration-tests/suites/express/tracing/updateName/test.ts b/dev-packages/node-integration-tests/suites/express/tracing/updateName/test.ts index f48b97265f59..30288eb58a25 100644 --- a/dev-packages/node-integration-tests/suites/express/tracing/updateName/test.ts +++ b/dev-packages/node-integration-tests/suites/express/tracing/updateName/test.ts @@ -1,6 +1,4 @@ import { SENTRY_SEGMENT_NAME_SOURCE } from '@sentry/conventions/attributes'; -import { SEMANTIC_ATTRIBUTE_SENTRY_CUSTOM_SPAN_NAME } from '@sentry/core'; - import { afterAll, describe, expect } from 'vitest'; import { cleanupChildProcesses, createCjsTests } from '../../../../utils/runner'; @@ -16,11 +14,10 @@ describe('express tracing - updateName', () => { test('calling just `span.updateName` updates the final name in express', async () => { const runner = createRunner() .expect({ - transaction: { - transaction: 'new-name', - transaction_info: { - source: 'custom', - }, + span: container => { + const serverSpan = container.items.find(item => item.is_segment); + expect(serverSpan?.name).toBe('new-name'); + expect(serverSpan?.attributes[SENTRY_SEGMENT_NAME_SOURCE]).toEqual({ type: 'string', value: 'custom' }); }, }) .start(); @@ -32,21 +29,15 @@ describe('express tracing - updateName', () => { test('calling `Sentry.updateSpanName` updates the final name and source in express', async () => { const runner = createRunner() .expect({ - transaction: txnEvent => { - expect(txnEvent).toMatchObject({ - transaction: 'new-name', - transaction_info: { - source: 'custom', - }, - contexts: { - trace: { - op: 'http.server', - data: { [SENTRY_SEGMENT_NAME_SOURCE]: 'custom' }, - }, - }, + span: container => { + const serverSpan = container.items.find(item => item.is_segment); + expect(serverSpan).toMatchObject({ + name: 'new-name', + attributes: expect.objectContaining({ + 'sentry.op': { type: 'string', value: 'http.server' }, + [SENTRY_SEGMENT_NAME_SOURCE]: { type: 'string', value: 'custom' }, + }), }); - // ensure we delete the internal attribute once we're done with it - expect(txnEvent.contexts?.trace?.data?.[SEMANTIC_ATTRIBUTE_SENTRY_CUSTOM_SPAN_NAME]).toBeUndefined(); }, }) .start(); @@ -58,21 +49,15 @@ describe('express tracing - updateName', () => { test('calling `Sentry.updateSpanName` and setting source subsequently updates the final name and sets correct source', async () => { const runner = createRunner() .expect({ - transaction: txnEvent => { - expect(txnEvent).toMatchObject({ - transaction: 'new-name', - transaction_info: { - source: 'component', - }, - contexts: { - trace: { - op: 'http.server', - data: { [SENTRY_SEGMENT_NAME_SOURCE]: 'component' }, - }, - }, + span: container => { + const serverSpan = container.items.find(item => item.is_segment); + expect(serverSpan).toMatchObject({ + name: 'new-name', + attributes: expect.objectContaining({ + 'sentry.op': { type: 'string', value: 'http.server' }, + [SENTRY_SEGMENT_NAME_SOURCE]: { type: 'string', value: 'component' }, + }), }); - // ensure we delete the internal attribute once we're done with it - expect(txnEvent.contexts?.trace?.data?.[SEMANTIC_ATTRIBUTE_SENTRY_CUSTOM_SPAN_NAME]).toBeUndefined(); }, }) .start(); diff --git a/dev-packages/node-integration-tests/suites/express/with-http/base/instrument.mjs b/dev-packages/node-integration-tests/suites/express/with-http/base/instrument.mjs index 170ad6f6a702..46a27dd03b74 100644 --- a/dev-packages/node-integration-tests/suites/express/with-http/base/instrument.mjs +++ b/dev-packages/node-integration-tests/suites/express/with-http/base/instrument.mjs @@ -2,7 +2,6 @@ import * as Sentry from '@sentry/node'; import { loggingTransport } from '@sentry-internal/node-integration-tests'; Sentry.init({ - traceLifecycle: 'static', dsn: 'https://public@dsn.ingest.sentry.io/1337', release: '1.0', tracesSampleRate: 1.0, diff --git a/dev-packages/node-integration-tests/suites/express/with-http/base/test.ts b/dev-packages/node-integration-tests/suites/express/with-http/base/test.ts index 40c74a3d8888..8a5360ae64ac 100644 --- a/dev-packages/node-integration-tests/suites/express/with-http/base/test.ts +++ b/dev-packages/node-integration-tests/suites/express/with-http/base/test.ts @@ -1,4 +1,4 @@ -import { afterAll, describe } from 'vitest'; +import { afterAll, describe, expect } from 'vitest'; import { cleanupChildProcesses, createEsmAndCjsTests } from '../../../../utils/runner'; describe('express with http import', () => { @@ -9,19 +9,17 @@ describe('express with http import', () => { createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => { test('it works when importing the http module', async () => { const runner = createRunner() + // `/test` calls `/test2` on the same server, so both requests share one trace and their + // spans are flushed together in a single envelope. .expect({ - transaction: { - transaction: 'GET /test2', + span: container => { + const segmentNames = container.items.filter(item => item.is_segment).map(item => item.name); + expect(segmentNames).toEqual(['GET /test2', 'GET /test']); }, }) .expect({ - transaction: { - transaction: 'GET /test', - }, - }) - .expect({ - transaction: { - transaction: 'GET /test3', + span: container => { + expect(container.items.find(item => item.is_segment)?.name).toBe('GET /test3'); }, }) .start();