Skip to content

Commit 0c2ee2f

Browse files
bizob2828bizob2828
authored
fix: Updated koa instrumentation to properly get the matched route name and to handle changes in @koa/[email protected] (#2486)
1 parent fd2d76f commit 0c2ee2f

File tree

3 files changed

+21
-40
lines changed

3 files changed

+21
-40
lines changed

‎lib/instrumentation/koa/instrumentation.js‎

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -115,10 +115,8 @@ function wrapMatchedRoute(shim, context) {
115115
Object.defineProperty(context, '_matchedRoute', {
116116
get: () => context[symbols.koaMatchedRoute],
117117
set: (val) => {
118-
const match = getLayerForTransactionName(context)
119-
120118
// match should never be undefined given _matchedRoute was set
121-
if (match) {
119+
if (val) {
122120
const currentSegment = shim.getActiveSegment()
123121

124122
// Segment/Transaction may be null, see:
@@ -131,7 +129,7 @@ function wrapMatchedRoute(shim, context) {
131129
transaction.nameState.popPath()
132130
}
133131

134-
transaction.nameState.appendPath(match.path)
132+
transaction.nameState.appendPath(val)
135133
transaction.nameState.markPath()
136134
}
137135
}
@@ -169,22 +167,6 @@ function wrapResponseStatus(shim, context) {
169167
})
170168
}
171169

172-
function getLayerForTransactionName(context) {
173-
// Context.matched might be null
174-
// See https://github.com/newrelic/node-newrelic-koa/pull/29
175-
if (!context.matched) {
176-
return null
177-
}
178-
for (let i = context.matched.length - 1; i >= 0; i--) {
179-
const layer = context.matched[i]
180-
if (layer.opts.end) {
181-
return layer
182-
}
183-
}
184-
185-
return null
186-
}
187-
188170
function getInheritedPropertyDescriptor(obj, property) {
189171
let proto = obj
190172
let descriptor = null

‎test/versioned/koa/package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@
5353
"samples": 5
5454
},
5555
"@koa/router": {
56-
"versions": ">=11.0.2 <13.0.0",
56+
"versions": ">=11.0.2",
5757
"samples": 5
5858
}
5959
},

‎test/versioned/koa/router-common.js‎

Lines changed: 18 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -422,7 +422,7 @@ module.exports = (pkg) => {
422422
)
423423
})
424424

