Skip to content

Commit

Permalink
fix: Updated koa instrumentation to properly get the matched route na…
Browse files Browse the repository at this point in the history 
…me and to handle changes in `@koa/router@13.0.0` (#2486)
  • Loading branch information
@bizob2828
bizob2828 committed Aug 16, 2024
1 parent fd2d76f commit 0c2ee2f
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 40 deletions.
22 changes: 2 additions & 20 deletions lib/instrumentation/koa/instrumentation.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -131,7 +129,7 @@ function wrapMatchedRoute(shim, context) {
transaction.nameState.popPath()
}

transaction.nameState.appendPath(match.path)
transaction.nameState.appendPath(val)
transaction.nameState.markPath()
}
}
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion test/versioned/koa/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@
"samples": 5
},
"@koa/router": {
"versions": ">=11.0.2 <13.0.0",
"versions": ">=11.0.2",
"samples": 5
}
},
Expand Down
37 changes: 18 additions & 19 deletions test/versioned/koa/router-common.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -546,23 +546,20 @@ 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')]]
]
])
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()
Expand All @@ -581,23 +578,20 @@ 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']]
]
])
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()
Expand All @@ -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')
? '<anonymous>'
: 'allowedMethods'
t.autoend()

t.test('with throw: true', (t) => {
Expand All @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down

0 comments on commit 0c2ee2f

Please sign in to comment.