diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index ad6212df499..a7b7a9deab1 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -40,6 +40,19 @@ Notes: [float] ===== Features +- Improve the captured information for Elasticsearch client instrumentation. + For all outgoing Elasticsearch client requests, the full HTTP url is + now captured (stored in the "url.original" field). For Elasticsearch requests + that do a search the outgoing request body is captured (to the + "span.db.statement" field), as before, but the format has changed to only + hold the request body. Before this change the "span.db.statement" would + also hold any HTTP query parameters. These are now more naturally captured + with "url.origin". ({issues}2019[#2019]) ++ +This change also introduces the <> +configuration option to enable controlling which Elasticsearch REST API +paths are considered for body capture. ({pull}2873[#2873]) + - Support instrumenting core modules when require'd with the optional https://nodejs.org/api/modules.html#core-modules['node:'-prefix]. For example `require('node:http')` will now be instrumented. diff --git a/docs/configuration.asciidoc b/docs/configuration.asciidoc index f6a49edc882..38477c1cb9a 100644 --- a/docs/configuration.asciidoc +++ b/docs/configuration.asciidoc @@ -1356,3 +1356,15 @@ require('elastic-apm-node').start({ exitSpanMinDuration: '10ms' }) ---- + +[[elasticsearch-capture-body-urls]] +==== `elasticsearchCaptureBodyUrls` + +* *Type:* Array of wildcard patterns +* *Default:* `['*/_search', '*/_search/template', '*/_msearch', '*/_msearch/template', '*/_async_search', '*/_count', '*/_sql', '*/_eql/search' ]` +* *Env:* `ELASTIC_APM_ELASTICSEARCH_CAPTURE_BODY_URLS` +// Spec name: elasticsearch_capture_body_urls + +The URL path patterns for which the APM agent will capture the request body of outgoing requests to Elasticsearch made with the `@elastic/elasticsearch` module (or the legacy `elasticsearch` module). The default setting captures the body for https://www.elastic.co/guide/en/elasticsearch/reference/current/rest-apis.html[Elasticsearch REST APIs] making a search. + +The captured request body (if any) is stored on the `span.db.statement` field. Captured request bodies are truncated to a maximum length defined by <>. diff --git a/examples/trace-elasticsearch7.js b/examples/trace-elasticsearch7.js index 84deddc10a5..c4f8994899b 100755 --- a/examples/trace-elasticsearch7.js +++ b/examples/trace-elasticsearch7.js @@ -18,6 +18,8 @@ const apm = require('../').start({ // elastic-apm-node logUncaughtExceptions: true }) +// Note that version 7 is *not* installed by default. To use v7 you'll need to: +// npm install @elastic/elasticsearch@7 const { Client } = require('@elastic/elasticsearch') const client = new Client({ diff --git a/index.d.ts b/index.d.ts index 00675d8fe6e..261cafd674f 100644 --- a/index.d.ts +++ b/index.d.ts @@ -249,6 +249,7 @@ declare namespace apm { contextPropagationOnly?: boolean; disableInstrumentations?: string | string[]; disableSend?: boolean; + elasticsearchCaptureBodyUrls?: Array; environment?: string; errorMessageMaxLength?: string; // DEPRECATED: use `longFieldMaxLength`. errorOnAbortedRequests?: boolean; diff --git a/lib/config.js b/lib/config.js index 3f7cd130ec6..44b0559b022 100644 --- a/lib/config.js +++ b/lib/config.js @@ -54,6 +54,10 @@ var DEFAULTS = { contextPropagationOnly: false, disableInstrumentations: [], disableSend: false, + elasticsearchCaptureBodyUrls: [ + '*/_search', '*/_search/template', '*/_msearch', '*/_msearch/template', + '*/_async_search', '*/_count', '*/_sql', '*/_eql/search' + ], environment: process.env.NODE_ENV || 'development', errorOnAbortedRequests: false, exitSpanMinDuration: '0ms', @@ -125,6 +129,7 @@ var ENV_TABLE = { environment: 'ELASTIC_APM_ENVIRONMENT', exitSpanMinDuration: 'ELASTIC_APM_EXIT_SPAN_MIN_DURATION', ignoreMessageQueues: ['ELASTIC_IGNORE_MESSAGE_QUEUES', 'ELASTIC_APM_IGNORE_MESSAGE_QUEUES'], + elasticsearchCaptureBodyUrls: 'ELASTIC_APM_ELASTICSEARCH_CAPTURE_BODY_URLS', errorMessageMaxLength: 'ELASTIC_APM_ERROR_MESSAGE_MAX_LENGTH', errorOnAbortedRequests: 'ELASTIC_APM_ERROR_ON_ABORTED_REQUESTS', filterHttpHeaders: 'ELASTIC_APM_FILTER_HTTP_HEADERS', @@ -289,6 +294,7 @@ var MINUS_ONE_EQUAL_INFINITY = [ var ARRAY_OPTS = [ 'disableInstrumentations', + 'elasticsearchCaptureBodyUrls', 'sanitizeFieldNames', 'transactionIgnoreUrls', 'ignoreMessageQueues' @@ -336,6 +342,7 @@ function initialConfig (logger) { cfg.ignoreUrlRegExp = [] cfg.ignoreUserAgentStr = [] cfg.ignoreUserAgentRegExp = [] + cfg.elasticsearchCaptureBodyUrlsRegExp = [] cfg.transactionIgnoreUrlRegExp = [] cfg.sanitizeFieldNamesRegExp = [] cfg.ignoreMessageQueuesRegExp = [] @@ -356,6 +363,7 @@ class Config { this.ignoreUrlRegExp = [] this.ignoreUserAgentStr = [] this.ignoreUserAgentRegExp = [] + this.elasticsearchCaptureBodyUrlsRegExp = [] this.transactionIgnoreUrlRegExp = [] this.sanitizeFieldNamesRegExp = [] this.ignoreMessageQueuesRegExp = [] @@ -700,6 +708,7 @@ function normalize (opts, logger) { normalizeDurationOptions(opts, logger) normalizeBools(opts, logger) normalizeIgnoreOptions(opts) + normalizeElasticsearchCaptureBodyUrls(opts) normalizeSanitizeFieldNames(opts) normalizeContextManager(opts, logger) // Must be after normalizeBools(). normalizeCloudProvider(opts, logger) @@ -926,6 +935,16 @@ function normalizeIgnoreOptions (opts) { } } +function normalizeElasticsearchCaptureBodyUrls (opts) { + if (opts.elasticsearchCaptureBodyUrls) { + const wildcard = new WildcardMatcher() + for (const ptn of opts.elasticsearchCaptureBodyUrls) { + const re = wildcard.compile(ptn) + opts.elasticsearchCaptureBodyUrlsRegExp.push(re) + } + } +} + function normalizeNumbers (opts) { for (const key of NUM_OPTS) { if (key in opts) opts[key] = Number(opts[key]) diff --git a/lib/instrumentation/elasticsearch-shared.js b/lib/instrumentation/elasticsearch-shared.js index ab56c14dd1d..75734b001fa 100644 --- a/lib/instrumentation/elasticsearch-shared.js +++ b/lib/instrumentation/elasticsearch-shared.js @@ -10,64 +10,50 @@ // - elasticsearch - the legacy Elasticsearch JS client // - @elastic/elasticsearch - the new Elasticsearch JS client -const querystring = require('querystring') - -// URL paths matching the following pattern will have their query params and -// request body captured in the span (as `context.db.statement`). We match -// a complete URL path component to attempt to avoid accidental matches of -// user data, like `GET /my_index_search/...`. -const pathIsAQuery = /\/(_search|_msearch|_count|_async_search|_sql|_eql)(\/|$)/ - -// (This is exported for testing.) -exports.pathIsAQuery = pathIsAQuery - -// Set the span's `context.db` from the Elasticsearch request querystring and -// body, if the request path looks like it is a query API. -// -// Some ES endpoints, e.g. '_search', support both query params and a body. -// We encode both into 'span.context.db.statement', separated by '\n\n' -// if both are present. E.g. for a possible msearch: -// -// search_type=query_then_fetch&typed_keys=false -// -// {} -// {"query":{"query_string":{"query":"pants"}}} -// -// `path`, `query`, and `body` can all be null or undefined. -exports.setElasticsearchDbContext = function (span, path, query, body) { - if (path && pathIsAQuery.test(path)) { - const parts = [] - if (query) { - if (typeof (query) === 'string') { - parts.push(query) - } else if (typeof (query) === 'object') { - const encodedQuery = querystring.encode(query) - if (encodedQuery) { - parts.push(encodedQuery) - } - } - } - if (body) { - if (typeof (body) === 'string') { - parts.push(body) - } else if (Buffer.isBuffer(body) || typeof body.pipe === 'function') { - // Never serialize a Buffer or a Readable. These guards mirror - // `shouldSerialize()` in the ES client, e.g.: - // https://github.com/elastic/elastic-transport-js/blob/069172506d1fcd544b23747d8c2d497bab053038/src/Transport.ts#L614-L618 - } else if (Array.isArray(body)) { - try { - parts.push(body.map(JSON.stringify).join('\n') + '\n') // ndjson - } catch (_ignoredErr) {} - } else if (typeof (body) === 'object') { - try { - parts.push(JSON.stringify(body)) - } catch (_ignoredErr) {} - } +// Only capture the ES request body if the request path matches the +// `elasticsearchCaptureBodyUrls` config. +function shouldCaptureBody (path, elasticsearchCaptureBodyUrlsRegExp) { + if (!path) { + return false + } + for (var i = 0; i < elasticsearchCaptureBodyUrlsRegExp.length; i++) { + const re = elasticsearchCaptureBodyUrlsRegExp[i] + if (re.test(path)) { + return true } + } + return false +} - span.setDbContext({ - type: 'elasticsearch', - statement: parts.join('\n\n') - }) +/** + * Get an appropriate `span.context.db.statement` for this ES client request, if any. + * https://github.com/elastic/apm/blob/main/specs/agents/tracing-instrumentation-db.md#elasticsearch_capture_body_urls-configuration + * + * @param {string | null} path + * @param {string | null} body + * @param {RegExp[]} elasticsearchCaptureBodyUrlsRegExp + * @return {string | undefined} + */ +function getElasticsearchDbStatement (path, body, elasticsearchCaptureBodyUrlsRegExp) { + if (body && shouldCaptureBody(path, elasticsearchCaptureBodyUrlsRegExp)) { + if (typeof (body) === 'string') { + return body + } else if (Buffer.isBuffer(body) || typeof body.pipe === 'function') { + // Never serialize a Buffer or a Readable. These guards mirror + // `shouldSerialize()` in the ES client, e.g.: + // https://github.com/elastic/elastic-transport-js/blob/069172506d1fcd544b23747d8c2d497bab053038/src/Transport.ts#L614-L618 + } else if (Array.isArray(body)) { + try { + return body.map(JSON.stringify).join('\n') + '\n' // ndjson + } catch (_ignoredErr) {} + } else if (typeof (body) === 'object') { + try { + return JSON.stringify(body) + } catch (_ignoredErr) {} + } } } + +module.exports = { + getElasticsearchDbStatement +} diff --git a/lib/instrumentation/modules/@elastic/elasticsearch.js b/lib/instrumentation/modules/@elastic/elasticsearch.js index 61a6a7de084..185d9b52874 100644 --- a/lib/instrumentation/modules/@elastic/elasticsearch.js +++ b/lib/instrumentation/modules/@elastic/elasticsearch.js @@ -34,9 +34,10 @@ // 2. Users cannot access `apm.currentSpan` inside a diagnostic event handler. const semver = require('semver') +const { URL, URLSearchParams } = require('url') const { getDBDestination } = require('../../context') -const { setElasticsearchDbContext } = require('../../elasticsearch-shared') +const { getElasticsearchDbStatement } = require('../../elasticsearch-shared') const shimmer = require('../../shimmer') module.exports = function (elasticsearch, agent, { version, enabled }) { @@ -53,6 +54,7 @@ module.exports = function (elasticsearch, agent, { version, enabled }) { const doubleCallsRequestIfNoCb = semver.lt(version, '7.7.0') const ins = agent._instrumentation const isGteV8 = semver.satisfies(version, '>=8', { includePrerelease: true }) + const elasticsearchCaptureBodyUrlsRegExp = agent._conf.elasticsearchCaptureBodyUrlsRegExp agent.logger.debug('shimming elasticsearch.Transport.prototype.{request,getConnection}') shimmer.wrap(elasticsearch.Transport && elasticsearch.Transport.prototype, 'request', wrapRequest) @@ -127,15 +129,20 @@ module.exports = function (elasticsearch, agent, { version, enabled }) { const spanRunContext = parentRunContext.enterSpan(span) const finish = ins.bindFunctionToRunContext(spanRunContext, (err, result) => { // Set DB context. - // In @elastic/elasticsearch@7, `Transport#request` encodes - // `params.{querystring,body}` in-place; use it. In >=8 this encoding is - // no longer in-place. A better eventual solution would be to wrap - // `Connection.request` to capture the serialized params. - setElasticsearchDbContext( - span, - params && params.path, - params && params.querystring, - params && (params.body || params.bulkBody)) + const dbContext = { + type: 'elasticsearch' + } + if (params) { + const statement = getElasticsearchDbStatement( + params.path, + params.body || params.bulkBody, + elasticsearchCaptureBodyUrlsRegExp + ) + if (statement) { + dbContext.statement = statement + } + } + span.setDbContext(dbContext) // Set destination context. // Use the connection from wrappedGetConnection() above, if that worked. @@ -152,7 +159,7 @@ module.exports = function (elasticsearch, agent, { version, enabled }) { span.setDestinationContext(getDBDestination(span, connUrl && connUrl.hostname, connUrl && connUrl.port)) - // Gather some HTTP context from the "DiagnosticResult" object. + // Gather some HTTP context. // We are *not* including the response headers b/c they are boring: // // X-elastic-product: Elasticsearch @@ -170,6 +177,8 @@ module.exports = function (elasticsearch, agent, { version, enabled }) { // The result is that with Promise usage of v7, ES client requests that // are queued behind the "product-check" and that reject, won't have a // `diagResult`. + const httpContext = {} + let haveHttpContext = false let diagResult = isGteV8 ? null : result if (!diagResult) { diagResult = diagResultFromSpan.get(span) @@ -178,12 +187,11 @@ module.exports = function (elasticsearch, agent, { version, enabled }) { } } if (diagResult) { - const httpContext = {} - let haveHttpContext = false if (diagResult.statusCode) { haveHttpContext = true httpContext.status_code = diagResult.statusCode } + // *Not* currently adding headers because if (diagResult.headers && 'content-length' in diagResult.headers) { const contentLength = Number(diagResult.headers['content-length']) @@ -192,9 +200,27 @@ module.exports = function (elasticsearch, agent, { version, enabled }) { httpContext.response = { encoded_body_size: contentLength } } } - if (haveHttpContext) { - span.setHttpContext(httpContext) - } + } + + // Reconstruct the full URL (including query params). + let origin + if (connUrl) { + origin = connUrl.origin + } else if (diagResult && diagResult.meta && diagResult.meta.connection && diagResult.meta.connection.url) { + try { + origin = new URL(diagResult.meta.connection.url).origin + } catch (_ignoredErr) {} + } + if (origin && params && params.path) { + const fullUrl = new URL(origin) + fullUrl.pathname = params.path + fullUrl.search = new URLSearchParams(params.querystring).toString() + httpContext.url = fullUrl.toString() + haveHttpContext = true + } + + if (haveHttpContext) { + span.setHttpContext(httpContext) } if (err) { diff --git a/lib/instrumentation/modules/elasticsearch.js b/lib/instrumentation/modules/elasticsearch.js index 0d8b07002fb..acebcf019c0 100644 --- a/lib/instrumentation/modules/elasticsearch.js +++ b/lib/instrumentation/modules/elasticsearch.js @@ -6,11 +6,11 @@ 'use strict' -const URL = require('url').URL +const { URL, URLSearchParams } = require('url') var shimmer = require('../shimmer') var { getDBDestination } = require('../context') -const { setElasticsearchDbContext } = require('../elasticsearch-shared') +const { getElasticsearchDbStatement } = require('../elasticsearch-shared') const startsWithProtocolRE = /^([a-z]+:)?\/\//i const DEFAULT_PORT = 9200 @@ -21,7 +21,8 @@ const DEFAULT_PORT_FROM_PROTO = { // This is an imperfect equivalent of the handling in the `Transport` // constructor and internal `Host` parsing function in the ES client. -function getHostAndPortFromTransportConfig (config) { +// This returns a `URL` object or null. +function getTargetUrlFromTransportConfig (config) { const transportHosts = config ? config.host || config.hosts : null if (!transportHosts) { return null @@ -39,21 +40,23 @@ function getHostAndPortFromTransportConfig (config) { if (!startsWithProtocolRE.test(firstTransportHost)) { firstTransportHost = 'http://' + firstTransportHost } - let u try { - u = new URL(firstTransportHost) + return new URL(firstTransportHost) } catch (_err) { return null } - if (!u.port) { - u.port = DEFAULT_PORT_FROM_PROTO[u.protocol] - } - return [u.hostname, u.port] } else if (typeof firstTransportHost === 'object') { - return [ - firstTransportHost.hostname || firstTransportHost.host, - firstTransportHost.port || DEFAULT_PORT - ] + let proto = firstTransportHost.protocol || 'http:' + if (!proto.endsWith(':')) { + proto += ':' + } + const hostname = firstTransportHost.hostname || firstTransportHost.host + const port = firstTransportHost.port || DEFAULT_PORT + try { + return new URL(proto + '//' + hostname + ':' + port) + } catch (_ignoredErr) { + return null + } } return null @@ -63,6 +66,7 @@ module.exports = function (elasticsearch, agent, { enabled }) { if (!enabled) return elasticsearch const ins = agent._instrumentation + const elasticsearchCaptureBodyUrlsRegExp = agent._conf.elasticsearchCaptureBodyUrlsRegExp agent.logger.debug('shimming elasticsearch.Transport.prototype.request') shimmer.wrap(elasticsearch.Transport && elasticsearch.Transport.prototype, 'request', wrapRequest) @@ -81,17 +85,35 @@ module.exports = function (elasticsearch, agent, { enabled }) { if (span && method && path) { span.name = `Elasticsearch: ${method} ${path}` - setElasticsearchDbContext(span, path, params && params.query, - params && params.body) + // Set DB context. + const dbContext = { + type: 'elasticsearch' + } + const statement = getElasticsearchDbStatement( + path, params && params.body, elasticsearchCaptureBodyUrlsRegExp) + if (statement) { + dbContext.statement = statement + } + span.setDbContext(dbContext) // Get the remote host information from elasticsearch Transport options. - let host, port - const hostAndPort = getHostAndPortFromTransportConfig(this._config) - if (hostAndPort) { - host = hostAndPort[0] - port = hostAndPort[1] + const targetUrl = getTargetUrlFromTransportConfig(this._config) + let port = targetUrl && targetUrl.port + if (!port && targetUrl) { + port = DEFAULT_PORT_FROM_PROTO[targetUrl.protocol] + } + span.setDestinationContext(getDBDestination( + span, + targetUrl && targetUrl.hostname, + port + )) + if (targetUrl) { + targetUrl.pathname = path + targetUrl.search = new URLSearchParams(params.query).toString() + span.setHttpContext({ + url: targetUrl.toString() + }) } - span.setDestinationContext(getDBDestination(span, host, port)) const parentRunContext = ins.currRunContext() const spanRunContext = parentRunContext.enterSpan(span) diff --git a/package-lock.json b/package-lock.json index f34d5ce81c5..0816905cb95 100644 --- a/package-lock.json +++ b/package-lock.json @@ -46,7 +46,7 @@ "@babel/cli": "^7.8.4", "@babel/core": "^7.8.4", "@babel/preset-env": "^7.8.4", - "@elastic/elasticsearch": "^8.2.0-patch.1", + "@elastic/elasticsearch": "^8.2.1", "@elastic/elasticsearch-canary": "^8.2.0-canary.2", "@fastify/formbody": "^7.0.1", "@hapi/hapi": "^20.1.2", diff --git a/package.json b/package.json index f5bf172bd9a..c933ee77d07 100644 --- a/package.json +++ b/package.json @@ -119,7 +119,7 @@ "@babel/cli": "^7.8.4", "@babel/core": "^7.8.4", "@babel/preset-env": "^7.8.4", - "@elastic/elasticsearch": "^8.2.0-patch.1", + "@elastic/elasticsearch": "^8.2.1", "@elastic/elasticsearch-canary": "^8.2.0-canary.2", "@fastify/formbody": "^7.0.1", "@hapi/hapi": "^20.1.2", diff --git a/test/config.test.js b/test/config.test.js index 9a4d6341b77..9e1eef39320 100644 --- a/test/config.test.js +++ b/test/config.test.js @@ -675,20 +675,26 @@ test('should separate strings and regexes into their own ignore arrays', functio t.end() }) -test('should compile wildcards from string', function (t) { +test('should prepare WildcardMatcher array config vars', function (t) { var agent = new Agent() agent.start(Object.assign( {}, agentOptsNoopTransport, { - transactionIgnoreUrls: ['foo', '/str1', '/wil*card'] + transactionIgnoreUrls: ['foo', 'bar', '/wil*card'], + elasticsearchCaptureBodyUrls: ['*/_search', '*/_eql/search'] } )) - t.strictEqual( - agent._conf.transactionIgnoreUrlRegExp.length, - 3, - 'was everything added?' + t.equal( + agent._conf.transactionIgnoreUrlRegExp.toString(), + '/^foo$/i,/^bar$/i,/^\\/wil.*card$/i', + 'transactionIgnoreUrlRegExp' + ) + t.equal( + agent._conf.elasticsearchCaptureBodyUrlsRegExp.toString(), + '/^.*\\/_search$/i,/^.*\\/_eql\\/search$/i', + 'elasticsearchCaptureBodyUrlsRegExp' ) agent.destroy() @@ -1231,6 +1237,11 @@ test('parsing of ARRAY and KEY_VALUE opts', function (t) { expect: { transactionIgnoreUrls: ['foo', 'bar bling'] } }, + { + opts: { elasticsearchCaptureBodyUrls: '*/_search, */_msearch/template ' }, + expect: { elasticsearchCaptureBodyUrls: ['*/_search', '*/_msearch/template'] } + }, + { opts: { disableInstrumentations: 'foo, bar' }, expect: { disableInstrumentations: ['foo', 'bar'] } diff --git a/test/instrumentation/modules/@elastic/elasticsearch.test.js b/test/instrumentation/modules/@elastic/elasticsearch.test.js index fca26173c0a..f447e5fd724 100644 --- a/test/instrumentation/modules/@elastic/elasticsearch.test.js +++ b/test/instrumentation/modules/@elastic/elasticsearch.test.js @@ -70,7 +70,7 @@ const clientOpts = { } test('client.ping with promise', function (t) { - resetAgent(checkDataAndEnd(t, 'HEAD', '/', null, 200)) + resetAgent(checkDataAndEnd(t, 'HEAD /', `http://${host}/`, 200)) agent.startTransaction('myTrans') @@ -85,7 +85,7 @@ test('client.ping with promise', function (t) { // Callback-style was dropped in ES client v8. if (!semver.satisfies(esVersion, '>=8', { includePrerelease: true })) { test('client.ping with callback', function (t) { - resetAgent(checkDataAndEnd(t, 'HEAD', '/', null, 200)) + resetAgent(checkDataAndEnd(t, 'HEAD /', `http://${host}/`, 200)) agent.startTransaction('myTrans') @@ -102,7 +102,7 @@ if (!semver.satisfies(esVersion, '>=8', { includePrerelease: true })) { test('client.search with promise', function (t) { const searchOpts = { q: 'pants' } - resetAgent(checkDataAndEnd(t, 'GET', '/_search', 'q=pants', 200)) + resetAgent(checkDataAndEnd(t, 'GET /_search', `http://${host}/_search?q=pants`, 200)) agent.startTransaction('myTrans') @@ -123,7 +123,7 @@ if (semver.gte(process.version, '10.0.0')) { test('client.child', function (t) { const searchOpts = { q: 'pants' } - resetAgent(checkDataAndEnd(t, 'GET', '/_search', 'q=pants', 200)) + resetAgent(checkDataAndEnd(t, 'GET /_search', `http://${host}/_search?q=pants`, 200)) agent.startTransaction('myTrans') @@ -143,7 +143,7 @@ if (semver.gte(process.version, '10.0.0')) { test('client.search with queryparam', function (t) { const searchOpts = { q: 'pants' } - resetAgent(checkDataAndEnd(t, 'GET', '/_search', 'q=pants', 200)) + resetAgent(checkDataAndEnd(t, 'GET /_search', `http://${host}/_search?q=pants`, 200)) agent.startTransaction('myTrans') @@ -170,7 +170,13 @@ if (semver.gte(process.version, '10.0.0')) { body: body } - resetAgent(checkDataAndEnd(t, 'POST', `/${searchOpts.index}/_search`, JSON.stringify(body), 200)) + resetAgent(checkDataAndEnd( + t, + `POST /${searchOpts.index}/_search`, + `http://${host}/${searchOpts.index}/_search`, + 200, + JSON.stringify(body) + )) agent.startTransaction('myTrans') @@ -199,7 +205,13 @@ if (semver.gte(process.version, '10.0.0')) { let expectedDbStatement = Object.assign({}, searchOpts) delete expectedDbStatement.index expectedDbStatement = JSON.stringify(expectedDbStatement) - resetAgent(checkDataAndEnd(t, 'POST', `/${searchOpts.index}/_search`, expectedDbStatement, 200)) + resetAgent(checkDataAndEnd( + t, + `POST /${searchOpts.index}/_search`, + `http://${host}/${searchOpts.index}/_search`, + 200, + expectedDbStatement + )) agent.startTransaction('myTrans') @@ -230,20 +242,24 @@ if (semver.gte(process.version, '10.0.0')) { size: 2, sort: 'myField:asc' } - let statement + let query, statement // ES client version 8 merges options into `body` differently from earlier // versions. if (semver.satisfies(esVersion, '>=8', { includePrerelease: true })) { - statement = `sort=myField%3Aasc - -{"query":{"match":{"request":"bar"}},"size":2}` + query = 'sort=myField%3Aasc' + statement = '{"query":{"match":{"request":"bar"}},"size":2}' } else { - statement = `size=2&sort=myField%3Aasc - -${JSON.stringify(body)}` + query = 'size=2&sort=myField%3Aasc' + statement = JSON.stringify(body) } - resetAgent(checkDataAndEnd(t, 'POST', `/${searchOpts.index}/_search`, statement, 200)) + resetAgent(checkDataAndEnd( + t, + `POST /${searchOpts.index}/_search`, + `http://${host}/${searchOpts.index}/_search?${query}`, + 200, + statement + )) agent.startTransaction('myTrans') @@ -271,7 +287,13 @@ ${JSON.stringify(body)}` } } - resetAgent(checkDataAndEnd(t, 'POST', '/_search/template', JSON.stringify(body), 200)) + resetAgent(checkDataAndEnd( + t, + 'POST /_search/template', + `http://${host}/_search/template`, + 200, + JSON.stringify(body) + )) agent.startTransaction('myTrans') @@ -301,12 +323,16 @@ ${JSON.stringify(body)}` typed_keys: false, body: body } - const statement = `search_type=query_then_fetch&typed_keys=false - -${body.map(JSON.stringify).join('\n')} -` + const query = 'search_type=query_then_fetch&typed_keys=false' + const statement = body.map(JSON.stringify).join('\n') + '\n' - resetAgent(checkDataAndEnd(t, 'POST', '/_msearch', statement, 200)) + resetAgent(checkDataAndEnd( + t, + 'POST /_msearch', + `http://${host}/_msearch?${query}`, + 200, + statement + )) agent.startTransaction('myTrans') @@ -338,7 +364,13 @@ ${body.map(JSON.stringify).join('\n')} ] const statement = body.map(JSON.stringify).join('\n') + '\n' - resetAgent(checkDataAndEnd(t, 'POST', '/_msearch/template', statement, 200)) + resetAgent(checkDataAndEnd( + t, + 'POST /_msearch/template', + `http://${host}/_msearch/template`, + 200, + statement + )) agent.startTransaction('myTrans') @@ -819,7 +851,7 @@ function checkSpanOutcomesSuccess (t) { } } -function checkDataAndEnd (t, method, path, dbStatement, statusCode) { +function checkDataAndEnd (t, expectedName, expectedHttpUrl, expectedStatusCode, expectedDbStatement) { return function (data) { t.equal(data.transactions.length, 1, 'should have 1 transaction') const trans = data.transactions[0] @@ -845,33 +877,35 @@ function checkDataAndEnd (t, method, path, dbStatement, statusCode) { t.strictEqual(esSpan.subtype, 'elasticsearch') t.strictEqual(esSpan.action, 'request') t.strictEqual(esSpan.sync, false, 'span.sync=false') - t.equal(esSpan.name, 'Elasticsearch: ' + method + ' ' + path, 'elasticsearch span should have expected name') + t.equal(esSpan.name, 'Elasticsearch: ' + expectedName, 'elasticsearch span should have expected name') - // Iff the test case provided a `dbStatement`, then we expect `.context.db`. - if (typeof dbStatement === 'string') { + if (expectedDbStatement) { t.deepEqual(esSpan.context.db, - { type: 'elasticsearch', statement: dbStatement }, - 'elasticsearch span has correct .context.db') + { type: 'elasticsearch', statement: expectedDbStatement }, + 'span.context.db') } else { - t.notOk(esSpan.context && esSpan.context.db, 'elasticsearch span should not have .context.db') + t.deepEqual(esSpan.context.db, { type: 'elasticsearch' }, 'span.context.db') } // Ensure "destination" context is set. t.equal(esSpan.context.destination.service.name, 'elasticsearch', 'elasticsearch span.context.destination.service.name=="elasticsearch"') - // Iff the test case provided an expected `statusCode`, then we expect - // `.context.http`. The exception is with @elastic/elasticsearch >=8 - // and `contextManager="patch"` (see "Limitations" section in the - // instrumentation code). - if (statusCode !== undefined) { - if (semver.satisfies(esVersion, '>=8', { includePrerelease: true }) && + if (expectedHttpUrl) { + t.equal(esSpan.context.http.url, expectedHttpUrl, 'span.context.http.url') + } else { + t.notOk(esSpan.context.http && esSpan.context.http.url, 'should not have span.context.http.url') + } + + // With @elastic/elasticsearch >=8 and `contextManager="patch"` there is + // a limitation such that some HTTP context fields cannot be captured. + // (See "Limitations" section in the instrumentation code.) + if (semver.satisfies(esVersion, '>=8', { includePrerelease: true }) && agent._conf.contextManager === config.CONTEXT_MANAGER_PATCH) { - t.comment('skip span.context.http check because of contextManager="patch" + esVersion>=8 limitation') - } else { - t.equal(esSpan.context.http.status_code, statusCode, 'context.http.status_code') - t.ok(esSpan.context.http.response.encoded_body_size, 'context.http.response.encoded_body_size is present') - } + t.comment('skip span.context.http.{status_code,response} check because of contextManager="patch" + esVersion>=8 limitation') + } else { + t.equal(esSpan.context.http.status_code, expectedStatusCode, 'context.http.status_code') + t.ok(esSpan.context.http.response.encoded_body_size, 'context.http.response.encoded_body_size is present') } t.end() diff --git a/test/instrumentation/modules/elasticsearch.test.js b/test/instrumentation/modules/elasticsearch.test.js index a8a96d3bb9b..2e2a4f2fc2c 100644 --- a/test/instrumentation/modules/elasticsearch.test.js +++ b/test/instrumentation/modules/elasticsearch.test.js @@ -6,14 +6,11 @@ 'use strict' -const { pathIsAQuery } = require('../../../lib/instrumentation/elasticsearch-shared') - process.env.ELASTIC_APM_TEST = true var host = (process.env.ES_HOST || 'localhost') + ':9200' var agent = require('../../..').start({ - serviceName: 'test', - secretToken: 'test', + serviceName: 'test-elasticsearch-legacy-client', captureExceptions: false, metricsInterval: 0, centralConfig: false, @@ -31,7 +28,7 @@ var mockClient = require('../../_mock_http_client') var findObjInArray = require('../../_utils').findObjInArray test('client.ping with callback', function userLandCode (t) { - resetAgent(done(t, 'HEAD', '/')) + resetAgent(assertApmDataAndEnd(t, 'HEAD /', `http://${host}/`)) agent.startTransaction('foo') @@ -46,7 +43,7 @@ test('client.ping with callback', function userLandCode (t) { }) test('client.ping with promise', function userLandCode (t) { - resetAgent(done(t, 'HEAD', '/')) + resetAgent(assertApmDataAndEnd(t, 'HEAD /', `http://${host}/`)) agent.startTransaction('foo') @@ -62,7 +59,7 @@ test('client.ping with promise', function userLandCode (t) { }) test('client.search with callback', function userLandCode (t) { - resetAgent(done(t, 'POST', '/_search', 'q=pants')) + resetAgent(assertApmDataAndEnd(t, 'POST /_search', `http://${host}/_search?q=pants`)) agent.startTransaction('foo') @@ -78,7 +75,7 @@ test('client.search with callback', function userLandCode (t) { }) test('client.search with abort', function userLandCode (t) { - resetAgent(done(t, 'POST', '/_search', 'q=pants')) + resetAgent(assertApmDataAndEnd(t, 'POST /_search', `http://${host}/_search?q=pants`)) agent.startTransaction('foo') @@ -110,7 +107,12 @@ if (semver.satisfies(pkg.version, '>= 10')) { } } - resetAgent(done(t, 'POST', '/_search/template', JSON.stringify(body))) + resetAgent(assertApmDataAndEnd( + t, + 'POST /_search/template', + `http://${host}/_search/template`, + JSON.stringify(body) + )) agent.startTransaction('foo') @@ -140,7 +142,7 @@ if (semver.satisfies(pkg.version, '>= 13')) { var statement = body.map(JSON.stringify).join('\n') + '\n' - resetAgent(done(t, 'POST', '/_msearch', statement)) + resetAgent(assertApmDataAndEnd(t, 'POST /_msearch', `http://${host}/_msearch`, statement)) agent.startTransaction('foo') @@ -173,7 +175,7 @@ if (semver.satisfies(pkg.version, '>= 13')) { var statement = body.map(JSON.stringify).join('\n') + '\n' - resetAgent(done(t, 'POST', '/_msearch/template', statement)) + resetAgent(assertApmDataAndEnd(t, 'POST /_msearch/template', `http://${host}/_msearch/template`, statement)) agent.startTransaction('foo') @@ -189,7 +191,7 @@ if (semver.satisfies(pkg.version, '>= 13')) { } test('client.count with callback', function userLandCode (t) { - resetAgent(done(t, 'POST', '/_count', '')) + resetAgent(assertApmDataAndEnd(t, 'POST /_count', `http://${host}/_count`)) agent.startTransaction('foo') @@ -203,7 +205,7 @@ test('client.count with callback', function userLandCode (t) { }) test('client with host=', function userLandCode (t) { - resetAgent(done(t, 'HEAD', '/')) + resetAgent(assertApmDataAndEnd(t, 'HEAD /', `http://${host}/`)) agent.startTransaction('foo') var client = new elasticsearch.Client({ host: [host] }) client.ping(function (err) { @@ -215,7 +217,7 @@ test('client with host=', function userLandCode (t) { }) test('client with hosts=', function userLandCode (t) { - resetAgent(done(t, 'HEAD', '/')) + resetAgent(assertApmDataAndEnd(t, 'HEAD /', `http://${host}/`)) agent.startTransaction('foo') var client = new elasticsearch.Client({ hosts: [host, host] }) client.ping(function (err) { @@ -227,7 +229,7 @@ test('client with hosts=', function userLandCode (t) { }) test('client with hosts="http://host:port"', function userLandCode (t) { - resetAgent(done(t, 'HEAD', '/')) + resetAgent(assertApmDataAndEnd(t, 'HEAD /', `http://${host}/`)) agent.startTransaction('foo') let hostWithProto = host if (!hostWithProto.startsWith('http')) { @@ -242,7 +244,22 @@ test('client with hosts="http://host:port"', function userLandCode (t) { t.ok(agent.currentSpan === null, 'no currentSpan in sync code after elasticsearch client command') }) -function done (t, method, path, query) { +test('client with host=', function userLandCode (t) { + resetAgent(assertApmDataAndEnd(t, 'HEAD /', `http://${host}/`)) + agent.startTransaction('foo') + const [hostname, port] = host.split(':') + var client = new elasticsearch.Client({ + host: [{ host: hostname, port: port }] + }) + client.ping(function (err) { + t.error(err) + agent.endTransaction() + agent.flush() + }) + t.ok(agent.currentSpan === null, 'no currentSpan in sync code after elasticsearch client command') +}) + +function assertApmDataAndEnd (t, expectedName, expectedHttpUrl, expectedDbStatement) { return function (data, cb) { t.strictEqual(data.transactions.length, 1, 'should have 1 transaction') t.strictEqual(data.spans.length, 1, 'should have 1 span') @@ -261,16 +278,22 @@ function done (t, method, path, query) { t.strictEqual(span.subtype, subtype) t.strictEqual(span.action, action) - t.strictEqual(span.name, 'Elasticsearch: ' + method + ' ' + path) + t.strictEqual(span.name, 'Elasticsearch: ' + expectedName) t.ok(span.stacktrace.some(function (frame) { return frame.function === 'userLandCode' }), 'include user-land code frame') - if (pathIsAQuery.test(path)) { - t.deepEqual(span.context.db, { statement: query, type: 'elasticsearch' }) + if (expectedDbStatement) { + t.deepEqual(span.context.db, { type: 'elasticsearch', statement: expectedDbStatement }, 'span.context.db') + } else { + t.deepEqual(span.context.db, { type: 'elasticsearch' }, 'span.context.db') + } + + if (expectedHttpUrl) { + t.equal(span.context.http.url, expectedHttpUrl, 'span.context.http.url') } else { - t.notOk(span.context.db, 'span should not have "context.db"') + t.notOk(span.context.http && span.context.http.url, 'should not have span.context.http.url') } const [address, port] = host.split(':')