425-
t.test('using multipler routers', (t) => {
425+
t.test('using multiple routers', (t) => {
426426
t.beforeEach(testSetup)
427427
t.afterEach(tearDown)
428428
t.autoend()
@@ -546,23 +546,20 @@ module.exports = (pkg) => {
546546
nestedRouter.get('/:second', function terminalMiddleware(ctx) {
547547
ctx.body = 'this is a test'
548548
})
549-
nestedRouter.get('/second', function secondMiddleware(ctx) {
550-
ctx.body = 'want this to set the name'
551-
})
552549
router.use('/:first', nestedRouter.routes())
553550
app.use(router.routes())
554551

555552
agent.on('transactionFinished', (tx) => {
556553
t.assertSegments(tx.trace.root, [
557-
'WebTransaction/WebFrameworkUri/Koa/GET//:first/second',
554+
'WebTransaction/WebFrameworkUri/Koa/GET//:first/:second',
558555
[
559556
'Nodejs/Middleware/Koa/appLevelMiddleware',
560557
['Koa/Router: /', [getNestedSpanName('terminalMiddleware')]]
561558
]
562559
])
563560
t.equal(
564561
tx.name,
565-
'WebTransaction/WebFrameworkUri/Koa/GET//:first/second',
562+
'WebTransaction/WebFrameworkUri/Koa/GET//:first/:second',
566563
'should be named after last matched route'
567564
)
568565
t.end()
@@ -581,23 +578,20 @@ module.exports = (pkg) => {
581578
router.get('/:second', function terminalMiddleware(ctx) {
582579
ctx.body = 'this is a test'
583580
})
584-
router.get('/second', function secondMiddleware(ctx) {
585-
ctx.body = 'want this to set the name'
586-
})
587581
router.prefix('/:first')
588582
app.use(router.routes())
589583

590584
agent.on('transactionFinished', (tx) => {
591585
t.assertSegments(tx.trace.root, [
592-
'WebTransaction/WebFrameworkUri/Koa/GET//:first/second',
586+
'WebTransaction/WebFrameworkUri/Koa/GET//:first/:second',
593587
[
594588
'Nodejs/Middleware/Koa/appLevelMiddleware',
595589
['Koa/Router: /', ['Nodejs/Middleware/Koa/terminalMiddleware//:first/:second']]
596590
]
597591
])
598592
t.equal(
599593
tx.name,
600-
'WebTransaction/WebFrameworkUri/Koa/GET//:first/second',
594+
'WebTransaction/WebFrameworkUri/Koa/GET//:first/:second',
601595
'should be named after the last matched path'
602596
)
603597
t.end()
@@ -607,6 +601,11 @@ module.exports = (pkg) => {
607601
})
608602

609603
t.test('using allowedMethods', (t) => {
604+
// `@koa/[email protected]` changed the allowedMethods middleware function from named to arrow function
605+
// update span name for assertions
606+
const allowedMethodsFnName = semver.gte(pkgVersion, '13.0.0')
607+
? '<anonymous>'
608+
: 'allowedMethods'
610609
t.autoend()
611610

612611
t.test('with throw: true', (t) => {
@@ -622,7 +621,7 @@ module.exports = (pkg) => {
622621
agent.on('transactionFinished', (tx) => {
623622
t.assertSegments(tx.trace.root, [
624623
'WebTransaction/WebFrameworkUri/Koa/GET/(method not allowed)',
625-
['Koa/Router: /', ['Nodejs/Middleware/Koa/allowedMethods']]
624+
['Koa/Router: /', [`Nodejs/Middleware/Koa/${allowedMethodsFnName}`]]
626625
])
627626
t.equal(
628627
tx.name,
@@ -645,7 +644,7 @@ module.exports = (pkg) => {
645644
agent.on('transactionFinished', (tx) => {
646645
t.assertSegments(tx.trace.root, [
647646
'WebTransaction/WebFrameworkUri/Koa/GET/(not implemented)',
648-
['Koa/Router: /', ['Nodejs/Middleware/Koa/allowedMethods']]
647+
['Koa/Router: /', [`Nodejs/Middleware/Koa/${allowedMethodsFnName}`]]
649648
])
650649
t.equal(
651650
tx.name,
@@ -683,7 +682,7 @@ module.exports = (pkg) => {
683682
'WebTransaction/NormalizedUri/*',
684683
[
685684
'Nodejs/Middleware/Koa/errorHandler',
686-
['Koa/Router: /', ['Nodejs/Middleware/Koa/allowedMethods']]
685+
['Koa/Router: /', [`Nodejs/Middleware/Koa/${allowedMethodsFnName}`]]
687686
]
688687
])
689688
t.equal(
@@ -722,7 +721,7 @@ module.exports = (pkg) => {
722721
'WebTransaction/WebFrameworkUri/Koa/GET/(method not allowed)',
723722
[
724723
'Nodejs/Middleware/Koa/baseMiddleware',
725-
['Koa/Router: /', ['Nodejs/Middleware/Koa/allowedMethods']]
724+
['Koa/Router: /', [`Nodejs/Middleware/Koa/${allowedMethodsFnName}`]]
726725
]
727726
])
728727
t.equal(
@@ -753,7 +752,7 @@ module.exports = (pkg) => {
753752
agent.on('transactionFinished', (tx) => {
754753
t.assertSegments(tx.trace.root, [
755754
'WebTransaction/WebFrameworkUri/Koa/GET/(method not allowed)',
756-
['Koa/Router: /', ['Nodejs/Middleware/Koa/allowedMethods']]
755+
['Koa/Router: /', [`Nodejs/Middleware/Koa/${allowedMethodsFnName}`]]
757756
])
758757
t.equal(
759758
tx.name,
@@ -777,7 +776,7 @@ module.exports = (pkg) => {
777776
agent.on('transactionFinished', (tx) => {
778777
t.assertSegments(tx.trace.root, [
779778
'WebTransaction/WebFrameworkUri/Koa/GET/(not implemented)',
780-
['Koa/Router: /', ['Nodejs/Middleware/Koa/allowedMethods']]
779+
['Koa/Router: /', [`Nodejs/Middleware/Koa/${allowedMethodsFnName}`]]
781780
])
782781
t.equal(
783782
tx.name,
@@ -811,7 +810,7 @@ module.exports = (pkg) => {
811810
'WebTransaction/WebFrameworkUri/Koa/GET/(method not allowed)',
812811
[
813812
'Nodejs/Middleware/Koa/appLevelMiddleware',
814-
['Koa/Router: /', ['Nodejs/Middleware/Koa/allowedMethods']]
813+
['Koa/Router: /', [`Nodejs/Middleware/Koa/${allowedMethodsFnName}`]]
815814
]
816815
])
817816
t.equal(
@@ -845,7 +844,7 @@ module.exports = (pkg) => {
845844
'WebTransaction/WebFrameworkUri/Koa/GET/(not implemented)',
846845
[
847846
'Nodejs/Middleware/Koa/appLevelMiddleware',
848-
['Koa/Router: /', ['Nodejs/Middleware/Koa/allowedMethods']]
847+
['Koa/Router: /', [`Nodejs/Middleware/Koa/${allowedMethodsFnName}`]]
849848
]
850849
])
851850
t.equal(

0 commit comments

Comments
 (0)