newrelic / node-newrelic

New Relic Node.js agent code base. Developers are welcome to create pull requests here, please see our contributing guidelines. For New Relic technical support, please go to http://support.newrelic.com.
Apache License 2.0
969 stars 398 forks source link

Consider adding framework support for ActionHero #121

Closed svperfecta closed 5 years ago

svperfecta commented 10 years ago

Hi Team -

We use ActionHero, a great framework in the spirit of Hapi.js and Restify to create apps for many of our customers, all of which also are or will become NewRelic customers by the time we're done with them :) In any case, I was wondering about the possibility of adding support for ActionHero to NewRelic. It's actually a fairly mature framework, and I think if someone could stub out the some basic support the community would continue to maintain it.

Here's a link: http://www.actionherojs.com

Also, the BDFL is @evantahler, who is absolutely awesome.

Cheers, Brian

othiym23 commented 10 years ago

Thanks for getting in touch! This is really a question for our product team, as they set priorities when it comes to framework support (and adding new instrumentation in general). The best way to let them know this is something worth supporting is to send a feature request to support@newrelic.com, and to encourage anyone else who wants ActionHero instrumentation to do the same. Even better is for everyone include info about the size of their deployments, because New Relic tries to optimize for bang for the buck when sorting out its roadmap (which is how hapi support got added so fast).

In the medium term, it should get easier for framework maintainers to add support for New Relic, because we will be adding a custom instrumentation API. That said, for most web frameworks we support today, there's really only two things we instrument, and one of those is optional:

  1. Mapping routes to transaction names. Any web framework that's built on the core http library and that uses middleware or life cycle plugins should already be able to use the New Relic API to set transaction / controller names by calling newrelic.setTransactionName or newrelic.setControllerName. First-class support requires us to convert that to monkeypatching so that it's transparent to framework users, but you can do it by hand today.
  2. Tracing errors raised during request processing. Depending on how comprehensive a framework tries to be about error-handling, this can range from doing nothing to a total MacGyver operation, which is why I consider it optional.

As you can probably tell by the length of this answer, I don't anticipate we'll get to ActionHero immediately, because we have a lot on our plates. That said, please do let us know if you're an Action Hero user and want to use New Relic. By emailing support@newrelic.com! Not by putting "+1" on this issue! The people who count won't see it!

Thanks!

othiym23 commented 10 years ago

I forgot a piece of instrumentation, which is fine because it can't really be handled using the current New Relic for Node API anyway:

svperfecta commented 10 years ago

Thanks for the comprehensive response!! This is enough for us do some of the work ourselves. We'll keep an eye on the upcoming api changes and would be happy to add AH support once its appropriate to do so!

On Mon, Mar 10, 2014 at 11:50 AM, Forrest L Norvell < notifications@github.com> wrote:

I forgot a piece of instrumentation, which is fine because it can't really be handled using the current New Relic for Node API anyway:

  • If a framework has built-in support for rendering templates, New Relic will wrap that, because template rendering in Node is comparatively veeeery sloooooooow, especially if you're using jade. This is something the custom instrumentation API will be able to handle, when it's available.

Reply to this email directly or view it on GitHubhttps://github.com/newrelic/node-newrelic/issues/121#issuecomment-37197639 .

evantahler commented 10 years ago

Hey, I'm the actionhero maintainer. Integrating with airbrake has been on my list for a while, and this issue was the kick I needed to try it out. I have to say it was REALLY EASY to do with @othiym23's great API (and comments above).

All you need is this initializer. (boom)

evantahler commented 10 years ago

And since actionhero doesn't render templates, I can just ignore https://github.com/newrelic/node-newrelic/issues/121#issuecomment-37197639 :D

evantahler commented 10 years ago

As a note, it looks like this will not work with non-http clients as well (raw socket, websocket, etc). It seems that the newrelic agent really requires an HTTP object to be present within the domain (?)

@othiym23 would you have any docs on how to stub a request-like object or pass one to newrelic directly? actionhero supports a number of transports (and allows users to make their own), but we always end up with this generic connection object.

