The consul.catalog.nodes_up should reflect the # of nodes actually up

[mtougeron]

The consul.catalog.nodes_up should reflect the # of nodes actually up so it is now checking the serfHealth for each service node. Technically, I think the consul.catalog.nodes_up should reflect the # of nodes passing the health checks but for backwards compatibility I left it with just the nodes that are running that are part of the service.

I also added 3 new stats: consul.catalog.nodes_passing, consul.catalog.nodes_warning, & consul.catalog.nodes_critical so that you can trend the actual # of nodes in each state for the service.

Future enhancement planned: Adjust the service checks to use the /v1/health/service/<service> endpoint instead of /v1/health/state/<state> since /v1/health/state/<state> only returns results for services with a Check defined. This means that services that are based on if the node is up/down won't have proper checks from the agent.

[irabinovitch]

Hi Mike, Thanks for the continued PRs on the agent! It looks like this PR might have caused the mock tests to start failing for this check.

test_get_nodes_with_service (tests.checks.mock.test_consul.TestCheckConsul) ... Exception 'str' object has no attribute 'get' during check Traceback (most recent call last): File "/home/travis/build/DataDog/dd-agent/tests/checks/common.py", line 164, in run_check self.check.check(copy.deepcopy(instance)) File "/home/travis/build/DataDog/dd-agent/checks.d/consul.py", line 264, in check node_id = node.get('Node') or None AttributeError: 'str' object has no attribute 'get'

I think the API key failure maybe a flakey test. @remh thoughts?

[mtougeron]

Actually, I think the problem with this one is that I didn't update the mock. Fixing that now.

[mtougeron]

@irabinovitch okay, now I think it is due to the problem with the unit tests. :/

[mtougeron]

@irabinovitch Can you help me out here? It looks like the unit tests are failing for something totally unrelated to my changes.,

[olivielpeau]

Thanks for looking into this @mtougeron, the tests that were failing were flaky, I've restarted them and they pass now.

[irabinovitch]

Thanks @olivielpeau @mtougeron. Apologies for the delay in responding, been on the road a bit.

[mtougeron]

Ya! Thanks! Now to get it merged. :)

[remh]

@talwai can you look at this one ?

[talwai]

Looks good @mtougeron , thanks once again. I'm going to add some cosmetic changes on top of your work and we can merge it in for our next bugfix release

[mtougeron]

Cool, sounds good! Thanks.

[mtougeron]

These changes were merged via #2130