Add sane default behavior for missing statsdKey

[tomuber]

cc @Raynos @kumikoda @mlmorg

Updated based on Jake's feedback: -Don't provide a middleware -I opted to provide a sane default for when the user does not provide a statsdKey

[Raynos]

If we are going to pave a cowpath we should probably do a little better then this.

We can change the express-statsd module to be

var statsd = require('express-statsd');
var computeKey = require('express-statsd/compute-key');

statsd(req, res, {
  statsdKey: computeKey(req, res)
}, cb)

We can put the code you have for computing the key value in a compute-key function.

We can then get rid of the middleware stuff.

[kumikoda]

+1 on what jake said. No middleware stuff!

[tomuber]

@Raynos @mlmorg @kumikoda I updated this based on jake's feedback, I opted to provide a sane default for when the user doesn't provide a statsdKey

[tomuber]

This is failing on lynx not being available for node 0.8

[mlmorg]

I think this should be two libraries: http-statsd and express-statsd. http-statsd can follow @Raynos' interface while express-statsd can build upon it and allow you to do something like:

server.get('/', 'root.statsd-key', function (req, res) {...});

Also, let me mention my dislike for trying to provide a "sane" default for three reasons:

1. With this approach, you'd now be logging any pathname + query param that a user decides to hit, regardless of whether it's an actual route that the service supports or not, e.g. /mistyped-route?foo=bar
2. You'd log all permutations of routes like /users/:id separately.
3. You may eventually run into collisions, e.g. /foo/bar and /foo_bar

We may be able to provide some ok defaults based on express' req.route interface but, again, it will never be perfect (e.g. * wildcards and collisions like issue 3 above) and that's why I would rather the eng just have to set it themselves.

[Raynos]

@mlmorg you are correct.

What you really want is integration with a routing table -.-

statsd(req, res, {
  routePattern: x
})

Which should work fine.

@mlmorg

• wildcards and collisions like issue 3 above

can we not just find a way of encoding a route pattern without running into collisions?

[tomuber]

The default is only for the top level URI, so # 2/3 won't apply. Re: # 1 it'll log the pathname and not the query params.

[mlmorg]

What's the point of only logging the content within the first /'s?

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}