riandyrn/otelchi

Use method in span name

Closed this issue · 8 comments

jcarter3 commented

Currently, the span name is set to the matching RoutePattern. If GET/POST/DELETE methods all apply to the same route, these all get lumped under the same name. Would it work to adjust this to include the method here or have you not ran into this issue?

riandyrn commented

Hello, @jcarter3

This feature has been discussed here.

In short, the span name used by otelchi following the convention used by other http handler middlewares for OpenTelemetry.

If you want to fetch the http request method you could get it from http.method attribute (it is being injected here). Tools like Jaeger or AWS X-Ray will also read from this attribute.

Or maybe your vendor tools doesn't provide this?

jcarter3 commented

Thank you for the context! We're using New Relic and they don't show the method either, just like elastic apparently doesn't. It's a pretty minor thing so we might just make a quick patch. Thanks!

riandyrn commented

Hmm.., I see..

This is quite interesting. I thought this problem is only specific to elastic.co. 😅

What do you think if I add option to add request method to the span name like this?

Does this solve your problem, @jcarter3?

cc: @tjefferson08

tjefferson08 commented

Normally, an application could override the span name by registering a middleware/handler downstream of the otelchi handler.

As I understand it, that's (uniquely) not possible with go-chi middleware due to the binding of RoutePattern occurring at response time.

A configuration option makes sense to me, although perhaps it could be made more generic? WithSpanNameSetter which takes a

func (w http.ResponseWriter, r *http.Request) (name string) {}
jcarter3 commented

@riandyrn I think that option would be fantastic! It would solve the problem and prevent some extra work on my end

  • 👍1
riandyrn commented

Thanks for the suggestion, @tjefferson08!

I still don't fully understand though, why do you suggest WithSpanNameSetter to accept func (w http.ResponseWriter, r *http.Request) (name string)?

Can you maybe create another PR that include example use case for this?

riandyrn commented

Ok, I have released v0.4.0 which contains this update, @jcarter3.

As for the case mentioned here, it could also be solved by setting WithChiRoutes() option. Thank you so much for sharing this use case @tjefferson08!

jcarter3 commented

@riandyrn This works as expected, thank you so much!

  • 👍1