Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom function for process metrics #81

Open
dnlup opened this issue Jan 8, 2021 · 6 comments
Open

Custom function for process metrics #81

dnlup opened this issue Jan 8, 2021 · 6 comments

Comments

@dnlup
Copy link
Contributor

dnlup commented Jan 8, 2021

🚀 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
Copy link
Member

mcollina commented Jan 9, 2021

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

@dnlup
Copy link
Contributor Author

dnlup commented Jan 10, 2021

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
Copy link
Member

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
Copy link
Contributor Author

dnlup commented Jan 10, 2021

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
Copy link
Member

zekth commented Jan 10, 2021

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:

function onRequest (req, reply, next) {

@dnlup
Copy link
Contributor Author

dnlup commented Jan 11, 2021

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:

function onRequest (req, reply, next) {

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants