Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include DTrace probes #2191

Closed
palmerabollo opened this issue Jun 21, 2014 · 17 comments
Closed

Include DTrace probes #2191

palmerabollo opened this issue Jun 21, 2014 · 17 comments

Comments

@palmerabollo
Copy link

It would be interesting to include a DTrace provider and some probes to express, in a similar way as it is already implemented in other frameworks such as resitfy:

  • 'route-start': // server_name, route_name, id, method, url, headers (json)
  • 'handler-start': // server_name, route_name, handler_name, id
  • 'handler-done': // server_name, route_name, handler_name, id
  • 'route-done': // server_name, route_name, id, statusCode, headers (json)

Express users could take advantage of these probes in DTrace-enabled operating systems (OSX, Solaris, SmartOS, Oracle Linux, BSD...).

Are you open to contributions in this line?

@dougwilson
Copy link
Contributor

I am OK with it, as long as it is not tied to DTrace. I would like to be able to use StatsD with the probes. Probably make it just fire events an people can hook up their tracer if choice to it.

@jonathanong
Copy link
Member

something like this: https://github.com/koajs/trace

@dougwilson
Copy link
Contributor

@jonathanong pretty much exactly that :D

@palmerabollo
Copy link
Author

It's a good idea. I don't see how koajs/trace chooses the tracer you want to use. Just to confirm, after require('koa-trace')(app) all you need to do is something like this:

app.instrument(function myHandler() {
    ... do whatever I want, e.g. fire a dtrace probe
})

@thomasfr
Copy link

would be great to have koajs/trace funcitonality in express
👍

@frankrousseau
Copy link

Hello there,

I'm interested in contributing to Express. From what I understand of your discussion, providing a tracing feature to Express consist in porting the koa trace module to Express. The module code doesn't look big. So it's probably something I will have time to handle. Is it still a feature you are interested in? Do you think I can handle it while being new at Express contributing?

@dougwilson
Copy link
Contributor

Is it still a feature you are interested in?

Yes, 100% :)

Do you think I can handle it while being new at Express contributing?

I don't see why not :) Especially if you're open to feedback on a potential PR.

@frankrousseau
Copy link

Ok so how do you recommend to develop it: should I make a separate module like the koa one or should I contribute directly to the Express platform?

@dougwilson
Copy link
Contributor

There is always the potential of an additional module, but right now, there is no way to hook into what is happening inside Express, so providing a general hook into the operations of the internals is the first step to be done :)

@Fishrock123
Copy link
Contributor

Doesn't debug already do this?

@frankrousseau
Copy link

Yep, should we include that in your debugging mode?
http://expressjs.com/guide/debugging.html

@frankrousseau
Copy link

Here is how I see things. I would like to be sure I'm correct before going further.

Goal

The main goal is to allow to trace the response life-cycle. Basically, the job of the http server is to build and return a response from a given request and eventually run external operation. The server itself is already "traced" through your debug mode as @Fishrock123 mention it.

Implementation

Usage example (port of the Koa example)

What do you think about adding a trace function to the response object?
That function would do nothing if the DEBUG environment flag is not set. If the flag is set, that function would fire listeners previously set at the applicaton level via an instrument function.

Listeners could be called with the same signature as in Koa-trace by just replacing context with res (res has pointers to the req and the app object).

@dougwilson
Copy link
Contributor

That function would do nothing if the DEBUG environment flag is not set

This tracing stuff needs to be always enabled and not require an environment variable to be set.

Listeners could be called with the same signature as in Koa-trace by just replacing context with res

It does, but there is the possibility people can alter those values. I would replace context with app, req, res personally.

@frankrousseau
Copy link

Thank you @dougwilson for the feedback. I will work on it this week.

@dougwilson
Copy link
Contributor

No problem! This would be a lovely contribution :)

@frankrousseau
Copy link

I published a module similar to koa-trace. We can work on it to match the needs of this issue and identify if some extra work is needed in the internal of express. I would be glad to have your feedback to improve this module:

https://github.com/frankrousseau/express-tracer

@dougwilson
Copy link
Contributor

I'm going to close this issue as generally stale, due to the age and lack of continued conversation. I see this was even a module published, which us great! If that is not the actual end solution and it is desired, this can be reopened for further discussion of course. It's been open, but hasn't materialized any sort of code contribution so far, so I'm not sure it needs to remain open indefinitely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants