diff --git a/lib/instrumentation/koa/lib/instrumentation.js b/lib/instrumentation/koa/lib/instrumentation.js index 2452ced92..fd8e874a6 100644 --- a/lib/instrumentation/koa/lib/instrumentation.js +++ b/lib/instrumentation/koa/lib/instrumentation.js @@ -4,6 +4,7 @@ */ 'use strict' +const symbols = require('./symbols') module.exports = function initialize(shim, Koa) { if (!shim || !Koa || Object.keys(Koa.prototype).length > 1) { @@ -68,23 +69,23 @@ function wrapCreateContext(shim, fn, fnName, context) { // The `context.body` and `context.response.body` properties are how users set // the response contents. It is roughly equivalent to `res.send()` in Express. // Under the hood, these set the `_body` property on the `context.response`. - context.__NR_body = context.response.body - context.__NR_bodySet = false + context[symbols.body] = context.response.body + context[symbols.bodySet] = false Object.defineProperty(context.response, '_body', { - get: () => context.__NR_body, + get: () => context[symbols.body], set: function setBody(val) { - if (!context.__NR_koaRouter) { + if (!context[symbols.koaRouter]) { shim.savePossibleTransactionName(context.req) } - context.__NR_body = val - context.__NR_bodySet = true + context[symbols.body] = val + context[symbols.bodySet] = true } }) - context.__NR_matchedRoute = null - context.__NR_koaRouter = false + context[symbols.matchedRoute] = null + context[symbols.koaRouter] = false Object.defineProperty(context, '_matchedRoute', { - get: () => context.__NR_matchedRoute, + get: () => context[symbols.matchedRoute], set: (val) => { const match = getLayerForTransactionName(context) @@ -98,7 +99,7 @@ function wrapCreateContext(shim, fn, fnName, context) { if (currentSegment) { const transaction = currentSegment.transaction - if (context.__NR_matchedRoute) { + if (context[symbols.matchedRoute]) { transaction.nameState.popPath() } @@ -107,10 +108,10 @@ function wrapCreateContext(shim, fn, fnName, context) { } } - context.__NR_matchedRoute = val + context[symbols.matchedRoute] = val // still true if somehow match is undefined because we are // using koa-router naming and don't want to allow default naming - context.__NR_koaRouter = true + context[symbols.koaRouter] = true } }) @@ -130,7 +131,7 @@ function wrapCreateContext(shim, fn, fnName, context) { Object.defineProperty(context.response, 'status', { get: () => statusDescriptor.get.call(context.response), set: function setStatus(val) { - if (!context.__NR_bodySet && !context.__NR_koaRouter) { + if (!context[symbols.bodySet] && !context[symbols.koaRouter]) { shim.savePossibleTransactionName(context.req) } return statusDescriptor.set.call(this, val) diff --git a/lib/instrumentation/koa/lib/router-instrumentation.js b/lib/instrumentation/koa/lib/router-instrumentation.js index 99a6a1e20..6623e9828 100644 --- a/lib/instrumentation/koa/lib/router-instrumentation.js +++ b/lib/instrumentation/koa/lib/router-instrumentation.js @@ -4,6 +4,7 @@ */ 'use strict' +const symbols = require('./symbols') module.exports = function instrumentRouter(shim, Router) { shim.setFramework(shim.KOA) @@ -76,7 +77,7 @@ function wrapAllowedMethods(shim, fn, name, allowedMethodsMiddleware) { function wrapAllowedMethodsMiddleware(shim, original) { return function setRouteHandledOnContextWrapper() { const [ctx] = shim.argsToArray.apply(shim, arguments) - ctx.__NR_koaRouter = true + ctx[symbols.koaRouter] = true return original.apply(this, arguments) } diff --git a/lib/instrumentation/koa/lib/symbols.js b/lib/instrumentation/koa/lib/symbols.js new file mode 100644 index 000000000..cf6168cdf --- /dev/null +++ b/lib/instrumentation/koa/lib/symbols.js @@ -0,0 +1,13 @@ +/* + * Copyright 2022 New Relic Corporation. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +'use strict' + +module.exports = { + body: Symbol('body'), + bodySet: Symbol('bodySet'), + koaRouter: Symbol('koaRouter'), + matchedRoute: Symbol('matchedRoute') +} diff --git a/lib/instrumentation/koa/tests/unit/koa.tap.js b/lib/instrumentation/koa/tests/unit/koa.tap.js index e4e1f82cf..b23b403c4 100644 --- a/lib/instrumentation/koa/tests/unit/koa.tap.js +++ b/lib/instrumentation/koa/tests/unit/koa.tap.js @@ -5,28 +5,29 @@ 'use strict' -var tap = require('tap') -var utils = require('@newrelic/test-utilities') +const tap = require('tap') +const utils = require('@newrelic/test-utilities') utils.tap tap.test('Koa instrumentation', function (t) { - var helper = utils.TestAgent.makeInstrumented() + const wrapped = ['createContext', 'use', 'emit'] + const notWrapped = ['handleRequest', 'listen', 'toJSON', 'inspect', 'callback', 'onerror'] + + const helper = utils.TestAgent.makeInstrumented() helper.registerInstrumentation({ moduleName: 'koa', type: 'web-framework', onRequire: require('../../lib/instrumentation') }) - var Koa = require('koa') - - var wrapped = ['createContext', 'use', 'emit'] - var notWrapped = ['handleRequest', 'listen', 'toJSON', 'inspect', 'callback', 'onerror'] + const Koa = require('koa') + const shim = helper.getShim() wrapped.forEach(function (method) { - t.ok(Koa.prototype[method].__NR_original, method + ' is wrapped, as expected') + t.ok(shim.isWrapped(Koa.prototype[method]), method + ' is wrapped, as expected') }) notWrapped.forEach(function (method) { - t.notOk(Koa.prototype[method].__NR_original, method + ' is not wrapped, as expected') + t.not(shim.isWrapped(Koa.prototype[method]), method + ' is not wrapped, as expected') }) helper && helper.unload() diff --git a/lib/instrumentation/koa/tests/unit/route.tap.js b/lib/instrumentation/koa/tests/unit/route.tap.js index df63c3527..de8b91aea 100644 --- a/lib/instrumentation/koa/tests/unit/route.tap.js +++ b/lib/instrumentation/koa/tests/unit/route.tap.js @@ -10,7 +10,7 @@ const utils = require('@newrelic/test-utilities') const { METHODS } = require('../../lib/http-methods') tap.test('koa-route', function (t) { - var helper = utils.TestAgent.makeInstrumented() + const helper = utils.TestAgent.makeInstrumented() t.teardown(function () { helper.unload() @@ -22,10 +22,12 @@ tap.test('koa-route', function (t) { onRequire: require('../../lib/route-instrumentation.js') }) + const shim = helper.getShim() + t.test('methods', function (t) { - var route = require('koa-route') + const route = require('koa-route') METHODS.forEach(function checkWrapped(method) { - t.type(route[method].__NR_original, 'function', method + ' should be wrapped') + t.ok(shim.isWrapped(route[method]), method + ' should be wrapped') }) t.end() }) diff --git a/lib/instrumentation/koa/tests/unit/router.tap.js b/lib/instrumentation/koa/tests/unit/router.tap.js index 19979ff62..1ba222c95 100644 --- a/lib/instrumentation/koa/tests/unit/router.tap.js +++ b/lib/instrumentation/koa/tests/unit/router.tap.js @@ -21,96 +21,49 @@ const UNWRAPPED_METHODS = METHODS.concat([ ]) const UNWRAPPED_STATIC_METHODS = ['url'] -tap.test('koa-router', function tests(t) { - var helper = utils.TestAgent.makeInstrumented() - t.teardown(function () { - helper.unload() - }) - helper.registerInstrumentation({ - type: 'web-framework', - moduleName: 'koa-router', - onRequire: instrumentation - }) - - t.test('mounting paramware', function (t) { - var Router = require('koa-router') - var router = new Router() - router.param('second', function () {}) - t.type(router.params.second.__NR_original, 'function', 'param function should be wrapped') - t.end() - }) +const koaRouterMods = ['koa-router', '@koa/router'] - t.test('methods', function (t) { - var Router = require('koa-router') - WRAPPED_METHODS.forEach(function checkWrapped(method) { - t.type( - Router.prototype[method].__NR_original, - 'function', - method + ' should be a wrapped method on the prototype' - ) +koaRouterMods.forEach((koaRouterMod) => { + tap.test(koaRouterMod, function tests(t) { + const helper = utils.TestAgent.makeInstrumented() + t.teardown(function () { + helper.unload() }) - UNWRAPPED_METHODS.forEach(function checkUnwrapped(method) { - t.type( - Router.prototype[method].__NR_original, - 'undefined', - method + ' should be a unwrapped method on the prototype' - ) - }) - UNWRAPPED_STATIC_METHODS.forEach(function checkUnwrappedStatic(method) { - t.type( - Router[method].__NR_original, - 'undefined', - method + ' should be an unwrapped static method' - ) - }) - t.end() - }) - t.autoend() -}) + const shim = helper.getShim() -tap.test('@koa/router', function tests(t) { - var helper = utils.TestAgent.makeInstrumented() - t.teardown(function () { - helper.unload() - }) - helper.registerInstrumentation({ - type: 'web-framework', - moduleName: '@koa/router', - onRequire: instrumentation - }) - - t.test('mounting paramware', function (t) { - var Router = require('@koa/router') - var router = new Router() - router.param('second', function () {}) - t.type(router.params.second.__NR_original, 'function', 'param function should be wrapped') - t.end() - }) - - t.test('methods', function (t) { - var Router = require('@koa/router') - WRAPPED_METHODS.forEach(function checkWrapped(method) { - t.type( - Router.prototype[method].__NR_original, - 'function', - method + ' should be a wrapped method on the prototype' - ) + helper.registerInstrumentation({ + type: 'web-framework', + moduleName: koaRouterMod, + onRequire: instrumentation }) - UNWRAPPED_METHODS.forEach(function checkUnwrapped(method) { - t.type( - Router.prototype[method].__NR_original, - 'undefined', - method + ' should be a unwrapped method on the prototype' - ) + + t.test('mounting paramware', function (t) { + var Router = require(koaRouterMod) + var router = new Router() + router.param('second', function () {}) + t.ok(shim.isWrapped(router.params.second), 'param function should be wrapped') + t.end() }) - UNWRAPPED_STATIC_METHODS.forEach(function checkUnwrappedStatic(method) { - t.type( - Router[method].__NR_original, - 'undefined', - method + ' should be an unwrapped static method' - ) + + t.test('methods', function (t) { + var Router = require(koaRouterMod) + WRAPPED_METHODS.forEach(function checkWrapped(method) { + t.ok( + shim.isWrapped(Router.prototype[method]), + method + ' should be a wrapped method on the prototype' + ) + }) + UNWRAPPED_METHODS.forEach(function checkUnwrapped(method) { + t.not( + shim.isWrapped(Router.prototype[method]), + method + ' should be a unwrapped method on the prototype' + ) + }) + UNWRAPPED_STATIC_METHODS.forEach(function checkUnwrappedStatic(method) { + t.not(shim.isWrapped(Router[method]), method + ' should be an unwrapped static method') + }) + t.end() }) - t.end() + t.autoend() }) - t.autoend() })