Code review comment for lp:~james-w/charms/precise/haproxy/metrics

Revision history for this message
Simon Davy (bloodearnest) wrote :

Shouldn't enabled_monitoring be added to config.yaml?

A thought: in the squid version, squid actually provides average aggregate measures for 1min/5min/60min windows, which is nice as your 5 min cron can use that for better accuracy.

I am assuming the we're sampling current unaggregated metrics here? So, it it worth thinking about a more frequent cron timing? Is 5min enough?

Another thing - I thing it would be worth trying to use statsd rather than carbon?

We'd need to do 2 things

1. change the "| nc ..." part to:

... | while read line; do echo $line | nc -u {{ statsd_host }} {{ statsd_port }}; done

which would send 1 metric per packet? Note: we could also update txstatsd to support multi-metric packets, which would then simplify this back to just "| nc -u ..."

2. add a function to output statsd format rather than carbonify (untested)

statsdify() {
   sed -r "s/([^ ]+) (.*)/${PREFIX}.\1.${PERIOD}:\2|g/"
}

(ps I like explicitly putting the period in there, rather than assuming it's in the metric_scheme, I may change the squid version to that, only reason I didn't was that I think it's custom to our statsd setup, wasn't sure it was valid to put upstream, but maybe its useful info even if your statsd setup doesn't do anything special with it)

« Back to merge proposal