contrib icon indicating copy to clipboard operation
contrib copied to clipboard

added StatsD middleware, for reporting metrics to a StatsD agent.

Open oryband opened this issue 8 years ago • 14 comments

  • currently supported metrics:
    • request throughput count
    • response time
    • status code count
    • successful requests count
    • erroneous requests count
      • can assign different buckets to error types.
  • using functional options, for easier future additions. see the following article for more info: http://dave.cheney.net/2014/10/17/functional-options-for-friendly-apis
  • accepts a statsd client interface instead of a specific package implementation. this practically means that any statsd client package can be used, as long as it supports Incr() and Timing(). if not, the developer can wrap its client to comply with the interace. i wrote this middleware to be used with the most popular package: github.com/quipo/statsd
  • configuration options:
    • per-metric bucket name
    • per-gin.HandlerFunc bucket name
    • per-error type bucket name
  • fully documented
  • includes example

oryband avatar Jul 26 '15 16:07 oryband

@bolshoy @doodyparizada @fuzzyami @avivl cc

oryband avatar Jul 26 '15 16:07 oryband

build fails because of the gzip middleware build failing, not this middleware

oryband avatar Jul 27 '15 07:07 oryband

Very nice! maybe add a readme with an image for example?

doodyparizada avatar Jul 27 '15 13:07 doodyparizada

i think we should add sub-readmes to all middlewares in this repo, along with a godoc link. but this is for another issue

oryband avatar Jul 27 '15 13:07 oryband

@manucorporat merge plz k thx bai

oryband avatar Aug 13 '15 16:08 oryband

@oryband sorry, I have been disconnected for a long time (vacations). but I am back to work! I want to rebuild the contrib repo.

let me a couple of weeks to clean everything up

manucorporat avatar Aug 16 '15 14:08 manucorporat

great

oryband avatar Aug 16 '15 14:08 oryband

@manucorporat i'm intending to update this subpackage in the future, to comply with https://github.com/go-kit/kit/tree/master/metrics

i've taken notice of this project lately, and i feel it's going to become some sort of a backbone of the language. might as well follow they're guidelines.

i'll just need to update the interface i think, that's all

will do this in a future commit though, if you won't do it before that

oryband avatar Aug 16 '15 14:08 oryband

@oryband not sure how the metrics api is, but the current api is terrible:

r.Use(middleware.Statsd(bufferedClient, middleware.SetThroughputBucket("request.received"))

how about something like:

    r.Use(middleware.Statsd(bufferedClient).
        SetThroughputBucket("request.received").
        SetShitBucket("request.received"))

the pipeline pattern is powerful and recognised by everybody. It is also used in Gin: routes and c.Error(). (in case you missed it: https://github.com/gin-gonic/gin/issues/336 )

UPDATED: ok, never mind. we can't do what I said, but anyway, we should find a better solution

manucorporat avatar Aug 16 '15 16:08 manucorporat

@manucorporat pipelining will indeed not work because it will require gin.Handlerfunc to support all these SetX() functions.

The other alternative will result in having a fixed signature for mwr.Statsd(..), which isn't future proof. If we want to add more settings in the future, this is the best option. It allows you to include whatever settings you wish. If you want to make it a bit more beautiful you can alias mwr instead of middleware or some other short word.

See Dave Cheney's article at the top of this discussion. It's also recommended by Rob Pike. I chose to use this pattern for this exact reason.

IMO, all contrib modules should use this, or the pipeline if necessary.

oryband avatar Aug 17 '15 09:08 oryband

Also, go-kit's guidelines just state the metrics client (e.g. statsd) interface, not how you set it up.

oryband avatar Aug 17 '15 09:08 oryband

@manucorporat @oryband agree, go-kit has some helpful paradigms for middlewares, worth having a look.

bolshoy avatar Aug 18 '15 23:08 bolshoy

@manucorporat merge plz?

oryband avatar Aug 23 '15 14:08 oryband

@manucorporat ?

oryband avatar Nov 01 '15 09:11 oryband