fastify / under-pressure Goto Github PK

🚀 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.

🚀 Feature Proposal

Node v14.10.0 shipped a more precise way to check on the event loop utilization:
https://nodejs.org/api/perf_hooks.html#perf_hooks_performance_eventlooputilization_utilization1_utilization2

This module should use that if it is available.

🚀 Feature Proposal

Custom error thrown & exported from lib. Allow to override Error instance.

Motivation

It's hard to distinguish if error comes from plugin, the only way is to check string message.

Example

In fastify error handler

const { UnderPressureError }  = require('under-pressure'); 

const errorHandler = async (err, req, reply) => {
    if (err instanceof UnderPressureError) {
        return sendCustomError(reply, 'Service is under pressure.', E_10000_GENERIC, SERVICE_UNAVAILABLE);
    }
}

🐛 Bug Report

When attempting to change the log level of the status route, the option is not passed along when under-pressure builds the route.

To Reproduce

Steps to reproduce the behavior:

fastify.register(require('under-pressure'), {
      maxEventLoopDelay: 1000,
      maxHeapUsedBytes: 100000000,
      maxRssBytes: 100000000,
      exposeStatusRoute: true,
      logLevel: 'silent'
    });

When hitting the ./status route I can still see the request and response logs, which will clutter up the log files.

Expected behavior

when setting options for the plugin, they should be forwarded to the route registration in under-pressure.

Your Environment

• node version: 8
• fastify version: ^2.10.0
• under-pressure version: ^3.1.0

I'm happy to submit a PR to resolve the issue if required.

The devDependency tap was updated from 12.5.2 to 12.5.3.

🚨 View failing branch.

This version is covered by your current version range and after updating it in your project the build failed.

tap is a devDependency of this project. It might not break your production code or affect downstream projects, but probably breaks your build or test tools, which may prevent deploying or publishing.

Your Greenkeeper Bot 🌴

Version 10.7.3 of tap was just published.

This version is covered by your current version range and after updating it in your project the build failed.

tap is a devDependency of this project. It might not break your production code or affect downstream projects, but probably breaks your build or test tools, which may prevent deploying or publishing.

Your Greenkeeper Bot 🌴

Prerequisites

• I have written a descriptive issue title
• I have searched existing issues to ensure the feature has not already been requested

🚀 Feature Proposal

In some cases you do not want to trigger the "Service Unavailable" 503 error, a very good example would be the Swagger documentation page.

Meaning there should be an option for some kind of whitelist.

Motivation

Some pages or routes shouldn't be blocked, for example I would like to keep showing the Swagger Docs page, even if the healthCheck fails for instance.

Example

Just an proto-type idea for such an interface by introducing an option like routeWhitelist:

fastify.register(require('@fastify/under-pressure'), {
  maxEventLoopDelay: 1000,
  routeWhitelist: ['/docs'],
})

An alternative solution is to provide a config option to the route config settings for under-pressure. Just like you can set a log level per route, you should be able to "ignore" the route from under-pressure plugin.

Prerequisites

• I have written a descriptive issue title
• I have searched existing issues to ensure the feature has not already been requested

🚀 Feature Proposal

Expand the plugin's configuration options to allow the Fastify route option for exposeHeadRoute to be set.

Happy to do the work for this if you think it's a good idea.

Motivation

Kubernetes needs an endpoint that responds with a 2XX or 3XX HTTP status code to check the health of the deployed service (ref). I think it would be great to be able to re-use the under pressure endpoint for this functionality but currently this isn't possible as the Kubernetes check is a HEAD request.

Example

fastify.register(require('under-pressure'), {
  maxEventLoopDelay: 1000,
  exposeStatusRoute: {
    routeOpts: {
      exposeHeadRoute: true,
      logLevel: 'debug',
      config: {
        someAttr: 'value'
      }
    },
    routeSchemaOpts: { // If you also want to set a custom route schema
      hide: true
    },
    url: '/alive' // If you also want to set a custom route path and pass options
  }
})

The devDependency fastify was updated from 2.0.0 to 2.0.1.

