spring-cloud / spring-cloud-consul

Spring Cloud Consul
http://cloud.spring.io/spring-cloud-consul/
Apache License 2.0
812 stars 541 forks source link

TTL Checks only report Pass, and ignore real health status from the application #676

Closed jdeex closed 2 years ago

jdeex commented 4 years ago

Describe the bug TTL seems to only reports pass, even when there are health indicators failing. Even when the Health Endpoint <aaplication/actuator/health reports DOWN (and HTTP+Interval would show that way in Consul), the TTL check reports OK instead.

Having a look at the code it could be because of this

TTL should check the application status and report that using Consul TTL endpoints

Sample Implement a test health indicator that will cause the status to be down: public class TestHealthIndicator implements HealthIndicator { @Override public Health health() { return Health .down() .withDetail("test", "App is failing") .build(); } }

Enable TTL in the spring configuration: spring: cloud: consul: discovery: heartbeat: enabled: true ttl-value: 30 ttl-unit: m

Review /actuator/health Down status and status reported in Consul

onobc commented 3 years ago

I picked this one up. I have a working solution - adding tests then will submit proposal.

aromanos commented 3 years ago

Hi @bono007 this is also useful for my use case. Is this commit going to be merged?

onobc commented 3 years ago

@spencergibb I know you and team are busy doing "all the things" :) When you can, could you please give this one a review? Thank you.

onobc commented 3 years ago

@spencergibb I bit the bullet and did the nasty rebase/merge - should be good now.

jdeex commented 3 years ago

Hey, how is it going? thought It was merged already. In what release is this expected to be available?

jdeex commented 3 years ago

@spencergibb did you have the time to review @bono007 commit?

spencergibb commented 3 years ago

@jdeex it is scheduled for later this year.

jdeex commented 2 years ago

@spencergibb I saw this one missed the 2021.0.0 release, what's the target for this issue?

jdeex commented 2 years ago

Any update on this one??