SkyAPM / go2sky

Distributed tracing and monitor SDK in Go for Apache SkyWalking APM
https://skywalking.apache.org/
Apache License 2.0
448 stars 123 forks source link

routeMap key is handler name, if different url paths route to same handler, only the last one is available to trace, consider using Method+Path as the key? #59

Closed Logic0 closed 4 years ago

Logic0 commented 4 years ago

Is your feature request related to a problem? Please describe. A clear and concise description of what the problem is. Ex. I'm always frustrated when [...]

Describe the solution you'd like A clear and concise description of what you want to happen.

Describe alternatives you've considered A clear and concise description of any alternative solutions or features you've considered.

routeMap key is handler name, if different url paths route to same handler, only the last one is available to trace, consider using Path as the key to solve this? or any other solutions?

mm[r.Handler] = routeInfo{ operationName: fmt.Sprintf("/%s%s", r.Method, r.Path), }

described situation can be avoided by using different handlers for different urls, but it takes time to find why when it happens.

Additional context Add any other context or screenshots about the feature request here.

arugal commented 4 years ago

Hi @Logic0 Glad to receive your feedback. I don't quite understand what you said. Could you give me an example :)

Logic0 commented 4 years ago

if we have a handler like: func doSomething( c *gin.Context ) {...}

then register 2 handler:

eg = gin.New()
eg.Use( Middleware(eg, tracer) )
eg.POST( "/api/v1/m1", doSomething )
eg.POST( "/api/v1/m2", doSomething )

according to the code in go2sky gin plugin: mm[r.Handler] = routeInfo{ operationName: fmt.Sprintf("/%s%s", r.Method, r.Path), }

the mm will be:

mm['doSomething'] = "/POST/api/v1/m1"         // will be overwrited by m2
mm['doSomething'] = "/POST/api/v1/m2"

all the request from "/POST/api/v1/m1" will be recognized as "/POST/api/v1/m2", as it's overwrited.

hope this helps.

arugal commented 4 years ago

Thank you. It does happen. Let me see how to solve it.

arugal commented 4 years ago

Hi @Logic0, If use http.Request.Method + http.Request.URL.Path as the operation name, so /user/:name will generate multiple operation name, I will keep the current solution until there is a better solution.

wu-sheng commented 4 years ago

We did a discussion, this should be plugin FAQ.

wu-sheng commented 4 years ago

@Logic0 As you have known the logic behind this, I recommend using a method wrapper to make this conflict gone. I have talked with @arugal, my suggestion would like to add a FAQ document with an example.

arugal commented 4 years ago

Hi @Logic0 https://github.com/SkyAPM/go2sky-plugins/tree/master/gin/v3 We provide a new version, welcome to use.