Add more memcached metrics: get_hit_percent, fill_percent, avg_item_size #283
Conversation
|
@jkoppe Nice github account :) It may be worth checking that divisors !=0 before doing the division. The try: except: will catch the / 0 but it will log an error. |
| float( | ||
| float(stats["get_hits"]) / float(stats["cmd_get"]) * 100 | ||
| ), | ||
| tags=tags |
There was a problem hiding this comment.
Stylistic note: Making this tags=tags, would mean that adding new keyword parameters on the end could be a 1-line diff, rather than needing to edit this line as well to add a comma.
|
@jkoppe Can you use except Exception (and maybe log the exception) instead of except ? as except would also catch keyboard interrupt and system exit |
| tags=tags | ||
| ) | ||
| except: | ||
| self.logger.exception("Cannot calculate memcache.fill_percent for tags: %s" % tags) |
There was a problem hiding this comment.
Don't do self.logger.exception('string' % arg), but instead self.logger.exception('string', arg). This will still apply arg as a formatting argument, but will do so only if the log level is high enough that this exception will ever be logged anywhere; otherwise, it avoids needing to do the formatting operation and is thus faster to run.
|
awesome feedback everyone, charles is going to take a stab at cleaning this up :) |
- Avoid using `%` for string formatting in log messages when this would force formatting for logs which aren't going to be printed. - Avoid catching throwables not derived from Exception - Avoid logging on ZeroDivisionError
|
I will take a look at this for the next agent release. |
|
@remh Can you look at this as well? |
|
sure |
Add more memcached metrics: get_hit_percent, fill_percent, avg_item_size
|
seems all good |
This is for issue #282.