[ElasticSearch] Improve tests and bug fixes #1487
Conversation
- Properly handle multiple instances - 100% metrics and service checks coverage - Flake8 (everything but line > 80) - Make sure that constants are constants Also: - Pretty format coverage report - Log more info when candidates assertion is failing
|
Failing on Apache test. |
| @@ -126,7 +159,7 @@ class ESCheck(AgentCheck): | |||
| "elasticsearch.relocating_shards": ("gauge", "relocating_shards"), | |||
| "elasticsearch.initializing_shards": ("gauge", "initializing_shards"), | |||
| "elasticsearch.unassigned_shards": ("gauge", "unassigned_shards"), | |||
| "elasticsearch.cluster_status": ("gauge", "status", lambda v: {"red":0, "yellow":1, "green":2}.get(v, -1)), | |||
| "elasticsearch.cluster_status": ("gauge", "status", lambda v: {"red": 0, "yellow": 1, "green": 2}.get(v, -1)), | |||
There was a problem hiding this comment.
nitpick: but this should probably use the class constants instead of bare integers to define the service check values.
There was a problem hiding this comment.
What class constants ? This is a mapping that is not related to service checks. This is sent as a metric, it was done before we had service checks so one could alert on the cluster status.
Those values are actually the opposite of the constants we use for service checks.
There was a problem hiding this comment.
Apologies, scratch that.. Should have looked more at the context.
Maybe at one point we could deprecate this guy, it's really confusing
| log.error("No candidates for {0} (value: {1}, tags: {2}, count: {3}, at_least: {4}, hostname: {5})"\ | ||
| .format(metric_name, value, tags, count, at_least, hostname)) | ||
| raise | ||
|
|
There was a problem hiding this comment.
I added the debug headers for that reason ("Looking for...") should we keep only one or another? I am good with both, also "No candidates" might not be a clear message for some cases like at_least=1.
There was a problem hiding this comment.
I didn't find the debug log visible enough when testing, especially when you have a lot of metrics. This log line made my debugging easier. I can change the message though.
There was a problem hiding this comment.
Works for me. Let's get rid of the debug lines then.
|
Failing on Apache. Merging. |
[ElasticSearch] Improve tests and bug fixes
Also: