diff --git a/plugins/node/opentelemetry-instrumentation-express/src/utils.ts b/plugins/node/opentelemetry-instrumentation-express/src/utils.ts index fb7446ba76..d888115a0f 100644 --- a/plugins/node/opentelemetry-instrumentation-express/src/utils.ts +++ b/plugins/node/opentelemetry-instrumentation-express/src/utils.ts @@ -154,6 +154,10 @@ export const asErrorAndMessage = ( * @returns The layer path */ export const getLayerPath = (args: unknown[]) => { + if (args[0] instanceof RegExp) { + return args[0].toString(); + } + if (typeof args[0] === 'string') { return args[0]; } diff --git a/plugins/node/opentelemetry-instrumentation-express/test/express.test.ts b/plugins/node/opentelemetry-instrumentation-express/test/express.test.ts index baac4d40ff..cc194c6244 100644 --- a/plugins/node/opentelemetry-instrumentation-express/test/express.test.ts +++ b/plugins/node/opentelemetry-instrumentation-express/test/express.test.ts @@ -567,7 +567,7 @@ describe('ExpressInstrumentation', () => { }); }); - it('should work with ESM usage and multiple routes including regex', async () => { + it('should set a correct transaction name for routes specified in RegEx', async () => { await testUtils.runTestFixture({ cwd: __dirname, argv: ['fixtures/use-express-regex.mjs'], @@ -575,6 +575,7 @@ describe('ExpressInstrumentation', () => { NODE_OPTIONS: '--experimental-loader=@opentelemetry/instrumentation/hook.mjs', NODE_NO_WARNINGS: '1', + TEST_REGEX_ROUTE: '/test/regex', }, checkResult: (err, stdout, stderr) => { assert.ifError(err); @@ -582,9 +583,7 @@ describe('ExpressInstrumentation', () => { checkCollector: (collector: testUtils.TestCollector) => { const spans = collector.sortedSpans; - console.debug(spans); - - assert.strictEqual(spans[0].name, 'GET /test/arr/:id,/\\\/test\\\/arr[0-9]*\\\/required(path)?(\\\/optionalPath)?\\\/(lastParam)?/'); + assert.strictEqual(spans[0].name, 'GET /\\/test\\/regex/'); assert.strictEqual(spans[0].kind, SpanKind.CLIENT); assert.strictEqual(spans[1].name, 'middleware - query'); assert.strictEqual(spans[1].kind, SpanKind.SERVER); @@ -595,10 +594,106 @@ describe('ExpressInstrumentation', () => { assert.strictEqual(spans[3].name, 'middleware - simpleMiddleware'); assert.strictEqual(spans[3].kind, SpanKind.SERVER); assert.strictEqual(spans[3].parentSpanId, spans[0].spanId); - assert.strictEqual(spans[4].name, 'request handler - /test/arr/:id,/\\\/test\\\/arr[0-9]*\\\/required(path)?(\\\/optionalPath)?\\\/(lastParam)?/'); + assert.strictEqual( + spans[4].name, + 'request handler - /\\/test\\/regex/' + ); assert.strictEqual(spans[4].kind, SpanKind.SERVER); assert.strictEqual(spans[4].parentSpanId, spans[0].spanId); }, }); }); + + for (const segment of ['array1', 'array5']) { + it('should set a correct transaction name for routes consisting of arrays of routes', async () => { + await testUtils.runTestFixture({ + cwd: __dirname, + argv: ['fixtures/use-express-regex.mjs'], + env: { + NODE_OPTIONS: + '--experimental-loader=@opentelemetry/instrumentation/hook.mjs', + NODE_NO_WARNINGS: '1', + TEST_REGEX_ROUTE: `/test/${segment}`, + }, + checkResult: err => { + assert.ifError(err); + }, + checkCollector: (collector: testUtils.TestCollector) => { + const spans = collector.sortedSpans; + + assert.strictEqual( + spans[0].name, + 'GET /test/array1,/\\/test\\/array[2-9]/' + ); + assert.strictEqual(spans[0].kind, SpanKind.CLIENT); + assert.strictEqual(spans[1].name, 'middleware - query'); + assert.strictEqual(spans[1].kind, SpanKind.SERVER); + assert.strictEqual(spans[1].parentSpanId, spans[0].spanId); + assert.strictEqual(spans[2].name, 'middleware - expressInit'); + assert.strictEqual(spans[2].kind, SpanKind.SERVER); + assert.strictEqual(spans[2].parentSpanId, spans[0].spanId); + assert.strictEqual(spans[3].name, 'middleware - simpleMiddleware'); + assert.strictEqual(spans[3].kind, SpanKind.SERVER); + assert.strictEqual(spans[3].parentSpanId, spans[0].spanId); + assert.strictEqual( + spans[4].name, + 'request handler - /test/array1,/\\/test\\/array[2-9]/' + ); + assert.strictEqual(spans[4].kind, SpanKind.SERVER); + assert.strictEqual(spans[4].parentSpanId, spans[0].spanId); + }, + }); + }); + } + + for (const segment of [ + 'arr/545', + 'arr/required', + 'arr/required', + 'arr/requiredPath', + 'arr/required/lastParam', + 'arr55/required/lastParam', + 'arr/requiredPath/optionalPath/', + 'arr/requiredPath/optionalPath/lastParam', + ]) { + it('should handle more complex regexes in route arrays correctly', async () => { + await testUtils.runTestFixture({ + cwd: __dirname, + argv: ['fixtures/use-express-regex.mjs'], + env: { + NODE_OPTIONS: + '--experimental-loader=@opentelemetry/instrumentation/hook.mjs', + NODE_NO_WARNINGS: '1', + TEST_REGEX_ROUTE: `/test/${segment}`, + }, + checkResult: err => { + assert.ifError(err); + }, + checkCollector: (collector: testUtils.TestCollector) => { + const spans = collector.sortedSpans; + + assert.strictEqual( + spans[0].name, + 'GET /test/arr/:id,/\\/test\\/arr[0-9]*\\/required(path)?(\\/optionalPath)?\\/(lastParam)?/' + ); + assert.strictEqual(spans[0].kind, SpanKind.CLIENT); + assert.strictEqual(spans[1].name, 'middleware - query'); + assert.strictEqual(spans[1].kind, SpanKind.SERVER); + assert.strictEqual(spans[1].parentSpanId, spans[0].spanId); + assert.strictEqual(spans[2].name, 'middleware - expressInit'); + assert.strictEqual(spans[2].kind, SpanKind.SERVER); + assert.strictEqual(spans[2].parentSpanId, spans[0].spanId); + assert.strictEqual(spans[3].name, 'middleware - simpleMiddleware'); + assert.strictEqual(spans[3].kind, SpanKind.SERVER); + assert.strictEqual(spans[3].parentSpanId, spans[0].spanId); + assert.strictEqual( + spans[4].name, + 'request handler - /test/arr/:id,/\\/test\\/arr[0-9]*\\/required(path)?(\\/optionalPath)?\\/(lastParam)?/' + ); + assert.strictEqual(spans[4].kind, SpanKind.SERVER); + assert.strictEqual(spans[4].parentSpanId, spans[0].spanId); + }, + }); + }); + } }); diff --git a/plugins/node/opentelemetry-instrumentation-express/test/fixtures/use-express-regex.mjs b/plugins/node/opentelemetry-instrumentation-express/test/fixtures/use-express-regex.mjs index e5aebae064..facba6b16c 100644 --- a/plugins/node/opentelemetry-instrumentation-express/test/fixtures/use-express-regex.mjs +++ b/plugins/node/opentelemetry-instrumentation-express/test/fixtures/use-express-regex.mjs @@ -25,11 +25,8 @@ import { ExpressInstrumentation } from '../../build/src/index.js'; const sdk = createTestNodeSdk({ serviceName: 'use-express-regex', - instrumentations: [ - new HttpInstrumentation(), - new ExpressInstrumentation() - ] -}) + instrumentations: [new HttpInstrumentation(), new ExpressInstrumentation()], +}); sdk.start(); import express from 'express'; @@ -46,8 +43,22 @@ app.use(async function simpleMiddleware(req, res, next) { next(); }); -app.get(['/test/arr/:id', /\/test\/arr[0-9]*\/required(path)?(\/optionalPath)?\/(lastParam)?/], (_req, res) => { - res.send({ response: 'response' }); +app.get( + [ + '/test/arr/:id', + /\/test\/arr[0-9]*\/required(path)?(\/optionalPath)?\/(lastParam)?/, + ], + (_req, res) => { + res.send({ response: 'response' }); + } +); + +app.get(/\/test\/regex/, (_req, res) => { + res.send({ response: 'response 2' }); +}); + +app.get(['/test/array1', /\/test\/array[2-9]/], (_req, res) => { + res.send({ response: 'response 3' }); }); const server = http.createServer(app); @@ -55,12 +66,15 @@ await new Promise(resolve => server.listen(0, resolve)); const port = server.address().port; await new Promise(resolve => { - http.get(`http://localhost:${port}/test/arr/requiredPath/optionalPath/lastParam`, (res) => { - res.resume(); - res.on('end', () => { - resolve(); - }); - }) + http.get( + `http://localhost:${port}${process.env.TEST_REGEX_ROUTE}`, + res => { + res.resume(); + res.on('end', () => { + resolve(); + }); + } + ); }); await new Promise(resolve => server.close(resolve));