diff --git a/THIRD_PARTY_NOTICES.md b/THIRD_PARTY_NOTICES.md index 5b4fec7336..9ec24c16bd 100644 --- a/THIRD_PARTY_NOTICES.md +++ b/THIRD_PARTY_NOTICES.md @@ -712,7 +712,7 @@ This product includes source derived from [@newrelic/aws-sdk](https://github.com ### @newrelic/koa -This product includes source derived from [@newrelic/koa](https://github.com/newrelic/node-newrelic-koa) ([v9.0.0](https://github.com/newrelic/node-newrelic-koa/tree/v9.0.0)), distributed under the [Apache-2.0 License](https://github.com/newrelic/node-newrelic-koa/blob/v9.0.0/LICENSE): +This product includes source derived from [@newrelic/koa](https://github.com/newrelic/node-newrelic-koa) ([v9.1.0](https://github.com/newrelic/node-newrelic-koa/tree/v9.1.0)), distributed under the [Apache-2.0 License](https://github.com/newrelic/node-newrelic-koa/blob/v9.1.0/LICENSE): ``` Apache License diff --git a/lib/instrumentation/cassandra-driver.js b/lib/instrumentation/cassandra-driver.js index 7bc89623b0..84db9d9e02 100644 --- a/lib/instrumentation/cassandra-driver.js +++ b/lib/instrumentation/cassandra-driver.js @@ -28,7 +28,9 @@ module.exports = function initialize(_agent, cassandra, _moduleName, shim) { shim.recordOperation( ClientProto, ['connect', 'shutdown'], - new OperationSpec({ callback: shim.LAST }) + function operationSpec(shim, _fn, name) { + return new OperationSpec({ callback: shim.LAST, name }) + } ) if (semver.satisfies(cassandraVersion, '>=4.4.0')) { diff --git a/lib/instrumentation/mongodb/common.js b/lib/instrumentation/mongodb/common.js index 50e1ac7069..94878af119 100644 --- a/lib/instrumentation/mongodb/common.js +++ b/lib/instrumentation/mongodb/common.js @@ -23,12 +23,9 @@ common.NR_ATTRS = Symbol('NR_ATTRS') common.instrumentCursor = function instrumentCursor(shim, Cursor) { if (Cursor && Cursor.prototype) { const proto = Cursor.prototype - for (let i = 0; i < CURSOR_OPS.length; i++) { - shim.recordQuery(proto, CURSOR_OPS[i], common.makeQueryDescFunc(shim, CURSOR_OPS[i])) - } - - shim.recordQuery(proto, 'each', common.makeQueryDescFunc(shim, 'each')) - shim.recordOperation(proto, 'pipe', new OperationSpec({ opaque: true })) + shim.recordQuery(proto, CURSOR_OPS, common.makeQueryDescFunc) + shim.recordQuery(proto, 'each', common.makeQueryDescFunc) + shim.recordOperation(proto, 'pipe', new OperationSpec({ opaque: true, name: 'pipe' })) } } @@ -42,9 +39,7 @@ common.instrumentCursor = function instrumentCursor(shim, Cursor) { common.instrumentCollection = function instrumentCollection(shim, Collection) { if (Collection && Collection.prototype) { const proto = Collection.prototype - for (let i = 0; i < COLLECTION_OPS.length; i++) { - shim.recordQuery(proto, COLLECTION_OPS[i], common.makeQueryDescFunc(shim, COLLECTION_OPS[i])) - } + shim.recordQuery(proto, COLLECTION_OPS, common.makeQueryDescFunc) } } @@ -72,13 +67,20 @@ common.instrumentBulkOperation = function instrumentBulkOperation(shim, BulkOper common.instrumentDb = function instrumentDb(shim, Db) { if (Db && Db.prototype) { const proto = Db.prototype + shim.recordOperation(proto, DB_OPS, function makeOperationDescFunc(shim, _fn, methodName) { + return new OperationSpec({ + callback: shim.LAST, + opaque: true, + promise: true, + name: methodName + }) + }) + // link to client.connect(removed in v4.0) shim.recordOperation( - proto, - DB_OPS, - new OperationSpec({ callback: shim.LAST, opaque: true, promise: true }) + Db, + 'connect', + new OperationSpec({ callback: shim.LAST, promise: true, name: 'connect' }) ) - // link to client.connect(removed in v4.0) - shim.recordOperation(Db, 'connect', new OperationSpec({ callback: shim.LAST, promise: true })) } } @@ -86,29 +88,27 @@ common.instrumentDb = function instrumentDb(shim, Db) { * Sets up the desc for all instrumented query methods * * @param {Shim} shim instance of shim - * @param {string} methodName name of method getting executed - * @returns {object} query spec + * @param {Function} fn function getting instrumented + * @param _fn + * @param {string} methodName name of function + * @returns {QuerySpec} query spec */ -common.makeQueryDescFunc = function makeQueryDescFunc(shim, methodName) { +common.makeQueryDescFunc = function makeQueryDescFunc(shim, _fn, methodName) { if (methodName === 'each') { - return function eachDescFunc() { - const parameters = getInstanceAttributeParameters(shim, this) - return new QuerySpec({ query: methodName, parameters, rowCallback: shim.LAST, opaque: true }) - } - } - - return function queryDescFunc() { - // segment name does not actually use query string - // method name is set as query so the query parser has access to the op name const parameters = getInstanceAttributeParameters(shim, this) - return new QuerySpec({ - query: methodName, - parameters, - promise: true, - callback: shim.LAST, - opaque: true - }) + return new QuerySpec({ query: methodName, parameters, rowCallback: shim.LAST, opaque: true }) } + + // segment name does not actually use query string + // method name is set as query so the query parser has access to the op name + const parameters = getInstanceAttributeParameters(shim, this) + return new QuerySpec({ + query: methodName, + parameters, + promise: true, + callback: shim.LAST, + opaque: true + }) } /** diff --git a/lib/instrumentation/mongodb/v2-mongo.js b/lib/instrumentation/mongodb/v2-mongo.js index 96b85b05d5..5ee1231460 100644 --- a/lib/instrumentation/mongodb/v2-mongo.js +++ b/lib/instrumentation/mongodb/v2-mongo.js @@ -38,7 +38,7 @@ module.exports = function instrument(shim, mongodb) { const recordDesc = { Gridstore: { isQuery: false, - makeDescFunc: function makeGridDesc(opName) { + makeDescFunc: function makeGridDesc(shim, fn, opName) { return new OperationSpec({ name: 'GridFS-' + opName, callback: shim.LAST }) } }, @@ -50,8 +50,8 @@ module.exports = function instrument(shim, mongodb) { Collection: { isQuery: true, makeDescFunc: makeQueryDescFunc }, Db: { isQuery: false, - makeDescFunc: function makeDbDesc() { - return new OperationSpec({ callback: shim.LAST }) + makeDescFunc: function makeDbDesc(shim, fn, method) { + return new OperationSpec({ callback: shim.LAST, name: method }) } } } @@ -95,10 +95,10 @@ module.exports = function instrument(shim, mongodb) { const { isQuery, makeDescFunc } = recordDesc[objectName] const proto = object.prototype if (isQuery) { - shim.recordQuery(proto, method, makeDescFunc(shim, method)) + shim.recordQuery(proto, method, makeDescFunc) } else if (isQuery === false) { // could be unset - shim.recordOperation(proto, method, makeDescFunc(shim, method)) + shim.recordOperation(proto, method, makeDescFunc) } else { shim.logger.trace('No wrapping method found for %s', objectName) } @@ -108,7 +108,11 @@ module.exports = function instrument(shim, mongodb) { // the cursor object implements Readable stream and internally calls nextObject on // each read, in which case we do not want to record each nextObject() call if (/Cursor$/.test(objectName)) { - shim.recordOperation(object.prototype, 'pipe', new OperationSpec({ opaque: true })) + shim.recordOperation( + object.prototype, + 'pipe', + new OperationSpec({ opaque: true, name: 'pipe' }) + ) } } } diff --git a/lib/instrumentation/mongodb/v3-mongo.js b/lib/instrumentation/mongodb/v3-mongo.js index 5e1493ddcf..4eb4b03a66 100644 --- a/lib/instrumentation/mongodb/v3-mongo.js +++ b/lib/instrumentation/mongodb/v3-mongo.js @@ -48,7 +48,7 @@ function instrumentClient(shim, mongodb) { // Add the connection url to the MongoClient to retrieve later in the `lib/instrumentation/mongo/common` // captureAttributesOnStarted listener this[NR_ATTRS] = args[0] - return new RecorderSpec({ callback: shim.LAST }) + return new RecorderSpec({ callback: shim.LAST, name: 'connect' }) }) } diff --git a/lib/instrumentation/mongodb/v4-mongo.js b/lib/instrumentation/mongodb/v4-mongo.js index c34b929b25..0943685ac2 100644 --- a/lib/instrumentation/mongodb/v4-mongo.js +++ b/lib/instrumentation/mongodb/v4-mongo.js @@ -73,7 +73,7 @@ function cmdStartedHandler(shim, evnt) { function wrapConnect(shim) { this.monitorCommands = true this.on('commandStarted', cmdStartedHandler.bind(this, shim)) - return new OperationSpec({ callback: shim.LAST }) + return new OperationSpec({ callback: shim.LAST, name: 'connect' }) } /** diff --git a/lib/instrumentation/pg.js b/lib/instrumentation/pg.js index d501ba4e2b..9bfe8b748d 100644 --- a/lib/instrumentation/pg.js +++ b/lib/instrumentation/pg.js @@ -6,7 +6,7 @@ 'use strict' const { nrEsmProxy } = require('../symbols') -const { RecorderSpec, QuerySpec } = require('../shim/specs') +const { RecorderSpec, QuerySpec, ClassWrapSpec } = require('../shim/specs') const DatastoreParameters = require('../shim/specs/params/datastore') function getQuery(shim, original, name, args) { @@ -58,12 +58,12 @@ module.exports = function initialize(agent, pgsql, moduleName, shim) { // wrapping for native function instrumentPGNative(pg) { shim.wrapReturn(pg, 'Client', clientFactoryWrapper) - shim.wrapClass(pg, 'Pool', { post: poolPostConstructor, es6: true }) + shim.wrapClass(pg, 'Pool', new ClassWrapSpec({ post: poolPostConstructor, es6: true })) } function poolPostConstructor(shim) { if (!shim.isWrapped(this.Client)) { - shim.wrapClass(this, 'Client', clientPostConstructor) + shim.wrapClass(this, 'Client', new ClassWrapSpec({ post: clientPostConstructor })) } } diff --git a/lib/shim/datastore-shim.js b/lib/shim/datastore-shim.js index d1dd332fcc..82bee9e62e 100644 --- a/lib/shim/datastore-shim.js +++ b/lib/shim/datastore-shim.js @@ -14,9 +14,8 @@ const ParsedStatement = require('../db/parsed-statement') const Shim = require('./shim') const urltils = require('../util/urltils') const util = require('util') -const { - params: { DatastoreParameters } -} = require('./specs/') +const specs = require('./specs') +const { DatastoreParameters } = specs.params /** * An enumeration of well-known datastores so that new instrumentations can use @@ -263,6 +262,8 @@ function recordOperation(nodule, properties, opSpec) { opSpec = properties properties = null } + + // TODO: not a fan of this. people should always pass in some sort of spec if (!opSpec) { opSpec = Object.create(null) } @@ -270,11 +271,19 @@ function recordOperation(nodule, properties, opSpec) { return this.record(nodule, properties, function operationRecorder(shim, fn, fnName, args) { shim.logger.trace('Recording datastore operation "%s"', fnName) - const segDesc = _getSpec.call(this, { spec: opSpec, shim, fn, fnName, args }) + const segDesc = _getSpec.call(this, { + spec: opSpec, + shim, + fn, + fnName, + args + }) // Adjust the segment name with the metric prefix and add a recorder. if (!hasOwnProperty(segDesc, 'record') || segDesc.record !== false) { - segDesc.name = shim._metrics.OPERATION + segDesc.name + if (!segDesc?.name?.startsWith(shim._metrics.OPERATION)) { + segDesc.name = shim._metrics.OPERATION + segDesc.name + } segDesc.recorder = _recordOperationMetrics.bind(shim) } @@ -534,6 +543,7 @@ function _recordQuery(suffix, nodule, properties, querySpec) { querySpec = properties properties = null } + if (!querySpec) { this.logger.debug('Missing query spec for recordQuery, not wrapping.') return nodule @@ -579,20 +589,13 @@ function _recordQuery(suffix, nodule, properties, querySpec) { * */ function _getSpec({ spec, shim, fn, fnName, args }) { - let dsSpec = null + let dsSpec = spec if (shim.isFunction(spec)) { dsSpec = spec.call(this, shim, fn, fnName, args) - } else { - dsSpec = { ...spec } } - dsSpec.name = dsSpec.name || fnName || 'other' - - const parameters = _normalizeParameters.call(shim, dsSpec.parameters) - return { - ...dsSpec, - parameters - } + dsSpec.parameters = _normalizeParameters.call(shim, dsSpec.parameters) + return dsSpec } /** diff --git a/lib/shim/message-shim/consume.js b/lib/shim/message-shim/consume.js index 5492c922d9..62dc39121a 100644 --- a/lib/shim/message-shim/consume.js +++ b/lib/shim/message-shim/consume.js @@ -26,9 +26,8 @@ function updateSpecFromArgs({ shim, fn, fnName, args, spec }) { let msgDesc = null if (shim.isFunction(spec)) { msgDesc = spec.call(this, shim, fn, fnName, args) - msgDesc = new specs.MessageSpec(msgDesc) } else { - msgDesc = new specs.MessageSpec(spec) + msgDesc = spec const destIdx = shim.normalizeIndex(args.length, spec.destinationName) if (destIdx !== null) { msgDesc.destinationName = args[destIdx] diff --git a/lib/shim/message-shim/index.js b/lib/shim/message-shim/index.js index ac17b11ecb..6313f60940 100644 --- a/lib/shim/message-shim/index.js +++ b/lib/shim/message-shim/index.js @@ -233,7 +233,7 @@ function recordProduce(nodule, properties, recordNamer) { return null } - const name = _nameMessageSegment(shim, msgDesc, shim._metrics.PRODUCE) + msgDesc.name = _nameMessageSegment(shim, msgDesc, shim._metrics.PRODUCE) if (!shim.agent.config.message_tracer.segment_parameters.enabled) { delete msgDesc.parameters } else if (msgDesc.routingKey) { @@ -242,19 +242,13 @@ function recordProduce(nodule, properties, recordNamer) { }) } - return { - name: name, - promise: msgDesc.promise || false, - callback: msgDesc.callback || null, - recorder: genericRecorder, - inContext: function generateCATHeaders() { - if (msgDesc.headers) { - shim.insertCATRequestHeaders(msgDesc.headers, true) - } - }, - parameters: msgDesc.parameters || null, - opaque: msgDesc.opaque || false + msgDesc.inContext = function generateCATHeaders() { + if (msgDesc.headers) { + shim.insertCATRequestHeaders(msgDesc.headers, true) + } } + msgDesc.recorder = genericRecorder + return msgDesc }) } /** @@ -292,9 +286,6 @@ function recordConsume(nodule, properties, spec) { spec = properties properties = null } - if (!this.isFunction(spec)) { - spec = new specs.MessageSpec(spec) - } // This is using wrap instead of record because the spec allows for a messageHandler // which is being used to handle the result of the callback or promise of the @@ -345,11 +336,7 @@ function recordPurgeQueue(nodule, properties, spec) { properties = null } - // Fill the spec with defaults. const specIsFunction = this.isFunction(spec) - if (!specIsFunction) { - spec = new specs.MessageSpec(spec || {}) - } return this.record(nodule, properties, function purgeRecorder(shim, fn, name, args) { let descriptor = spec @@ -363,20 +350,16 @@ function recordPurgeQueue(nodule, properties, spec) { queue = args[queueIdx] } - return { - name: _nameMessageSegment( - shim, - { - destinationType: shim.QUEUE, - destinationName: queue - }, - shim._metrics.PURGE - ), - recorder: genericRecorder, - callback: descriptor.callback, - promise: descriptor.promise, - internal: descriptor.internal - } + descriptor.name = _nameMessageSegment( + shim, + { + destinationType: shim.QUEUE, + destinationName: queue + }, + shim._metrics.PURGE + ) + descriptor.recorder = genericRecorder + return descriptor }) } @@ -421,8 +404,6 @@ function recordSubscribedConsume(nodule, properties, spec) { properties = null } - spec = new specs.MessageSubscribeSpec(spec) - // Make sure our spec has what we need. if (!this.isFunction(spec.messageHandler)) { this.logger.debug('spec.messageHandler should be a function') @@ -456,13 +437,12 @@ function recordSubscribedConsume(nodule, properties, spec) { cbIdx = null } - return { + return new specs.RecorderSpec({ name: spec.name || name, callback: cbIdx, promise: spec.promise, - stream: false, internal: false - } + }) }) } diff --git a/lib/shim/message-shim/subscribe-consume.js b/lib/shim/message-shim/subscribe-consume.js index 0f1d18fd62..8a65e22bc7 100644 --- a/lib/shim/message-shim/subscribe-consume.js +++ b/lib/shim/message-shim/subscribe-consume.js @@ -77,12 +77,11 @@ function createSubscriberWrapper({ shim, fn, spec, destNameIsArg }) { * @returns {Function} wrapped consumer function */ function makeWrapConsumer({ spec, queue, destinationName, destNameIsArg }) { - const msgDescDefaults = new specs.MessageSubscribeSpec(spec) if (destNameIsArg && destinationName != null) { - msgDescDefaults.destinationName = destinationName + spec.destinationName = destinationName } if (queue != null) { - msgDescDefaults.queue = queue + spec.queue = queue } return function wrapConsumer(shim, consumer, cName) { @@ -90,7 +89,7 @@ function makeWrapConsumer({ spec, queue, destinationName, destNameIsArg }) { return consumer } - const consumerWrapper = createConsumerWrapper({ shim, consumer, cName, spec: msgDescDefaults }) + const consumerWrapper = createConsumerWrapper({ shim, consumer, cName, spec }) return shim.bindCreateTransaction( consumerWrapper, new specs.TransactionSpec({ diff --git a/lib/shim/promise-shim.js b/lib/shim/promise-shim.js index e95f7b2582..ef3c864927 100644 --- a/lib/shim/promise-shim.js +++ b/lib/shim/promise-shim.js @@ -8,6 +8,7 @@ const logger = require('../logger').child({ component: 'PromiseShim' }) const Shim = require('./shim') const symbols = require('../symbols') +const { ClassWrapSpec } = require('./specs') /** * Checks if function is actually not a function @@ -101,35 +102,39 @@ class PromiseShim extends Shim { * @see PromiseShim#wrapExecutorCaller */ wrapConstructor(nodule, properties) { - return this.wrapClass(nodule, properties, { - pre: function prePromise(shim, Promise, name, args) { - // We are expecting one function argument for executor, anything else is - // non-standard, do not attempt to wrap. Also do not attempt to wrap if - // we are not in a transaction. - if (args.length !== 1 || !shim.isFunction(args[0]) || !shim.getActiveSegment()) { - return - } - _wrapExecutorContext(shim, args) - }, - post: function postPromise(shim, Promise, name, args) { - // This extra property is added by `_wrapExecutorContext` in the pre step. - const executor = args[0] - const context = executor && executor[symbols.executorContext] - if (!context || !shim.isFunction(context.executor)) { - return - } + return this.wrapClass( + nodule, + properties, + new ClassWrapSpec({ + pre: function prePromise(shim, Promise, name, args) { + // We are expecting one function argument for executor, anything else is + // non-standard, do not attempt to wrap. Also do not attempt to wrap if + // we are not in a transaction. + if (args.length !== 1 || !shim.isFunction(args[0]) || !shim.getActiveSegment()) { + return + } + _wrapExecutorContext(shim, args) + }, + post: function postPromise(shim, Promise, name, args) { + // This extra property is added by `_wrapExecutorContext` in the pre step. + const executor = args[0] + const context = executor && executor[symbols.executorContext] + if (!context || !shim.isFunction(context.executor)) { + return + } - context.promise = this - Contextualizer.link(null, this, shim.getSegment()) - try { - // Must run after promise is defined so that `__NR_wrapper` can be set. - context.executor.apply(context.self, context.args) - } catch (e) { - const reject = context.args[1] - reject(e) + context.promise = this + Contextualizer.link(null, this, shim.getSegment()) + try { + // Must run after promise is defined so that `__NR_wrapper` can be set. + context.executor.apply(context.self, context.args) + } catch (e) { + const reject = context.args[1] + reject(e) + } } - } - }) + }) + ) } /** diff --git a/lib/shim/shim.js b/lib/shim/shim.js index f3c8cf0d9f..1a0a49a307 100644 --- a/lib/shim/shim.js +++ b/lib/shim/shim.js @@ -321,9 +321,6 @@ function wrap(nodule, properties, spec, args) { }) } - // TODO: Add option for omitting symbols.original; unwrappable: false - spec = this.setDefaults(spec, { matchArity: false }) - // If we're just wrapping one thing, just wrap it and return. if (properties == null) { const name = this.getName(nodule) @@ -536,12 +533,7 @@ function wrapClass(nodule, properties, spec, args) { spec = properties properties = null } - if (this.isFunction(spec)) { - spec = new specs.ClassWrapSpec({ pre: null, post: spec }) - } else { - spec.pre = spec.pre || null - spec.post = spec.post || null - } + if (!this.isArray(args)) { args = [] } @@ -673,12 +665,11 @@ function recordWrapper({ shim, fn, name, recordNamer }) { return function wrapper() { // Create the segment that will be recorded. const args = argsToArray.apply(shim, arguments) - let segDesc = recordNamer.call(this, shim, fn, name, args) + const segDesc = recordNamer.call(this, shim, fn, name, args) if (!segDesc) { shim.logger.trace('No segment descriptor for "%s", not recording.', name) return fnApply.call(fn, this, args) } - segDesc = new specs.RecorderSpec(segDesc) // See if we're in an active transaction. let parent @@ -1230,18 +1221,21 @@ function createSegment(name, recorder, parent) { * `null` is returned. */ function _rawCreateSegment(shim, opts) { - // Grab parent segment when none in opts so we can check opaqueness - opts.parent = opts.parent || shim.getActiveSegment() + // Grab parent segment when none in opts so we can check opaqueness. + // Also, saving reference and not assigning to opts to avoid hoisting + // this value to other executions of the same method or a shared spec + // definition + const parent = opts.parent || shim.getActiveSegment() // When parent exists and is opaque, no new segment will be created // by tracer.createSegment and the parent will be returned. We bail // out early so we do not risk modifying the parent segment. - if (opts.parent && opts.parent.opaque) { + if (parent?.opaque) { shim.logger.trace(opts, 'Did not create segment because parent is opaque') - return opts.parent + return parent } - const segment = shim.tracer.createSegment(opts.name, opts.recorder, opts.parent) + const segment = shim.tracer.createSegment(opts.name, opts.recorder, parent) if (segment) { segment.internal = opts.internal segment.opaque = opts.opaque diff --git a/lib/shim/specs/wrap.js b/lib/shim/specs/wrap.js index 83b4aa7457..c70b5f3cc0 100644 --- a/lib/shim/specs/wrap.js +++ b/lib/shim/specs/wrap.js @@ -46,21 +46,13 @@ class WrapSpec extends SegmentSpec { /* eslint-disable jsdoc/require-param-description */ /** - * The parameters may be a function; if so, that function is used as the - * wrapper. Otherwise, the parameters must be an object with a `wrapper` - * property set to the function that should be the wrapper. - * - * @param {WrapSpecParams|function} params + * @param {WrapSpecParams} params */ constructor(params) { super(params) this.matchArity = params.matchArity ?? false - if (typeof params === 'function') { - this.wrapper = params - } else { - this.wrapper = params.wrapper - } + this.wrapper = params.wrapper } } diff --git a/lib/shim/webframework-shim/common.js b/lib/shim/webframework-shim/common.js index b11cc46cc3..b4456c8912 100644 --- a/lib/shim/webframework-shim/common.js +++ b/lib/shim/webframework-shim/common.js @@ -20,26 +20,6 @@ common.MIDDLEWARE_TYPE_NAMES = { ROUTER: 'ROUTER' } -/** - * Copy the keys expected from source to destination. - * - * @private - * @param {object} destination - * The spec object receiving the expected values - * @param {object} source - * The spec object the values are coming from - */ -common.copyExpectedSpecParameters = function copyExpectedSpecParameters(destination, source) { - const keys = ['matchArity'] - - for (let i = 0; i < keys.length; ++i) { - const key = keys[i] - if (source[key] != null) { - destination[key] = source[key] - } - } -} - /** * Retrieves the cached transaction information from the given object if it is * available. diff --git a/lib/shim/webframework-shim/index.js b/lib/shim/webframework-shim/index.js index 4944535414..91faa49360 100644 --- a/lib/shim/webframework-shim/index.js +++ b/lib/shim/webframework-shim/index.js @@ -12,13 +12,7 @@ const TransactionShim = require('../transaction-shim') const Shim = require('../shim') const specs = require('../specs') const util = require('util') -const { - assignError, - copyExpectedSpecParameters, - getTransactionInfo, - isError, - MIDDLEWARE_TYPE_NAMES -} = require('./common') +const { assignError, getTransactionInfo, isError, MIDDLEWARE_TYPE_NAMES } = require('./common') const wrapMiddlewareMounter = require('./middleware-mounter') const { _recordMiddleware } = require('./middleware') @@ -183,15 +177,6 @@ function recordRender(nodule, properties, spec) { properties = null } - spec = this.setDefaults( - spec, - new specs.RenderSpec({ - view: this.FIRST, - callback: null, - promise: null - }) - ) - return this.record(nodule, properties, function renderRecorder(shim, fn, name, args) { const viewIdx = shim.normalizeIndex(args.length, spec.view) if (viewIdx === null) { @@ -199,17 +184,9 @@ function recordRender(nodule, properties, spec) { return null } - return { - name: metrics.VIEW.PREFIX + args[viewIdx] + metrics.VIEW.RENDER, - callback: spec.callback, - promise: spec.promise, - recorder: genericRecorder, - - // Hidden class stuff - rowCallback: null, - stream: null, - internal: false - } + spec.recorder = genericRecorder + spec.name = metrics.VIEW.PREFIX + args[viewIdx] + metrics.VIEW.RENDER + return spec }) } @@ -237,15 +214,14 @@ function recordMiddleware(nodule, properties, spec) { spec = properties properties = null } - spec = spec || Object.create(null) - const mwSpec = new specs.MiddlewareSpec(spec) - const wrapSpec = new specs.WrapSpec(function wrapMiddleware(shim, middleware) { - return _recordMiddleware(shim, middleware, mwSpec) + const wrapSpec = new specs.WrapSpec({ + matchArity: spec.matchArity, + wrapper: function wrapMiddleware(shim, middleware) { + return _recordMiddleware(shim, middleware, spec) + } }) - copyExpectedSpecParameters(wrapSpec, spec) - return this.wrap(nodule, properties, wrapSpec) } @@ -279,23 +255,21 @@ function recordParamware(nodule, properties, spec) { spec = properties properties = null } - spec = spec || Object.create(null) - - const mwSpec = new specs.MiddlewareSpec(spec) if (spec && this.isString(spec.name)) { - mwSpec.route = '[param handler :' + spec.name + ']' + spec.route = '[param handler :' + spec.name + ']' } else { - mwSpec.route = '[param handler]' + spec.route = '[param handler]' } - mwSpec.type = MIDDLEWARE_TYPE_NAMES.PARAMWARE + spec.type = MIDDLEWARE_TYPE_NAMES.PARAMWARE - const wrapSpec = new specs.WrapSpec(function wrapParamware(shim, middleware, name) { - mwSpec.name = name - return _recordMiddleware(shim, middleware, mwSpec) + const wrapSpec = new specs.WrapSpec({ + matchArity: spec.matchArity, + wrapper: function wrapParamware(shim, middleware, name) { + spec.name = name + return _recordMiddleware(shim, middleware, spec) + } }) - copyExpectedSpecParameters(wrapSpec, spec) - return this.wrap(nodule, properties, wrapSpec) } diff --git a/lib/shim/webframework-shim/middleware-mounter.js b/lib/shim/webframework-shim/middleware-mounter.js index 94aa7ba6ae..7057e99d3e 100644 --- a/lib/shim/webframework-shim/middleware-mounter.js +++ b/lib/shim/webframework-shim/middleware-mounter.js @@ -5,7 +5,6 @@ 'use strict' -const { copyExpectedSpecParameters } = require('./common') const { MiddlewareMounterSpec } = require('../specs') /** @@ -109,17 +108,10 @@ module.exports = function wrapMiddlewareMounter(nodule, properties, spec) { properties = null } - // TODO: we should remove this. it looks like koa still uses this method - if (this.isFunction(spec)) { - // wrapMiddlewareMounter(nodule [, properties], wrapper) - spec = new MiddlewareMounterSpec({ wrapper: spec }) - } - const wrapSpec = new MiddlewareMounterSpec({ + matchArity: spec.matchArity, wrapper: wrapMounter.bind(null, spec) }) - copyExpectedSpecParameters(wrapSpec, spec) - return this.wrap(nodule, properties, wrapSpec) } diff --git a/lib/shim/webframework-shim/middleware.js b/lib/shim/webframework-shim/middleware.js index 2bd50a5630..1144ae757b 100644 --- a/lib/shim/webframework-shim/middleware.js +++ b/lib/shim/webframework-shim/middleware.js @@ -12,6 +12,7 @@ const { MIDDLEWARE_TYPE_NAMES } = require('./common') const { assignCLMSymbol } = require('../../util/code-level-metrics') +const { RecorderSpec } = require('../specs') const MIDDLEWARE_TYPE_DETAILS = { APPLICATION: { name: 'Mounted App: ', path: true, record: false }, @@ -169,11 +170,11 @@ function middlewareWithCallbackRecorder({ spec, typeDetails, metricName, isError const segmentName = getSegmentName(metricName, typeDetails, route) // Finally, return the segment descriptor. - return { + return new RecorderSpec({ name: segmentName, callback: nextWrapper, parent: txInfo.segmentStack[txInfo.segmentStack.length - 1], - recorder: recorder, + recorder, parameters: params, after: function afterExec(shim, _fn, _name, err) { const errIsError = isError(shim, err) @@ -186,7 +187,7 @@ function middlewareWithCallbackRecorder({ spec, typeDetails, metricName, isError txInfo.segmentStack.pop() } } - } + }) } } @@ -235,12 +236,12 @@ function middlewareWithPromiseRecorder({ spec, typeDetails, metricName, isErrorW const segmentName = getSegmentName(metricName, typeDetails, route) // Finally, return the segment descriptor. - return { + return new RecorderSpec({ name: segmentName, parent: txInfo.segmentStack[txInfo.segmentStack.length - 1], promise: spec.promise, callback: nextWrapper, - recorder: recorder, + recorder, parameters: params, after: function afterExec(shim, _fn, _name, err, result) { if (shim._responsePredicate(args, result)) { @@ -257,7 +258,7 @@ function middlewareWithPromiseRecorder({ spec, typeDetails, metricName, isErrorW } txInfo.segmentStack.pop() } - } + }) } } diff --git a/package-lock.json b/package-lock.json index 740c97db5a..af1e7164d7 100644 --- a/package-lock.json +++ b/package-lock.json @@ -12,7 +12,7 @@ "@grpc/grpc-js": "^1.9.4", "@grpc/proto-loader": "^0.7.5", "@newrelic/aws-sdk": "^7.3.0", - "@newrelic/koa": "^9.0.0", + "@newrelic/koa": "^9.1.0", "@newrelic/ritm": "^7.2.0", "@newrelic/security-agent": "^1.1.1", "@newrelic/superagent": "^7.0.1", @@ -857,9 +857,9 @@ } }, "node_modules/@newrelic/koa": { - "version": "9.0.0", - "resolved": "https://registry.npmjs.org/@newrelic/koa/-/koa-9.0.0.tgz", - "integrity": "sha512-3PS14tkK0mNy9z01wZE7U5IYdI57bh/j//n1rCQNvl+5zTeiPsTmaSeKPQ21XI0RXDkSR5P909jIDt05M1+OsA==", + "version": "9.1.0", + "resolved": "https://registry.npmjs.org/@newrelic/koa/-/koa-9.1.0.tgz", + "integrity": "sha512-huLV/11IZ1CByVlNzU79bUV1p/SHpglFNPT1DJV5NfcfW+czZ0VIWH9gJd8PK1azaZ1Gy2+HV+nZ1mFuoIANnA==", "engines": { "node": ">=16.0.0" } diff --git a/package.json b/package.json index 330cadeef2..79da6cf169 100644 --- a/package.json +++ b/package.json @@ -194,7 +194,7 @@ "@grpc/grpc-js": "^1.9.4", "@grpc/proto-loader": "^0.7.5", "@newrelic/aws-sdk": "^7.3.0", - "@newrelic/koa": "^9.0.0", + "@newrelic/koa": "^9.1.0", "@newrelic/ritm": "^7.2.0", "@newrelic/security-agent": "^1.1.1", "@newrelic/superagent": "^7.0.1", diff --git a/test/unit/shim/datastore-shim.test.js b/test/unit/shim/datastore-shim.test.js index 3fa4af4642..09a3490d98 100644 --- a/test/unit/shim/datastore-shim.test.js +++ b/test/unit/shim/datastore-shim.test.js @@ -321,7 +321,10 @@ test('DatastoreShim', function (t) { t.test( 'should create a datastore operation segment but no metric when `record` is false', function (t) { - shim.recordOperation(wrappable, 'getActiveSegment', { record: false }) + shim.recordOperation(wrappable, 'getActiveSegment', { + record: false, + name: 'getActiveSegment' + }) helper.runInTransaction(agent, function (tx) { const startingSegment = agent.tracer.getSegment() @@ -336,7 +339,10 @@ test('DatastoreShim', function (t) { ) t.test('should create a datastore operation metric when `record` is true', function (t) { - shim.recordOperation(wrappable, 'getActiveSegment', { record: true }) + shim.recordOperation(wrappable, 'getActiveSegment', { + record: true, + name: 'getActiveSegment' + }) helper.runInTransaction(agent, function (tx) { const startingSegment = agent.tracer.getSegment() @@ -350,7 +356,7 @@ test('DatastoreShim', function (t) { }) t.test('should create a datastore operation metric when `record` is defaulted', function (t) { - shim.recordOperation(wrappable, 'getActiveSegment') + shim.recordOperation(wrappable, 'getActiveSegment', { name: 'getActiveSegment' }) helper.runInTransaction(agent, function (tx) { const startingSegment = agent.tracer.getSegment() @@ -623,7 +629,8 @@ test('DatastoreShim', function (t) { function (t) { shim.recordQuery(wrappable, 'getActiveSegment', { query: shim.FIRST, - record: false + record: false, + name: 'getActiveSegment' }) helper.runInTransaction(agent, function (tx) { diff --git a/test/unit/shim/webframework-shim.test.js b/test/unit/shim/webframework-shim.test.js index c64789c1c7..09cc192f09 100644 --- a/test/unit/shim/webframework-shim.test.js +++ b/test/unit/shim/webframework-shim.test.js @@ -11,6 +11,7 @@ const helper = require('../../lib/agent_helper') const Shim = require('../../../lib/shim/shim') const WebFrameworkShim = require('../../../lib/shim/webframework-shim') const symbols = require('../../../lib/symbols') +const { MiddlewareSpec, RenderSpec } = require('../../../lib/shim/specs') test.runOnly = true @@ -283,8 +284,10 @@ test('WebFrameworkShim', function (t) { t.equal(b, 2) t.equal(c, 3) }, - function () { - return ++wrapperCallCount + { + wrapper: function () { + return ++wrapperCallCount + } } ) @@ -420,14 +423,14 @@ test('WebFrameworkShim', function (t) { t.afterEach(afterEach) t.test('should not wrap non-function objects', function (t) { - const wrapped = shim.recordMiddleware(wrappable) + const wrapped = shim.recordMiddleware(wrappable, new MiddlewareSpec({})) t.equal(wrapped, wrappable) t.notOk(shim.isWrapped(wrapped)) t.end() }) t.test('should wrap the first parameter if no properties are given', function (t) { - const wrapped = shim.recordMiddleware(wrappable.bar, {}) + const wrapped = shim.recordMiddleware(wrappable.bar, new MiddlewareSpec({})) t.not(wrapped, wrappable.bar) t.ok(shim.isWrapped(wrapped)) t.equal(shim.unwrap(wrapped), wrappable.bar) @@ -435,7 +438,7 @@ test('WebFrameworkShim', function (t) { }) t.test('should wrap the first parameter if `null` is given for properties', function (t) { - const wrapped = shim.recordMiddleware(wrappable.bar, null, {}) + const wrapped = shim.recordMiddleware(wrappable.bar, null, new MiddlewareSpec({})) t.not(wrapped, wrappable.bar) t.ok(shim.isWrapped(wrapped)) t.equal(shim.unwrap(wrapped), wrappable.bar) @@ -444,7 +447,7 @@ test('WebFrameworkShim', function (t) { t.test('should replace wrapped properties on the original object', function (t) { const original = wrappable.bar - shim.recordMiddleware(wrappable, 'bar', {}) + shim.recordMiddleware(wrappable, 'bar', new MiddlewareSpec({})) t.not(wrappable.bar, original) t.ok(shim.isWrapped(wrappable.bar)) t.equal(shim.unwrap(wrappable.bar), original) @@ -452,7 +455,7 @@ test('WebFrameworkShim', function (t) { }) t.test('should not mark unwrapped properties as wrapped', function (t) { - shim.recordMiddleware(wrappable, 'name', {}) + shim.recordMiddleware(wrappable, 'name', new MiddlewareSpec({})) t.notOk(shim.isWrapped(wrappable.name)) t.end() }) @@ -465,7 +468,7 @@ test('WebFrameworkShim', function (t) { t.equal(a, 'a') t.equal(b, 'b') t.equal(c, 'c') - }) + }, new MiddlewareSpec({})) helper.runInTransaction(agent, function (tx) { txInfo.transaction = tx @@ -480,10 +483,13 @@ test('WebFrameworkShim', function (t) { testType(shim.ERRORWARE, 'Nodejs/Middleware/Restify/getActiveSegment//foo/bar') function testType(type, expectedName) { - const wrapped = shim.recordMiddleware(wrappable.getActiveSegment, { - type: type, - route: '/foo/bar' - }) + const wrapped = shim.recordMiddleware( + wrappable.getActiveSegment, + new MiddlewareSpec({ + type: type, + route: '/foo/bar' + }) + ) helper.runInTransaction(agent, function (tx) { txInfo.transaction = tx sinon.spy(tx.nameState, 'appendPath') @@ -507,10 +513,13 @@ test('WebFrameworkShim', function (t) { testType(shim.PARAMWARE, 'Nodejs/Middleware/Restify/getActiveSegment//foo/bar') function testType(type, expectedName) { - const wrapped = shim.recordMiddleware(wrappable.getActiveSegment, { - type: type, - route: '/foo/bar' - }) + const wrapped = shim.recordMiddleware( + wrappable.getActiveSegment, + new MiddlewareSpec({ + type: type, + route: '/foo/bar' + }) + ) helper.runInTransaction(agent, function (tx) { txInfo.transaction = tx const segment = wrapped(req) @@ -530,10 +539,13 @@ test('WebFrameworkShim', function (t) { testType(shim.PARAMWARE, 'Nodejs/Middleware/Restify/getActiveSegment') function testType(type, expectedName) { - const wrapped = shim.recordMiddleware(wrappable.getActiveSegment, { - type: type, - route: '' - }) + const wrapped = shim.recordMiddleware( + wrappable.getActiveSegment, + new MiddlewareSpec({ + type: type, + route: '' + }) + ) helper.runInTransaction(agent, function (tx) { txInfo.transaction = tx const segment = wrapped(req) @@ -553,10 +565,13 @@ test('WebFrameworkShim', function (t) { testType(shim.PARAMWARE, 'Nodejs/Middleware/Restify/getActiveSegment//one,/two') function testType(type, expectedName) { - const wrapped = shim.recordMiddleware(wrappable.getActiveSegment, { - type: type, - route: ['/one', '/two'] - }) + const wrapped = shim.recordMiddleware( + wrappable.getActiveSegment, + new MiddlewareSpec({ + type: type, + route: ['/one', '/two'] + }) + ) helper.runInTransaction(agent, function (tx) { txInfo.transaction = tx const segment = wrapped(req) @@ -571,10 +586,13 @@ test('WebFrameworkShim', function (t) { testType(shim.MIDDLEWARE, 'Nodejs/Middleware/Restify/getActiveSegment') function testType(type, expectedName) { - const wrapped = shim.recordMiddleware(wrappable.getActiveSegment, { - type: type, - route: '' - }) + const wrapped = shim.recordMiddleware( + wrappable.getActiveSegment, + new MiddlewareSpec({ + type: type, + route: '' + }) + ) const tx = helper.runInTransaction(agent, function (_tx) { return _tx }) @@ -590,10 +608,13 @@ test('WebFrameworkShim', function (t) { t.test('should capture route parameters when high_security is off', function (t) { agent.config.high_security = false - const wrapped = shim.recordMiddleware(wrappable.getActiveSegment, { - type: shim.MIDDLEWARE, - route: ['/one', '/two'] - }) + const wrapped = shim.recordMiddleware( + wrappable.getActiveSegment, + new MiddlewareSpec({ + type: shim.MIDDLEWARE, + route: ['/one', '/two'] + }) + ) helper.runInTransaction(agent, function (tx) { txInfo.transaction = tx const segment = wrapped(req) @@ -605,7 +626,7 @@ test('WebFrameworkShim', function (t) { const filePathSplit = attrs['code.filepath'].split('/') t.equal(filePathSplit[filePathSplit.length - 1], 'webframework-shim.test.js') t.equal(attrs['code.function'], 'getActiveSegment') - t.equal(attrs['code.lineno'], 39) + t.equal(attrs['code.lineno'], 40) t.equal(attrs['code.column'], 50) t.end() }) @@ -613,10 +634,13 @@ test('WebFrameworkShim', function (t) { t.test('should not capture route parameters when high_security is on', function (t) { agent.config.high_security = true - const wrapped = shim.recordMiddleware(wrappable.getActiveSegment, { - type: shim.MIDDLEWARE, - route: ['/one', '/two'] - }) + const wrapped = shim.recordMiddleware( + wrappable.getActiveSegment, + new MiddlewareSpec({ + type: shim.MIDDLEWARE, + route: ['/one', '/two'] + }) + ) helper.runInTransaction(agent, function (tx) { txInfo.transaction = tx const segment = wrapped(req) @@ -633,7 +657,7 @@ test('WebFrameworkShim', function (t) { t.test('should notice thrown exceptions', function (t) { const wrapped = shim.recordMiddleware(function () { throw new Error('foobar') - }) + }, new MiddlewareSpec({})) helper.runInTransaction(agent, function (tx) { txInfo.transaction = tx @@ -648,12 +672,9 @@ test('WebFrameworkShim', function (t) { }) t.test('pops the name if error was thrown and there is no next handler', function (t) { - const wrapped = shim.recordMiddleware( - function () { - throw new Error('foobar') - }, - { route: '/foo/bar' } - ) + const wrapped = shim.recordMiddleware(function () { + throw new Error('foobar') + }, new MiddlewareSpec({ route: '/foo/bar' })) helper.runInTransaction(agent, function (tx) { tx.nameState.appendPath('/') @@ -668,12 +689,9 @@ test('WebFrameworkShim', function (t) { }) t.test('does not pop the name if there was an error and a next handler', function (t) { - const wrapped = shim.recordMiddleware( - function () { - throw new Error('foobar') - }, - { route: '/foo/bar', next: shim.SECOND } - ) + const wrapped = shim.recordMiddleware(function () { + throw new Error('foobar') + }, new MiddlewareSpec({ route: '/foo/bar', next: shim.SECOND })) helper.runInTransaction(agent, function (tx) { tx.nameState.appendPath('/') @@ -688,7 +706,8 @@ test('WebFrameworkShim', function (t) { }) t.test('should pop the namestate if there was no error', function (t) { - const wrapped = shim.recordMiddleware(function () {}, { route: '/foo/bar' }) + const wrapped = shim.recordMiddleware(function () {}, + new MiddlewareSpec({ route: '/foo/bar' })) helper.runInTransaction(agent, function (tx) { tx.nameState.appendPath('/') @@ -701,12 +720,9 @@ test('WebFrameworkShim', function (t) { }) t.test('should pop the namestate if error is not an error', function (t) { - const wrapped = shim.recordMiddleware( - function (r, obj, next) { - next(obj) - }, - { route: '/foo/bar' } - ) + const wrapped = shim.recordMiddleware(function (r, obj, next) { + next(obj) + }, new MiddlewareSpec({ route: '/foo/bar' })) const err = new Error() shim.setErrorPredicate(function (obj) { @@ -726,12 +742,9 @@ test('WebFrameworkShim', function (t) { }) }) t.test('should notice errors handed to the callback', function (t) { - const wrapped = shim.recordMiddleware( - function (_req, next) { - setTimeout(next, 10, new Error('foobar')) - }, - { next: shim.LAST } - ) + const wrapped = shim.recordMiddleware(function (_req, next) { + setTimeout(next, 10, new Error('foobar')) + }, new MiddlewareSpec({ next: shim.LAST })) helper.runInTransaction(agent, function (tx) { txInfo.transaction = tx @@ -747,12 +760,9 @@ test('WebFrameworkShim', function (t) { }) t.test('should not pop the name if there was an error', function (t) { - const wrapped = shim.recordMiddleware( - function (_req, next) { - setTimeout(next, 10, new Error('foobar')) - }, - { route: '/foo/bar', next: shim.LAST } - ) + const wrapped = shim.recordMiddleware(function (_req, next) { + setTimeout(next, 10, new Error('foobar')) + }, new MiddlewareSpec({ route: '/foo/bar', next: shim.LAST })) helper.runInTransaction(agent, function (tx) { tx.nameState.appendPath('/') @@ -765,15 +775,12 @@ test('WebFrameworkShim', function (t) { }) t.test('should pop the namestate if there was no error', function (t) { - const wrapped = shim.recordMiddleware( - function (_req, next) { - setTimeout(function () { - t.equal(txInfo.transaction.nameState.getPath(), '/foo/bar') - next() - }, 10) - }, - { route: '/foo/bar', next: shim.LAST } - ) + const wrapped = shim.recordMiddleware(function (_req, next) { + setTimeout(function () { + t.equal(txInfo.transaction.nameState.getPath(), '/foo/bar') + next() + }, 10) + }, new MiddlewareSpec({ route: '/foo/bar', next: shim.LAST })) helper.runInTransaction(agent, function (tx) { tx.nameState.appendPath('/') @@ -786,11 +793,11 @@ test('WebFrameworkShim', function (t) { }) t.test('should not append path and should not pop path', function (t) { - const spec = { + const spec = new MiddlewareSpec({ route: '/foo/bar', appendPath: false, next: shim.LAST - } + }) const wrapped = shim.recordMiddleware(function (_req, next) { setTimeout(function () { @@ -816,12 +823,15 @@ test('WebFrameworkShim', function (t) { t.test('should mark the error as handled', function (t) { const wrapped = shim.recordMiddleware(function () { throw new Error('foobar') - }) + }, new MiddlewareSpec({})) - const errorware = shim.recordMiddleware(function () {}, { - type: shim.ERRORWARE, - req: shim.SECOND - }) + const errorware = shim.recordMiddleware( + function () {}, + new MiddlewareSpec({ + type: shim.ERRORWARE, + req: shim.SECOND + }) + ) helper.runInTransaction(agent, function (tx) { txInfo.transaction = tx @@ -842,14 +852,11 @@ test('WebFrameworkShim', function (t) { t.test('should notice if the errorware errors', function (t) { const wrapped = shim.recordMiddleware(function () { throw new Error('foobar') - }) + }, new MiddlewareSpec({})) - const errorware = shim.recordMiddleware( - function () { - throw new Error('errorware error') - }, - { type: shim.ERRORWARE, req: shim.SECOND } - ) + const errorware = shim.recordMiddleware(function () { + throw new Error('errorware error') + }, new MiddlewareSpec({ type: shim.ERRORWARE, req: shim.SECOND })) helper.runInTransaction(agent, function (tx) { txInfo.transaction = tx @@ -918,11 +925,14 @@ test('WebFrameworkShim', function (t) { }) } - wrapped = shim.recordMiddleware(middleware, { - route: '/foo/bar', - next: shim.LAST, - promise: true - }) + wrapped = shim.recordMiddleware( + middleware, + new MiddlewareSpec({ + route: '/foo/bar', + next: shim.LAST, + promise: true + }) + ) }) t.afterEach(afterEach) @@ -1058,12 +1068,15 @@ test('WebFrameworkShim', function (t) { t.afterEach(afterEach) t.test('should not append path when spec.appendPath is false', () => { - wrapped = shim.recordMiddleware(middleware, { - route: '/foo/bar', - appendPath: false, - next: shim.LAST, - promise: true - }) + wrapped = shim.recordMiddleware( + middleware, + new MiddlewareSpec({ + route: '/foo/bar', + appendPath: false, + next: shim.LAST, + promise: true + }) + ) return helper.runInTransaction(agent, (tx) => { tx.nameState.appendPath('/') txInfo.transaction = tx @@ -1086,14 +1099,14 @@ test('WebFrameworkShim', function (t) { t.afterEach(afterEach) t.test('should not wrap non-function objects', function (t) { - const wrapped = shim.recordParamware(wrappable) + const wrapped = shim.recordParamware(wrappable, new MiddlewareSpec({})) t.equal(wrapped, wrappable) t.notOk(shim.isWrapped(wrapped)) t.end() }) t.test('should wrap the first parameter if no properties are given', function (t) { - const wrapped = shim.recordParamware(wrappable.bar, {}) + const wrapped = shim.recordParamware(wrappable.bar, new MiddlewareSpec({})) t.not(wrapped, wrappable.bar) t.ok(shim.isWrapped(wrapped)) t.equal(shim.unwrap(wrapped), wrappable.bar) @@ -1101,7 +1114,7 @@ test('WebFrameworkShim', function (t) { }) t.test('should wrap the first parameter if `null` is given for properties', function (t) { - const wrapped = shim.recordParamware(wrappable.bar, null, {}) + const wrapped = shim.recordParamware(wrappable.bar, null, new MiddlewareSpec({})) t.not(wrapped, wrappable.bar) t.ok(shim.isWrapped(wrapped)) t.equal(shim.unwrap(wrapped), wrappable.bar) @@ -1110,7 +1123,7 @@ test('WebFrameworkShim', function (t) { t.test('should replace wrapped properties on the original object', function (t) { const original = wrappable.bar - shim.recordParamware(wrappable, 'bar', {}) + shim.recordParamware(wrappable, 'bar', new MiddlewareSpec({})) t.not(wrappable.bar, original) t.ok(shim.isWrapped(wrappable.bar)) t.equal(shim.unwrap(wrappable.bar), original) @@ -1118,7 +1131,7 @@ test('WebFrameworkShim', function (t) { }) t.test('should not mark unwrapped properties as wrapped', function (t) { - shim.recordParamware(wrappable, 'name', {}) + shim.recordParamware(wrappable, 'name', new MiddlewareSpec({})) t.notOk(shim.isWrapped(wrappable.name)) t.end() }) @@ -1131,7 +1144,7 @@ test('WebFrameworkShim', function (t) { t.equal(a, 'a') t.equal(b, 'b') t.equal(c, 'c') - }) + }, new MiddlewareSpec({})) helper.runInTransaction(agent, function (tx) { txInfo.transaction = tx @@ -1146,10 +1159,13 @@ test('WebFrameworkShim', function (t) { testType(shim.PARAMWARE, 'Nodejs/Middleware/Restify/getActiveSegment//[param handler :foo]') function testType(type, expectedName) { - const wrapped = shim.recordParamware(wrappable.getActiveSegment, { - type: type, - name: 'foo' - }) + const wrapped = shim.recordParamware( + wrappable.getActiveSegment, + new MiddlewareSpec({ + type: type, + name: 'foo' + }) + ) helper.runInTransaction(agent, function (tx) { txInfo.transaction = tx const segment = wrapped(req) @@ -1163,7 +1179,7 @@ test('WebFrameworkShim', function (t) { t.test('should notice thrown exceptions', function (t) { const wrapped = shim.recordParamware(function () { throw new Error('foobar') - }) + }, new MiddlewareSpec({})) helper.runInTransaction(agent, function (tx) { txInfo.transaction = tx @@ -1182,12 +1198,9 @@ test('WebFrameworkShim', function (t) { }) t.test('should not pop the name if there was an error', function (t) { - const wrapped = shim.recordParamware( - function () { - throw new Error('foobar') - }, - { name: 'bar' } - ) + const wrapped = shim.recordParamware(function () { + throw new Error('foobar') + }, new MiddlewareSpec({ name: 'bar' })) helper.runInTransaction(agent, function (tx) { tx.nameState.appendPath('/foo/') @@ -1202,7 +1215,7 @@ test('WebFrameworkShim', function (t) { }) t.test('should pop the namestate if there was no error', function (t) { - const wrapped = shim.recordParamware(function () {}, { name: 'bar' }) + const wrapped = shim.recordParamware(function () {}, new MiddlewareSpec({ name: 'bar' })) helper.runInTransaction(agent, function (tx) { tx.nameState.appendPath('/foo') @@ -1215,12 +1228,9 @@ test('WebFrameworkShim', function (t) { }) t.test('should notice errors handed to the callback', function (t) { - const wrapped = shim.recordParamware( - function (_req, next) { - setTimeout(next, 10, new Error('foobar')) - }, - { next: shim.LAST } - ) + const wrapped = shim.recordParamware(function (_req, next) { + setTimeout(next, 10, new Error('foobar')) + }, new MiddlewareSpec({ next: shim.LAST })) helper.runInTransaction(agent, function (tx) { txInfo.transaction = tx @@ -1236,12 +1246,9 @@ test('WebFrameworkShim', function (t) { }) t.test('should not pop the name if there was an error', function (t) { - const wrapped = shim.recordParamware( - function (_req, next) { - setTimeout(next, 10, new Error('foobar')) - }, - { name: 'bar', next: shim.LAST } - ) + const wrapped = shim.recordParamware(function (_req, next) { + setTimeout(next, 10, new Error('foobar')) + }, new MiddlewareSpec({ name: 'bar', next: shim.LAST })) helper.runInTransaction(agent, function (tx) { tx.nameState.appendPath('/foo') @@ -1254,15 +1261,12 @@ test('WebFrameworkShim', function (t) { }) t.test('should pop the namestate if there was no error', function (t) { - const wrapped = shim.recordParamware( - function (_req, next) { - setTimeout(function () { - t.equal(txInfo.transaction.nameState.getPath(), '/foo/[param handler :bar]') - next() - }, 10) - }, - { name: 'bar', next: shim.LAST } - ) + const wrapped = shim.recordParamware(function (_req, next) { + setTimeout(function () { + t.equal(txInfo.transaction.nameState.getPath(), '/foo/[param handler :bar]') + next() + }, 10) + }, new MiddlewareSpec({ name: 'bar', next: shim.LAST })) helper.runInTransaction(agent, function (tx) { tx.nameState.appendPath('/foo') @@ -1281,14 +1285,14 @@ test('WebFrameworkShim', function (t) { t.afterEach(afterEach) t.test('should not wrap non-function objects', function (t) { - const wrapped = shim.recordRender(wrappable) + const wrapped = shim.recordRender(wrappable, new RenderSpec({ view: shim.FIRST })) t.equal(wrapped, wrappable) t.notOk(shim.isWrapped(wrapped)) t.end() }) t.test('should wrap the first parameter if no properties are given', function (t) { - const wrapped = shim.recordRender(wrappable.bar, {}) + const wrapped = shim.recordRender(wrappable.bar, new RenderSpec({ view: shim.FIRST })) t.not(wrapped, wrappable.bar) t.ok(shim.isWrapped(wrapped)) t.equal(shim.unwrap(wrapped), wrappable.bar) @@ -1296,7 +1300,7 @@ test('WebFrameworkShim', function (t) { }) t.test('should wrap the first parameter if `null` is given for properties', function (t) { - const wrapped = shim.recordRender(wrappable.bar, null, {}) + const wrapped = shim.recordRender(wrappable.bar, null, new RenderSpec({ view: shim.FIRST })) t.not(wrapped, wrappable.bar) t.ok(shim.isWrapped(wrapped)) t.equal(shim.unwrap(wrapped), wrappable.bar) @@ -1305,7 +1309,7 @@ test('WebFrameworkShim', function (t) { t.test('should replace wrapped properties on the original object', function (t) { const original = wrappable.bar - shim.recordRender(wrappable, 'bar', {}) + shim.recordRender(wrappable, 'bar', new RenderSpec({ view: shim.FIRST })) t.not(wrappable.bar, original) t.ok(shim.isWrapped(wrappable.bar)) t.equal(shim.unwrap(wrappable.bar), original) @@ -1313,7 +1317,7 @@ test('WebFrameworkShim', function (t) { }) t.test('should not mark unwrapped properties as wrapped', function (t) { - shim.recordRender(wrappable, 'name', {}) + shim.recordRender(wrappable, 'name', new RenderSpec({ view: shim.FIRST })) t.notOk(shim.isWrapped(wrappable.name)) t.end() }) @@ -1322,7 +1326,7 @@ test('WebFrameworkShim', function (t) { let called = false const wrapped = shim.recordRender(function () { called = true - }) + }, new RenderSpec({ view: shim.FIRST })) t.notOk(called) wrapped() @@ -1331,7 +1335,7 @@ test('WebFrameworkShim', function (t) { }) t.test('should create a segment', function (t) { - shim.recordRender(wrappable, 'getActiveSegment') + shim.recordRender(wrappable, 'getActiveSegment', new RenderSpec({ view: shim.FIRST })) helper.runInTransaction(agent, function () { const segment = wrappable.getActiveSegment('viewToRender') t.equal(segment.name, 'View/viewToRender/Rendering') diff --git a/test/versioned-external/external-repos.js b/test/versioned-external/external-repos.js index 5494091380..15f625fa3e 100644 --- a/test/versioned-external/external-repos.js +++ b/test/versioned-external/external-repos.js @@ -20,7 +20,7 @@ const repos = [ { name: 'koa', repository: 'https://github.com/newrelic/node-newrelic-koa.git', - branch: 'main' + branch: 'fix-paramware' }, { name: 'next', diff --git a/third_party_manifest.json b/third_party_manifest.json index b29c46d12f..55bd3f666d 100644 --- a/third_party_manifest.json +++ b/third_party_manifest.json @@ -1,5 +1,5 @@ { - "lastUpdated": "Thu Mar 21 2024 15:16:56 GMT+0530 (India Standard Time)", + "lastUpdated": "Mon Apr 01 2024 16:40:41 GMT-0400 (Eastern Daylight Time)", "projectName": "New Relic Node Agent", "projectUrl": "https://github.com/newrelic/node-newrelic", "includeOptDeps": true, @@ -81,15 +81,15 @@ "publisher": "New Relic Node.js agent team", "email": "nodejs@newrelic.com" }, - "@newrelic/koa@9.0.0": { + "@newrelic/koa@9.1.0": { "name": "@newrelic/koa", - "version": "9.0.0", - "range": "^9.0.0", + "version": "9.1.0", + "range": "^9.1.0", "licenses": "Apache-2.0", "repoUrl": "https://github.com/newrelic/node-newrelic-koa", - "versionedRepoUrl": "https://github.com/newrelic/node-newrelic-koa/tree/v9.0.0", + "versionedRepoUrl": "https://github.com/newrelic/node-newrelic-koa/tree/v9.1.0", "licenseFile": "node_modules/@newrelic/koa/LICENSE", - "licenseUrl": "https://github.com/newrelic/node-newrelic-koa/blob/v9.0.0/LICENSE", + "licenseUrl": "https://github.com/newrelic/node-newrelic-koa/blob/v9.1.0/LICENSE", "licenseTextSource": "file", "publisher": "New Relic Node.js agent team", "email": "nodejs@newrelic.com"