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
970 stars 125 forks source link

incompatible with socket.io@3 and @4 - Exception thrown "TypeError: Cannot read property 'on' of undefined" #649

Open frostmar opened 3 years ago

frostmar commented 3 years ago

The socket.io probe https://github.com/RuntimeTools/appmetrics/blob/master/probes/socketio-probe.js works fine with socket.io@2

However socket.io was rewritten for it's v3 and v4. The socket.io interface has changed slightly, causing the following exception to be thrown when socket.io is require'd and the probe attempts to hook in:

FATAL Node server exiting due to exception: TypeError: Cannot read property 'on' of undefined
                        at C:\firefly\repos\firefly-ui\node_modules\appmetrics\lib\aspect.js:56:26
                        at Array.forEach (<anonymous>)
                        at Object.exports.before (C:\firefly\repos\firefly-ui\node_modules\appmetrics\lib\aspect.js:55:9)
                        at SocketioProbe.attach (C:\firefly\repos\firefly-ui\node_modules\appmetrics\probes\socketio-probe.js:79:12)

I've done a little debugging and can see:

frostmar commented 3 years ago

Until this is fixed, is there any way to prevent the socket.io probe from installing? We only use data from other probes.

mattcolegate commented 3 years ago

Does https://github.com/RuntimeTools/appmetrics#appmetricsdisabletype work?

frostmar commented 3 years ago

appMetrics.disable('socketio') doesn't help. It looks to me that all probes are installed very early, by the appmetrics index.js, and whether a probe is disabled doesn't affect it attempting to hook in during the require of the target library.

Our code is approximately:

    appMetrics = require('appmetrics')
    appMetrics.configure({ mqtt: 'off' })
    const appMetricsMonitor = appMetrics.monitor()
    appMetrics.disable('socketio')

    // ... later ...
    require('socket.io')    
EMJzero commented 3 years ago

Same issue, I need socket.io v3, and that impedes me to use appmetrics-dash due to this issue with appmatrics. I used it in the past, before using v3, and I'd like to continue... Has this problem been acknowledged by the devs?

streboryaj commented 2 years ago

If you require socket.io BEFORE appmetrics then it will not be instrumented by appmetrics regardless of the later disable call.