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

Not possible to set the span operation name to the matched route #10

Open
krosen040 opened this issue Mar 17, 2021 · 5 comments
Open

Not possible to set the span operation name to the matched route #10

krosen040 opened this issue Mar 17, 2021 · 5 comments

Comments

@krosen040
Copy link

We would like to be able to set the span operation name to the matched gin route, however currently this is difficult. For example, if there is a route /user/:name, and a request is made to /user/foo, we would like to set the span name to the former.

The default span operation name is HTTP + method, e.g. HTTP GET. It's possible to override this default using the OperationNameFunc option, which gives access to the http.Request object, however I don't think that can be used to get the matched route (it's obviously possible to get the path, but that could contain path parameters, which we want to avoid).

As a side note, the gorilla/mux Opentracing middleware implementation defaults to the matched route as the operation name, see https://github.com/opentracing-contrib/go-gorilla/blob/3ee496ae11d5bebfd34703cee4efa29e0439a33d/gorilla/server.go#L35, which I think makes more sense than just the HTTP method used.

@stuart-mclaren-hpe
Copy link

Is it possible to get from the request -> context -> FullPath()?

gin-gonic/gin#748
gin-gonic/gin#1826

@krosen040
Copy link
Author

FullPath() is what we want to use, however the Context() method on http.Request gives context.Context, not gin.Context, so it's not possible to do so.

@stuart-mclaren-hpe
Copy link

Ok, I understand.

Are you thinking about proposing a patch?

@krosen040
Copy link
Author

I'd like to, but I'm not sure what that would look like and still keep everything backwards compatible.

@stuart-mclaren-hpe
Copy link

Yeah, I agree backwards compatible is important.

A simple option would be to add a gin.fullpath Tag, the same way ext.HTTPUrl.Set sets the http.url Tag -- for example on line 88. But I'm not sure that's exactly what you want.

It would be possible to put gin.fullpath in the request context so it would be available in opNameFunc, eg via a function such as ginhttp.GetFullpath(r). But I'm not keen on duplicating stuff that's already in the gin context that the request context is a subset of; and it also feels a bit "loose".

We could potentially add a new, "extended" flavour of OperationNameFunc, eg OperationNameExtFunc which would have access to a limited set of attributes (eg fullpath) in addition to the request object.

func OperationNameExtFunc(f func(r *http.Request, attrs *Attributes) string) MWOption {

The original function could be left for backwards compatibility.
What do you think?

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

No branches or pull requests

2 participants