Skip to content

Commit

Permalink
use URL path instead of handler
Browse files Browse the repository at this point in the history
  • Loading branch information
Zachary Sais committed Aug 1, 2017
1 parent 18dc60e commit 94faa35
Showing 1 changed file with 1 addition and 2 deletions.
3 changes: 1 addition & 2 deletions middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,6 @@ func (p *Prometheus) handlerFunc() gin.HandlerFunc {
}

start := time.Now()

reqSz := make(chan int)
go computeApproximateRequestSize(c.Request, reqSz)

Expand All @@ -124,7 +123,7 @@ func (p *Prometheus) handlerFunc() gin.HandlerFunc {
resSz := float64(c.Writer.Size())

p.reqDur.Observe(elapsed)
p.reqCnt.WithLabelValues(status, c.Request.Method, c.HandlerName()).Inc()
p.reqCnt.WithLabelValues(status, c.Request.Method, c.Request.URL.Path).Inc()
p.reqSz.Observe(float64(<-reqSz))
p.resSz.Observe(resSz)
}
Expand Down

2 comments on commit 94faa35

@pmoncadaisla
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @zsais,

I've watched this commit, and I'm not sure it is a good idea to include the URL path as a prometheus label, I'll explain why:

If you have URLs paths like:

  • /page1
  • /page2

this would be perfect, but if you have URL paths with path params like:

  • /page1/:param1/action/:param2

Every param combination will result in different URL paths, and therefore, different prometheus labels. This would make harder to group when quering prometheus.

What is your opinion?

Regards,

@zsais
Copy link
Owner

@zsais zsais commented on 94faa35 Aug 2, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @pmoncadaisla,

Thanks for your comments! You're definitely right, once the URL path has params we lose the ability to make consistent queries. I'd initially had c.HandlerName() but wasn't a fan of the way it was getting stored. Any ideas on how to get the proper handler or base URL path written? I can revert back to c.HandlerName() for the time being.

Please sign in to comment.