Merge pull request #1146 from NodeJS-agent/nwolfe/n7-net

NODE-1446 net: support net module on node 7.10.0 and later

lib/instrumentation/core/http.js

@@ -510,14 +510,15 @@ module.exports = function initialize(agent, http, moduleName) {
                        wrapWriteHead.bind(null, agent))
   }
 
-  /**
-   * As of node 0.8, http.request() is the right way to originate outbound
-   * requests.
-   */
-  if (http && http.Agent && http.Agent.prototype && http.Agent.prototype.request) {
+  var agentProto = http && http.Agent && http.Agent.prototype
+
+  // As of node 0.8, http.request() is the right way to originate outbound
+  // requests.
+  // TODO: Remove this check when deprecating Node <0.8.
+  if (agentProto && agentProto.request) {
     // Node 0.11+ always uses an Agent.
     shimmer.wrapMethod(
-      http.Agent.prototype,
+      agentProto,
       'http.Agent.prototype',
       'request',
       wrapRequest.bind(null, agent)
@@ -531,6 +532,34 @@ module.exports = function initialize(agent, http, moduleName) {
     )
   }
 
+  // Agent#createConnection was added in 0.11.
+  // TODO: Remove this check when deprecating Node 0.10
+  if (agentProto && agentProto.createConnection) {
+    shimmer.wrapMethod(
+      agentProto,
+      'http.Agent.prototype',
+      'createConnection',
+      function wrapCreateConnection(original) {
+        return function wrappedCreateConnection() {
+          if (!agent.getTransaction()) {
+            return original.apply(this, arguments)
+          }
+
+          var segment = agent.tracer.createSegment('http.Agent#createConnection')
+
+          var args = agent.tracer.slice(arguments)
+          if (typeof args[1] === 'function') {
+            args[1] = agent.tracer.bindFunction(args[1], segment, true)
+          }
+
+          var sock = agent.tracer.bindFunction(original, segment, true)
+            .apply(this, args)
+          return sock
+        }
+      }
+    )
+  }
+
   // http.Client is deprecated, but still in use
   var DeprecatedClient, deprecatedCreateClient
   function clearGetters() {

lib/instrumentation/core/net.js

@@ -1,12 +1,35 @@
 'use strict'
 
-var wrap = require('../../shimmer').wrapMethod
+var shimmer = require('../../shimmer')
 
-module.exports = initialize
 
-function initialize(agent, net) {
-  wrap(net.Server.prototype, 'net.Server.prototype', '_listen2', wrapListen2)
-  wrap(net.Socket.prototype, 'net.Socket.prototype', 'connect', wrapConnect)
+module.exports = function initialize(agent, net) {
+  shimmer.wrapMethod(net, 'net', ['connect', 'createConnection'], wrapCreate)
+  function wrapCreate(original, name) {
+    return function wrappedCreateConnection() {
+      if (!agent.getTransaction()) {
+        return original.apply(this, arguments)
+      }
+
+      var segment = agent.tracer.createSegment('net.' + name)
+      var sock = agent.tracer.bindFunction(original, segment, true).apply(this, arguments)
+      wrapSocket(sock, segment)
+      return sock
+    }
+  }
+
+  shimmer.wrapMethod(
+    net.Server.prototype,
+    'net.Server.prototype',
+    '_listen2',
+    wrapListen2
+  )
+  shimmer.wrapMethod(
+    net.Socket.prototype,
+    'net.Socket.prototype',
+    'connect',
+    wrapConnect
+  )
 
   function wrapListen2(original) {
     return function wrappedListen2() {
@@ -59,6 +82,12 @@ function initialize(agent, net) {
       }
     }
   }
+
+  function wrapSocket(sock, segment) {
+    shimmer.wrapMethod(sock, 'net.Socket', 'emit', function emitWrapper(emit) {
+      return agent.tracer.bindFunction(emit, segment)
+    })
+  }
 }
 
 // taken from node master on 2013/10/30

test/integration/core/net.tap.js

@@ -4,8 +4,13 @@ var test = require('tap').test
 var net = require('net')
 var helper = require('../../lib/agent_helper')
 
+function id(tx) {
+  return tx && tx.id
+}
+
 test('createServer', function createServerTest(t) {
   var agent = setupAgent(t)
+
   helper.runInTransaction(agent, function transactionWrapper(transaction) {
     var server = net.createServer(handler)
 
@@ -17,7 +22,7 @@ test('createServer', function createServerTest(t) {
     })
 
     function handler(socket) {
-      t.equal(transaction, agent.getTransaction())
+      t.equal(id(agent.getTransaction()), id(transaction), 'should maintain tx')
       socket.end('test')
       t.equal(
         agent.tracer.getSegment().name,
@@ -26,7 +31,7 @@ test('createServer', function createServerTest(t) {
       )
 
       socket.on('data', function onData(data) {
-        t.equal(transaction, agent.getTransaction())
+        t.equal(id(agent.getTransaction()), id(transaction), 'should maintain tx')
         t.equal(data.toString(), 'test123')
         socket.end()
         setTimeout(server.close.bind(server, onClose), 0)
@@ -87,32 +92,42 @@ test('connect', function connectTest(t) {
     var count = 0
     var socket = net.createConnection({port: 4123})
     socket.on('data', function onData(data) {
-      t.equal(agent.getTransaction(), transaction)
+      t.equal(id(agent.getTransaction()), id(transaction), 'should maintain tx')
       t.equal(data.toString(), 'end data')
       ++count
     })
     socket.on('end', function onEnd() {
-      t.equal(agent.getTransaction(), transaction)
+      t.equal(id(agent.getTransaction()), id(transaction), 'should maintain tx')
       t.equal(count, 1)
       setTimeout(verify, 0)
     })
 
     socket.on('connect', function onConnet() {
-      t.equal(agent.getTransaction(), transaction)
+      t.equal(id(agent.getTransaction()), id(transaction), 'should maintain tx')
       socket.write('some data')
     })
 
     function verify() {
+      if (!t.passing()) {
+        return t.end()
+      }
+
       var root = agent.getTransaction().trace.root
       t.equal(root.children.length, 1, 'should have a single child')
       var connectSegment = root.children[0]
       t.equal(
         connectSegment.name,
-        'net.Socket.connect',
+        'net.createConnection',
         'connect segment should have correct name'
       )
       t.ok(connectSegment.timer.touched, 'connect should started and ended')
 
+      // Depending on the version of Node there may be another connection segment
+      // floating in the trace.
+      if (connectSegment.children[0].name === 'net.Socket.connect') {
+        connectSegment = connectSegment.children[0]
+      }
+
       var timeoutSegment
       // 0.12 has dns lookup, 0.10 and under does not
       if (connectSegment.children.length > 1) {
@@ -170,7 +185,7 @@ test('createServer and connect', function createServerTest(t) {
     })
 
     function handler(socket) {
-      t.equal(transaction, agent.getTransaction())
+      t.equal(id(agent.getTransaction()), id(transaction), 'should maintain tx')
       socket.end('test')
       t.equal(
         agent.tracer.getSegment().name,
@@ -179,7 +194,7 @@ test('createServer and connect', function createServerTest(t) {
       )
 
       socket.on('data', function onData(data) {
-        t.equal(transaction, agent.getTransaction())
+        t.equal(id(agent.getTransaction()), id(transaction), 'should maintain tx')
         t.equal(data.toString(), 'test123')
         socket.end()
         server.close(onClose)
@@ -192,11 +207,17 @@ test('createServer and connect', function createServerTest(t) {
       var clientSegment = root.children[0]
       t.equal(
         clientSegment.name,
-        'net.Socket.connect',
+        'net.connect',
         'server segment should have correct name'
       )
       t.ok(clientSegment.timer.touched, 'server should started and ended')
 
+      // Depending on the version of Node there may be another connection segment
+      // floating in the trace.
+      if (clientSegment.children[0].name === 'net.Socket.connect') {
+        clientSegment = clientSegment.children[0]
+      }
+
       // 0.12 gets a DNS segment, 0.10 or less doesn't, yay conditional tests.
       if (clientSegment.children.length > 0) {
         t.equal(

test/integration/instrumentation/http-outbound.tap.js

@@ -81,10 +81,20 @@ tap.test('external requests', function(t) {
       t.ok(external.timer.duration, 'should have ended')
       t.ok(external.children.length, 'should have children')
 
+      // TODO: Change this to a simple equal when deprecating Node v0.10
       var connect = external.children[0]
-      t.equal(connect.name, 'net.Socket.connect', 'should be connect segment')
+      t.match(
+        connect.name,
+        /^(?:http\.Agent#createConnection|net\.Socket\.connect)$/,
+        'should be connect segment'
+      )
       t.equal(connect.children.length, 1, 'connect should have 1 child')
 
+      // There is potentially an extra layer of create/connect segments.
+      if (connect.children[0].name === 'net.Socket.connect') {
+        connect = connect.children[0]
+      }
+
       var dnsLookup = connect.children[0]
       t.equal(dnsLookup.name, 'dns.lookup', 'should be dns.lookup segment')