search

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

appmetrics trace cannot handle function with more than 9 parameters #601

Closed yuecchen closed 4 years ago

yuecchen commented 5 years ago

Steps to recreate the problem:

  1. use acmeair or other tested application
  2. modify any method to have more than 9 parameters, e.g. I modify bookflights function in routes/index.js as following:
 module.bookflights = function(req, res, a,b,c,d,e,f,g,h) {
  1. add the following line in the first line of this app.js: 
    
var appmetrics=require('appmetrics');
appmetrics.enable('trace');
4. start the application

Error message is as following:

undefined:1 x = function(a,b,c,d,e,f,g,h,i,j) {return fn.apply(this,arguments);}; ^

ReferenceError: x is not defined at eval (eval at generateF (/root/Node/App/acmeair-demo/node_modules/appmetrics/probes/trace-probe.js:123:16), :1:3) at generateF (/root/Node/App/acmeair-demo/node_modules/appmetrics/probes/trace-probe.js:123:16) at instrument (/root/Node/App/acmeair-demo/node_modules/appmetrics/probes/trace-probe.js:220:18) at traceMethod (/root/Node/App/acmeair-demo/node_modules/appmetrics/probes/trace-probe.js:246:5) at instrumentMethods (/root/Node/App/acmeair-demo/node_modules/appmetrics/probes/trace-probe.js:277:9) at ret (/root/Node/App/acmeair-demo/node_modules/appmetrics/probes/trace-probe.js:51:9) at Object. (/root/Node/App/acmeair-demo/app.js:69:46) at Module._compile (module.js:653:30) at Object.Module._extensions..js (module.js:664:10) at Module.load (module.js:566:32)

mattcolegate commented 5 years ago

Hi @yuecchen , As you know, we have to return a function in https://github.com/RuntimeTools/appmetrics/blob/master/probes/trace-probe.js#L71 containing the exact number of arguments to the target function, and you are right, our current limit is nine. Can you please suggest a suitable upper limit for number of arguments to a function that you have in mind?

yuecchen commented 5 years ago

We currently have a requirement from customer to support a function with 12 parameters. No more parameters have been seen by now.

yuecchen commented 5 years ago

add some code for this fix, could you please help check and merge the pull request?

mattcolegate commented 4 years ago

@yuechen fix for this issue has now been merged