search

grafana / pyroscope-nodejs

Pyroscope NodeJS integration
Apache License 2.0
27 stars 21 forks source link

Use @datadog/pprof instead of the old, neglected pprof #31

Closed ben-xD closed 3 months ago

ben-xD commented 1 year ago

https://www.npmjs.com/package/@datadog/pprof (1,788,492 weekly downloads) is used a lot more than https://www.npmjs.com/package/pprof (142,535 weekly downloads).

It's also difficult to use the old pprof, because there are no binaries available compatible with newer versions of Node. Therefore, installing it will use node-gyp to build pprof, which requires python and make to be installed. @datadog/pprof really easily (within seconds).

Is it possible to switch to consider if @datadog/pprof is better, and switch to it?

korniltsev commented 1 year ago

there is a PR https://github.com/grafana/pyroscope-nodejs/pull/19

bzurkowski commented 3 months ago

@ben-xD @korniltsev Any progress on this matter? The linked PR only adds datadog/pprof as a dependency, while the original pprof is still present in package.json. Installing pyroscope-nodejs in the app still requires heavy dependencies (python, gcc, make) for higher Node.js versions.

korniltsev commented 3 months ago

The PR was merged and released. Does it work with the latest release?

bzurkowski commented 3 months ago

It does not work for me. The pprof dependency is still there. When building the app with pyroscope-nodejs in the latest version, I get errors in CI related to missing make tool - it tries building the pprof dependency.

simonswine commented 3 months ago

As of v0.3.8 we should no longer use the pprof dependency now

simonswine commented 3 months ago

@bzurkowski Thanks for reporting this, if you find more problems, please let us know.