Adding foundation for dtrace probe support #959
Conversation
| @@ -0,0 +1,44 @@ | |||
| // Load modules | |||
|
|
|||
| var DTraceProvider = require('dtrace-provider'); | |||
There was a problem hiding this comment.
Should use conditional load like Restify:
https://github.com/mcavage/node-restify/blob/master/lib/dtrace.js
|
Looks fine for a first pass. |
|
Related: #972 |
|
I'm not sure this is sufficiently abstracted to not have any performance impact when dtrace is not used. Needs to turn the exported interface into empty functions so v8 will inline them everywhere called. |
| @@ -436,11 +436,13 @@ internals.Request.bindPre = function (pre) { | |||
| request.pre[pre.assign] = result; | |||
| } | |||
|
|
|||
| request.server._dtrace['pre.end'](pre.assign, result); | |||
There was a problem hiding this comment.
I want this to become an empty operation. It still makes a function call with some logic.
There was a problem hiding this comment.
Currently it will end up calling function () {} (https://github.com/spumko/hapi/pull/959/files#L0R37)
Would you rather this becomes a conditional call:
request.server._dtrace && request.server._dtrace['pre.end'](pre.assign, result);
where _dtrace is undefined when dtrace-provider is not installed?
There was a problem hiding this comment.
More like:
request.server._dtrace.report('pre.end', pre.assign, result);Where report() is an empty function when not enabled.
| }; | ||
|
|
||
|
|
||
| internals.DTrace.prototype.report = function (key, args) { |
There was a problem hiding this comment.
args is?? If I have to build an array or object, that's bad.
| internals.DTrace.prototype.report = function (key/* arg1, arg2 */) { | ||
|
|
||
| var args = arguments; | ||
| this._probes[key].fire(function () { |
There was a problem hiding this comment.
Why not create probes on first use?
There was a problem hiding this comment.
They will only be created when dtrace-provider is installed. In which case, if we create a probe on first use then that may lead to additional overhead after the server has started, versus initializing during the startup.
There was a problem hiding this comment.
I don't like the idea of having to manage this list in one place, and every time we add a probe also register it here. How slow is creating probes? What about user-defined probes?
Adding foundation for dtrace probe support
Adding foundation for dtrace probe support
No description provided.