diff --git a/lib/db/parsed-statement.js b/lib/db/parsed-statement.js index c509e22ab2..ac876fe977 100644 --- a/lib/db/parsed-statement.js +++ b/lib/db/parsed-statement.js @@ -5,9 +5,6 @@ 'use strict' -const { DB, ALL } = require('../metrics/names') -const { DESTINATIONS } = require('../config/attribute-filter') - function ParsedStatement(type, operation, collection, raw) { this.type = type this.operation = operation @@ -21,55 +18,4 @@ function ParsedStatement(type, operation, collection, raw) { } } -ParsedStatement.prototype.recordMetrics = function recordMetrics(segment, scope) { - const duration = segment.getDurationInMillis() - const exclusive = segment.getExclusiveDurationInMillis() - const transaction = segment.transaction - const type = transaction.isWeb() ? DB.WEB : DB.OTHER - const thisTypeSlash = this.type + '/' - const operation = DB.OPERATION + '/' + thisTypeSlash + this.operation - - // Note, an operation metric should _always_ be created even if the action was - // a statement. This is part of the spec. - - // Rollups - transaction.measure(operation, null, duration, exclusive) - transaction.measure(DB.PREFIX + type, null, duration, exclusive) - transaction.measure(DB.PREFIX + thisTypeSlash + type, null, duration, exclusive) - transaction.measure(DB.PREFIX + thisTypeSlash + ALL, null, duration, exclusive) - transaction.measure(DB.ALL, null, duration, exclusive) - - // If we can parse the SQL statement, create a 'statement' metric, and use it - // as the scoped metric for transaction breakdowns. Otherwise, skip the - // 'statement' metric and use the 'operation' metric as the scoped metric for - // transaction breakdowns. - let collection - if (this.collection) { - collection = DB.STATEMENT + '/' + thisTypeSlash + this.collection + '/' + this.operation - transaction.measure(collection, null, duration, exclusive) - if (scope) { - transaction.measure(collection, scope, duration, exclusive) - } - } else if (scope) { - transaction.measure(operation, scope, duration, exclusive) - } - - // This recorder is side-effectful Because we are depending on the recorder - // setting the transaction name, recorders must always be run before generating - // the final transaction trace - segment.name = collection || operation - - // Datastore instance metrics. - const attributes = segment.attributes.get(DESTINATIONS.TRANS_SEGMENT) - if (attributes.host && attributes.port_path_or_id) { - const instanceName = - DB.INSTANCE + '/' + thisTypeSlash + attributes.host + '/' + attributes.port_path_or_id - transaction.measure(instanceName, null, duration, exclusive) - } - - if (this.raw) { - transaction.agent.queries.add(segment, this.type.toLowerCase(), this.raw, this.trace) - } -} - module.exports = ParsedStatement diff --git a/lib/metrics/recorders/database-operation.js b/lib/metrics/recorders/database-operation.js new file mode 100644 index 0000000000..11e4c6ea46 --- /dev/null +++ b/lib/metrics/recorders/database-operation.js @@ -0,0 +1,62 @@ +/* + * Copyright 2024 New Relic Corporation. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +'use strict' + +const metrics = require('../names') + +/** + * Records all the metrics required for database operations. + * + * - `recordOperationMetrics(segment [, scope])` + * + * @private + * @this DatastoreShim + * @implements {MetricFunction} + * @param {TraceSegment} segment - The segment being recorded. + * @param {string} [scope] - The scope of the segment. + * @see DatastoreShim#recordOperation + * @see MetricFunction + */ +function recordOperationMetrics(segment, scope) { + if (!segment) { + return + } + + const duration = segment.getDurationInMillis() + const exclusive = segment.getExclusiveDurationInMillis() + const transaction = segment.transaction + const type = transaction.isWeb() ? 'allWeb' : 'allOther' + const operation = segment.name + + if (scope) { + transaction.measure(operation, scope, duration, exclusive) + } + + transaction.measure(operation, null, duration, exclusive) + transaction.measure(metrics.DB.PREFIX + type, null, duration, exclusive) + transaction.measure(metrics.DB.ALL, null, duration, exclusive) + transaction.measure(this._metrics.ALL, null, duration, exclusive) + transaction.measure( + metrics.DB.PREFIX + this._metrics.PREFIX + '/' + type, + null, + duration, + exclusive + ) + + const attributes = segment.getAttributes() + if (attributes.host && attributes.port_path_or_id) { + const instanceName = [ + metrics.DB.INSTANCE, + this._metrics.PREFIX, + attributes.host, + attributes.port_path_or_id + ].join('/') + + transaction.measure(instanceName, null, duration, exclusive) + } +} + +module.exports = recordOperationMetrics diff --git a/lib/metrics/recorders/database.js b/lib/metrics/recorders/database.js new file mode 100644 index 0000000000..7d4e172ec6 --- /dev/null +++ b/lib/metrics/recorders/database.js @@ -0,0 +1,68 @@ +/* + * Copyright 2024 New Relic Corporation. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +'use strict' + +const { DB, ALL } = require('../names') +const { DESTINATIONS } = require('../../config/attribute-filter') + +/** + * @this ParsedStatement + * @param {TraceSegment} segment - The segment being recorded. + * @param {string} [scope] - The scope of the segment. + */ + +function recordQueryMetrics(segment, scope) { + const duration = segment.getDurationInMillis() + const exclusive = segment.getExclusiveDurationInMillis() + const transaction = segment.transaction + const type = transaction.isWeb() ? DB.WEB : DB.OTHER + const thisTypeSlash = this.type + '/' + const operation = DB.OPERATION + '/' + thisTypeSlash + this.operation + + // Note, an operation metric should _always_ be created even if the action was + // a statement. This is part of the spec. + + // Rollups + transaction.measure(operation, null, duration, exclusive) + transaction.measure(DB.PREFIX + type, null, duration, exclusive) + transaction.measure(DB.PREFIX + thisTypeSlash + type, null, duration, exclusive) + transaction.measure(DB.PREFIX + thisTypeSlash + ALL, null, duration, exclusive) + transaction.measure(DB.ALL, null, duration, exclusive) + + // If we can parse the SQL statement, create a 'statement' metric, and use it + // as the scoped metric for transaction breakdowns. Otherwise, skip the + // 'statement' metric and use the 'operation' metric as the scoped metric for + // transaction breakdowns. + let collection + if (this.collection) { + collection = DB.STATEMENT + '/' + thisTypeSlash + this.collection + '/' + this.operation + transaction.measure(collection, null, duration, exclusive) + if (scope) { + transaction.measure(collection, scope, duration, exclusive) + } + } else if (scope) { + transaction.measure(operation, scope, duration, exclusive) + } + + // This recorder is side-effectful Because we are depending on the recorder + // setting the transaction name, recorders must always be run before generating + // the final transaction trace + segment.name = collection || operation + + // Datastore instance metrics. + const attributes = segment.attributes.get(DESTINATIONS.TRANS_SEGMENT) + if (attributes.host && attributes.port_path_or_id) { + const instanceName = + DB.INSTANCE + '/' + thisTypeSlash + attributes.host + '/' + attributes.port_path_or_id + transaction.measure(instanceName, null, duration, exclusive) + } + + if (this.raw) { + segment.transaction.agent.queries.add(segment, this.type.toLowerCase(), this.raw, this.trace) + } +} + +module.exports = recordQueryMetrics diff --git a/lib/metrics/recorders/middleware.js b/lib/metrics/recorders/middleware.js new file mode 100644 index 0000000000..61693cfb88 --- /dev/null +++ b/lib/metrics/recorders/middleware.js @@ -0,0 +1,29 @@ +/* + * Copyright 2024 New Relic Corporation. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +'use strict' + +/** + * Creates a recorder for middleware metrics. + * + * @private + * @param {object} _shim instance of shim + * @param {string} metricName name of metric + * @returns {Function} recorder for middleware + */ +function makeMiddlewareRecorder(_shim, metricName) { + return function middlewareMetricRecorder(segment, scope) { + const duration = segment.getDurationInMillis() + const exclusive = segment.getExclusiveDurationInMillis() + const transaction = segment.transaction + + if (scope) { + transaction.measure(metricName, scope, duration, exclusive) + } + transaction.measure(metricName, null, duration, exclusive) + } +} + +module.exports = makeMiddlewareRecorder diff --git a/lib/shim/datastore-shim.js b/lib/shim/datastore-shim.js index 9c1b85bfcc..70d103ee0e 100644 --- a/lib/shim/datastore-shim.js +++ b/lib/shim/datastore-shim.js @@ -15,7 +15,9 @@ const Shim = require('./shim') const urltils = require('../util/urltils') const util = require('util') const specs = require('./specs') +const recordOperationMetrics = require('../../lib/metrics/recorders/database-operation') const { DatastoreParameters } = specs.params +const recordQueryMetrics = require('../../lib/metrics/recorders/database') /** * An enumeration of well-known datastores so that new instrumentations can use @@ -284,7 +286,7 @@ function recordOperation(nodule, properties, opSpec) { if (!segDesc?.name?.startsWith(shim._metrics.OPERATION)) { segDesc.name = shim._metrics.OPERATION + segDesc.name } - segDesc.recorder = _recordOperationMetrics.bind(shim) + segDesc.recorder = recordOperationMetrics.bind(shim) } return segDesc @@ -567,7 +569,7 @@ function _recordQuery(suffix, nodule, properties, querySpec) { const name = (parsed.collection || 'other') + '/' + parsed.operation + suffix shim.logger.trace('Found and parsed query %s -> %s', parsed.type, name) segDesc.name = shim._metrics.STATEMENT + name - segDesc.recorder = _recordQueryMetrics.bind(null, parsed) + segDesc.recorder = recordQueryMetrics.bind(parsed) } return segDesc @@ -598,72 +600,6 @@ function _getSpec({ spec, shim, fn, fnName, args }) { return dsSpec } -/** - * Records all query metrics when a segment is active - * - * @private - * @param {ParsedStatement} parsed instance of ParsedStatement - * @param {TraceSegment} segment active segment - * @param {string} scope scope of metrics if it exists - */ -function _recordQueryMetrics(parsed, segment, scope) { - if (segment) { - parsed.recordMetrics(segment, scope) - } -} - -/** - * Records all the metrics required for database operations. - * - * - `_recordOperationMetrics(segment [, scope])` - * - * @private - * @this DatastoreShim - * @implements {MetricFunction} - * @param {TraceSegment} segment - The segment being recorded. - * @param {string} [scope] - The scope of the segment. - * @see DatastoreShim#recordOperation - * @see MetricFunction - */ -function _recordOperationMetrics(segment, scope) { - if (!segment) { - return - } - - const duration = segment.getDurationInMillis() - const exclusive = segment.getExclusiveDurationInMillis() - const transaction = segment.transaction - const type = transaction.isWeb() ? 'allWeb' : 'allOther' - const operation = segment.name - - if (scope) { - transaction.measure(operation, scope, duration, exclusive) - } - - transaction.measure(operation, null, duration, exclusive) - transaction.measure(metrics.DB.PREFIX + type, null, duration, exclusive) - transaction.measure(metrics.DB.ALL, null, duration, exclusive) - transaction.measure(this._metrics.ALL, null, duration, exclusive) - transaction.measure( - metrics.DB.PREFIX + this._metrics.PREFIX + '/' + type, - null, - duration, - exclusive - ) - - const attributes = segment.getAttributes() - if (attributes.host && attributes.port_path_or_id) { - const instanceName = [ - metrics.DB.INSTANCE, - this._metrics.PREFIX, - attributes.host, - attributes.port_path_or_id - ].join('/') - - transaction.measure(instanceName, null, duration, exclusive) - } -} - /** * Extracts the query string from the arguments according to the given spec. * diff --git a/lib/shim/webframework-shim/middleware.js b/lib/shim/webframework-shim/middleware.js index d76f4cab86..ed7cde37e7 100644 --- a/lib/shim/webframework-shim/middleware.js +++ b/lib/shim/webframework-shim/middleware.js @@ -13,6 +13,7 @@ const { } = require('./common') const { assignCLMSymbol } = require('../../util/code-level-metrics') const { RecorderSpec } = require('../specs') +const makeMiddlewareRecorder = require('../../metrics/recorders/middleware') const MIDDLEWARE_TYPE_DETAILS = { APPLICATION: { name: 'Mounted App: ', path: true, record: false }, @@ -88,7 +89,7 @@ function constructRecorder({ txInfo, typeDetails, shim, metricName }) { let recorder = null if (typeDetails.record) { const stackPath = txInfo.transaction.nameState.getPath() || '' - recorder = _makeMiddlewareRecorder(shim, metricName + '/' + stackPath) + recorder = makeMiddlewareRecorder(shim, metricName + '/' + stackPath) } return recorder } @@ -325,27 +326,6 @@ module.exports._recordMiddleware = function _recordMiddleware(shim, middleware, ) } -/** - * Creates a recorder for middleware metrics. - * - * @private - * @param {object} _shim instance of shim - * @param {string} metricName name of metric - * @returns {Function} recorder for middleware - */ -function _makeMiddlewareRecorder(_shim, metricName) { - return function middlewareMetricRecorder(segment, scope) { - const duration = segment.getDurationInMillis() - const exclusive = segment.getExclusiveDurationInMillis() - const transaction = segment.transaction - - if (scope) { - transaction.measure(metricName, scope, duration, exclusive) - } - transaction.measure(metricName, null, duration, exclusive) - } -} - /** * Wrap the `next` middleware function and push on our name state if we find it. We only want to * push the name state if there is a next so that we can safely remove it diff --git a/test/unit/parsed-statement.test.js b/test/unit/metrics-recorder/database-metrics-recorder.test.js similarity index 94% rename from test/unit/parsed-statement.test.js rename to test/unit/metrics-recorder/database-metrics-recorder.test.js index 33b9569699..6625c3cf6d 100644 --- a/test/unit/parsed-statement.test.js +++ b/test/unit/metrics-recorder/database-metrics-recorder.test.js @@ -8,11 +8,12 @@ const test = require('node:test') const assert = require('node:assert') -const helper = require('../lib/agent_helper') -const { match } = require('../lib/custom-assertions') +const helper = require('../../lib/agent_helper') +const { match } = require('../../lib/custom-assertions') -const Transaction = require('../../lib/transaction') -const ParsedStatement = require('../../lib/db/parsed-statement') +const Transaction = require('../../../lib/transaction') +const ParsedStatement = require('../../../lib/db/parsed-statement') +const recordQueryMetrics = require('../../../lib/metrics/recorders/database') function checkMetric(metrics, name, scope) { match(metrics.getMetric(name, scope), { total: 0.333 }) @@ -32,7 +33,7 @@ test('recording database metrics', async (t) => { transaction.type = Transaction.TYPES.BG segment.setDurationInMillis(333) - ps.recordMetrics(segment, 'TEST') + recordQueryMetrics.bind(ps)(segment, 'TEST') transaction.end() ctx.nr.metrics = transaction.metrics @@ -100,7 +101,7 @@ test('recording database metrics', async (t) => { transaction.type = Transaction.TYPES.BG segment.setDurationInMillis(333) - ps.recordMetrics(segment, 'TEST') + recordQueryMetrics.bind(ps)(segment, 'TEST') transaction.end() ctx.nr.metrics = transaction.metrics @@ -165,7 +166,7 @@ test('recording database metrics', async (t) => { transaction.type = Transaction.TYPES.BG segment.setDurationInMillis(333) - ps.recordMetrics(segment, null) + recordQueryMetrics.bind(ps)(segment, null) transaction.end() ctx.nr.metrics = transaction.metrics @@ -228,7 +229,7 @@ test('recording database metrics', async (t) => { transaction.type = Transaction.TYPES.BG segment.setDurationInMillis(333) - ps.recordMetrics(segment, null) + recordQueryMetrics.bind(ps)(segment, null) transaction.end() ctx.nr.metrics = transaction.metrics @@ -297,13 +298,13 @@ test('recording slow queries', async (t) => { ctx.nr.segment = segment segment.setDurationInMillis(503) - ps.recordMetrics(segment, 'TEST') + recordQueryMetrics.bind(ps)(segment, 'TEST') const ps2 = new ParsedStatement('MySql', 'select', 'foo', 'select * from foo where b=2') const segment2 = transaction.trace.add('test') segment2.setDurationInMillis(501) - ps2.recordMetrics(segment2, 'TEST') + recordQueryMetrics.bind(ps2)(segment2, 'TEST') transaction.end() }) @@ -358,13 +359,13 @@ test('recording slow queries', async (t) => { ctx.nr.segment = segment segment.setDurationInMillis(503) - ps.recordMetrics(segment, 'TEST') + recordQueryMetrics.bind(ps)(segment, 'TEST') const ps2 = new ParsedStatement('MySql', 'select', null, 'select * from foo where b=2') const segment2 = transaction.trace.add('test') segment2.setDurationInMillis(501) - ps2.recordMetrics(segment2, 'TEST') + recordQueryMetrics.bind(ps2)(segment2, 'TEST') transaction.end() }) @@ -427,13 +428,13 @@ test('recording slow queries', async (t) => { ctx.nr.segment = segment segment.setDurationInMillis(503) - ps.recordMetrics(segment, 'TEST') + recordQueryMetrics.bind(ps)(segment, 'TEST') const ps2 = new ParsedStatement('MySql', 'select', null, null) const segment2 = transaction.trace.add('test') segment2.setDurationInMillis(501) - ps2.recordMetrics(segment2, 'TEST') + recordQueryMetrics.bind(ps2)(segment2, 'TEST') transaction.end() })