Merge pull request #50 from eexit/master

Express middleware: improve route name sanitation + added an option

README.md

@@ -132,6 +132,8 @@ As the names can become rather odd in corner-cases (esp. regexes and non-REST
 interfaces), you can specify another value by setting `res.locals.statsdUrlKey`
 at a later point.
 
+The `/` page will appear as `root` (e.g. `GET_root`) in metrics while any not found route will appear as `{METHOD}_unknown_express_route`. You can change that name by setting the `notFoundRouteName` in the middleware options.
+
 ### Stopping gracefully
 
 By default, the socket is closed if it hasn't been used for a second (see

lib/helpers/getExpressMiddleware.js

@@ -1,3 +1,33 @@
+
+// Removes ":", heading/trailing / and replaces / by _ in a given route name
+function sanitize(routeName) {
+    return routeName.replace(/:/g, "").replace(/^\/|\/$/g, "").replace(/\//g, "_");
+}
+
+// Extracts a route name from the request or response
+function findRouteName(req, res) {
+    // Did we get a hardcoded name, or should we figure one out?
+    if (res.locals && res.locals.statsdUrlKey) {
+
+        return res.locals.statsdUrlKey;
+    }
+
+    if (req.route && req.route.path) {
+        var routeName = req.route.path;
+
+        if (Object.prototype.toString.call(routeName) === '[object RegExp]') {
+            routeName = routeName.source;
+        }
+
+        if (routeName === '/') {
+            routeName = 'root';
+        }
+
+        // Appends the HTTP method
+        return req.method + '_' + sanitize(routeName);
+    }
+}
+
 /*
  * Return express middleware that measures overall performance.
  *
@@ -11,10 +41,12 @@
  */
 function factory(parentClient) {
     return function (prefix, options) {
-        var client = parentClient.getChildClient(prefix || '');
         options = options || {};
+
+        var client = parentClient.getChildClient(prefix || '');
         var timeByUrl = options.timeByUrl || false;
-        var onResponseEnd = options.onResponseEnd;
+        var notFoundRouteName = options.notFoundRouteName || 'unknown_express_route';
+        var onResponseEnd = options.onResponseEnd || undefined;
 
         return function (req, res, next) {
             var startTime = new Date();
@@ -24,33 +56,17 @@ function factory(parentClient) {
             res.end = function () {
                 end.apply(res, arguments);
 
+                var timingName = 'response_time';
                 client.increment('response_code.' + res.statusCode);
 
                 // Time by URL?
                 if (timeByUrl) {
-                    var routeName = "unknown_express_route";
-
-                    // Did we get a harc-coded name, or should we figure one out?
-                    if (res.locals && res.locals.statsdUrlKey) {
-                        routeName = res.locals.statsdUrlKey;
-                    } else if (req.route && req.route.path) {
-                        routeName = req.route.path;
-                        if (Object.prototype.toString.call(routeName) === '[object RegExp]') {
-                            // Might want to do some sanitation here?
-                            routeName = routeName.source;
-                        }
-                        if (routeName === "/") routeName = "root";
-                        routeName = req.method + '_' + routeName;
-                    }
-
-                    // Get rid of : in route names, remove first and last /,
-                    // and replace rest with _.
-                    routeName = 'response_time.' + routeName.replace(/:/g, "").replace(/^\/|\/$/g, "").replace(/\//g, "_");
-                    client.timing(routeName, startTime);
-                } else {
-                    client.timing('response_time', startTime);
+                    timingName += '.';
+                    timingName += findRouteName(req, res) || notFoundRouteName;
                 }
 
+                client.timing(timingName, startTime);
+
                 if (onResponseEnd) {
                     onResponseEnd(client, startTime, req, res);
                 }