Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: Updated mysql instrumentation to properly wrap the connection pool.getConnection and poolCluster.of #1647

Merged
merged 12 commits into from
May 23, 2023
Merged

fix: Updated mysql instrumentation to properly wrap the connection pool.getConnection and poolCluster.of #1647

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
033676e
Set state of callback to not-wrapped at the beginning of initialization
mrickard May 17, 2023
2647571
updates tests to show failing tests, does expose new issue around clu…
bizob2828 May 17, 2023
f316369
Changed mysql basic-pool test variable names to match mysql2. Fixed t…
mrickard May 19, 2023
b1f627a
Fixed typos in mysql/mysql2 basic-pool tests
mrickard May 19, 2023
f0d0342
Fixed typos in mysql basic-pool assertions
mrickard May 22, 2023
90a5a44
Replaced deprecated .notEqual with recommended .not in mysql/mysql2 b…
mrickard May 22, 2023
98cfdf6
Moved getDomainSocketPath in mysql2 basic-pool.tap.js, to align it mo…
mrickard May 22, 2023
572430b
Removed calls to setup in mysql2/basic-pool.tap.js. The setup functio…
mrickard May 22, 2023
565ac6d
Fixed assertion typos
mrickard May 22, 2023
f38cd80
feat: added proper instrumentation for PoolNamespace.prototype.query
bizob2828 May 22, 2023
23332bb
Standardized datastore_tracer.instance_reporting tests in both basic-…
mrickard May 23, 2023
c75f383
Removed unneeded timeout from mysql basic-pool callback test
mrickard May 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions lib/instrumentation/mysql/mysql.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const symbols = require('../../symbols')