othiym23 commented 10 years ago

Hey, Evan. Rad. I'm glad it was so easy for you to put something together! Like I said to @genexp, it's the product team's decision when support for a framework gets added to the core module, but that middleware looks like it does the trick (probably more straightforwardly than doing it automatically from inside the module).

To answer your last question, no, we don't have documentation like that. New Relic is pretty heavily biased towards transactional web services, and adding support for monitoring applications that don't follow that model is something that we're only doing as we figure out ways to do it without diluting our very strong (and somewhat opinionated) product focus. The big issue you would have were you to hand-instrument session-based performance data (like websockets or raw network connections) is that even if you were able to send us complete transaction traces, we don't really have a good way to display them at the moment (if you look in lib/transaction/tracer.js and lib/transaction/trace/segment.js you can see how the transaction trace gets built up – not too magic once you understand continuation-local-storage, but it's a small part of the overall picture). Even once we add custom instrumentation, it will still be intended for use in situations that look more transactional than session-like.

The part that we will be able to handle more easily in the not-too-distant future is reporting aggregated metrics like number of concurrent websockets, memory overhead, etc using a custom metrics API call. This is something the other New Relic language agents support, and so we already have the front-end support for it you'd need. We don't have a date set for when that will be ready, but it's fairly high on our list of priorities. Maybe that will be enough?

svperfecta commented 10 years ago

Thanks all. Wow, I was actually going to post something like this we put together. This is better, we forgot to hook the exceptionHandler.

On Tue, Mar 11, 2014 at 1:45 AM, Forrest L Norvell <notifications@github.com

wrote:

Hey, Evan. Rad. I'm glad it was so easy for you to put something together! Like I said to @genexp https://github.com/genexp, it's the product team's decision when support for a framework gets added to the core module, but that middleware looks like it does the trick (probably more straightforwardly than doing it automatically from inside the module).

To answer your last question, no, we don't have documentation like that. New Relic is pretty heavily biased towards transactional web services, and adding support for monitoring applications that don't follow that model is something that we're only doing as we figure out ways to do it without diluting our very strong (and somewhat opinionated) product focus. The big issue you would have were you to hand-instrument session-based performance data (like websockets or raw network connections) is that even if you were able to send us complete transaction traces, we don't really have a good way to display them at the moment (if you look in lib/transaction/tracer.js and lib/transaction/trace/segment.js you can see how the transaction trace gets built up - not too magic once you understand continuation-local-storagehttps://github.com/othiym23/continuation-local-storage, but it's a small part of the overall picture). Even once we add custom instrumentation, it will still be intended for use in situations that look more transactional than session-like.

The part that we will be able to handle more easily in the not-too-distant future is reporting aggregated metrics like number of concurrent websockets, memory overhead, etc using a custom metrics API call. This is something the other New Relic language agents support, and so we already have the front-end support for it you'd need. We don't have a date set for when that will be ready, but it's fairly high on our list of priorities. Maybe that will be enough?

Reply to this email directly or view it on GitHubhttps://github.com/newrelic/node-newrelic/issues/121#issuecomment-37265354 .

evantahler commented 10 years ago

Thanks everyone

I'll update my example to only try to trace web requests for now to be explicit.

While the aggregated metrics look cool, I guess what I'm looking for is a more generic way to stop and start a transaction manually. For "persistent" connections (websocket, etc), it's possible for the connection to ask the server to do many things at once.

In a general sense, I would like to be able to instrument one level above a controller to do something like this:

// somehow a connection is created in the server
var connection = {};

// connections can call "controllers"
connection.do = function(controller_name, action, callback){
  var self = this;
  var nr_trasnaction_id = newrelic.startTransaction(connection_id, controller_name, action);
  // do the stuff
  ControllerManager.do(self, controller_name, action, function(err){
    if(err){ newrelic.noticeError(err); }
    newrelic.endTransaction(nr_trasnaction_id); 
    callback();
  })
})

Either way, there's no rush for me on this, just a suggestion. Thanks!