statful / statful-client-nodejs Goto Github PK

There are some improvements we need to do on the client:

• Make validator to not allow that flushInterval < timeout.
• Tests for system stats, blocked, and default config values.
• Verify merge order between global, global defaults and method parameters.
• Add start e stop methods to the client. The stop method should assure that all metrics in buffer are sent before client closes.
• Create a constants file.
• Remove close from prototype.
• Move some helper request functions into a request class.

One use case that seems to become a pattern in expressjs applications is to measure the interval between receiving and responding to a request.

This pattern can be abstracted with a simple middleware function similar to:

function timer(start = new Date()) {
  return (end = new Date()) => (end - start);
}

module.exports = (client) => (req, res, next) => {
  req.__timer = timer();
  res.on('finish', () => client.timer('response_time', req.__timer()));
  next();
};

And could be used in a similar manner to:

const Statful = require('statful');
const statfulMiddleware = require('statful/middleware')
const statfulClient = new Statful(/*config*/);
const app = express();

app.use(statfulMiddlware(statfulClient))

I can start a PR with this, but I was wondering if it's desirable to add this directly to the statful client, or if it should be in a separate project.

There's a memory leak when systemStats are set to true. The function "sendFlushStats" puts the system stats metrics in the buffers, the "flush" method is called if the buffers are full and the "sendFlushStats" is called again. Because of that there's a loop every time the buffers are at the "maximum capacity" before sending the metrics.

flush is not exposed and close only closes the socket. This means that when exiting, buffered metrics might not be sent.

My proposal is to expose flush and also flush when close is invoked.

I might prepare a PR for this.