From 34130a28c3dcfe900806dab5faa2ea58022b6702 Mon Sep 17 00:00:00 2001 From: Stephen Belanger Date: Tue, 4 Jun 2019 18:47:07 -0700 Subject: [PATCH] feat: allow manual instrumentation with instrument: false Fixes #858 --- docs/configuration.asciidoc | 2 +- lib/instrumentation/index.js | 48 ++++++++++++++---------------------- test/_mock_http_client.js | 2 +- test/config.js | 22 +++++++++++++++++ 4 files changed, 43 insertions(+), 31 deletions(-) diff --git a/docs/configuration.asciidoc b/docs/configuration.asciidoc index 149c2fb0c49..3fa106e9ed5 100644 --- a/docs/configuration.asciidoc +++ b/docs/configuration.asciidoc @@ -98,7 +98,7 @@ var options = { * *Default:* `true` * *Env:* `ELASTIC_APM_INSTRUMENT` -A boolean specifying if the agent should collect performance metrics for the app. +A boolean specifying if the agent should automatically apply instrumentation to supported modules when they are loaded. Note that both `active` and `instrument` needs to be `true` for instrumentation to be running. diff --git a/lib/instrumentation/index.js b/lib/instrumentation/index.js index acbe95aa1dc..a064e957289 100644 --- a/lib/instrumentation/index.js +++ b/lib/instrumentation/index.js @@ -104,9 +104,7 @@ Instrumentation.prototype.clearPatches = function (name) { Instrumentation.modules = Object.freeze(MODULES) Instrumentation.prototype.start = function () { - if (!this._agent._conf.instrument) return if (this._started) return - this._started = true if (this._agent._conf.asyncHooks && semver.gte(process.version, '8.2.0')) { @@ -138,7 +136,7 @@ Instrumentation.prototype._startHook = function () { this._agent.logger.debug('adding hook to Node.js module loader') this._hook = hook(this._patches.keys, function (exports, name, basedir) { - var enabled = !disabled.has(name) + var enabled = !disabled.has(name) && self._agent._conf.instrument var pkg, version if (basedir) { @@ -184,40 +182,32 @@ Instrumentation.prototype._patchModule = function (exports, name, version, enabl Instrumentation.prototype.addEndedTransaction = function (transaction) { var agent = this._agent - if (this._started) { - var payload = agent._transactionFilters.process(transaction._encode()) - if (!payload) return agent.logger.debug('transaction ignored by filter %o', { trans: transaction.id, trace: transaction.traceId }) - agent.logger.debug('sending transaction %o', { trans: transaction.id, trace: transaction.traceId }) - agent._transport.sendTransaction(payload) - } else { - agent.logger.debug('ignoring transaction %o', { trans: transaction.id, trace: transaction.traceId }) - } + var payload = agent._transactionFilters.process(transaction._encode()) + if (!payload) return agent.logger.debug('transaction ignored by filter %o', { trans: transaction.id, trace: transaction.traceId }) + agent.logger.debug('sending transaction %o', { trans: transaction.id, trace: transaction.traceId }) + agent._transport.sendTransaction(payload) } Instrumentation.prototype.addEndedSpan = function (span) { var agent = this._agent - if (this._started) { - agent.logger.debug('encoding span %o', { span: span.id, parent: span.parentId, trace: span.traceId, name: span.name, type: span.type }) - span._encode(function (err, payload) { - if (err) { - agent.logger.error('error encoding span %o', { span: span.id, parent: span.parentId, trace: span.traceId, name: span.name, type: span.type, error: err.message }) - return - } + agent.logger.debug('encoding span %o', { span: span.id, parent: span.parentId, trace: span.traceId, name: span.name, type: span.type }) + span._encode(function (err, payload) { + if (err) { + agent.logger.error('error encoding span %o', { span: span.id, parent: span.parentId, trace: span.traceId, name: span.name, type: span.type, error: err.message }) + return + } - payload = agent._spanFilters.process(payload) + payload = agent._spanFilters.process(payload) - if (!payload) { - agent.logger.debug('span ignored by filter %o', { span: span.id, parent: span.parentId, trace: span.traceId, name: span.name, type: span.type }) - return - } + if (!payload) { + agent.logger.debug('span ignored by filter %o', { span: span.id, parent: span.parentId, trace: span.traceId, name: span.name, type: span.type }) + return + } - agent.logger.debug('sending span %o', { span: span.id, parent: span.parentId, trace: span.traceId, name: span.name, type: span.type }) - if (agent._transport) agent._transport.sendSpan(payload) - }) - } else { - agent.logger.debug('ignoring span %o', { span: span.id, parent: span.parentId, trace: span.traceId, name: span.name, type: span.type }) - } + agent.logger.debug('sending span %o', { span: span.id, parent: span.parentId, trace: span.traceId, name: span.name, type: span.type }) + if (agent._transport) agent._transport.sendSpan(payload) + }) } Instrumentation.prototype.startTransaction = function (name, type, opts) { diff --git a/test/_mock_http_client.js b/test/_mock_http_client.js index 76a9f36a892..a681c395339 100644 --- a/test/_mock_http_client.js +++ b/test/_mock_http_client.js @@ -50,7 +50,7 @@ module.exports = function (expected, done) { if (timer) clearTimeout(timer) timer = setTimeout(function () { done(client._writes) - }, 100) + }, 200) } } diff --git a/test/config.js b/test/config.js index 3bd29a2acd9..0ae84e3b651 100644 --- a/test/config.js +++ b/test/config.js @@ -760,6 +760,28 @@ test('globalLabels should be received by transport', function (t) { }) }) +test('instrument: false allows manual instrumentation', function (t) { + var trans + var opts = { + metricsInterval: 0, + instrument: false + } + + var server = APMServer(opts, { expect: 'transaction' }) + .on('listening', function () { + trans = this.agent.startTransaction('trans') + trans.end() + }) + .on('data-transaction', (data) => { + assertEncodedTransaction(t, trans, data) + t.end() + }) + + t.on('end', function () { + server.destroy() + }) +}) + function assertEncodedTransaction (t, trans, result) { t.comment('transaction') t.equal(result.id, trans.id, 'id matches')