routers.patch

From 357ac14276b19fd760a9c3441a81ba2469e4e42a Mon Sep 17 00:00:00 2001
From: Michael Hayes <mhayes@newrelic.com>
Date: Fri, 10 Oct 2014 13:49:15 -0700
Subject: [PATCH] wip

---
 lib/instrumentation/express.js | 71 ++++++++++++++++++++++++++++++++++--------
 1 file changed, 58 insertions(+), 13 deletions(-)

diff --git a/lib/instrumentation/express.js b/lib/instrumentation/express.js
index 032cacd..7cf5684 100644
--- a/lib/instrumentation/express.js
+++ b/lib/instrumentation/express.js
@@ -60,8 +60,7 @@ function nameFromRoute(segment, route, params) {
 
   urltils.copyParameters(transaction.agent.config, params, segment.parameters)
 
-  transaction.partialName = NAMES.EXPRESS.PREFIX + transaction.verb +
-                            NAMES.ACTION_DELIMITER + path
+  transaction.partialName += path
 }
 
 module.exports = function initialize(agent, express) {
@@ -200,9 +199,9 @@ module.exports = function initialize(agent, express) {
    * Use eval. This once. For this one specific purpose. Not anywhere else for
    * any reason.
    */
-  function wrapHandle(__NR_handle) {
+  function wrapHandle(__NR_handle, path) {
+    var name = ''
     var arglist
-      , name = ''
 
 
     // reiterated: testing function arity is stupid
@@ -238,12 +237,20 @@ module.exports = function initialize(agent, express) {
       __NR_handle.apply(this, args)
     }
 
+    var routerTemplate = function () {
+      return wrappedHandle.call(this, path, template, [].slice.call(arguments))
+    }
+
+    var handlerTemplate = Object.getPrototypeOf(__NR_handle) === express.Router ?
+      routerTemplate :
+      template
+
     // I am a bad person and this makes me feel bad.
     // We use eval because we need to insert the function with a specific name to allow for lookups.
     // jshint evil:true
     var wrapped = eval(
       '(function(){return function ' + name + arglist +
-      template.toString().substring(11) + '}())'
+      handlerTemplate.toString().substring(11) + '}())'
     )
     wrapped[ORIGINAL] = __NR_handle
     // jshint evil:false
@@ -260,6 +267,7 @@ module.exports = function initialize(agent, express) {
        * for us so we don't have to also do argument type checking.
        */
       var app = use.apply(this, arguments)
+      var path = typeof arguments[0] === 'string' ? arguments[0] : '/'
 
       /* Express adds routes to the same stack as middlewares. We need to wrap
        * that adder too but we only want to wrap the middlewares that are
@@ -267,13 +275,14 @@ module.exports = function initialize(agent, express) {
        */
       if (!route) {
         // wrap most recently added unwrapped handler
-        var top = this.stack[this.stack.length - 1]
-        if (top) {
-            if (top.handle &&
-                typeof top.handle === 'function' &&
-                !top.handle[ORIGINAL]) {
-              top.handle = wrapHandle(top.handle)
-            }
+        var i = this.stack.length
+        var top
+        while(top = this.stack[--i]) {
+          if(!top.handle || typeof top.handle !== 'function' || top.handle[ORIGINAL]) {
+            break
+          }
+
+          top.handle = wrapHandle(top.handle, path)
         }
       }
 
@@ -322,6 +331,37 @@ module.exports = function initialize(agent, express) {
     }
   }
 
+  function wrapAppHandle(handle) {
+    return function(req, res, next) {
+      return wrappedHandle.call(this, this.mountpath, handle, [].slice.call(arguments))
+    }
+  }
+
+  function wrappedHandle(path, handle, args) {
+    var transaction = agent.tracer.getTransaction()
+    var orignal = transaction.partialName
+    var next = args[2]
+
+    if(next) {
+      args[2] = cleanup
+    }
+
+    if(!transaction.partialName || transaction.partialName.lastIndexOf(NAMES.EXPRESS.PREFIX, 0) !== 0) {
+      transaction.partialName = NAMES.EXPRESS.PREFIX + transaction.verb + NAMES.ACTION_DELIMITER
+    }
+
+    if(path && path !== '/') {
+      transaction.partialName += path
+    }
+
+    handle.apply(this, args)
+
+    function cleanup() {
+      transaction.partialName = orignal
+      next.apply(this, arguments)
+    }
+  }
+
   function addInterceptor(app) {
     /* Give the error tracer a better chance of intercepting errors by
      * putting it before the first error handler (a middleware that takes 4
@@ -465,10 +505,15 @@ module.exports = function initialize(agent, express) {
                          'route',
                          wrapMiddlewareStack.bind(null, true))
 
-       shimmer.wrapMethod(express.application,
+      shimmer.wrapMethod(express.application,
                          'express.application',
                          'use',
                          wrapAppUse)
+
+      shimmer.wrapMethod(express.application,
+                         'express.application',
+                         'handle',
+                         wrapAppHandle)
       break
     default:
       logger.warn("Unrecognized version %d of Express detected; not instrumenting",
-- 
1.9.3 (Apple Git-50)