Merge pull request #1143 from NodeJS-agent/nwolfe/generic-pool

NODE-1440 Fix generic pool 3 instrumentation

lib/instrumentation/generic-pool.js

@@ -4,38 +4,41 @@ var shimmer = require('../shimmer')
 
 
 module.exports = function initialize(agent, generic) {
-  shimmer.wrapMethod(generic, 'generic-pool', 'Pool', function cb_wrapMethod(Pool) {
-    return function cls_wrapMethod() {
-      var pooler = Pool.apply(this, arguments)
+  if (!generic || !generic.Pool || !generic.Pool.prototype) {
+    return false
+  }
 
-      shimmer.wrapMethod(pooler, 'Pool', 'acquire', function cb_wrapMethod(acquire) {
-        return function propagateTransactionThroughPool(callback, priority) {
-          if (typeof callback === 'function') {
-            /* See adjustCallback in generic-pool.js for the motivation behind
-             * this grotesque hack. Tl;dr: depending on Function.length is evil.
-             */
-            var proxied = agent.tracer.bindFunction(callback)
-            switch (callback.length) {
-              case 2:
-                callback = function moveAlongNothingToSeeHere(error, client) {
-                  return proxied.call(this, error, client)
-                }
-                break
-              case 1:
-                callback = function moveAlongNothingToSeeHere(client) {
-                  return proxied.call(this, client)
-                }
-                break
-              default:
-                callback = proxied
+  var proto = generic.Pool.prototype
+  shimmer.wrapMethod(proto, 'Pool', 'acquire', function wrapAcquire(acquire) {
+    return function wrappedAcquire(callback, priority) {
+      if (typeof callback === 'function') {
+        /* See adjustCallback in generic-pool.js for the motivation behind
+         * this grotesque hack. Tl;dr: depending on Function.length is evil.
+         */
+        var proxied = agent.tracer.bindFunction(callback)
+        switch (callback.length) {
+          case 2:
+            callback = function moveAlongNothingToSeeHere(error, client) {
+              return proxied.call(this, error, client)
             }
-          }
-
-          return acquire.call(this, callback, priority)
+            break
+          case 1:
+            callback = function moveAlongNothingToSeeHere(client) {
+              return proxied.call(this, client)
+            }
+            break
+          default:
+            callback = proxied
         }
-      })
+      }
+
+      return acquire.call(this, callback, priority)
+    }
+  })
 
-      return pooler
+  shimmer.wrapMethod(proto, 'Pool', ['drain', 'destroyAllNow'], function wrap(original) {
+    return function wrapper(cb) {
+      return original.call(this, agent.tracer.bindFunction(cb))
     }
   })
 }

test/versioned/generic-pool-2/basic.tap.js

@@ -0,0 +1,117 @@
+'use strict'
+
+var a = require('async')
+var helper = require('../../lib/agent_helper')
+var tap = require('tap')
+
+
+tap.test('generic-pool', function(t) {
+  t.autoend()
+
+  var agent = null
+  var pool = null
+  var PoolClass = require('generic-pool').Pool
+
+  t.beforeEach(function(done) {
+    agent = helper.instrumentMockedAgent()
+    pool = require('generic-pool')
+    done()
+  })
+
+  t.afterEach(function(done) {
+    helper.unloadAgent(agent)
+    pool = null
+    done()
+  })
+
+  var tasks = []
+  var decontextInterval = setInterval(function() {
+    if (tasks.length > 0) {
+      tasks.pop()()
+    }
+  }, 10)
+
+  t.tearDown(function() {
+    clearInterval(decontextInterval)
+  })
+
+  function addTask(cb, args) {
+    tasks.push(function() {
+      return cb.apply(null, args || [])
+    })
+  }
+
+  function id(tx) {
+    return tx && tx.id
+  }
+
+  t.test('instantiation', function(t) {
+    t.plan(4)
+
+    t.doesNotThrow(function() {
+      var p = pool.Pool({ // eslint-disable-line new-cap
+        create: function(cb) { addTask(cb, [null, {}]) },
+        destroy: function(o, cb) { addTask(cb) }
+      })
+      t.type(p, PoolClass, 'should create a Pool')
+    }, 'should be able to instantiate without new')
+
+    t.doesNotThrow(function() {
+      var p = new pool.Pool({
+        create: function(cb) { addTask(cb, [null, {}]) },
+        destroy: function(o, cb) { addTask(cb) }
+      })
+      t.type(p, PoolClass, 'should create a Pool')
+    }, 'should be able to instantiate with new')
+  })
+
+  t.test('context maintenance', function(t) {
+    var p = new pool.Pool({
+      max: 2,
+      min: 0,
+      create: function(cb) { addTask(cb, [null, {}]) },
+      destroy: function(o, cb) { addTask(cb) }
+    })
+
+    a.times(6, run, function(err) {
+      t.error(err, 'should not error when acquiring')
+      drain()
+    })
+
+    function run(n, cb) {
+      helper.runInTransaction(agent, function(tx) {
+        p.acquire(function(err, c) {
+          if (err) {
+            return cb(err)
+          }
+
+          t.equal(id(agent.getTransaction()), id(tx), n + ': should maintain tx state')
+          addTask(function() {
+            p.release(c)
+            cb()
+          })
+        })
+      })
+    }
+
+    function drain() {
+      run('drain', function(err) {
+        t.error(err, 'should not error when acquired before draining')
+      })
+
+      helper.runInTransaction(agent, function(tx) {
+        p.drain(function() {
+          t.equal(id(agent.getTransaction()), id(tx), 'should have context through drain')
+
+          p.destroyAllNow(function() {
+            t.equal(
+              id(agent.getTransaction()), id(tx),
+              'should have context through destroy'
+            )
+            t.end()
+          })
+        })
+      })
+    }
+  })
+})

test/versioned/generic-pool-2/package.json

@@ -0,0 +1,7 @@
+{
+  "name": "generic-pool-2-test",
+  "dependencies": {
+    "generic-pool": "^2"
+  },
+  "private": true
+}

test/versioned/generic-pool-latest/basic.tap.js

@@ -0,0 +1,117 @@
+'use strict'
+
+var a = require('async')
+var helper = require('../../lib/agent_helper')
+var semver = require('semver')
+var tap = require('tap')
+
+
+tap.test('generic-pool', function(t) {
+  t.autoend()
+
+  if (semver.lt(process.version, '4.0.0')) {
+    return
+  }
+
+  var agent = null
+  var pool = null
+  var PoolClass = require('generic-pool').Pool
+
+  t.beforeEach(function(done) {
+    agent = helper.instrumentMockedAgent()
+    pool = require('generic-pool')
+    done()
+  })
+
+  t.afterEach(function(done) {
+    helper.unloadAgent(agent)
+    pool = null
+    done()
+  })
+
+  var tasks = []
+  var decontextInterval = setInterval(function() {
+    if (tasks.length > 0) {
+      tasks.pop()()
+    }
+  }, 10)
+
+  t.tearDown(function() {
+    clearInterval(decontextInterval)
+  })
+
+  function addTask(cb, args) {
+    tasks.push(function() {
+      return cb.apply(null, args || [])
+    })
+  }
+
+  function id(tx) {
+    return tx && tx.id
+  }
+
+  t.test('instantiation', function(t) {
+    t.plan(2)
+
+    // As of generic-pool 3, it is not possible to instantiate Pool without `new`.
+
+    t.doesNotThrow(function() {
+      var p = pool.createPool({
+        create: function() { return new Promise(function(res) { addTask(res, {}) }) },
+        destroy: function() { return new Promise(function(res) { addTask(res) }) }
+      })
+      t.type(p, PoolClass, 'should create a Pool')
+    }, 'should be able to instantiate with createPool')
+  })
+
+  t.test('context maintenance', function(t) {
+    var p = pool.createPool({
+      create: function() { return new Promise(function(res) { addTask(res, {}) }) },
+      destroy: function() { return new Promise(function(res) { addTask(res) }) }
+    }, {
+      max: 2,
+      min: 0
+    })
+
+    a.times(6, run, function(err) {
+      t.error(err, 'should not error when acquiring')
+      drain()
+    })
+
+    function run(n, cb) {
+      helper.runInTransaction(agent, function(tx) {
+        p.acquire().then(function(c) {
+          t.equal(id(agent.getTransaction()), id(tx), n + ': should maintain tx state')
+          addTask(function() {
+            p.release(c)
+            cb()
+          })
+        }, cb)
+      })
+    }
+
+    function drain() {
+      run('drain', function(err) {
+        t.error(err, 'should not error when acquired before draining')
+      })
+
+      helper.runInTransaction(agent, function(tx) {
+        p.drain().then(function() {
+          t.equal(id(agent.getTransaction()), id(tx), 'should have context through drain')
+
+          return p.clear().then(function() {
+            t.equal(
+              id(agent.getTransaction()), id(tx),
+              'should have context through destroy'
+            )
+          })
+        }).then(function() {
+          t.end()
+        }, function(err) {
+          t.error(err)
+          t.end()
+        })
+      })
+    }
+  })
+})

test/versioned/generic-pool-latest/package.json

@@ -0,0 +1,7 @@
+{
+  "name": "generic-pool-2-test",
+  "dependencies": {
+    "generic-pool": "*"
+  },
+  "private": true
+}