go-http-metrics
go-http-metrics copied to clipboard
Reduce HandlerID cardinality by default in gin middleware
Here's the doc for FullPath()
FullPath returns a matched route full path. For not found routes
returns an empty string.
router.GET("/user/:id", func(c *gin.Context) {
c.FullPath() == "/user/:id" // true
})
IMO it's better to do the correct thing by default when possible; existing users would benefit from performance increases but it may break some dashboards.
Let me know what you think, Thanks!
Hi @arwineap!
Thanks for the contribution, You are right, safety should be a default. However, this will have unexpected behaviour for users at this moment, and that could be worse. Maybe would be better to have a flag on middleware creation and break the API so when people upgrade, they are aware of this safer behavior change that will be by default. e.g:
func Handler(handlerID string, m middleware.Middleware, rawPaths bool) gin.HandlerFunc
Let me think about it!
Just a bump on this as I found myself wondering why there wasn't an option for this, so I ended up just implementing it myself locally.
+1 for FullPath()
@arwineap @ImVexed Ok, let's do this, add an option on middleware creation so it breaks the API and users are aware of this change. Similar to what is above, by default would use FullPath and if the flag of RawPaths is used then, we use the current way of measuring it.
WDYT?
I'm doing something similar for gorestful in #150. Instead of adding a rawPaths bool to Handler, why not consider adding a Config object specific for gin so that we can easily add more options later if that ends up being important.
In #150, I also introduced HandlerWithConfig and left Handler alone (Handler would then be backward compatible and just call HandlerWithConfig with an empty Config{}).