go-gin
go-gin copied to clipboard
fix panic risk in func Middleware
In func Middleware, c.Next
would be panic, then span.Finish will not be executed.
//...
return func(c *gin.Context) {
carrier := opentracing.HTTPHeadersCarrier(c.Request.Header)
ctx, _ := tr.Extract(opentracing.HTTPHeaders, carrier)
op := opts.opNameFunc(c.Request)
sp := tr.StartSpan(op, ext.RPCServerOption(ctx))
ext.HTTPMethod.Set(sp, c.Request.Method)
ext.HTTPUrl.Set(sp, opts.urlTagFunc(c.Request.URL))
opts.spanObserver(sp, c.Request)
// set component name, use "net/http" if caller does not specify
componentName := opts.componentName
if componentName == "" {
componentName = defaultComponentName
}
ext.Component.Set(sp, componentName)
c.Request = c.Request.WithContext(
opentracing.ContextWithSpan(c.Request.Context(), sp))
c.Next()
ext.HTTPStatusCode.Set(sp, uint16(c.Writer.Status()))
sp.Finish()
}
//...
@stuart-mclaren
It seems much more difficult to fix panic risk with gin comparing to stdlib(https://github.com/opentracing-contrib/go-stdlib/pull/38). As you pointed out, gin's Writer.Status()
will be 200 other than 500 when inner handler panicked. This is misleading, and not the same as golang's http implementation. Maybe I need to create an issue to gin's project first.
Dears, any progress on the status code?
Just FYI, my current fix is just to leave the status code as it is: https://github.com/polyrabbit/go-gin/commit/e7a519105e6f2aec8c1cfc001c43278f043ec007