Merge pull request newrelic#125 from jordigh/remove-dunder

NEWRELIC-4422: remove __NR prefix properties and usage

Author: bizob2828

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)

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)
   }

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')
+}

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()

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()
   })

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()
 })