Skip to content

Commit

Permalink
fix: make 'elasticsearch' instrumentation host/port parsing more resi…
Browse files Browse the repository at this point in the history
…lient (#2312)

In particular support the `host` or `hosts` config options being arrays.

Refs: #2306
  • Loading branch information
trentm authored Sep 1, 2021
1 parent 3326b60 commit 32e4e92
Show file tree
Hide file tree
Showing 3 changed files with 95 additions and 4 deletions.
6 changes: 5 additions & 1 deletion CHANGELOG.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ Notes:
[float]
===== Bug fixes
* Fix a crash in instrumentation of the old Elasticsearch client
(`elasticsearch`) for some rarer cases of client options -- for example
passing multiple hosts. ({pull}2312[#2312])
* Ensure the internal HTTP(S) client requests made by the APM agent to APM
server are not themselves traced. ({issues}1168[#1168], {issues}1136[#1136])
Expand Down Expand Up @@ -90,7 +94,7 @@ Notes:
* Add instrumentation of the AWS SNS publish method when using the
https://www.npmjs.com/package/aws-sdk[JavaScript AWS SDK v2] (`aws-sdk`). ({pull}2157[#2157])
[float]
===== Bug fixes
Expand Down
56 changes: 53 additions & 3 deletions lib/instrumentation/modules/elasticsearch.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,58 @@
'use strict'

const URL = require('url').URL

var shimmer = require('../shimmer')
var { getDBDestination } = require('../context')
const { setElasticsearchDbContext } = require('../elasticsearch-shared')

const startsWithProtocolRE = /^([a-z]+:)?\/\//i
const DEFAULT_PORT = 9200
const DEFAULT_PORT_FROM_PROTO = {
'http:': 80,
'https:': 443
}

// This is an imperfect equivalent of the handling in the `Transport`
// constructor and internal `Host` parsing function in the ES client.
function getHostAndPortFromTransportConfig (config) {
const transportHosts = config ? config.host || config.hosts : null
if (!transportHosts) {
return null
}

let firstTransportHost = Array.isArray(transportHosts)
? transportHosts[0] : transportHosts
if (!firstTransportHost) {
return null
}

if (typeof firstTransportHost === 'string') {
// "example.com:42" or "someprotocol://example.com:42" or
// "someprotocol://example.com".
if (!startsWithProtocolRE.test(firstTransportHost)) {
firstTransportHost = 'http://' + firstTransportHost
}
let u
try {
u = 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
]
}

return null
}

module.exports = function (elasticsearch, agent, { enabled }) {
if (!enabled) return elasticsearch

Expand All @@ -28,10 +77,11 @@ module.exports = function (elasticsearch, agent, { enabled }) {
params && params.body)

// Get the remote host information from elasticsearch Transport options.
const transportConfig = this._config
let host, port
if (typeof transportConfig === 'object' && transportConfig.host) {
[host, port] = transportConfig.host.split(':')
const hostAndPort = getHostAndPortFromTransportConfig(this._config)
if (hostAndPort) {
host = hostAndPort[0]
port = hostAndPort[1]
}
span.setDestinationContext(getDBDestination(span, host, port))

Expand Down
37 changes: 37 additions & 0 deletions test/instrumentation/modules/elasticsearch.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,43 @@ test('client.count with callback', function userLandCode (t) {
})
})

test('client with host=<array of host:port>', function userLandCode (t) {
resetAgent(done(t, 'HEAD', '/'))
agent.startTransaction('foo')
var client = new elasticsearch.Client({ host: [host] })
client.ping(function (err) {
t.error(err)
agent.endTransaction()
agent.flush()
})
})

test('client with hosts=<array of host:port>', function userLandCode (t) {
resetAgent(done(t, 'HEAD', '/'))
agent.startTransaction('foo')
var client = new elasticsearch.Client({ hosts: [host, host] })
client.ping(function (err) {
t.error(err)
agent.endTransaction()
agent.flush()
})
})

test('client with hosts="http://host:port"', function userLandCode (t) {
resetAgent(done(t, 'HEAD', '/'))
agent.startTransaction('foo')
let hostWithProto = host
if (!hostWithProto.startsWith('http')) {
hostWithProto = 'http://' + host
}
var client = new elasticsearch.Client({ hosts: hostWithProto })
client.ping(function (err) {
t.error(err)
agent.endTransaction()
agent.flush()
})
})

function done (t, method, path, query, abort = false) {
return function (data, cb) {
t.strictEqual(data.transactions.length, 1, 'should have 1 transaction')
Expand Down

0 comments on commit 32e4e92

Please sign in to comment.