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();