RuntimeTools / appmetrics

Node Application Metrics provides a foundational infrastructure for collecting resource and performance monitoring data for Node.js-based applications.
https://developer.ibm.com/open/node-application-metrics/
Apache License 2.0
976 stars 127 forks source link

appmetrics should be pre-loadable #434

Open sam-github opened 7 years ago

sam-github commented 7 years ago

It should be possible to both require appmetrics, and cause .monitor() to be called with a one-shot from the node command line. Perhaps node -r appmetrics/start?

This will interact well with https://github.com/nodejs/node/pull/12028 when it lands, and be consistent with https://github.com/nodejs/node-report#usage (though node-report privileges the -r use-case by having the main require auto-start, and making people use node-report/api to get the API with no auto-starting, which is OK, different use-case).

cc: @rnchamberlain @mhdawson

tobespc commented 7 years ago

we already have a version of this with "node_hc" although how much its used I don't know.

gibfahn commented 7 years ago

The neat thing about this would be that you could autostart from a local install, so you could do

npm i --save appmetrics
export NODE_OPTIONS="-r appmetrics/start" # Requires nodejs/node#12028
npm start

and you'd have appmetrics working without having to globally install it, so it'd be portable.

Thinking about it though, it's probably more useful for appmetrics-dash. Does that already work? Can you do node -r appmetrics-dash/monitor test.js or node -r appmetrics-dash/attach test.js?

sam-github commented 7 years ago

node_hc is a different use-case.

With a requireable entrypoint I could (soon) do NODE_OPTIONS="-r /my/appmetrics/start" npm start in a Dockerfile, node_hc requires rewriting the package.json for the app.

sjanuary commented 7 years ago

So this would need a new javascript file called 'start.js' or 'autostart.js' and then -r appmetrics/start or -r appmetrics/autostart would cause that file to be invoked. New file would simply need to require appmetrics and call start on it.

sjanuary commented 7 years ago

No reason why we couldn't do this for appmetrics-dash too, there are valid use cases for both.

sjanuary commented 7 years ago

Implemented here but still needs tests and documenting, so it's a WIP at the moment.