From b5a4fb26d3df8216cfdccfd19aef75e863c3aac3 Mon Sep 17 00:00:00 2001 From: Bob Evans Date: Fri, 16 Aug 2024 14:15:10 -0400 Subject: [PATCH] fix: Updated koa instrumentation to properly get the matched route name and to handle changes in `@koa/router@13.0.0` --- lib/instrumentation/koa/instrumentation.js | 22 ++----------- test/versioned/koa/package.json | 2 +- test/versioned/koa/router-common.js | 37 +++++++++++----------- 3 files changed, 21 insertions(+), 40 deletions(-) diff --git a/lib/instrumentation/koa/instrumentation.js b/lib/instrumentation/koa/instrumentation.js index 1eadd3819b..7514529017 100644 --- a/lib/instrumentation/koa/instrumentation.js +++ b/lib/instrumentation/koa/instrumentation.js @@ -115,10 +115,8 @@ function wrapMatchedRoute(shim, context) { Object.defineProperty(context, '_matchedRoute', { get: () => context[symbols.koaMatchedRoute], set: (val) => { - const match = getLayerForTransactionName(context) - // match should never be undefined given _matchedRoute was set - if (match) { + if (val) { const currentSegment = shim.getActiveSegment() // Segment/Transaction may be null, see: @@ -131,7 +129,7 @@ function wrapMatchedRoute(shim, context) { transaction.nameState.popPath() } - transaction.nameState.appendPath(match.path) + transaction.nameState.appendPath(val) transaction.nameState.markPath() } } @@ -169,22 +167,6 @@ function wrapResponseStatus(shim, context) { }) } -function getLayerForTransactionName(context) { - // Context.matched might be null - // See https://github.com/newrelic/node-newrelic-koa/pull/29 - if (!context.matched) { - return null - } - for (let i = context.matched.length - 1; i >= 0; i--) { - const layer = context.matched[i] - if (layer.opts.end) { - return layer - } - } - - return null -} - function getInheritedPropertyDescriptor(obj, property) { let proto = obj let descriptor = null diff --git a/test/versioned/koa/package.json b/test/versioned/koa/package.json index 664c1ccacf..50a1775d0f 100644 --- a/test/versioned/koa/package.json +++ b/test/versioned/koa/package.json @@ -53,7 +53,7 @@ "samples": 5 }, "@koa/router": { - "versions": ">=11.0.2 <13.0.0", + "versions": ">=11.0.2", "samples": 5 } }, diff --git a/test/versioned/koa/router-common.js b/test/versioned/koa/router-common.js index 50492b76ea..7927391e36 100644 --- a/test/versioned/koa/router-common.js +++ b/test/versioned/koa/router-common.js @@ -422,7 +422,7 @@ module.exports = (pkg) => { ) }) - t.test('using multipler routers', (t) => { + t.test('using multiple routers', (t) => { t.beforeEach(testSetup) t.afterEach(tearDown) t.autoend() @@ -546,15 +546,12 @@ module.exports = (pkg) => { nestedRouter.get('/:second', function terminalMiddleware(ctx) { ctx.body = 'this is a test' }) - nestedRouter.get('/second', function secondMiddleware(ctx) { - ctx.body = 'want this to set the name' - }) router.use('/:first', nestedRouter.routes()) app.use(router.routes()) agent.on('transactionFinished', (tx) => { t.assertSegments(tx.trace.root, [ - 'WebTransaction/WebFrameworkUri/Koa/GET//:first/second', + 'WebTransaction/WebFrameworkUri/Koa/GET//:first/:second', [ 'Nodejs/Middleware/Koa/appLevelMiddleware', ['Koa/Router: /', [getNestedSpanName('terminalMiddleware')]] @@ -562,7 +559,7 @@ module.exports = (pkg) => { ]) t.equal( tx.name, - 'WebTransaction/WebFrameworkUri/Koa/GET//:first/second', + 'WebTransaction/WebFrameworkUri/Koa/GET//:first/:second', 'should be named after last matched route' ) t.end() @@ -581,15 +578,12 @@ module.exports = (pkg) => { router.get('/:second', function terminalMiddleware(ctx) { ctx.body = 'this is a test' }) - router.get('/second', function secondMiddleware(ctx) { - ctx.body = 'want this to set the name' - }) router.prefix('/:first') app.use(router.routes()) agent.on('transactionFinished', (tx) => { t.assertSegments(tx.trace.root, [ - 'WebTransaction/WebFrameworkUri/Koa/GET//:first/second', + 'WebTransaction/WebFrameworkUri/Koa/GET//:first/:second', [ 'Nodejs/Middleware/Koa/appLevelMiddleware', ['Koa/Router: /', ['Nodejs/Middleware/Koa/terminalMiddleware//:first/:second']] @@ -597,7 +591,7 @@ module.exports = (pkg) => { ]) t.equal( tx.name, - 'WebTransaction/WebFrameworkUri/Koa/GET//:first/second', + 'WebTransaction/WebFrameworkUri/Koa/GET//:first/:second', 'should be named after the last matched path' ) t.end() @@ -607,6 +601,11 @@ module.exports = (pkg) => { }) t.test('using allowedMethods', (t) => { + // `@koa/router@13.0.0` changed the allowedMethods middleware function from named to arrow function + // update span name for assertions + const allowedMethodsFnName = semver.gte(pkgVersion, '13.0.0') + ? '' + : 'allowedMethods' t.autoend() t.test('with throw: true', (t) => { @@ -622,7 +621,7 @@ module.exports = (pkg) => { agent.on('transactionFinished', (tx) => { t.assertSegments(tx.trace.root, [ 'WebTransaction/WebFrameworkUri/Koa/GET/(method not allowed)', - ['Koa/Router: /', ['Nodejs/Middleware/Koa/allowedMethods']] + ['Koa/Router: /', [`Nodejs/Middleware/Koa/${allowedMethodsFnName}`]] ]) t.equal( tx.name, @@ -645,7 +644,7 @@ module.exports = (pkg) => { agent.on('transactionFinished', (tx) => { t.assertSegments(tx.trace.root, [ 'WebTransaction/WebFrameworkUri/Koa/GET/(not implemented)', - ['Koa/Router: /', ['Nodejs/Middleware/Koa/allowedMethods']] + ['Koa/Router: /', [`Nodejs/Middleware/Koa/${allowedMethodsFnName}`]] ]) t.equal( tx.name, @@ -683,7 +682,7 @@ module.exports = (pkg) => { 'WebTransaction/NormalizedUri/*', [ 'Nodejs/Middleware/Koa/errorHandler', - ['Koa/Router: /', ['Nodejs/Middleware/Koa/allowedMethods']] + ['Koa/Router: /', [`Nodejs/Middleware/Koa/${allowedMethodsFnName}`]] ] ]) t.equal( @@ -722,7 +721,7 @@ module.exports = (pkg) => { 'WebTransaction/WebFrameworkUri/Koa/GET/(method not allowed)', [ 'Nodejs/Middleware/Koa/baseMiddleware', - ['Koa/Router: /', ['Nodejs/Middleware/Koa/allowedMethods']] + ['Koa/Router: /', [`Nodejs/Middleware/Koa/${allowedMethodsFnName}`]] ] ]) t.equal( @@ -753,7 +752,7 @@ module.exports = (pkg) => { agent.on('transactionFinished', (tx) => { t.assertSegments(tx.trace.root, [ 'WebTransaction/WebFrameworkUri/Koa/GET/(method not allowed)', - ['Koa/Router: /', ['Nodejs/Middleware/Koa/allowedMethods']] + ['Koa/Router: /', [`Nodejs/Middleware/Koa/${allowedMethodsFnName}`]] ]) t.equal( tx.name, @@ -777,7 +776,7 @@ module.exports = (pkg) => { agent.on('transactionFinished', (tx) => { t.assertSegments(tx.trace.root, [ 'WebTransaction/WebFrameworkUri/Koa/GET/(not implemented)', - ['Koa/Router: /', ['Nodejs/Middleware/Koa/allowedMethods']] + ['Koa/Router: /', [`Nodejs/Middleware/Koa/${allowedMethodsFnName}`]] ]) t.equal( tx.name, @@ -811,7 +810,7 @@ module.exports = (pkg) => { 'WebTransaction/WebFrameworkUri/Koa/GET/(method not allowed)', [ 'Nodejs/Middleware/Koa/appLevelMiddleware', - ['Koa/Router: /', ['Nodejs/Middleware/Koa/allowedMethods']] + ['Koa/Router: /', [`Nodejs/Middleware/Koa/${allowedMethodsFnName}`]] ] ]) t.equal( @@ -845,7 +844,7 @@ module.exports = (pkg) => { 'WebTransaction/WebFrameworkUri/Koa/GET/(not implemented)', [ 'Nodejs/Middleware/Koa/appLevelMiddleware', - ['Koa/Router: /', ['Nodejs/Middleware/Koa/allowedMethods']] + ['Koa/Router: /', [`Nodejs/Middleware/Koa/${allowedMethodsFnName}`]] ] ]) t.equal(