contrib/zenazn/goji/web: add goji integration

[knusbaum]

This implements middleware for goji that will trace incoming requests.

Suggestions for improvements in the implementation are welcome.

Unfortunately it looks like out of the box, there's no easy way to get the route, so the resource will usually be just the method. If the customer uses the Router middleware, that puts the route in the context, which the tracer middleware then picks up and adds to the resource name.

[gbbr]

Thanks Kyle! Did you try this out with an actual dummy app and get results in Datadog?

I can't prove it, but the options and test files give me a copy/paste feeling. It's not wrong, but always makes me think if we can do something better to reduce the duplication of both of these (maybe in v2).

[knusbaum]

I can't prove it, but the options and test files give me a copy/paste feeling.

That is correct. This is largely lifted from our go-chi/chi integration.

Did you try this out with an actual dummy app and get results in Datadog?

Yes, I did quite a lot of testing with a dummy app trying to improve the middleware and get more detailed traces.

[...] always makes me think if we can do something better to reduce the duplication of both of these

This would be good to do. There are a few integrations that are similar to this (gin-gonic/gin, go-chi/chi), and I think that a lot of the web router frameworks out there support this style of tracing. At very least, we can possibly unify the options and maybe have some standard tests, because those are almost identical.

[gbbr]

Yes, I did quite a lot of testing with a dummy app trying to improve the middleware and get more detailed traces.

Excellent! 🎉

Left just one comment about a log message. Let me know if it doesn't make sense.

[knusbaum]

Thanks, @gbbr

I added a log message, but didn't mention goji.DefaultMux.Router specifically. Just the Router middleware. Each Mux has its own Router middleware, and the right one for the right router needs to be chosen, depending on how they are using our middleware.

It's totally reasonable to use our tracing middleware on a sub-Mux, in which case they need to use that Mux's Router, not the DefaultMux. I'm sorry, the situation is a bit confusing.

If you think I can word the warning better, I'm open for suggestions.

[gbbr]

I think your log message is perfect. Just made a small comment on naming. Many times I see sync.Once variable suffixed with *once where it makes sense. It makes sense here, I think...

[gbbr]

👍

[knusbaum]

@gbbr I just realized that goji has released v1.0.0. Do I need to suffix this integration directory name with v1, or are we only going to start doing that with tracer version 2?

[gbbr]

Damn. Great catch! I forgot about this. We definitely should. Otherwise we risk this becoming ambiguous later.

[gbbr]

LGTM

[gbbr]

👍