Skip to content

Commit

Permalink
chore: migrated koa versioned tests to use agent_helper, moved symbol…
Browse files Browse the repository at this point in the history
…s to lib/symbols, consolidated koa instrumentation into one folder
  • Loading branch information
bizob2828 committed Apr 17, 2024
1 parent a338a21 commit 7667ce6
Show file tree
Hide file tree
Showing 19 changed files with 514 additions and 864 deletions.
4 changes: 2 additions & 2 deletions lib/instrumentation/grpc-js/grpc.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -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)
Expand Down
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -22,7 +23,7 @@ module.exports = function initialize(shim, Koa) {
shim.wrapMiddlewareMounter(
proto,
'use',
new shim.specs.MiddlewareMounterSpec({
new MiddlewareMounterSpec({
wrapper: wrapMiddleware
})
)
Expand Down Expand Up @@ -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,
Expand All @@ -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)

Expand All @@ -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()
}

Expand All @@ -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
Expand All @@ -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)
Expand Down
13 changes: 0 additions & 13 deletions lib/instrumentation/koa/lib/symbols.js

This file was deleted.

8 changes: 4 additions & 4 deletions lib/instrumentation/koa/nr-hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
}
]
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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]),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
})
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down
3 changes: 1 addition & 2 deletions lib/instrumentation/superagent.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
4 changes: 4 additions & 0 deletions lib/symbols.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
Expand Down
9 changes: 9 additions & 0 deletions test/lib/agent_helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -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]
}
2 changes: 1 addition & 1 deletion test/unit/instrumentation/koa/instrumentation.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
2 changes: 1 addition & 1 deletion test/unit/instrumentation/koa/koa.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ 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'
})

Expand Down
4 changes: 2 additions & 2 deletions test/unit/instrumentation/koa/route.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
'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')
Expand All @@ -15,7 +15,7 @@ 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'
})

Expand Down
4 changes: 2 additions & 2 deletions test/unit/instrumentation/koa/router.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@

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')
Expand Down
Loading

0 comments on commit 7667ce6

Please sign in to comment.