shipharbor / merry

:ocean::ocean::sailboat::ocean::ocean: - cute streaming API framework
MIT License
312 stars 21 forks source link

add APM API #47

Open yoshuawuyts opened 7 years ago

yoshuawuyts commented 7 years ago

It'd be cool if we could plug in opbeat and the like without too much hassle. I'm quite convinced we can do without a lot of the monkey patching their default package has because we use streams + already catch all uncaught errors - having a proper plugin system would allow for seamless integration with monitoring providers (:

cc/ @watson @roncohen - thoughts? This'd be super neat to have I think (:

roncohen commented 7 years ago

I'll let @watson comment on the details, but I think it would be relatively easy to do.

If route names were available to middlewares, it could be as simple as a one-line middleware that calls this: https://opbeat.com/docs/articles/opbeat-for-nodejs-api/#settransactionname

lrlna commented 7 years ago

If route names were available to middlewares

hell ya, req.url should get that for us and we would be all set; nice!

watson commented 7 years ago

All Opbeat need to know is the name of the route (with name I mean /user/:id as opposed to /user/42).

So if you expose the route name in some easy to get way then we can use that.

What we usually find that we need to do is hook into the framework to get access to the matched route name in some way. Here's an example of how we do this with the koa-router module: https://github.com/opbeat/opbeat-node/blob/master/lib/instrumentation/modules/koa-router.js

watson commented 7 years ago

I've been looking over merry, server-router and wayfarer now and can't seem to find a place where this string is accessible. If it ins't, would you consider exposing it in someway? Beside being used by Opbeat, this would be convenient to have for logging purposes as well.

lrlna commented 7 years ago

can't seem to find a place where this string is accessible

@watson you mean the name of the route? if so, it's currently not being exposed, but def considering it doing it. and good point on having it there for logging purposes!

On Sat, 14 Jan 2017 at 21:08 Thomas Watson Steen notifications@github.com wrote:

I've been looking over merry, server-router and wayfarer now and can't seem to find a place where this string is accessible. If it ins't, would you consider exposing it in someway? Beside being used by Opbeat, this would be convenient to have for logging purposes as well.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/yoshuawuyts/merry/issues/47#issuecomment-272653284, or mute the thread https://github.com/notifications/unsubscribe-auth/AHu3CG6-JgfWBsZTwOpoJwjQRKOHiPbXks5rSTligaJpZM4LjpSd .

watson commented 7 years ago

Yeah, the route name. If that could be exposed we'd have full support 😄 Here's an explanation: https://opbeat.com/docs/articles/get-started-with-a-custom-nodejs-stack/#route-naming

watson commented 7 years ago

I guess the Trie.prototype.match function needed to be modified. But I only spend a few minutes looking over it, so I'm not sure if there's a better spot

yoshuawuyts commented 7 years ago

Reference material for opbeat support from scratch:

yoshuawuyts commented 7 years ago

MongoDB exposes an APM API; we should do something similar so instrumentation vendors have a better time instrumenting merry apps! :tada: