[couchdb] Improve error handling and add Blacklist support. #1760
Conversation
irabinovitch
commented
Jul 9, 2015
irabinovitch
commented
- fail gracefully when one or more individual dbs are not readable by the configured user.
- allow blacklisting of specific databases via db_blacklist in couch.yaml
- append to blacklist at run time if unable to query a given db's stats
| @@ -18,6 +18,7 @@ class CouchDb(AgentCheck): | |||
| SERVICE_CHECK_NAME = 'couchdb.can_connect' | |||
| SOURCE_TYPE_NAME = 'couchdb' | |||
| TIMEOUT = 5 | |||
| db_blacklist = [] | |||
There was a problem hiding this comment.
So, this won't play well in the (unlikely) case where you are monitoring multiple instances of couchdb on a single server, and if they have some database names in common.
If you want to blacklist one of these db in one of this instance, you'll end up blacklisting it for all instances.
So, the best (but kind of ugly) solution is to use a dictionary instead of a list whose keys are unique keys of an instance (e.g. the "server" parameter of the configuration).
Let me know if you have more questions
|
Nice first pass! I made a few comments (mostly nitpicks). Do you think it's possible to extend the tests to test this scenario as well ? |
|
@remh thanks for the feedback. Yes, we should be able to extend the tests. I'll take a stab at it tomorrow. |
|
This fixes #1749 |
| couchdb['databases'][dbName] = None | ||
| if e.response.status_code == 403: | ||
| self.db_blacklist[server].append(dbName) | ||
| self.warning('Database %s is not readable by the configured user. It will be added to the blacklist. Please restart then agent to clear.' % dbName) |
There was a problem hiding this comment.
typo, should be:
Please restart THE agent to clear
- fail gracefully when one or more individual dbs are not readable by the configured user. - allow blacklisting of specific databases via db_blacklist in couch.yaml - append to blacklist at run time if unable to query a given db's stats - add tests for whitelist and blacklist functionality - add tests for global metrics in addition to per db metrics - add tests to verify error handling around restricted databases. - add tests to test user authentication
|
@remh @JohnLZeller OK. All tests are passing and I believe this is complete. Let me know if you have any further comments you'd like me to address. |
|
Thanks @irabinovitch! I'll take a look here shortly |
|
lgtm, good work :) 👍 |
[couchdb] Improve error handling and add Blacklist support
