fastify / under-pressure

Measure process load with automatic handling of "Service Unavailable" plugin for Fastify.
MIT License
337 stars 35 forks source link

Custom function for process metrics #81

Open dnlup opened 3 years ago

dnlup commented 3 years ago

🚀 Feature Proposal

Expose a custom function to gather process metrics.

Motivation

There are cases in which there's already a module/library that monitors process metrics, for example, to report to statsd/prometeus and similar platforms. This would allow reusing those modules for a similar task.

Example

fastify.register(require('under-pressure'), {
  maxEventLoopUtilization:0.98,
  // This custom function will be bound to the fastify instance
  usage() {
    const { eventLoopUtilization } = this.customMetrics
    return { eventLoopUtilization }
 }
})

fastify.get('/', (req, reply) => {
  reply.send({ hello: 'world'})
})

fastify.listen(3000, err => {
  if (err) throw err
  console.log(`server listening on ${fastify.server.address().port}`)
})

Notes

There are a lot of details to figure out here, like defining a way to make sure the object returned by the custom usage function is safe to use (i.e., it defines all the properties needed), but I just wanted to start a discussion to understand if it's something this plugin could provide.

mcollina commented 3 years ago

I think this would be really useful to provide. It should also be easy to publish the data from under-pressure to prometheus.

dnlup commented 3 years ago

In my view, I thought that when the user passes a custom usage function, the plugin will not perform sampling (i.e., not timer is set up) but just call the function when a request comes in to check that the metrics values are under the threshold.

const fastify = require('fastify')()
const doc = require('@dnlup/doc') // A module to sample process metrics

// This might be a plugin in the ecosystem
fastify.register(function metrics (instance, opts, next) {
  const sampler = doc(opts)
  instance.decorate('sampler', sampler)
  instance.decorate('metrics', {
    eventLoopUtilization: 0
  })
  sampler.on('sample', () => {
    instance.metrics.eventLoopUtilization = sampler.eventLoopUtilization.raw
  })
  next()
})

fastify.register(require('under-pressure'), {
  maxEventLoopUtilization:0.98,
  // This custom function will be bound to the fastify instance
  usage() {
    return this.metrics
 }
})

fastify.get('/', (req, reply) => {
  reply.send({ hello: 'world'})
})

fastify.listen(3000, err => {
  if (err) throw err
  console.log(`server listening on ${fastify.server.address().port}`)
})

I am not sure this is a nice DX, though, because I am not using the plugins' built-in dependencies check of fastify-plugin. I am also not sure how to publish metrics to prometeus in a setup like this one, but I agree this is something the plugin could do.

mcollina commented 3 years ago

It seems fair! My only concern is that under-pressure works specifically with a certain set of metrics. My making those optional, what would be left?

It might be better to figure out the prometheus integration and adapt this module accordingly.

dnlup commented 3 years ago

It seems fair! My only concern is that under-pressure works specifically with a certain set of metrics. My making those optional, what would be left?

Exactly, that was my concern also, but I don't see a way to validate the metrics returned at plugin-registration time atm. All that would be left is the user common sense, unfortunately.

zekth commented 3 years ago

What about providing a map to the under-pressure registration and this map would be updated in the fastify instance by the business cases. And this map would be consumed in the onRequest hook: https://github.com/fastify/under-pressure/blob/441c552d8af55657dcd15ac8b592273605b14dca/index.js#L156

dnlup commented 3 years ago

What about providing a map to the under-pressure registration and this map would be updated in the fastify instance by the business cases. And this map would be consumed in the onRequest hook:

https://github.com/fastify/under-pressure/blob/441c552d8af55657dcd15ac8b592273605b14dca/index.js#L156

Yes, that would work too. So, for example, something like this?


fastify.decorate('metrics', {
  eventLoopUtilization: 0,
  ....
})

fastify.register(require('under-pressure'), {
  maxEventLoopUtilization:0.98,
  metrics: fastify.metrics
})

fastify.get('/', (req, reply) => {
  reply.send({ hello: 'world'})
})

fastify.listen(3000, err => {
  if (err) throw err
  console.log(`server listening on ${fastify.server.address().port}`)
})

I think it might share the same problem of metrics definition, though.