🚨 View failing branch.

This version is covered by your current version range and after updating it in your project the build failed.

fastify is a devDependency of this project. It might not break your production code or affect downstream projects, but probably breaks your build or test tools, which may prevent deploying or publishing.

Your Greenkeeper Bot 🌴

The devDependency tap was updated from 12.4.0 to 12.4.1.

🚨 View failing branch.

This version is covered by your current version range and after updating it in your project the build failed.

tap is a devDependency of this project. It might not break your production code or affect downstream projects, but probably breaks your build or test tools, which may prevent deploying or publishing.

Your Greenkeeper Bot 🌴

Example

fastify.register(async function (fastify) {
fastify.register(underPressure, {

  maxEventLoopDelay: fastify.config.MAX_EVENT_LOOP_DELAY,
  maxHeapUsedBytes: fastify.config.MAX_HEAP_USED_BYTES,
  maxRssBytes: fastify.config.MAX_RSS_BYTES,

  exposeStatusRoute: "/live",
});

Prerequisites

• I have written a descriptive issue title
• I have searched existing issues to ensure the bug has not already been reported

Fastify version

3.20.1

Plugin version

5.7.0

Node.js version

14.17.1

Operating system

macOS

Operating system version (i.e. 20.04, 11.3, 10)

10.14.6 (Mojave)

Description

In some cases, on very high load (or some time-consuming sync actions) the node's histogram.mean can be NaN and produce a false-negative condition that does not trigger under-pressure. In extreme cases, it can lead to node process being unresponsive.

Histogram {
  min: 9223372036854776000,
  max: 0,
  mean: NaN,
  exceeds: 0,
  stddev: NaN,
  percentiles: SafeMap(1) [Map] { 100 => 0 }
}

The min value however is always present and could be used in the comparison instead. The second option is to use Infinity to compare

Steps to Reproduce

Both scenarios were reproduced using autocannon:

a) Time consuming synchronous action used in fastify route handler, e.g.:

function sleep(ms) {
    const start = new Date().getTime();
    const expire = start + ms;
    while (new Date().getTime() < expire) { }
}

b) Big amount of connections to real-life service (500 or more in our case)

Expected Behavior

under-pressure being calculated properly and triggered

Prerequisites

• I have written a descriptive issue title
• I have searched existing issues to ensure the issue has not already been raised

Issue