exports.callbackInitialize = function callbackInitialize(shim, mysql) {
shim.setDatastore(shim.MYSQL)
shim[symbols.wrappedPoolConnection] = true
shim[symbols.wrappedPoolConnection] = false

shim.wrapReturn(mysql, 'createConnection', wrapCreateConnection)
function wrapCreateConnection(shim, fn, fnName, connection) {
Expand Down Expand Up @@ -46,11 +46,12 @@ exports.callbackInitialize = function callbackInitialize(shim, mysql) {
const proto = Object.getPrototypeOf(poolCluster)
shim.wrapReturn(proto, 'of', wrapPoolClusterOf)
function wrapPoolClusterOf(shim, of, _n, poolNamespace) {
if (shim[symbols.clusterOf]) {
if (poolNamespace[symbols.clusterOf]) {
return
}
if (wrapGetConnection(shim, poolNamespace)) {
shim[symbols.clusterOf] = true

if (wrapGetConnection(shim, poolNamespace) && wrapQueryable(shim, poolNamespace, false)) {
poolNamespace[symbols.clusterOf] = true
}
}

Expand Down
147 changes: 75 additions & 72 deletions test/versioned/mysql/basic-pool.tap.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,11 @@ tap.test('mysql built-in connection pools', function (t) {
let mysql = null
let pool = null

t.beforeEach(function () {
t.beforeEach(async function () {
await setup(require('mysql'))
agent = helper.instrumentMockedAgent()
mysql = require('mysql')
pool = mysql.createPool(config)
return setup(mysql)
})

t.afterEach(function () {
Expand Down Expand Up @@ -150,8 +150,8 @@ tap.test('mysql built-in connection pools', function (t) {
const seg = getDatastoreSegment(agent.tracer.getSegment())
t.error(err, 'should not error making query')
t.ok(seg, 'should have a segment')
const attributes = seg.getAttributes()

const attributes = seg.getAttributes()
t.notOk(attributes.host, 'should have no host parameter')
t.notOk(attributes.port_path_or_id, 'should have no port parameter')
t.equal(attributes.database_name, DATABASE, 'should set database name')
Expand Down Expand Up @@ -259,13 +259,13 @@ tap.test('mysql built-in connection pools', function (t) {
t.test('pool.query', function (t) {
helper.runInTransaction(agent, function transactionInScope(txn) {
pool.query('SELECT 1 + 1 AS solution123123123123', function (err) {
const transxn = agent.getTransaction()
const transaction = agent.getTransaction()
const segment = agent.tracer.getSegment().parent

t.error(err, 'no error ocurred')
t.ok(transxn, 'transaction should exist')
t.error(err, 'no error occurred')
t.ok(transaction, 'transaction should exist')
t.ok(segment, 'segment should exist')
t.ok(segment.timer.start > 0, 'starts at a postitive time')
t.ok(segment.timer.start > 0, 'starts at a positive time')
t.ok(segment.timer.start <= Date.now(), 'starts in past')
t.equal(segment.name, 'MySQL Pool#query', 'is named')
txn.end()
Expand All @@ -277,13 +277,13 @@ tap.test('mysql built-in connection pools', function (t) {
t.test('pool.query with values', function (t) {
helper.runInTransaction(agent, function transactionInScope(txn) {
pool.query('SELECT ? + ? AS solution', [1, 1], function (err) {
const transxn = agent.getTransaction()
const transaction = agent.getTransaction()
t.error(err)
t.ok(transxn, 'should not lose transaction')
if (transxn) {
t.ok(transaction, 'should not lose transaction')
if (transaction) {
const segment = agent.tracer.getSegment().parent
t.ok(segment, 'segment should exist')
t.ok(segment.timer.start > 0, 'starts at a postitive time')
t.ok(segment.timer.start > 0, 'starts at a positive time')
t.ok(segment.timer.start <= Date.now(), 'starts in past')
t.equal(segment.name, 'MySQL Pool#query', 'is named')
}
Expand All @@ -304,13 +304,13 @@ tap.test('mysql built-in connection pools', function (t) {
})

connection.query('SELECT 1 + 1 AS solution', function (err) {
const transxn = agent.getTransaction()
const transaction = agent.getTransaction()
const segment = agent.tracer.getSegment().parent

t.error(err, 'no error ocurred')
t.ok(transxn, 'transaction should exist')
t.error(err, 'no error occurred')
t.ok(transaction, 'transaction should exist')
t.ok(segment, 'segment should exist')
t.ok(segment.timer.start > 0, 'starts at a postitive time')
t.ok(segment.timer.start > 0, 'starts at a positive time')
t.ok(segment.timer.start <= Date.now(), 'starts in past')
t.equal(segment.name, 'Datastore/statement/MySQL/unknown/select', 'is named')
txn.end()
Expand All @@ -330,13 +330,13 @@ tap.test('mysql built-in connection pools', function (t) {
})

connection.query('SELECT ? + ? AS solution', [1, 1], function (err) {
const transxn = agent.getTransaction()
const transaction = agent.getTransaction()
t.error(err)
t.ok(transxn, 'should not lose transaction')
if (transxn) {
t.ok(transaction, 'should not lose transaction')
if (transaction) {
const segment = agent.tracer.getSegment().parent
t.ok(segment, 'segment should exist')
t.ok(segment.timer.start > 0, 'starts at a postitive time')
t.ok(segment.timer.start > 0, 'starts at a positive time')
t.ok(segment.timer.start <= Date.now(), 'starts in past')
t.equal(segment.name, 'Datastore/statement/MySQL/unknown/select', 'is named')
}
Expand Down Expand Up @@ -393,16 +393,15 @@ tap.test('poolCluster', function (t) {
let mysql = null
let poolCluster = null

t.beforeEach(function () {
t.beforeEach(async function () {
await setup(require('mysql'))
agent = helper.instrumentMockedAgent()
mysql = require('mysql')
return setup(mysql).then(() => {
poolCluster = mysql.createPoolCluster()
poolCluster = mysql.createPoolCluster()

poolCluster.add(config) // anonymous group
poolCluster.add('MASTER', config)
poolCluster.add('REPLICA', config)
})
poolCluster.add(config) // anonymous group
poolCluster.add('MASTER', config)
poolCluster.add('REPLICA', config)
})

t.afterEach(function () {
Expand Down Expand Up @@ -449,15 +448,16 @@ tap.test('poolCluster', function (t) {

helper.runInTransaction(agent, function (txn) {
connection.query('SELECT ? + ? AS solution', [1, 1], function (err) {
t.error(err, 'no error ocurred')
const transxn = agent.getTransaction()
t.ok(transxn, 'transaction should exist')
t.equal(transxn.id, txn.id, 'transaction must be same')
t.error(err, 'no error occurred')
const transaction = agent.getTransaction()
t.ok(transaction, 'transaction should exist')
t.equal(transaction.id, txn.id, 'transaction must be same')
const segment = agent.tracer.getSegment().parent
t.ok(segment, 'segment should exist')
t.ok(segment.timer.start > 0, 'starts at a postitive time')
t.ok(segment.timer.start > 0, 'starts at a positive time')
t.ok(segment.timer.start <= Date.now(), 'starts in past')
t.equal(segment.name, 'Datastore/statement/MySQL/unknown/select', 'is named')

txn.end()
connection.release()
t.end()
Expand All @@ -484,15 +484,16 @@ tap.test('poolCluster', function (t) {
poolCluster.getConnection('MASTER', function (err, connection) {
helper.runInTransaction(agent, function (txn) {
connection.query('SELECT ? + ? AS solution', [1, 1], function (err) {
t.error(err, 'no error ocurred')
const transxn = agent.getTransaction()
t.ok(transxn, 'transaction should exist')
t.equal(transxn.id, txn.id, 'transaction must be same')
t.error(err, 'no error occurred')
const transaction = agent.getTransaction()
t.ok(transaction, 'transaction should exist')
t.equal(transaction.id, txn.id, 'transaction must be same')
const segment = agent.tracer.getSegment().parent
t.ok(segment, 'segment should exist')
t.ok(segment.timer.start > 0, 'starts at a postitive time')
t.ok(segment.timer.start > 0, 'starts at a positive time')
t.ok(segment.timer.start <= Date.now(), 'starts in past')
t.equal(segment.name, 'Datastore/statement/MySQL/unknown/select', 'is named')

txn.end()
connection.release()
t.end()
Expand All @@ -519,15 +520,16 @@ tap.test('poolCluster', function (t) {
poolCluster.getConnection('REPLICA*', 'ORDER', function (err, connection) {
helper.runInTransaction(agent, function (txn) {
connection.query('SELECT ? + ? AS solution', [1, 1], function (err) {
t.error(err, 'no error ocurred')
const transxn = agent.getTransaction()
t.ok(transxn, 'transaction should exist')
t.equal(transxn.id, txn.id, 'transaction must be same')
t.error(err, 'no error occurred')
const transaction = agent.getTransaction()
t.ok(transaction, 'transaction should exist')
t.equal(transaction.id, txn.id, 'transaction must be same')
const segment = agent.tracer.getSegment().parent
t.ok(segment, 'segment should exist')
t.ok(segment.timer.start > 0, 'starts at a postitive time')
t.ok(segment.timer.start > 0, 'starts at a positive time')
t.ok(segment.timer.start <= Date.now(), 'starts in past')
t.equal(segment.name, 'Datastore/statement/MySQL/unknown/select', 'is named')

txn.end()
connection.release()
t.end()
Expand All @@ -553,13 +555,13 @@ tap.test('poolCluster', function (t) {
poolCluster.of('*').getConnection(function (err, connection) {
helper.runInTransaction(agent, function (txn) {
connection.query('SELECT ? + ? AS solution', [1, 1], function (err) {
t.error(err, 'no error ocurred')
const transxn = agent.getTransaction()
t.ok(transxn, 'transaction should exist')
t.equal(transxn.id, txn.id, 'transaction must be same')
t.error(err, 'no error occurred')
const transaction = agent.getTransaction()
t.ok(transaction, 'transaction should exist')
t.equal(transaction.id, txn.id, 'transaction must be same')
const segment = agent.tracer.getSegment().parent
t.ok(segment, 'segment should exist')
t.ok(segment.timer.start > 0, 'starts at a postitive time')
t.ok(segment.timer.start > 0, 'starts at a positive time')
t.ok(segment.timer.start <= Date.now(), 'starts in past')
t.equal(segment.name, 'Datastore/statement/MySQL/unknown/select', 'is named')

Expand All @@ -575,7 +577,7 @@ tap.test('poolCluster', function (t) {
helper.runInTransaction(agent, function () {
const pool = poolCluster.of('REPLICA*', 'RANDOM')
pool.getConnection(function (err, connection) {
t.notOk(err)
t.error(err)
t.ok(agent.getTransaction(), 'should have transaction')

agent.getTransaction().end()
Expand All @@ -590,15 +592,16 @@ tap.test('poolCluster', function (t) {
pool.getConnection(function (err, connection) {
helper.runInTransaction(agent, function (txn) {
connection.query('SELECT ? + ? AS solution', [1, 1], function (err) {
t.error(err, 'no error ocurred')
const transxn = agent.getTransaction()
t.ok(transxn, 'transaction should exist')
t.equal(transxn.id, txn.id, 'transaction must be same')
t.error(err, 'no error occurred')
const currentTransaction = agent.getTransaction()
t.ok(currentTransaction, 'transaction should exist')
t.equal(currentTransaction.id, txn.id, 'transaction must be same')
const segment = agent.tracer.getSegment().parent
t.ok(segment, 'segment should exist')
t.ok(segment.timer.start > 0, 'starts at a postitive time')
t.ok(segment.timer.start > 0, 'starts at a positive time')
t.ok(segment.timer.start <= Date.now(), 'starts in past')
t.equal(segment.name, 'Datastore/statement/MySQL/unknown/select', 'is named')

txn.end()
connection.release()
t.end()
Expand All @@ -615,42 +618,42 @@ tap.test('poolCluster', function (t) {
const replicaPool = poolCluster.of('REPLICA', 'RANDOM')
helper.runInTransaction(agent, function (txn) {
replicaPool.query('SELECT ? + ? AS solution', [1, 1], function (err) {
let transxn = agent.getTransaction()
t.ok(transxn, 'transaction should exist')
t.equal(transxn, txn, 'transaction must be same')
let transaction = agent.getTransaction()
t.ok(transaction, 'transaction should exist')
t.equal(transaction, txn, 'transaction must be same')

let segment = agent.tracer.getSegment().children[1]
let segment = agent.tracer.getSegment().parent
t.ok(segment, 'segment should exist')
t.ok(segment.timer.start > 0, 'starts at a postitive time')
t.ok(segment.timer.start > 0, 'starts at a positive time')
t.ok(segment.timer.start <= Date.now(), 'starts in past')

t.equal(segment.name, 'Datastore/statement/MySQL/unknown/select', 'is named')

t.error(err, 'no error ocurred')
t.ok(transxn, 'transaction should exit')
t.equal(transxn, txn, 'transaction must be same')
t.ok(segment, 'segment should exit')
t.ok(segment.timer.start > 0, 'starts at a postitive time')
t.error(err, 'no error occurred')
t.ok(transaction, 'transaction should exist')
t.equal(transaction, txn, 'transaction must be same')
t.ok(segment, 'segment should exist')
t.ok(segment.timer.start > 0, 'starts at a positive time')
t.ok(segment.timer.start <= Date.now(), 'starts in past')
t.equal(segment.name, 'Datastore/statement/MySQL/unknown/select', 'is named')

masterPool.query('SELECT ? + ? AS solution', [1, 1], function (err) {
transxn = agent.getTransaction()
t.ok(transxn, 'transaction should exist')
t.equal(transxn, txn, 'transaction must be same')
transaction = agent.getTransaction()
t.ok(transaction, 'transaction should exist')
t.equal(transaction, txn, 'transaction must be same')

segment = agent.tracer.getSegment().children[1]
segment = agent.tracer.getSegment().parent
t.ok(segment, 'segment should exist')
t.ok(segment.timer.start > 0, 'starts at a postitive time')
t.ok(segment.timer.start > 0, 'starts at a positive time')
t.ok(segment.timer.start <= Date.now(), 'starts in past')

t.equal(segment.name, 'Datastore/statement/MySQL/unknown/select', 'is named')

t.error(err, 'no error ocurred')
t.ok(transxn, 'transaction should exit')
t.equal(transxn, txn, 'transaction must be same')
t.ok(segment, 'segment should exit')
t.ok(segment.timer.start > 0, 'starts at a postitive time')
t.error(err, 'no error occurred')
t.ok(transaction, 'transaction should exist')
t.equal(transaction, txn, 'transaction must be same')
t.ok(segment, 'segment should exist')
t.ok(segment.timer.start > 0, 'starts at a positive time')
t.ok(segment.timer.start <= Date.now(), 'starts in past')
t.equal(segment.name, 'Datastore/statement/MySQL/unknown/select', 'is named')

Expand Down
6 changes: 6 additions & 0 deletions test/versioned/mysql/helpers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,14 @@
const params = require('../../lib/params')

module.exports = async function setupDb(user, db, table, mysql) {
const regex = new RegExp(/mysql/)
await createDb(mysql, user, db)
await createTable(mysql, user, db, table)
Object.keys(require.cache).forEach((key) => {
if (regex.test(key)) {
delete require.cache[key]
}
})
}

function runCommand(client, cmd) {
Expand Down
Loading