diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 79886da6a8..4ac45e44a8 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -60,6 +60,17 @@ This is similar to <>, but differs in that https://github.com/elastic/apm/issues/509[helpful for APM Server log analysis]. ({issues}2364[#2364]) +* Zero configuration support. The only required agent configuration option + is <>. Normally the agent will attempt to + infer `serviceName` for the "name" field in a package.json file. However, + that could fail. With this version, the agent will cope with: a scoped + package name (`@scope/name` is normalized to `scope-name`), a "name" that + isn't a valid `serviceName`, not being able to find a "package.json" file, + etc. Ultimately it will fallback to "nodejs_service". ({issues}1944[#1944]) ++ +One consequence of this change is that `apm.getServiceName()` will return +`undefined` until the agent is started (check with `apm.isStarted()`). + [float] ===== Bug fixes diff --git a/docs/agent-api.asciidoc b/docs/agent-api.asciidoc index 123f12b866..252bd92731 100644 --- a/docs/agent-api.asciidoc +++ b/docs/agent-api.asciidoc @@ -67,10 +67,8 @@ otherwise returns `false`. Get the configured <>. If a service name was not explicitly configured, this value may have been automatically determined. - -This may be called before `agent.start()`, but the values before and after -might differ as config is reloaded during and can be set from options given to -`.start()`. A misconfigured agent can have an `undefined` service name. +The service name is not determined until `agent.start()`, so will be `undefined` +until then. A misconfigured agent can have a `null` service name. [[apm-set-framework]] ==== `apm.setFramework(options)` diff --git a/docs/configuration.asciidoc b/docs/configuration.asciidoc index 0a94a0f8d0..9c455ef075 100644 --- a/docs/configuration.asciidoc +++ b/docs/configuration.asciidoc @@ -20,13 +20,19 @@ the agent will use the `name` from `package.json` by default if available. ==== `serviceName` * *Type:* String -* *Default:* `name` field of `package.json` +* *Default:* `name` field of `package.json`, or "nodejs_service" * *Env:* `ELASTIC_APM_SERVICE_NAME` -Your Elastic APM service name. - +The name to identify this service in Elastic APM. Multiple instances of the +same service should use the same name. Allowed characters: `a-z`, `A-Z`, `0-9`, `-`, `_`, and space. +If serviceName is not provided, the agent will attempt to use the "name" field +from "package.json" -- looking up from the current working directory. The name +will be normalized to the allowed characters. If the name cannot be inferred +from package.json, then a fallback value of "nodejs_service" is used. + + [float] [[service-node-name]] ==== `serviceNodeName` diff --git a/lib/agent.js b/lib/agent.js index 58ebf99339..bfde1b219a 100644 --- a/lib/agent.js +++ b/lib/agent.js @@ -26,17 +26,12 @@ var version = require('../package').version module.exports = Agent function Agent () { - this.middleware = { connect: connect.bind(this) } + // Early configuration to ensure `agent.logger` works before `agent.start()`. + this.logger = config.configLogger() - this.logger = null this._conf = null this._httpClient = null this._uncaughtExceptionListener = null - - // Early configuration to ensure `agent.logger` works before `.start()` - // is called. - this._config() - this._inflightEvents = new InflightEventSet() this._instrumentation = new Instrumentation(this) this._metrics = new Metrics(this) @@ -46,6 +41,7 @@ function Agent () { this._transport = null this.lambda = elasticApmAwsLambda(this) + this.middleware = { connect: connect.bind(this) } } Object.defineProperty(Agent.prototype, 'currentTransaction', { @@ -113,7 +109,7 @@ Agent.prototype.destroy = function () { // for tests that may use multiple Agent instances in a single test process. global[symbols.agentInitialized] = null - if (Error.stackTraceLimit === this._conf.stackTraceLimit) { + if (this._origStackTraceLimit && Error.stackTraceLimit !== this._origStackTraceLimit) { Error.stackTraceLimit = this._origStackTraceLimit } } @@ -127,7 +123,7 @@ Agent.prototype._getStats = function () { const stats = { frameCache: frameCacheStats } - if (typeof this._transport._getStats === 'function') { + if (this._transport && typeof this._transport._getStats === 'function') { stats.apmclient = this._transport._getStats() } return stats @@ -186,7 +182,7 @@ Agent.prototype.setSpanOutcome = function (outcome) { } Agent.prototype._config = function (opts) { - this._conf = config(opts) + this._conf = config.createConfig(opts, this.logger) this.logger = this._conf.logger const { host, port, protocol } = this._conf.serverUrl @@ -204,7 +200,9 @@ Agent.prototype.isStarted = function () { } Agent.prototype.start = function (opts) { - if (this.isStarted()) throw new Error('Do not call .start() more than once') + if (this.isStarted()) { + throw new Error('Do not call .start() more than once') + } global[symbols.agentInitialized] = true this._config(opts) @@ -217,15 +215,11 @@ Agent.prototype.start = function (opts) { this.logger.debug('Elastic APM agent disabled (`active` is false)') return this } else if (!this._conf.serviceName) { - this.logger.error('Elastic APM isn\'t correctly configured: Missing serviceName') - this._conf.active = false - return this - } else if (!/^[a-zA-Z0-9 _-]+$/.test(this._conf.serviceName)) { - this.logger.error('Elastic APM isn\'t correctly configured: serviceName "%s" contains invalid characters! (allowed: a-z, A-Z, 0-9, _, -, )', this._conf.serviceName) + this.logger.error('Elastic APM is incorrectly configured: Missing serviceName (APM will be disabled)') this._conf.active = false return this } else if (!(this._conf.serverPort >= 1 && this._conf.serverPort <= 65535)) { - this.logger.error('Elastic APM isn\'t correctly configured: serverUrl "%s" contains an invalid port! (allowed: 1-65535)', this._conf.serverUrl) + this.logger.error('Elastic APM is incorrectly configured: serverUrl "%s" contains an invalid port! (allowed: 1-65535)', this._conf.serverUrl) this._conf.active = false return this } else if (this._conf.logLevel === 'trace') { @@ -266,11 +260,13 @@ Agent.prototype.start = function (opts) { } Agent.prototype.getServiceName = function () { - return this._conf.serviceName + return this._conf ? this._conf.serviceName : undefined } Agent.prototype.setFramework = function ({ name, version, overwrite = true }) { - if (!this._transport || !this._conf) return + if (!this._transport || !this._conf) { + return + } const conf = {} if (name && (overwrite || !this._conf.frameworkName)) this._conf.frameworkName = conf.frameworkName = name if (version && (overwrite || !this._conf.frameworkVersion)) this._conf.frameworkVersion = conf.frameworkVersion = version @@ -408,6 +404,13 @@ Agent.prototype.captureError = function (err, opts, cb) { const id = errors.generateErrorId() + if (!this.isStarted()) { + if (cb) { + cb(new Error('cannot capture error before agent is started'), id) + } + return + } + // Avoid unneeded error/stack processing if only propagating trace-context. if (this._conf.contextPropagationOnly) { if (cb) { @@ -534,8 +537,7 @@ Agent.prototype.captureError = function (err, opts, cb) { } else { inflightEvents.delete(id) if (cb) { - // TODO: Swallow this error just as it's done in agent.flush()? - cb(new Error('cannot capture error before agent is started'), id) + cb(new Error('cannot send error: missing transport'), id) } } }) @@ -556,9 +558,9 @@ Agent.prototype.handleUncaughtExceptions = function (cb) { this._uncaughtExceptionListener = function (err) { agent.logger.debug({ err }, 'Elastic APM caught unhandled exception') // The stack trace of uncaught exceptions are normally written to STDERR. - // The `uncaughtException` listener inhibits this behavor, and it's + // The `uncaughtException` listener inhibits this behavior, and it's // therefore necessary to manually do this to not break expectations. - if (agent._conf.logUncaughtExceptions === true) { + if (agent._conf && agent._conf.logUncaughtExceptions === true) { console.error(err) } diff --git a/lib/config.js b/lib/config.js index 0c9e18bd23..5a5a14fa9f 100644 --- a/lib/config.js +++ b/lib/config.js @@ -18,13 +18,6 @@ const { isLambdaExecutionEnviornment } = require('./lambda') let confFile = loadConfigFile() -let serviceName, serviceVersion -try { - const { name, version } = readPkgUp.sync().packageJson - serviceName = name - serviceVersion = version -} catch (err) {} - const INTAKE_STRING_MAX_SIZE = 1024 const CAPTURE_ERROR_LOG_STACK_TRACES_NEVER = 'never' const CAPTURE_ERROR_LOG_STACK_TRACES_MESSAGES = 'messages' @@ -226,12 +219,39 @@ var KEY_VALUE_OPTS = [ 'globalLabels' ] -function config (opts) { - return new Config(opts) +// Configure a logger for the agent. +// +// This is separate from `createConfig` to allow the agent to have an early +// logger before `agent.start()` is called. +function configLogger (opts) { + const logLevel = ( + process.env[ENV_TABLE.logLevel] || + (opts && opts.logLevel) || + (confFile && confFile.logLevel) || + DEFAULTS.logLevel + ) + + // `ELASTIC_APM_LOGGER=false` is provided as a mechanism to *disable* a + // custom logger for troubleshooting because a wrapped custom logger does + // not get structured log data. + // https://www.elastic.co/guide/en/apm/agent/nodejs/current/troubleshooting.html#debug-mode + let customLogger = null + if (process.env.ELASTIC_APM_LOGGER !== 'false') { + customLogger = ( + (opts && opts.logger) || + (confFile && confFile.logger) + ) + } + + return logging.createLogger(logLevel, customLogger) +} + +function createConfig (opts, logger) { + return new Config(opts, logger) } class Config { - constructor (opts) { + constructor (opts, logger) { this.ignoreUrlStr = [] this.ignoreUrlRegExp = [] this.ignoreUserAgentStr = [] @@ -239,6 +259,7 @@ class Config { this.transactionIgnoreUrlRegExp = [] this.sanitizeFieldNamesRegExp = [] this.ignoreMessageQueuesRegExp = [] + // If we didn't find a config file on process boot, but a path to one is // provided as a config option, let's instead try to load that if (confFile === null && opts && opts.configFile) { @@ -253,19 +274,45 @@ class Config { readEnv() // options read from environment variables ) - // Custom logic for setting serviceName so that an empty string in the config - // doesn't overwrite the serviceName read from package.json - if (!this.serviceName) this.serviceName = serviceName - if (!this.serviceVersion) this.serviceVersion = serviceVersion - - // The logger is used in config process, so create it first. - // - // `ELASTIC_APM_LOGGER=false` is provided as a mechanism to *disable* a - // custom logger for troubleshooting because a wrapped custom logger does - // not get structured log data. - // https://www.elastic.co/guide/en/apm/agent/nodejs/current/troubleshooting.html#debug-mode + // The logger is used later in this function, so create/update it first. + // Unless a new custom `logger` was provided, we use the one created earlier + // in `configLogger()`. const customLogger = (process.env.ELASTIC_APM_LOGGER === 'false' ? null : this.logger) - this.logger = logging.createLogger(this.logLevel, customLogger) + if (!customLogger && logger) { + logging.setLogLevel(logger, this.logLevel) + this.logger = logger + } else { + this.logger = logging.createLogger(this.logLevel, customLogger) + } + + // Fallback and validation handling for `serviceName` and `serviceVersion`. + if (this.serviceName) { + // A value here means an explicit value was given. Error out if invalid. + try { + validateServiceName(this.serviceName) + } catch (err) { + this.logger.error('serviceName "%s" is invalid: %s', this.serviceName, err.message) + this.serviceName = null + } + } else { + // Zero-conf support: use package.json#name, else // "nodejs_service". + try { + this.serviceName = serviceNameFromPackageJson() + } catch (err) { + this.logger.warn(err.message) + } + if (!this.serviceName) { + this.serviceName = 'nodejs_service' + } + } + if (!this.serviceVersion) { + // Zero-conf support: use package.json#version, if possible. + try { + this.serviceVersion = serviceVersionFromPackageJson() + } catch (err) { + // pass + } + } normalize(this, this.logger) @@ -409,6 +456,78 @@ function readEnv () { return opts } +function validateServiceName (s) { + if (typeof s !== 'string') { + throw new Error('not a string') + } else if (!/^[a-zA-Z0-9 _-]+$/.test(s)) { + throw new Error('contains invalid characters (allowed: a-z, A-Z, 0-9, _, -, )') + } +} + +// getPkg() attempts to find a "package.json" file (looking up from the current +// working directory), read it, and return an object of the form: +// { +// path: "/the/full/path/to/package.json", +// packageJson: { /* the loaded package.json object */ } +// } +let pkgCache +function getPkg () { + if (pkgCache === undefined) { + pkgCache = readPkgUp.sync() || null + } + return pkgCache +} + +function serviceNameFromPackageJson () { + const pkg = getPkg() + if (!pkg) { + throw new Error(`could not infer serviceName: could not find package.json up from ${process.cwd()}`) + } + if (!pkg.packageJson.name) { + throw new Error(`could not infer serviceName: "${pkg.path}" does not contain a "name"`) + } + if (typeof pkg.packageJson.name !== 'string') { + throw new Error(`could not infer serviceName: "name" in "${pkg.path}" is not a string`) + } + let serviceName = pkg.packageJson.name + + // Normalize a namespaced npm package name, '@ns/name', to 'ns-name'. + const match = /^@([^/]+)\/([^/]+)$/.exec(serviceName) + if (match) { + serviceName = match[1] + '-' + match[2] + } + + // Sanitize, by replacing invalid service name chars with an underscore. + const SERVICE_NAME_BAD_CHARS = /[^a-zA-Z0-9 _-]/g + serviceName = serviceName.replace(SERVICE_NAME_BAD_CHARS, '_') + + // Disallow some weird sanitized values. For example, it is better to + // have the fallback "nodejs_service" than "_" or "____" or " ". + const ALL_NON_ALPHANUMERIC = /^[ _-]*$/ + if (ALL_NON_ALPHANUMERIC.test(serviceName)) { + serviceName = null + } + if (!serviceName) { + throw new Error(`could not infer serviceName from name="${pkg.packageJson.name}" in "${pkg.path}"`) + } + + return serviceName +} + +function serviceVersionFromPackageJson () { + const pkg = getPkg() + if (!pkg) { + throw new Error(`could not infer serviceVersion: could not find package.json up from ${process.cwd()}`) + } + if (!pkg.packageJson.version) { + throw new Error(`could not infer serviceName: "${pkg.path}" does not contain a "version"`) + } + if (typeof pkg.packageJson.version !== 'string') { + throw new Error(`could not infer serviceName: "version" in "${pkg.path}" is not a string`) + } + return pkg.packageJson.version +} + function normalize (opts, logger) { normalizeKeyValuePairs(opts) normalizeNumbers(opts) @@ -807,13 +926,16 @@ function getBaseClientConfig (conf, agent) { } // Exports. -module.exports = config -module.exports.INTAKE_STRING_MAX_SIZE = INTAKE_STRING_MAX_SIZE -module.exports.CAPTURE_ERROR_LOG_STACK_TRACES_NEVER = CAPTURE_ERROR_LOG_STACK_TRACES_NEVER -module.exports.CAPTURE_ERROR_LOG_STACK_TRACES_MESSAGES = CAPTURE_ERROR_LOG_STACK_TRACES_MESSAGES -module.exports.CAPTURE_ERROR_LOG_STACK_TRACES_ALWAYS = CAPTURE_ERROR_LOG_STACK_TRACES_ALWAYS - -// The following are exported for tests. -module.exports.DEFAULTS = DEFAULTS -module.exports.secondsFromTimeStr = secondsFromTimeStr -module.exports.userAgentFromConf = userAgentFromConf +module.exports = { + configLogger, + createConfig, + INTAKE_STRING_MAX_SIZE, + CAPTURE_ERROR_LOG_STACK_TRACES_NEVER, + CAPTURE_ERROR_LOG_STACK_TRACES_MESSAGES, + CAPTURE_ERROR_LOG_STACK_TRACES_ALWAYS, + + // The following are exported for tests. + DEFAULTS, + secondsFromTimeStr, + userAgentFromConf +} diff --git a/lib/instrumentation/index.js b/lib/instrumentation/index.js index a6d6f12d23..0f2967ce06 100644 --- a/lib/instrumentation/index.js +++ b/lib/instrumentation/index.js @@ -59,8 +59,7 @@ function Instrumentation (agent) { this._hook = null // this._hook is only exposed for testing purposes this._started = false this._runCtxMgr = null - - this._log = agent.logger.child({ 'event.module': 'instrumentation' }) + this._log = agent.logger // NOTE: we need to track module names for patches // in a separate array rather than using Object.keys() @@ -148,6 +147,9 @@ Instrumentation.prototype.start = function () { if (this._started) return this._started = true + // Could have changed in Agent.start(). + this._log = this._agent.logger + if (this._agent._conf.asyncHooks) { this._runCtxMgr = new AsyncHooksRunContextManager(this._log) } else { diff --git a/lib/logging.js b/lib/logging.js index b3605ccee8..77109ad65c 100644 --- a/lib/logging.js +++ b/lib/logging.js @@ -163,8 +163,9 @@ function setLogLevel (logger, levelName) { const pinoLevel = PINO_LEVEL_FROM_LEVEL_NAME[levelName] if (!pinoLevel) { logger.warn('unknown log levelName "%s": cannot setLogLevel', levelName) + } else { + logger.level = pinoLevel } - logger.level = pinoLevel } module.exports = { diff --git a/package.json b/package.json index 4697185ac3..9297f97dc2 100644 --- a/package.json +++ b/package.json @@ -172,7 +172,6 @@ "numeral": "^2.0.6", "nyc": "^15.0.0", "once": "^1.4.0", - "p-finally": "^2.0.1", "pg": "^8.7.1", "pug": "^2.0.4", "redis": "^3.0.2", diff --git a/test/agent.test.js b/test/agent.test.js index 33de6e6e4c..12b0e1ff6f 100644 --- a/test/agent.test.js +++ b/test/agent.test.js @@ -91,14 +91,10 @@ function deep (depth, n) { test('#getServiceName()', function (t) { const agent = new Agent() - // Before agent.start(), config will have already been loaded once, which - // typically means a `serviceName` determined from package.json. + // Before agent.start() the agent hasn't configured yet. t.ok(!agent.isStarted(), 'agent should not have been started yet') - t.strictEqual(agent.getServiceName(), packageJson.name) - t.strictEqual(agent.getServiceName(), agent._conf.serviceName) + t.strictEqual(agent.getServiceName(), undefined) - // After agent.start() config will be loaded again, this time with possible - // provided config. agent.start(Object.assign( {}, agentOptsNoopTransport, diff --git a/test/config.test.js b/test/config.test.js index 1247e02c3a..287a781dd2 100644 --- a/test/config.test.js +++ b/test/config.test.js @@ -9,7 +9,6 @@ var util = require('util') var isRegExp = require('core-util-is').isRegExp var mkdirp = require('mkdirp') -var pFinally = require('p-finally') var rimraf = require('rimraf') var semver = require('semver') var test = require('tape') @@ -18,7 +17,7 @@ const Agent = require('../lib/agent') const { MockAPMServer } = require('./_mock_apm_server') const { NoopTransport } = require('../lib/noop-transport') const { safeGetPackageVersion } = require('./_utils') -var config = require('../lib/config') +const config = require('../lib/config') var Instrumentation = require('../lib/instrumentation') var apmVersion = require('../package').version var apmName = require('../package').name @@ -538,14 +537,6 @@ test('should overwrite option property active by ELASTIC_APM_ACTIVE', function ( t.end() }) -test('should default serviceName to package name', function (t) { - var agent = new Agent() - agent.start() - t.strictEqual(agent._conf.serviceName, 'elastic-apm-node') - agent.destroy() - t.end() -}) - test('should default to empty request blacklist arrays', function (t) { var agent = new Agent() agent.start(agentOptsNoopTransport) @@ -605,9 +596,16 @@ test('should compile wildcards from string', function (t) { }) test('invalid serviceName => inactive', function (t) { + var logger = new CaptureLogger() var agent = new Agent() - agent.start(Object.assign({}, agentOptsNoopTransport, { serviceName: 'foo&bar' })) - t.strictEqual(agent._conf.active, false) + agent.start(Object.assign( + {}, + agentOptsNoopTransport, + { serviceName: 'foo&bar', logger } + )) + t.ok(logger.calls[0].type === 'error' && logger.calls[0].message.indexOf('serviceName') !== -1, + 'there was a log.error mentioning "serviceName"') + t.strictEqual(agent._conf.active, false, 'active is false') agent.destroy() t.end() }) @@ -620,144 +618,105 @@ test('valid serviceName => active', function (t) { t.end() }) -test('serviceName defaults to package name', function (t) { - var mkdirpPromise = util.promisify(mkdirp) - var rimrafPromise = util.promisify(rimraf) - var writeFile = util.promisify(fs.writeFile) - var symlink = util.promisify(fs.symlink) - var exec = util.promisify(cp.exec) - - function testServiceConfig (pkg, handle) { - var tmp = path.join(os.tmpdir(), 'elastic-apm-node-test', String(Date.now())) - var files = [ - { - action: 'mkdirp', - dir: tmp - }, - { - action: 'create', - path: path.join(tmp, 'package.json'), - contents: JSON.stringify(pkg) - }, - { - action: 'create', - path: path.join(tmp, 'index.js'), - contents: ` - var apm = require('elastic-apm-node').start({ - centralConfig: false, - metricsInterval: '0s', - logLevel: 'off' - }) - console.log(JSON.stringify(apm._conf)) - ` - }, - { - action: 'mkdirp', - dir: path.join(tmp, 'node_modules') - } - ] - - if (process.platform === 'win32') { - files.push({ - action: 'npm link', - from: path.resolve(__dirname, '..'), - to: tmp - }) - } else { - files.push({ - action: 'symlink', - from: path.resolve(__dirname, '..'), - to: path.join(tmp, 'node_modules/elastic-apm-node') - }) - } - - // NOTE: Reduce the sequence to a promise chain rather - // than using Promise.all(), as the tasks are dependent. - let promise = files.reduce((p, file) => { - return p.then(() => { - switch (file.action) { - case 'create': { - return writeFile(file.path, file.contents) - } - case 'mkdirp': { - return mkdirpPromise(file.dir) - } - case 'symlink': { - return symlink(file.from, file.to) - } - case 'npm link': { - return exec('npm link', { - cwd: file.from - }).then(() => { - return exec('npm link elastic-apm-node', { - cwd: file.to - }) - }) - } - } - }) - }, Promise.resolve()) - - promise = promise - .then(() => { - return exec('node index.js', { - cwd: tmp - }) - }) - .then(result => { - return JSON.parse(result.stdout) - }) - .catch(err => { - t.error(err) - }) - - return pFinally(promise, () => { - return rimrafPromise(tmp) - }) - } - - t.test('should be active when valid', function (t) { - var pkg = { - name: 'valid' - } - - return testServiceConfig(pkg).then(conf => { - t.strictEqual(conf.active, true) - t.strictEqual(conf.serviceName, pkg.name) - t.end() - }) +test('serviceName/serviceVersion zero-conf: valid', function (t) { + cp.execFile(process.execPath, ['index.js'], { + timeout: 3000, + cwd: path.join(__dirname, 'fixtures', 'pkg-zero-conf-valid') + }, function (err, stdout, stderr) { + t.error(err, 'no error running index.js: ' + err) + t.equal(stderr, '', 'no stderr') + const lines = stdout.trim().split('\n') + const conf = JSON.parse(lines[lines.length - 1]) + t.equal(conf.serviceName, 'validName', 'serviceName was inferred from package.json') + t.equal(conf.serviceVersion, '1.2.3', 'serviceVersion was inferred from package.json') + t.end() }) +}) - t.test('should be inactive when blank', function (t) { - var pkg = { - name: '' - } - - return testServiceConfig(pkg).then(conf => { - t.strictEqual(conf.active, false) - t.strictEqual(conf.serviceName, pkg.name) - t.end() - }) +test('serviceName/serviceVersion zero-conf: no package.json to find', function (t) { + const indexJs = path.join(__dirname, 'fixtures', 'pkg-zero-conf-valid', 'index.js') + cp.execFile(process.execPath, [indexJs], { + timeout: 3000, + // Set CWD to top-level to ensure the agent cannot find a package.json file. + cwd: '/' + }, function (err, stdout, stderr) { + t.error(err, 'no error running index.js: ' + err) + t.equal(stderr, '', 'no stderr') + const lines = stdout.trim().split('\n') + const conf = JSON.parse(lines[lines.length - 1]) + t.equal(conf.serviceName, 'nodejs_service', 'serviceName is the "nodejs_service" zero-conf fallback') + t.equal(conf.serviceVersion, undefined, 'serviceVersion is undefined') + t.end() }) +}) - t.test('should be inactive when missing', function (t) { - var pkg = {} +test('serviceName/serviceVersion zero-conf: no "name" in package.json', function (t) { + cp.execFile(process.execPath, ['index.js'], { + timeout: 3000, + cwd: path.join(__dirname, 'fixtures', 'pkg-zero-conf-noname') + }, function (err, stdout, stderr) { + t.error(err, 'no error running index.js: ' + err) + t.equal(stderr, '', 'no stderr') + const lines = stdout.trim().split('\n') + const conf = JSON.parse(lines[lines.length - 1]) + t.equal(conf.serviceName, 'nodejs_service', 'serviceName is the "nodejs_service" zero-conf fallback') + t.equal(conf.serviceVersion, '1.2.3', 'serviceVersion was inferred from package.json') + t.end() + }) +}) - return testServiceConfig(pkg).then(conf => { - t.strictEqual(conf.active, false) - t.end() - }) +// A package.json#name that uses a scoped npm name, e.g. @ns/name, should get +// a normalized serviceName='ns-name'. +test('serviceName/serviceVersion zero-conf: namespaced package name', function (t) { + cp.execFile(process.execPath, ['index.js'], { + timeout: 3000, + cwd: path.join(__dirname, 'fixtures', 'pkg-zero-conf-nsname') + }, function (err, stdout, stderr) { + t.error(err, 'no error running index.js: ' + err) + t.equal(stderr, '', 'no stderr') + const lines = stdout.trim().split('\n') + const conf = JSON.parse(lines[lines.length - 1]) + t.equal(conf.serviceName, 'ns-name', 'serviceName was inferred and normalized from package.json') + t.equal(conf.serviceVersion, '1.2.3', 'serviceVersion was inferred from package.json') + t.end() }) +}) - t.test('serviceVersion should default to package version', function (t) { - var pkg = { - version: '1.2.3' - } +test('serviceName/serviceVersion zero-conf: a package name that requires sanitization', function (t) { + cp.execFile(process.execPath, ['index.js'], { + timeout: 3000, + cwd: path.join(__dirname, 'fixtures', 'pkg-zero-conf-sanitize') + }, function (err, stdout, stderr) { + t.error(err, 'no error running index.js: ' + err) + t.equal(stderr, '', 'no stderr') + const lines = stdout.trim().split('\n') + const conf = JSON.parse(lines[lines.length - 1]) + // serviceName sanitization changes any disallowed char to an underscore. + // The pkg-zero-conf-sanitize/package.json has a name starting with the + // 7 characters that an npm package name can have, but a serviceName + // cannot. + // "name": "~*.!'()validNpmName" + t.equal(conf.serviceName, '_______validNpmName', 'serviceName was inferred and sanitized from package.json') + t.equal(conf.serviceVersion, '1.2.3', 'serviceVersion was inferred from package.json') + t.end() + }) +}) - return testServiceConfig(pkg).then(conf => { - t.strictEqual(conf.serviceVersion, pkg.version) - t.end() - }) +test('serviceName/serviceVersion zero-conf: weird "name" in package.json', function (t) { + cp.execFile(process.execPath, ['index.js'], { + timeout: 3000, + cwd: path.join(__dirname, 'fixtures', 'pkg-zero-conf-weird') + }, function (err, stdout, stderr) { + t.error(err, 'no error running index.js: ' + err) + t.equal(stderr, '', 'no stderr') + const lines = stdout.trim().split('\n') + const logWarn = JSON.parse(lines[0]) + t.ok(logWarn['log.level'] === 'warn' && logWarn.message.indexOf('serviceName') !== -1, + 'there is a log.warn about "serviceName"') + const conf = JSON.parse(lines[lines.length - 1]) + t.equal(conf.serviceName, 'nodejs_service', 'serviceName is the "nodejs_service" zero-conf fallback') + t.equal(conf.serviceVersion, '1.2.3', 'serviceVersion was inferred from package.json') + t.end() }) }) @@ -1125,7 +1084,7 @@ test('parsing of ARRAY and KEY_VALUE opts', function (t) { if (env) { process.env = Object.assign({}, origEnv, env) } - var cfg = config(opts) + var cfg = config.createConfig(opts) for (var field in expect) { t.deepEqual(cfg[field], expect[field], util.format('opts=%j env=%j -> %j', opts, env, expect)) @@ -1184,7 +1143,7 @@ test('transactionSampleRate precision', function (t) { if (env) { process.env = Object.assign({}, origEnv, env) } - var cfg = config(opts) + var cfg = config.createConfig(opts) for (var field in expect) { t.deepEqual(cfg[field], expect[field], util.format('opts=%j env=%j -> %j', opts, env, expect)) diff --git a/test/filters.test.js b/test/filters.test.js index 08612c5ed2..a53a4c3b05 100644 --- a/test/filters.test.js +++ b/test/filters.test.js @@ -1,5 +1,11 @@ 'use strict' +// `filterHttpHeaders` below indirectly (*very* indirectly) depends on there +// being a configured agent, because "is-secret.js" reaches into `agent._conf`. +require('..').start({ + disableSend: true +}) + const test = require('tape') const Filters = require('object-filter-sequence') diff --git a/test/fixtures/pkg-zero-conf-noname/index.js b/test/fixtures/pkg-zero-conf-noname/index.js new file mode 100644 index 0000000000..b4e5c4cd02 --- /dev/null +++ b/test/fixtures/pkg-zero-conf-noname/index.js @@ -0,0 +1,4 @@ +const apm = require('../../..').start({ + disableSend: true +}) +console.log(JSON.stringify(apm._conf)) diff --git a/test/fixtures/pkg-zero-conf-noname/package.json b/test/fixtures/pkg-zero-conf-noname/package.json new file mode 100644 index 0000000000..a8d7893ce7 --- /dev/null +++ b/test/fixtures/pkg-zero-conf-noname/package.json @@ -0,0 +1,4 @@ +{ + "version": "1.2.3", + "description": "a package without a 'name' field for testing serviceName and serviceVersion inference" +} diff --git a/test/fixtures/pkg-zero-conf-nsname/index.js b/test/fixtures/pkg-zero-conf-nsname/index.js new file mode 100644 index 0000000000..b4e5c4cd02 --- /dev/null +++ b/test/fixtures/pkg-zero-conf-nsname/index.js @@ -0,0 +1,4 @@ +const apm = require('../../..').start({ + disableSend: true +}) +console.log(JSON.stringify(apm._conf)) diff --git a/test/fixtures/pkg-zero-conf-nsname/package.json b/test/fixtures/pkg-zero-conf-nsname/package.json new file mode 100644 index 0000000000..c2981f2ab8 --- /dev/null +++ b/test/fixtures/pkg-zero-conf-nsname/package.json @@ -0,0 +1,5 @@ +{ + "name": "@ns/name", + "version": "1.2.3", + "description": "a package with a scoped npm name for testing serviceName and serviceVersion inference" +} diff --git a/test/fixtures/pkg-zero-conf-sanitize/index.js b/test/fixtures/pkg-zero-conf-sanitize/index.js new file mode 100644 index 0000000000..b4e5c4cd02 --- /dev/null +++ b/test/fixtures/pkg-zero-conf-sanitize/index.js @@ -0,0 +1,4 @@ +const apm = require('../../..').start({ + disableSend: true +}) +console.log(JSON.stringify(apm._conf)) diff --git a/test/fixtures/pkg-zero-conf-sanitize/package.json b/test/fixtures/pkg-zero-conf-sanitize/package.json new file mode 100644 index 0000000000..c89018c2ab --- /dev/null +++ b/test/fixtures/pkg-zero-conf-sanitize/package.json @@ -0,0 +1,5 @@ +{ + "name": "~*.!'()validNpmName", + "version": "1.2.3", + "description": "a package with a valid npm name, but invalid for serviceName, to test serviceName inference" +} diff --git a/test/fixtures/pkg-zero-conf-valid/index.js b/test/fixtures/pkg-zero-conf-valid/index.js new file mode 100644 index 0000000000..b4e5c4cd02 --- /dev/null +++ b/test/fixtures/pkg-zero-conf-valid/index.js @@ -0,0 +1,4 @@ +const apm = require('../../..').start({ + disableSend: true +}) +console.log(JSON.stringify(apm._conf)) diff --git a/test/fixtures/pkg-zero-conf-valid/package.json b/test/fixtures/pkg-zero-conf-valid/package.json new file mode 100644 index 0000000000..29b26065de --- /dev/null +++ b/test/fixtures/pkg-zero-conf-valid/package.json @@ -0,0 +1,5 @@ +{ + "name": "validName", + "version": "1.2.3", + "description": "a package with valid values for serviceName and serviceVersion inference" +} diff --git a/test/fixtures/pkg-zero-conf-weird/index.js b/test/fixtures/pkg-zero-conf-weird/index.js new file mode 100644 index 0000000000..b4e5c4cd02 --- /dev/null +++ b/test/fixtures/pkg-zero-conf-weird/index.js @@ -0,0 +1,4 @@ +const apm = require('../../..').start({ + disableSend: true +}) +console.log(JSON.stringify(apm._conf)) diff --git a/test/fixtures/pkg-zero-conf-weird/package.json b/test/fixtures/pkg-zero-conf-weird/package.json new file mode 100644 index 0000000000..5a49efa9d8 --- /dev/null +++ b/test/fixtures/pkg-zero-conf-weird/package.json @@ -0,0 +1,5 @@ +{ + "name": "~~-~~", + "version": "1.2.3", + "description": "a package with a valid npm name that sanitizes to all non-alphanumeric for testing serviceName inference" +} diff --git a/test/redact-secrets/is-secret.test.js b/test/redact-secrets/is-secret.test.js index 026cec2936..165b055d05 100644 --- a/test/redact-secrets/is-secret.test.js +++ b/test/redact-secrets/is-secret.test.js @@ -22,6 +22,12 @@ // LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE // SOFTWARE. + +// `isSecret` below assumes there is a configured agent. +require('../..').start({ + disableSend: true +}) + var test = require('tape') var isSecret = require('../../lib/redact-secrets/is-secret') diff --git a/test/redact-secrets/redact-secrets.test.js b/test/redact-secrets/redact-secrets.test.js index 8c4caf2f20..17e2371245 100644 --- a/test/redact-secrets/redact-secrets.test.js +++ b/test/redact-secrets/redact-secrets.test.js @@ -22,6 +22,12 @@ // LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE // SOFTWARE. + +// `redact` below assumes there is a configured agent. +require('../..').start({ + disableSend: true +}) + var test = require('tape') var clone = require('clone') var redact = require('../../lib/redact-secrets') diff --git a/test/start/env/test.test.js b/test/start/env/test.test.js index 39aa994576..b0513f3933 100644 --- a/test/start/env/test.test.js +++ b/test/start/env/test.test.js @@ -1,6 +1,8 @@ 'use strict' -var agent = require('../../..') +var agent = require('../../..').start({ + disableSend: true +}) const tape = require('tape') tape('from-env service name test', function (t) { diff --git a/test/start/file/test.test.js b/test/start/file/test.test.js index ef3b835cdd..86f072cee0 100644 --- a/test/start/file/test.test.js +++ b/test/start/file/test.test.js @@ -1,6 +1,8 @@ 'use strict' -var agent = require('../../..') +var agent = require('../../..').start({ + disableSend: true +}) const tape = require('tape')