Skip to content

Commit

Permalink
fix: Dropped support for ElasticSearch < 7.16.0 (#1940)
Browse files Browse the repository at this point in the history
  • Loading branch information
mrickard committed Jan 10, 2024
1 parent d3e393d commit e017768
Show file tree
Hide file tree
Showing 3 changed files with 180 additions and 10 deletions.
5 changes: 3 additions & 2 deletions lib/instrumentation/@elastic/elasticsearch.js
Expand Up @@ -20,10 +20,10 @@ const { isNotEmpty } = require('../../util/objects')
*/
module.exports = function initialize(_agent, elastic, _moduleName, shim) {
const pkgVersion = shim.pkgVersion
if (semver.lt(pkgVersion, '7.13.0')) {
if (semver.lt(pkgVersion, '7.16.0')) {
shim &&
shim.logger.debug(
`ElasticSearch support is for versions 7.13.0 and above. Not instrumenting ${pkgVersion}.`
`ElasticSearch support is for versions 7.16.0 and above. Not instrumenting ${pkgVersion}.`
)
return
}
Expand Down Expand Up @@ -70,6 +70,7 @@ function queryParser(params) {
} else if (Array.isArray(params.bulkBody) && params.bulkBody.length) {
queryParam = params.bulkBody
}
// The helper interface provides a simpler API:

const query = JSON.stringify(queryParam)

Expand Down
181 changes: 175 additions & 6 deletions test/versioned/elastic/elasticsearch.tap.js
Expand Up @@ -15,6 +15,8 @@ const { readFile } = require('fs/promises')
const semver = require('semver')
const DB_INDEX = `test-${randomString()}`
const DB_INDEX_2 = `test2-${randomString()}`
const DB_INDEX_3 = `test3-${randomString()}`
const SEARCHTERM_1 = randomString()

function randomString() {
return crypto.randomBytes(5).toString('hex')
Expand Down Expand Up @@ -130,7 +132,10 @@ test('Elasticsearch instrumentation', (t) => {
{ index: { _index: DB_INDEX_2 } },
{ title: 'Fifth Bulk Doc', body: 'Content of fifth bulk document' },
{ index: { _index: DB_INDEX_2 } },
{ title: 'Sixth Bulk Doc', body: 'Content of sixth bulk document.' },
{
title: 'Sixth Bulk Doc',
body: `Content of sixth bulk document. Has search term: ${SEARCHTERM_1}`
},
{ index: { _index: DB_INDEX_2 } },
{ title: 'Seventh Bulk Doc', body: 'Content of seventh bulk document.' },
{ index: { _index: DB_INDEX_2 } },
Expand All @@ -150,13 +155,54 @@ test('Elasticsearch instrumentation', (t) => {
})
})

t.test('should record bulk operations triggered by client helpers', async (t) => {
await helper.runInTransaction(agent, async function transactionInScope(transaction) {
const operations = [
{ title: 'Ninth Bulk Doc from helpers', body: 'Content of ninth bulk document' },
{ title: 'Tenth Bulk Doc from helpers', body: 'Content of tenth bulk document.' },
{ title: 'Eleventh Bulk Doc from helpers', body: 'Content of eleventh bulk document.' },
{ title: 'Twelfth Bulk Doc from helpers', body: 'Content of twelfth bulk document.' },
{
title: 'Thirteenth Bulk Doc from helpers',
body: 'Content of thirteenth bulk document'
},
{
title: 'Fourteenth Bulk Doc from helpers',
body: 'Content of fourteenth bulk document.'
},
{ title: 'Fifteenth Bulk Doc from helpers', body: 'Content of fifteenth bulk document.' },
{ title: 'Sixteenth Bulk Doc from helpers', body: 'Content of sixteenth bulk document.' }
]
await client.helpers.bulk({
datasource: operations,
onDocument() {
return {
index: { _index: DB_INDEX_2 }
}
},
refreshOnCompletion: true
})
t.ok(transaction, 'transaction should still be visible after bulk create')
const trace = transaction.trace
t.ok(trace?.root?.children?.[0], 'trace, trace root, and first child should exist')
t.ok(trace?.root?.children?.[1], 'trace, trace root, and second child should exist')
// helper interface results in a first child of timers.setTimeout, with the second child related to the operation
const secondChild = trace.root.children[1]
t.equal(
secondChild.name,
'Datastore/statement/ElasticSearch/any/bulk.create',
'should record bulk operation'
)
})
})

t.test('should record search with query string', async function (t) {
// enable slow queries
agent.config.transaction_tracer.explain_threshold = 0
agent.config.transaction_tracer.record_sql = 'raw'
agent.config.slow_sql.enabled = true
await helper.runInTransaction(agent, async function transactionInScope(transaction) {
const expectedQuery = { q: 'sixth' }
const expectedQuery = { q: SEARCHTERM_1 }
const search = await client.search({ index: DB_INDEX_2, ...expectedQuery })
t.ok(search, 'search should return a result')
t.ok(transaction, 'transaction should still be visible after search')
Expand Down Expand Up @@ -263,29 +309,29 @@ test('Elasticsearch instrumentation', (t) => {
await helper.runInTransaction(agent, async function transactionInScope(transaction) {
const expectedQuery = [
{}, // cross-index searches have can have an empty metadata section
{ query: { match: { body: 'sixth' } } },
{ query: { match: { body: SEARCHTERM_1 } } },
{},
{ query: { match: { body: 'bulk' } } }
]
const requestBody = setMsearch(expectedQuery, pkgVersion)
const search = await client.msearch(requestBody)
// 7 and 8 have different result responses
let results = search?.responses
if (semver.lt(pkgVersion, '8.0.0')) {
if (!search?.responses && semver.lt(pkgVersion, '8.0.0')) {
results = search?.body?.responses
}

t.ok(results, 'msearch should return results')
t.equal(results?.length, 2, 'there should be two responses--one per search')
t.equal(results?.[0]?.hits?.hits?.length, 1, 'first search should return one result')
t.equal(results?.[1]?.hits?.hits?.length, 8, 'second search should return eight results')
t.equal(results?.[1]?.hits?.hits?.length, 10, 'second search should return ten results')
t.ok(transaction, 'transaction should still be visible after search')
const trace = transaction.trace
t.ok(trace?.root?.children?.[0], 'trace, trace root, and first child should exist')
const firstChild = trace.root.children[0]
t.match(
firstChild.name,
'Datastore/statement/ElasticSearch/any/msearch',
'Datastore/statement/ElasticSearch/any/msearch.create',
'child name should show msearch'
)
const attrs = firstChild.getAttributes()
Expand All @@ -304,6 +350,39 @@ test('Elasticsearch instrumentation', (t) => {
})
})

t.test('should record msearch via helpers', async function (t) {
agent.config.transaction_tracer.explain_threshold = 0
agent.config.transaction_tracer.record_sql = 'raw'
agent.config.slow_sql.enabled = true
await helper.runInTransaction(agent, async function transactionInScope(transaction) {
const m = client.helpers.msearch()
const searchA = await m.search({}, { query: { match: { body: SEARCHTERM_1 } } })
const searchB = await m.search({}, { query: { match: { body: 'bulk' } } })
const resultsA = searchA?.body?.hits
const resultsB = searchB?.body?.hits

t.ok(resultsA, 'msearch for sixth should return results')
t.ok(resultsB, 'msearch for bulk should return results')
t.equal(resultsA?.hits?.length, 1, 'first search should return one result')
t.equal(resultsB?.hits?.length, 10, 'second search should return ten results')
t.ok(transaction, 'transaction should still be visible after search')
const trace = transaction.trace
t.ok(trace?.root?.children?.[0], 'trace, trace root, and first child should exist')
const firstChild = trace.root.children[0]
t.match(
firstChild.name,
'timers.setTimeout',
'helpers, for some reason, generates a setTimeout metric first'
)
transaction.end()
t.ok(agent.queries.samples.size > 0, 'there should be a query sample')
for (const query of agent.queries.samples.values()) {
// which query gets captured in helper.msearch is non-deterministic
t.ok(query.total > 0, 'the samples should have positive duration')
}
})
})

t.test('should create correct metrics', async function (t) {
const id = `key-${randomString()}`
t.plan(28)
Expand Down Expand Up @@ -417,6 +496,96 @@ test('Elasticsearch instrumentation', (t) => {
})
})

test('Elasticsearch uninstrumented behavior, to check helpers', { skip: false }, (t) => {
t.autoend()

let client
// eslint-disable-next-line no-unused-vars
let pkgVersion

t.before(async () => {
// Determine version. ElasticSearch v7 did not export package, so we have to read the file
// instead of requiring it, as we can with 8+.
const pkg = await readFile(`${__dirname}/node_modules/@elastic/elasticsearch/package.json`)
;({ version: pkgVersion } = JSON.parse(pkg.toString()))

const { Client } = require('@elastic/elasticsearch')
client = new Client({
node: `http://${params.elastic_host}:${params.elastic_port}`
})

return Promise.all([client.indices.create({ index: DB_INDEX_3 })])
})

t.teardown(() => {
return Promise.all([client.indices.delete({ index: DB_INDEX_3 })])
})

t.test('should record bulk operations triggered by client helpers', async (t) => {
const operations = [
{
title: 'Uninstrumented First Bulk Doc from helpers',
body: 'Content of uninstrumented first bulk document'
},
{
title: 'Uninstrumented Second Bulk Doc from helpers',
body: 'Content of uninstrumented second bulk document.'
},
{
title: 'Uninstrumented Third Bulk Doc from helpers',
body: 'Content of uninstrumented third bulk document.'
},
{
title: 'Uninstrumented Fourth Bulk Doc from helpers',
body: 'Content of uninstrumented fourth bulk document.'
},
{
title: 'Uninstrumented Fifth Bulk Doc from helpers',
body: 'Content of uninstrumented fifth bulk document'
},
{
title: 'Uninstrumented Sixth Bulk Doc from helpers',
body: 'Content of uninstrumented sixth bulk document.'
},
{
title: 'Uninstrumented Seventh Bulk Doc from helpers',
body: 'Content of uninstrumented seventh bulk document.'
},
{
title: 'Uninstrumented Eighth Bulk Doc from helpers',
body: 'Content of uninstrumented eighth bulk document.'
}
]
const result = await client.helpers.bulk({
datasource: operations,
onDocument() {
return {
index: { _index: DB_INDEX_3 }
}
}
// refreshOnCompletion: true
}) // setBulkBody(operations, pkgVersion)
t.ok(result, 'We should have been able to create bulk entries without error')
t.equal(result.total, 8, 'We should have been inserted eight records')
})
t.test('should be able to check bulk insert with msearch via helpers', async function (t) {
const m = client.helpers.msearch()
const searchA = await m.search({ index: DB_INDEX_3 }, { query: { match: { body: 'sixth' } } })
const searchB = await m.search(
{ index: DB_INDEX_3 },
{ query: { match: { body: 'uninstrumented' } } }
)
const resultsA = searchA?.body?.hits
const resultsB = searchB?.body?.hits

t.ok(resultsA, 'msearch should return a response for A')
t.ok(resultsB, 'msearch should return results for B')
// some versions of helper msearch seem not to return results for the first search.
// t.equal(resultsA?.hits?.length, 1, 'first search should return one result')
t.equal(resultsB?.hits?.length, 8, 'second search should return eight results')
})
})

function checkMetrics(t, metrics, expected) {
Object.keys(expected).forEach(function (name) {
t.ok(metrics[name], 'should have metric ' + name)
Expand Down
4 changes: 2 additions & 2 deletions test/versioned/elastic/package.json
Expand Up @@ -11,7 +11,7 @@
"node": ">=16"
},
"dependencies": {
"@elastic/elasticsearch": "7.10.0"
"@elastic/elasticsearch": "7.13.0"
},
"files": [
"elasticsearchNoop.tap.js"
Expand All @@ -22,7 +22,7 @@
"node": ">=16"
},
"dependencies": {
"@elastic/elasticsearch": ">=7.13.0"
"@elastic/elasticsearch": ">=7.16.0"
},
"files": [
"elasticsearch.tap.js"
Expand Down

0 comments on commit e017768

Please sign in to comment.