datadog / node-connect-datadog Goto Github PK

When routing requests using app.use(path, handlers), the req.route object is not set and so metrics are not reported for those requests. Below is an example route that you may want metrics for which currently doesn't work:

var proxy = require('express-http-proxy');
var app = require('express')();
 
app.use('/proxy', proxy('www.google.com'));

The request-received middleware at https://github.com/cabinjs/request-received exposes a Symbol.for property, which this package could conditionally consume if interested (as opposed to just generating a new Date().

e.g.

According to https://expressjs.com/en/4x/api.html#app.get the path parameter can be a string, regexp or array containing any of the above. It would appear that if the path is an array it will cause the following unhandled exception:

TypeError: str.replace is not a function
    at replacePipeChar (/home/stieg/Devel/airfordable/af-beta/node_modules/connect-datadog/lib/index.js:27:21)
    at getRoute (/home/stieg/Devel/airfordable/af-beta/node_modules/connect-datadog/lib/index.js:33:20)
    at ServerResponse.res.end (/home/stieg/Devel/airfordable/af-beta/node_modules/connect-datadog/lib/index.js:48:18)
    at ServerResponse.res.end (/home/stieg/Devel/airfordable/af-beta/node_modules/express-winston/index.js:316:17)
    at Gzip.onStreamEnd (/home/stieg/Devel/airfordable/af-beta/node_modules/compression/index.js:212:14)
    at Gzip.emit (events.js:214:15)
    at Gzip.EventEmitter.emit (domain.js:476:20)
    at endReadableNT (_stream_readable.js:1178:12)
    at processTicksAndRejections (internal/process/task_queues.js:80:21)

Hi !
I've notived that localhost is hardcoded
let datadog = options.dogstatsd || new hotShots.StatsD("localhost", 8125);

I can use the options.dogstatsd to create myself a new object with correct value but it may be easier for developpers to config host and port directly.

I've just pushed a PR which does it.
This is backward-compatible.

Since Express 4.x it has been valid syntax to use an array for a path, ie app.get(['/','/login'], ...). Since the pipe character bug fix was added in 0.0.7 this results in the following bug when the connect-datadog middleware gets called:

TypeError: str.replace is not a function
    at replacePipeChar (/usr/src/app/node_modules/connect-datadog/lib/index.js:27:21)
    at ServerResponse.res.end (/usr/src/app/node_modules/connect-datadog/lib/index.js:46:39)
    at ServerResponse.send (/usr/src/app/node_modules/express/lib/response.js:221:10)

It fails in the replacePipeChar function because it assumes that req.route.path will be a string, but it is actually an an array.

Docs says the statsd lib is node-dogstatsd but I couldn't find the options I was looking for so I had a look in the code https://github.com/DataDog/node-connect-datadog/blob/master/lib/index.js#L4 and realized the doc was misleading: the statsd client is hot-shots.

Moreover typing definitions seems wrong:

I get the typescript error: Type 'StatsD' is missing the following properties from type 'StatsDClient': incrementBy, decrementBy

because the expected type comes from node-dogstatsd: import { StatsDClient } from 'node-dogstatsd'

With connect-datadog 0.0.9 it is not possible to add route specific tags. I would like to do so to associate routes to a SLO domain with a tag: domain:my-domain.

I don't see an elegant way to do so. One way would be to adjust this line to read tags that could have been added in middlewares defined after datadog middleware:

let statTags = [...tags, ...req.ddTags]

Would you merge such a PR?

In the Readme you state that:

[DEPRECATED] Datadog middleware for Connect JS / Express

Yet you do not talk about the newer alternatives. What is the current alternative to datadog-connect?