chore: migrated koa versioned tests to use agent_helper, moved symbol…

…s to lib/symbols, consolidated koa instrumentation into one folder

lib/instrumentation/grpc-js/grpc.js

@@ -7,7 +7,7 @@
 
 const recordExternal = require('../../metrics/recorders/http_external')
 const recordHttp = require('../../metrics/recorders/http')
-const specs = require('../../shim/specs')
+const { TransactionSpec } = require('../../shim/specs')
 const { DESTINATIONS } = require('../../config/attribute-filter')
 const DESTINATION = DESTINATIONS.TRANS_EVENT | DESTINATIONS.ERROR_EVENT
 const semver = require('semver')
@@ -149,7 +149,7 @@ function wrapRegister(shim, original) {
 
     args[1] = shim.bindCreateTransaction(
       instrumentedHandler,
-      new specs.TransactionSpec({ type: shim.WEB })
+      new TransactionSpec({ type: shim.WEB })
     )
 
     return original.apply(this, args)

lib/instrumentation/koa/lib/instrumentation.js → lib/instrumentation/koa/instrumentation.js

@@ -4,7 +4,8 @@
  */
 
 'use strict'
-const symbols = require('./symbols')
+const symbols = require('../../symbols')
+const { MiddlewareSpec, MiddlewareMounterSpec } = require('../../shim/specs')
 
 module.exports = function initialize(shim, Koa) {
   // Koa's exports are different depending on using CJS or MJS - https://github.com/koajs/koa/issues/1513
@@ -22,7 +23,7 @@ module.exports = function initialize(shim, Koa) {
   shim.wrapMiddlewareMounter(
     proto,
     'use',
-    new shim.specs.MiddlewareMounterSpec({
+    new MiddlewareMounterSpec({
       wrapper: wrapMiddleware
     })
   )
@@ -58,7 +59,7 @@ function wrapMiddleware(shim, middleware) {
 
   return shim.recordMiddleware(
     middleware,
-    new shim.specs.MiddlewareSpec({
+    new MiddlewareSpec({
       type: shim.MIDDLEWARE,
       promise: true,
       appendPath: true,
@@ -70,34 +71,49 @@ function wrapMiddleware(shim, middleware) {
   )
 }
 
-function wrapCreateContext(shim, fn, fnName, context) {
-  // Many of the properties on the `context` object are just aliases for the same
-  // property on the `request` or `response` objects. We take advantage of this
-  // by just intercepting the `request` or `response` property and don't touch
-  // the `context` property.
-  //
-  // See: https://github.com/koajs/koa/blob/master/lib/context.js#L186-L241
+/**
+ * Many of the properties on the `context` object are just aliases for the same
+ * property on the `request` or `response` objects. We take advantage of this
+ * by just intercepting the `request` or `response` property and don't touch
+ * the `context` property.
+ * See: https://github.com/koajs/koa/blob/master/lib/context.js#L186-L241
+ *
+ * @param {Shim} shim instance of shim
+ * @param {function} _fn createContext function
+ * @param {string} _fnName name of function
+ * @param {object} context koa ctx object
+ */
+function wrapCreateContext(shim, _fn, _fnName, context) {
+  wrapResponseBody(shim, context)
+  wrapMatchedRoute(shim, context)
+  wrapResponseStatus(shim, context)
+}
 
+function wrapResponseBody(shim, 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[symbols.body] = context.response.body
-  context[symbols.bodySet] = false
+  context[symbols.koaBody] = context.response.body
+  context[symbols.koaBodySet] = false
+
   Object.defineProperty(context.response, '_body', {
-    get: () => context[symbols.body],
+    get: () => context[symbols.koaBody],
     set: function setBody(val) {
       if (!context[symbols.koaRouter]) {
         shim.savePossibleTransactionName(context.req)
       }
-      context[symbols.body] = val
-      context[symbols.bodySet] = true
+      context[symbols.koaBody] = val
+      context[symbols.koaBodySet] = true
     }
   })
+}
 
-  context[symbols.matchedRoute] = null
+function wrapMatchedRoute(shim, context) {
+  context[symbols.koaMatchedRoute] = null
   context[symbols.koaRouter] = false
+
   Object.defineProperty(context, '_matchedRoute', {
-    get: () => context[symbols.matchedRoute],
+    get: () => context[symbols.koaMatchedRoute],
     set: (val) => {
       const match = getLayerForTransactionName(context)
 
@@ -111,7 +127,7 @@ function wrapCreateContext(shim, fn, fnName, context) {
         if (currentSegment) {
           const transaction = currentSegment.transaction
 
-          if (context[symbols.matchedRoute]) {
+          if (context[symbols.koaMatchedRoute]) {
             transaction.nameState.popPath()
           }
 
@@ -120,13 +136,15 @@ function wrapCreateContext(shim, fn, fnName, context) {
         }
       }
 
-      context[symbols.matchedRoute] = val
+      context[symbols.koaMatchedRoute] = val
       // still true if somehow match is undefined because we are
       // using koa-router naming and don't want to allow default naming
       context[symbols.koaRouter] = true
     }
   })
+}
 
+function wrapResponseStatus(shim, context) {
   // Sometimes people just set `context.status` or `context.response.status`
   // without setting a body. When this happens we'll want to use that as the
   // response point to name the transaction. `context.status` is just an alias
@@ -143,7 +161,7 @@ function wrapCreateContext(shim, fn, fnName, context) {
   Object.defineProperty(context.response, 'status', {
     get: () => statusDescriptor.get.call(context.response),
     set: function setStatus(val) {
-      if (!context[symbols.bodySet] && !context[symbols.koaRouter]) {
+      if (!context[symbols.koaBodySet] && !context[symbols.koaRouter]) {
         shim.savePossibleTransactionName(context.req)
       }
       return statusDescriptor.set.call(this, val)

lib/instrumentation/koa/nr-hooks.js

@@ -12,24 +12,24 @@ module.exports = [
     type: InstrumentationDescriptor.TYPE_WEB_FRAMEWORK,
     moduleName: 'koa',
     shimName: 'koa',
-    onRequire: require('./lib/instrumentation')
+    onRequire: require('./instrumentation')
   },
   {
     type: InstrumentationDescriptor.TYPE_WEB_FRAMEWORK,
     moduleName: 'koa-router',
     shimName: 'koa',
-    onRequire: require('./lib/router-instrumentation')
+    onRequire: require('./router-instrumentation')
   },
   {
     type: InstrumentationDescriptor.TYPE_WEB_FRAMEWORK,
     moduleName: '@koa/router',
     shimName: 'koa',
-    onRequire: require('./lib/router-instrumentation')
+    onRequire: require('./router-instrumentation')
   },
   {
     type: InstrumentationDescriptor.TYPE_WEB_FRAMEWORK,
     moduleName: 'koa-route',
     shimName: 'koa',
-    onRequire: require('./lib/route-instrumentation')
+    onRequire: require('./route-instrumentation')
   }
 ]

lib/instrumentation/koa/lib/route-instrumentation.js → lib/instrumentation/koa/route-instrumentation.js

@@ -5,7 +5,8 @@
 
 'use strict'
 
-const { METHODS } = require('./http-methods')
+const { METHODS } = require('../http-methods')
+const { MiddlewareSpec } = require('../../shim/specs')
 
 module.exports = function instrumentRoute(shim, route) {
   shim.setFramework(shim.KOA)
@@ -16,7 +17,7 @@ module.exports = function instrumentRoute(shim, route) {
         const middleware = methodFn.apply(route, arguments)
         return shim.recordMiddleware(
           middleware,
-          new shim.specs.MiddlewareSpec({
+          new MiddlewareSpec({
             route: arguments[0],
             next: shim.LAST,
             name: shim.getName(arguments[1]),

lib/instrumentation/koa/lib/router-instrumentation.js → lib/instrumentation/koa/router-instrumentation.js

@@ -4,7 +4,8 @@
  */
 
 'use strict'
-const symbols = require('./symbols')
+const symbols = require('../../symbols')
+const { MiddlewareSpec, MiddlewareMounterSpec } = require('../../shim/specs')
 
 module.exports = function instrumentRouter(shim, Router) {
   shim.setFramework(shim.KOA)
@@ -19,7 +20,7 @@ module.exports = function instrumentRouter(shim, Router) {
   shim.wrapMiddlewareMounter(
     proto,
     'param',
-    new shim.specs.MiddlewareMounterSpec({
+    new MiddlewareMounterSpec({
       route: shim.FIRST,
       wrapper: wrapParamware
     })
@@ -29,7 +30,7 @@ module.exports = function instrumentRouter(shim, Router) {
 function wrapParamware(shim, paramware, fnName, route) {
   return shim.recordParamware(
     paramware,
-    new shim.specs.MiddlewareSpec({
+    new MiddlewareSpec({
       name: route,
       next: shim.LAST,
       promise: true,
@@ -46,7 +47,7 @@ function wrapMiddleware(shim, fn, name, layer) {
     return
   }
 
-  const spec = new shim.specs.MiddlewareSpec({
+  const spec = new MiddlewareSpec({
     route: () => layer.path, // defer retrieval
     next: shim.LAST,
     promise: true,
@@ -72,7 +73,7 @@ function wrapAllowedMethods(shim, fn, name, allowedMethodsMiddleware) {
 
   return shim.recordMiddleware(
     wrapped,
-    new shim.specs.MiddlewareSpec({
+    new MiddlewareSpec({
       name: allowedMethodsMiddleware.name,
       promise: true,
       appendPath: false,
@@ -99,7 +100,7 @@ function wrapRoutes(shim, fn, name, dispatchMiddleware) {
   }
   const wrappedDispatch = shim.recordMiddleware(
     dispatchMiddleware,
-    new shim.specs.MiddlewareSpec({
+    new MiddlewareSpec({
       type: shim.ROUTER,
       promise: true,
       appendPath: false,

lib/instrumentation/superagent.js

@@ -5,8 +5,7 @@
 
 'use strict'
 
-const http = require('http')
-const METHODS = http.METHODS.map((method) => method.toLowerCase())
+const { METHODS } = require('./http-methods')
 
 module.exports = function instrument(agent, superagent, moduleName, shim) {
   shim.wrapExport(superagent, function wrapRequest(shim, request) {

lib/symbols.js

@@ -11,6 +11,10 @@ module.exports = {
   databaseName: Symbol('databaseName'),
   disableDT: Symbol('Disable distributed tracing'), // description for backwards compatibility
   executorContext: Symbol('executorContext'),
+  koaBody: Symbol('body'),
+  koaBodySet: Symbol('bodySet'),
+  koaRouter: Symbol('koaRouter'),
+  koaMatchedRoute: Symbol('matchedRoute'),
   name: Symbol('name'),
   onceExecuted: Symbol('onceExecuted'),
   offTheRecord: Symbol('offTheRecord'),

test/lib/agent_helper.js

@@ -653,3 +653,12 @@ helper.isSupportedVersion = function isSupportedVersion(version) {
 helper.destroyProxyAgent = function destroyProxyAgent() {
   require('../../lib/collector/http-agents').proxyAgent().destroy()
 }
+
+/**
+ * Gets a shim instance for a package.
+ * @param {object} pkg exported obj that is instrumented
+ * @returns The existing or newly created shim.
+ */
+helper.getShim = function getShim(pkg) {
+  return pkg?.[symbols.shim]
+}

test/unit/instrumentation/koa/instrumentation.test.js

@@ -7,7 +7,7 @@
 
 const tap = require('tap')
 const sinon = require('sinon')
-const initialize = require('../../../../lib/instrumentation/koa/lib/instrumentation')
+const initialize = require('../../../../lib/instrumentation/koa/instrumentation')
 
 tap.test('Koa instrumentation', (t) => {
   t.beforeEach((t) => {

test/unit/instrumentation/koa/koa.test.js

@@ -8,18 +8,17 @@
 const tap = require('tap')
 const helper = require('../../../lib/agent_helper')
 const InstrumentationDescriptor = require('../../../../lib/instrumentation-descriptor')
-const symbols = require('../../../../lib/symbols')
 
 tap.beforeEach((t) => {
   t.context.agent = helper.instrumentMockedAgent({
     moduleName: 'koa',
     type: InstrumentationDescriptor.TYPE_WEB_FRAMEWORK,
-    onRequire: require('../../../../lib/instrumentation/koa/lib/instrumentation'),
+    onRequire: require('../../../../lib/instrumentation/koa/instrumentation'),
     shimName: 'koa'
   })
 
   t.context.Koa = require('koa')
-  t.context.shim = t.context.Koa[symbols.shim]
+  t.context.shim = helper.getShim(t.context.Koa)
 })
 
 tap.afterEach((t) => {

test/unit/instrumentation/koa/route.test.js

@@ -6,21 +6,20 @@
 'use strict'
 
 const tap = require('tap')
-const { METHODS } = require('../../../../lib/instrumentation/koa/lib/http-methods')
+const { METHODS } = require('../../../../lib/instrumentation/http-methods')
 const helper = require('../../../lib/agent_helper')
 const InstrumentationDescriptor = require('../../../../lib/instrumentation-descriptor')
-const symbols = require('../../../../lib/symbols')
 
 tap.beforeEach((t) => {
   t.context.agent = helper.instrumentMockedAgent({
     moduleName: 'koa-route',
     type: InstrumentationDescriptor.TYPE_WEB_FRAMEWORK,
-    onRequire: require('../../../../lib/instrumentation/koa/lib/route-instrumentation'),
+    onRequire: require('../../../../lib/instrumentation/koa/route-instrumentation'),
     shimName: 'koa'
   })
 
   t.context.KoaRoute = require('koa-route')
-  t.context.shim = t.context.KoaRoute[symbols.shim]
+  t.context.shim = helper.getShim(t.context.KoaRoute)
 })
 
 tap.afterEach((t) => {

test/unit/instrumentation/koa/router.test.js

@@ -7,11 +7,10 @@
 
 const tap = require('tap')
 
-const instrumentation = require('../../../../lib/instrumentation/koa/lib/router-instrumentation')
-const { METHODS } = require('../../../../lib/instrumentation/koa/lib/http-methods')
+const instrumentation = require('../../../../lib/instrumentation/koa/router-instrumentation')
+const { METHODS } = require('../../../../lib/instrumentation/http-methods')
 const helper = require('../../../lib/agent_helper')
 const InstrumentationDescriptor = require('../../../../lib/instrumentation-descriptor')
-const symbols = require('../../../../lib/symbols')
 const WRAPPED_METHODS = ['param', 'register', 'routes', 'middleware', 'allowedMethods']
 const UNWRAPPED_METHODS = METHODS.concat([
   'use',
@@ -41,7 +40,7 @@ tap.test('koa-router', (t) => {
     })
 
     t.context.mod = require(koaRouterMod)
-    t.context.shim = t.context.mod[symbols.shim]
+    t.context.shim = helper.getShim(t.context.mod)
   })
 
   t.afterEach((t) => {
@@ -95,7 +94,7 @@ tap.test('koa-router', (t) => {
     })
 
     t.context.mod = require(koaRouterMod)
-    t.context.shim = t.context.mod[symbols.shim]
+    t.context.shim = helper.getShim(t.context.mod)
   })
 
   t.afterEach((t) => {