Would be nice to release 4023ba6 and 13e9781. Installing latest pulled in @fastify/error@2 into the lockfile - all other @fastify/* deps in our project is using v3 😀

          I had a second look at it seems we have bugs in that part, before you even touched it.

When opts.exposeStatusRoute.routeSchemaOpts is in Object.assign before our changes, than it means we can not overwrite the error statuscodes.

The same here:

We could set a custom error handler like this:

    fastify.setErrorHandler((err, request, reply, done) => {
      reply.status(err.statusCode)
        .send({
          code: err.code,
          message: err.message
        })
      done()
    })

a potential unit test would be:

test('check error handler', async t => {
  t.plan(1)
  const fastify = Fastify({ exposeStatusRoute: true })

  fastify.register(underPressure, {
    healthCheck: () => {
      throw new Error('Arbitrary Error')
    },    
    exposeStatusRoute: {
      routeOpts: {
        logLevel: 'silent'
      }
    }
  })

  t.equal((await fastify.inject().get('/').end()).body, '{"code":"FST_UNDER_PRESSURE","message":"Service Unavailable"}')
})

Originally posted by @Uzlopak in #198 (comment)

Prerequisites

• I have written a descriptive issue title
• I have searched existing issues to ensure the bug has not already been reported

Fastify version

x.xx

Plugin version

x.xx

Node.js version

x.xx

Operating system

Linux

Operating system version (i.e. 20.04, 11.3, 10)

xxx

Description

Currently documentation states this:
///
Since eventLoopUtilization is only available in Node version 14.0.0 and 12.19.0 the check will be disabled in other versions.
///
I assume it actually works in 14.0.0+, not just that specific version, but documentation claims otherwise. It would be helpful to improve wording to make the intent clearer.

Steps to Reproduce

Documentation issue

Expected Behavior

///
Since eventLoopUtilization is only available in Node versions 12.19.0, 14.0.0, and newer, the check will be disabled in other versions.
///

I see you've already added under-pressure to the todo list for 2.0, but still: Do you expect anthing to break by just bumping the version requirement?

Dependabot encountered the following error when parsing your .dependabot/config.yml:

The property '#/update_configs/0/ignored_updates' of type null did not match the following type: array

Please update the config file to conform with Dependabot's specification using our docs and online validator.

Prerequisites

• I have written a descriptive issue title
• I have searched existing issues to ensure the issue has not already been raised

Issue

setInterval will repeatedly call at the scheduled time whether or not the previous call has finished. if the server is already under load, this will likely add to the problem, because those setInterval calls will start to stack. i imagine this would be especially bad in node v8 with setInterval(..., 5ms).

for node >= v10.2.0, using setTimeout and timer.refresh would avoid this issue. i swapped it in and tests pass in node v14, at least:

  const timer = setTimeout(() => {
    updateMemoryUsage()
    timer.refresh()
  }, sampleInterval)

Prerequisites

• I have written a descriptive issue title
• I have searched existing issues to ensure the issue has not already been raised

Issue

No response

See https://github.com/mafintosh/event-loop-delay.

Prerequisites

• I have written a descriptive issue title
• I have searched existing issues to ensure the issue has not already been raised

Issue

Is custom healthCheck called after checking under pressure?
If no - we need In addition to the Db and Redis state, to know if the service is under pressure in order to form the correct answer.

🚀 Feature Proposal

Allow healthCheck to return a custom response that will override the default { status: 'ok'} response.

Motivation

Sometime the "status" or "health" endpoints are used to return multiple informations about the system status: db connection, external services, etc. .

Example

In the above scenario, having just a { status: 'ok' } may not be enough.

ie:

{
  status: 'ok',
  db: 'ok',
  externalService: 'ko',
  externalService2: 'ok',
  ...
}

Prerequisites

• I have written a descriptive issue title
• I have searched existing issues to ensure the feature has not already been requested

🚀 Feature Proposal

Give the user the option to change the default state of the health-check to be healthy. Meaning in case of a new deployment, the server can be reached directly if needed, without waiting on the healthCheckInterval.

Motivation

I run the server with cluster support, and I want to increase the healthCheckInterval to reduce the load for health checks on my database. However, increasing the internal will cause 503 to be present for longer during start-up.

Example

Introducing a new option for example, like: healthCheckDefaultHealthy (default value is false to be backwards compatible):

{
       maxEventLoopDelay: 1000,
       healthCheckDefaultHealthy: true
}

The devDependency @types/node was updated from 12.12.8 to 12.12.9.

🚨 View failing branch.

This version is covered by your current version range and after updating it in your project the build failed.

@types/node is a devDependency of this project. It might not break your production code or affect downstream projects, but probably breaks your build or test tools, which may prevent deploying or publishing.

Your Greenkeeper Bot 🌴

Prerequisites

• I have written a descriptive issue title
• I have searched existing issues to ensure the bug has not already been reported

Fastify version

3.28.0

Plugin version

5.8.1

Node.js version

18.0.0

Operating system

Windows

Operating system version (i.e. 20.04, 11.3, 10)

10.‎06.‎2020

Description

.register(underPressurePligin, {
        exposeStatusRoute: {
            routeOpts: {
                logLevel: 'debug',
            },
            url: '/alive',
        },
        healthCheck: async function (fastifyInstance) {
            // do some magic to check if your db connection is healthy, etc...
            return true;
        },
    })

the contents of the log after executing the request

{
    "level": "info",
    "time": 1651439672804,
    "instance": "unknown:3000",
    "version": "1.0.0",
    "reqId": "req-1",
    "req": {
        "method": "GET",
        "url": "/alive",
        "path": "/alive",
        "parameters": {},
    },
    "msg": "incoming request"
}

log level is info instead of debug

Steps to Reproduce

setup server with under-pressure plugin with above config an run

Expected Behavior

expected loglevel for status route to be as defined in config

🐛 Bug Report

Breaking fastify server!

The main purpose of [under-pressure][1] plugin is to protect the server against the high load. I created a fake load using [autocannon][2] and the server suddenly starts throwing this error:

<--- JS stacktrace --->

==== JS stack trace =========================================

    0: ExitFrame [pc: 00007FF7D342A9AC]
Security context: 0x01b3e02408d1 <JSObject>
    1: _write [000001EBE7CBA7B1] [_stream_transform.js:~169] [pc=000002737CBBD07D](this=0x02fe03573099 <LineStream map = 00000149AA79E209>,0x00f819d83789 <Uint8Array map = 000002F9BBE3F279>,0x00c0496035d9 <String[#6]: buffer>,0x02fe03575639 <JSBoundFunction (BoundTargetFunction 000003C5D37BC781)>)        
    2: ondata [000002FE03577FC9] [_stream_readable.js:~712]...

FATAL ERROR: Ineffective mark-compacts near heap limit Allocation failed - JavaScript heap out of memory
 1: 00007FF7D28A363F napi_wrap+128063
 2: 00007FF7D2842836 v8::base::CPU::has_sse+35142
 3: 00007FF7D28434F6 v8::base::CPU::has_sse+38406
 4: 00007FF7D3059F4E v8::Isolate::ReportExternalAllocationLimitReached+94
 5: 00007FF7D3042021 v8::SharedArrayBuffer::Externalize+833
 6: 00007FF7D2F0E57C v8::internal::Heap::EphemeronKeyWriteBarrierFromCode+1436
 7: 00007FF7D2F197D0 v8::internal::Heap::ProtectUnprotectedMemoryChunks+1312
 8: 00007FF7D2F162F4 v8::internal::Heap::PageFlagsAreConsistent+3204
 9: 00007FF7D2F0BB13 v8::internal::Heap::CollectGarbage+1283
10: 00007FF7D2F0A184 v8::internal::Heap::AddRetainedMap+2452
11: 00007FF7D2F31E1F v8::internal::Factory::NewRawTwoByteString+95
12: 00007FF7D2F349CB v8::internal::Factory::NewStringFromUtf8+187
13: 00007FF7D305681A v8::String::NewFromUtf8+298
14: 00007FF7D27A9ABF node::tracing::TraceEventHelper::SetAgent+40751
15: 00007FF7D285A46D v8::internal::Malloced::operator delete+1661
16: 00007FF7D342A9AC v8::internal::SetupIsolateDelegate::SetupHeap+45852
17: 000002737CBBD07D

To Reproduce

Steps to reproduce the behavior:

fastify.register(require('under-pressure') , {
    maxEventLoopDelay: 0,
    maxHeapUsedBytes: 0,
    maxRssBytes: 0
})

console.log(fastify.memoryUsage())

Expected behavior

The very fastify plugin which is supposed to protect against load is breaking the fastify server, need some help with another plugin or some other way to implement load protection

Your Environment

• node version: v12.16.1
• fastify version: ^3.4.1
• os: Windows 10
• npm: 6.13.4

Cross posted on Stackoverflow: https://stackoverflow.com/q/64137079/2404470

The dependency fastify-plugin was updated from 1.4.0 to 1.5.0.

🚨 View failing branch.

This version is covered by your current version range and after updating it in your project the build failed.

fastify-plugin is a direct dependency of this project, and it is very likely causing it to break. If other packages depend on yours, this update is probably also breaking those in turn.

Your Greenkeeper Bot 🌴

The devDependency tap was updated from 12.5.0 to 12.5.1.

🚨 View failing branch.

This version is covered by your current version range and after updating it in your project the build failed.

tap is a devDependency of this project. It might not break your production code or affect downstream projects, but probably breaks your build or test tools, which may prevent deploying or publishing.

Your Greenkeeper Bot 🌴