xlts-dev / verdaccio-prometheus-middleware Goto Github PK

We should publish this package to the public npm registry. @Splaktar I was thinking of setting up CI to publish this package on tag creation. I was planning on following this and this article.

Additionally, we could leverage semantic-release to automatically increment version numbers using our CI which could then trigger the publish workflow. There is a good article on how to set this up with CircleCI here. If we go this route, I'm assuming you would want to stick with the default AngularJS commit message inspired format. If we went this route I think it would make sense to tackle #5 first and then leverage conventional-changelog/commitlint to ensure our commit messages follow the standard.

Let me know your thoughts.

We should License under MIT with Copyright XLTS.dev

An option should be added that allows for collecting metrics on all incoming HTTP requests. It should provide an exclusion configuration that allows excluding certain paths (e.g. /favicon.ico, web ui paths, etc.). The metric name should be named different then the metric name used to track tarball downloads.

prettier has been added as a dependency but the necessary config and integration with eslint was never completed.

I started doing real world testing against a Verdaccio instance running in a Docker container that uses this plugin and discovered that the metrics being collected were not 100% accurate. The counts of package downloads were always slightly less then what I was expecting based on the tests I was running.

I ran and debugged a local instance and performed the same tests that were producing inaccurate results. What I discovered is that the Express response finish event does not always fire. The plugin currently listens for the finish event in order to collect final response data and label metrics appropriately. You can see the line of code for this here.

There isn't a lot of documentation available on when the different Node/Express response event types fire. After doing some additional research, I found two good references:

• Node.js: Difference between http finish event, response close event, and response end event
• Measuring requests duration in a Node.js + Express application properly

Specifically from the 2nd link above:

When does the request-response cycle end? The first and the obvious answer is when the response is sent. Node.js has a default event that is fired when the response has been sent. This hook is res.on('finish').

99% of software developers use only “finish” event, but I should notice that not every request comes to the “finish” state, in the real life client can close the connection before the response has been sent. In this case Node.js emits only the res.on('close') event.

This is exactly the type of behavior I was seeing in my testing and debugging. It seems that sometimes the npm client kills the connection prior to Express thinking the full response has been sent. The solution seems to be to rely on the close event instead of the finish event on the response object.

This project was based on a template that Verdaccio provides. See here and here. Note that the repo in the second link is archived. All of the repos related to Verdaccio 5.x have been archived by the maintainers at this point even though version 6.x is not stable and has not been released.

Because of this both Babel and Typescript are used. We should move to using just TypeScript since using Babel is mostly redundant. See comment here from the first PR.

@Splaktar - do you want to address the glob-parent vulnerability before we make this public? I'm of the mindset of dismissing this alert for now... see reasons below:

1. The glob-parent dependency is a transitive dependency from a development dependency: @verdaccio/[email protected]>@babel/[email protected]>[email protected]>[email protected].
2. It's a regular expression denial of service vulnerability but since this is not a production dependency the code path can never be invoked from the plugin. It's extremely unlikely to ever be invoked even during transpiling when building.

Addressing this related issue I just opened would resolve the glob-parent vulnerability.

The prom-client package used to collect Prometheus compatible metrics has the ability to collect default metrics. We should add a corresponding configuration option and code to allow these default metrics to be collected and exposed. This could be useful when Verdaccio is not running in an environment that already handles collecting CPU/Memory related metrics.