grafana / pyroscope-nodejs

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

Express middleware starts heap profiling but does not stop it #15

Closed lizthegrey closed 2 years ago

lizthegrey commented 2 years ago

if, for instance, in a testsuite, app() entry point is called once for each testcase, a new express is provisioned (correctly), and middleware/handlers attempt to register each time.

but the pyroscope middleware is not idempotent -- if heap profiling is already on, it'll attempt to enable it again and crash. better to either register a shutdown hook, or to not attempt to enable heap profiling a second time if called twice in same nodejs runtime.

lizthegrey commented 2 years ago

Thanks @shaleynikov. One more test to add, if you wouldn't mind (but I'm confident your fix does fix our testsuite problem which is this one):

        const app = express();
        app.use(expressMiddleware());
        const app2 = express();
        app2.use(expressMiddleware());
shaleynikov commented 2 years ago

@lizthegrey added!