From 6d98851fb51b22f42cb2e34464e1dbe4ccbb5d09 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Mon, 15 Oct 2018 11:37:34 +0100 Subject: [PATCH] Use Couchbase's DiagnosticsReport to determine its health Previously, Couchbase's health was determined by retrieving the bucket info from the cluster info. This retrieval could take over one minute in some cases even when Couchbase is health. This latency is too large for a health check. The Couchbase team have recommended the of a Cluster#diagnostics instead. This provides a much lower latency view of the cluster's health. This commit updates CouchbaseHealthIndicator to use Cluster#diagnostics while retaining support, in a deprecated form, for the old info-based mechanism should anyone want to opt back into that in 2.0.x. Closes gh-14685 --- ...hbaseHealthIndicatorAutoConfiguration.java | 37 ++--- .../CouchbaseHealthIndicatorProperties.java | 2 + ...HealthIndicatorAutoConfigurationTests.java | 18 +-- .../couchbase/CouchbaseHealthIndicator.java | 127 +++++++++++++++--- .../CouchbaseHealthIndicatorTests.java | 62 ++++++++- .../appendix-application-properties.adoc | 1 - 6 files changed, 186 insertions(+), 61 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/couchbase/CouchbaseHealthIndicatorAutoConfiguration.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/couchbase/CouchbaseHealthIndicatorAutoConfiguration.java index 5d685be9eb..c279bce32b 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/couchbase/CouchbaseHealthIndicatorAutoConfiguration.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/couchbase/CouchbaseHealthIndicatorAutoConfiguration.java @@ -18,7 +18,7 @@ package org.springframework.boot.actuate.autoconfigure.couchbase; import java.util.Map; -import com.couchbase.client.java.Bucket; +import com.couchbase.client.java.Cluster; import org.springframework.boot.actuate.autoconfigure.health.CompositeHealthIndicatorConfiguration; import org.springframework.boot.actuate.autoconfigure.health.ConditionalOnEnabledHealthIndicator; @@ -31,11 +31,9 @@ import org.springframework.boot.autoconfigure.EnableAutoConfiguration; import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; -import org.springframework.boot.autoconfigure.data.couchbase.CouchbaseDataAutoConfiguration; -import org.springframework.boot.context.properties.EnableConfigurationProperties; +import org.springframework.boot.autoconfigure.couchbase.CouchbaseAutoConfiguration; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; -import org.springframework.data.couchbase.core.CouchbaseOperations; /** * {@link EnableAutoConfiguration Auto-configuration} for @@ -43,40 +41,33 @@ import org.springframework.data.couchbase.core.CouchbaseOperations; * * @author Eddú Meléndez * @author Stephane Nicoll + * @author Andy Wilkinson Nicoll * @since 2.0.0 */ @Configuration -@ConditionalOnClass({ CouchbaseOperations.class, Bucket.class }) -@ConditionalOnBean(CouchbaseOperations.class) +@ConditionalOnClass(Cluster.class) +@ConditionalOnBean(Cluster.class) @ConditionalOnEnabledHealthIndicator("couchbase") @AutoConfigureBefore(HealthIndicatorAutoConfiguration.class) -@AutoConfigureAfter(CouchbaseDataAutoConfiguration.class) -@EnableConfigurationProperties(CouchbaseHealthIndicatorProperties.class) -public class CouchbaseHealthIndicatorAutoConfiguration extends - CompositeHealthIndicatorConfiguration { +@AutoConfigureAfter(CouchbaseAutoConfiguration.class) +public class CouchbaseHealthIndicatorAutoConfiguration + extends CompositeHealthIndicatorConfiguration { - private final Map couchbaseOperations; + private final Map clusters; - private final CouchbaseHealthIndicatorProperties properties; - - public CouchbaseHealthIndicatorAutoConfiguration( - Map couchbaseOperations, - CouchbaseHealthIndicatorProperties properties) { - this.couchbaseOperations = couchbaseOperations; - this.properties = properties; + public CouchbaseHealthIndicatorAutoConfiguration(Map clusters) { + this.clusters = clusters; } @Bean @ConditionalOnMissingBean(name = "couchbaseHealthIndicator") public HealthIndicator couchbaseHealthIndicator() { - return createHealthIndicator(this.couchbaseOperations); + return createHealthIndicator(this.clusters); } @Override - protected CouchbaseHealthIndicator createHealthIndicator( - CouchbaseOperations couchbaseOperations) { - return new CouchbaseHealthIndicator(couchbaseOperations, - this.properties.getTimeout()); + protected CouchbaseHealthIndicator createHealthIndicator(Cluster cluster) { + return new CouchbaseHealthIndicator(cluster); } } diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/couchbase/CouchbaseHealthIndicatorProperties.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/couchbase/CouchbaseHealthIndicatorProperties.java index 2b4c3162a2..f25f76ef56 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/couchbase/CouchbaseHealthIndicatorProperties.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/couchbase/CouchbaseHealthIndicatorProperties.java @@ -26,7 +26,9 @@ import org.springframework.boot.context.properties.ConfigurationProperties; * * @author Stephane Nicoll * @since 2.0.5 + * @deprecated since 2.0.6 */ +@Deprecated @ConfigurationProperties(prefix = "management.health.couchbase") public class CouchbaseHealthIndicatorProperties { diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/couchbase/CouchbaseHealthIndicatorAutoConfigurationTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/couchbase/CouchbaseHealthIndicatorAutoConfigurationTests.java index 21fce0037b..20d5aa3f2a 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/couchbase/CouchbaseHealthIndicatorAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/couchbase/CouchbaseHealthIndicatorAutoConfigurationTests.java @@ -16,6 +16,7 @@ package org.springframework.boot.actuate.autoconfigure.couchbase; +import com.couchbase.client.java.Cluster; import org.junit.Test; import org.springframework.boot.actuate.autoconfigure.health.HealthIndicatorAutoConfiguration; @@ -26,8 +27,6 @@ import org.springframework.boot.autoconfigure.AutoConfigureBefore; import org.springframework.boot.test.context.runner.ApplicationContextRunner; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; -import org.springframework.data.couchbase.core.CouchbaseOperations; -import org.springframework.test.util.ReflectionTestUtils; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; @@ -52,17 +51,6 @@ public class CouchbaseHealthIndicatorAutoConfigurationTests { .doesNotHaveBean(ApplicationHealthIndicator.class)); } - @Test - public void runWithCustomTimeoutShouldCreateIndicator() { - this.contextRunner.withPropertyValues("management.health.couchbase.timeout=2s") - .run((context) -> { - assertThat(context).hasSingleBean(CouchbaseHealthIndicator.class); - assertThat(ReflectionTestUtils.getField( - context.getBean(CouchbaseHealthIndicator.class), "timeout")) - .isEqualTo(2000L); - }); - } - @Test public void runWhenDisabledShouldNotCreateIndicator() { this.contextRunner.withPropertyValues("management.health.couchbase.enabled:false") @@ -76,8 +64,8 @@ public class CouchbaseHealthIndicatorAutoConfigurationTests { protected static class CouchbaseConfiguration { @Bean - public CouchbaseOperations couchbaseOperations() { - return mock(CouchbaseOperations.class); + public Cluster cluster() { + return mock(Cluster.class); } } diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/couchbase/CouchbaseHealthIndicator.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/couchbase/CouchbaseHealthIndicator.java index 42ed4cdaca..a2342d43b5 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/couchbase/CouchbaseHealthIndicator.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/couchbase/CouchbaseHealthIndicator.java @@ -17,14 +17,22 @@ package org.springframework.boot.actuate.couchbase; import java.time.Duration; +import java.util.HashMap; +import java.util.Map; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; +import java.util.stream.Collectors; +import com.couchbase.client.core.message.internal.DiagnosticsReport; +import com.couchbase.client.core.message.internal.EndpointHealth; +import com.couchbase.client.core.state.LifecycleState; +import com.couchbase.client.java.Cluster; import com.couchbase.client.java.bucket.BucketInfo; import com.couchbase.client.java.cluster.ClusterInfo; import org.springframework.boot.actuate.health.AbstractHealthIndicator; import org.springframework.boot.actuate.health.Health; +import org.springframework.boot.actuate.health.Health.Builder; import org.springframework.boot.actuate.health.HealthIndicator; import org.springframework.data.couchbase.core.CouchbaseOperations; import org.springframework.util.Assert; @@ -39,23 +47,23 @@ import org.springframework.util.StringUtils; */ public class CouchbaseHealthIndicator extends AbstractHealthIndicator { - private final CouchbaseOperations operations; - - private final long timeout; + private final HealthCheck healthCheck; /** * Create an indicator with the specified {@link CouchbaseOperations} and * {@code timeout}. * @param couchbaseOperations the couchbase operations * @param timeout the request timeout + * @deprecated since 2.0.6 in favour of {@link #CouchbaseHealthIndicator(Cluster)} */ + @Deprecated public CouchbaseHealthIndicator(CouchbaseOperations couchbaseOperations, Duration timeout) { super("Couchbase health check failed"); Assert.notNull(couchbaseOperations, "CouchbaseOperations must not be null"); Assert.notNull(timeout, "Timeout must not be null"); - this.operations = couchbaseOperations; - this.timeout = timeout.toMillis(); + this.healthCheck = new OperationsHealthCheck(couchbaseOperations, + timeout.toMillis()); } /** @@ -69,27 +77,108 @@ public class CouchbaseHealthIndicator extends AbstractHealthIndicator { this(couchbaseOperations, Duration.ofSeconds(1)); } + /** + * Create an indicator with the specified {@link Cluster}. + * @param cluster the Couchbase Cluster + * @since 2.0.6 + */ + public CouchbaseHealthIndicator(Cluster cluster) { + super("Couchbase health check failed"); + Assert.notNull(cluster, "Cluster must not be null"); + this.healthCheck = new ClusterHealthCheck(cluster); + } + @Override protected void doHealthCheck(Health.Builder builder) throws Exception { - ClusterInfo cluster = this.operations.getCouchbaseClusterInfo(); - BucketInfo bucket = getBucketInfo(); - String versions = StringUtils - .collectionToCommaDelimitedString(cluster.getAllVersions()); - String nodes = StringUtils.collectionToCommaDelimitedString(bucket.nodeList()); - builder.up().withDetail("versions", versions).withDetail("nodes", nodes); + this.healthCheck.checkHealth(builder); + } + + private interface HealthCheck { + + void checkHealth(Builder builder) throws Exception; + + } + + private static final class OperationsHealthCheck implements HealthCheck { + + private final CouchbaseOperations operations; + + private final long timeout; + + OperationsHealthCheck(CouchbaseOperations operations, long timeout) { + this.operations = operations; + this.timeout = timeout; + } + + @Override + public void checkHealth(Builder builder) throws Exception { + ClusterInfo cluster = this.operations.getCouchbaseClusterInfo(); + BucketInfo bucket = getBucketInfo(); + String versions = StringUtils + .collectionToCommaDelimitedString(cluster.getAllVersions()); + String nodes = StringUtils + .collectionToCommaDelimitedString(bucket.nodeList()); + builder.up().withDetail("versions", versions).withDetail("nodes", nodes); + } + + private BucketInfo getBucketInfo() throws Exception { + try { + return this.operations.getCouchbaseBucket().bucketManager() + .info(this.timeout, TimeUnit.MILLISECONDS); + } + catch (RuntimeException ex) { + if (ex.getCause() instanceof TimeoutException) { + throw (TimeoutException) ex.getCause(); + } + throw ex; + } + } + } - private BucketInfo getBucketInfo() throws Exception { - try { - return this.operations.getCouchbaseBucket().bucketManager().info(this.timeout, - TimeUnit.MILLISECONDS); + private static class ClusterHealthCheck implements HealthCheck { + + private final Cluster cluster; + + ClusterHealthCheck(Cluster cluster) { + this.cluster = cluster; } - catch (RuntimeException ex) { - if (ex.getCause() instanceof TimeoutException) { - throw (TimeoutException) ex.getCause(); + + @Override + public void checkHealth(Builder builder) throws Exception { + DiagnosticsReport diagnostics = this.cluster.diagnostics(); + if (isCouchbaseUp(diagnostics)) { + builder.up(); + } + else { + builder.down(); + } + builder.withDetail("sdk", diagnostics.sdk()); + builder.withDetail("endpoints", diagnostics.endpoints().stream() + .map(this::describe).collect(Collectors.toList())); + } + + private boolean isCouchbaseUp(DiagnosticsReport diagnostics) { + for (EndpointHealth health : diagnostics.endpoints()) { + LifecycleState state = health.state(); + if (state != LifecycleState.CONNECTED && state != LifecycleState.IDLE) { + return false; + } } - throw ex; + return true; + } + + private Map describe(EndpointHealth endpointHealth) { + Map map = new HashMap<>(); + map.put("id", endpointHealth.id()); + map.put("lastActivity", endpointHealth.lastActivity()); + map.put("local", endpointHealth.local().toString()); + map.put("remote", endpointHealth.remote().toString()); + map.put("state", endpointHealth.state()); + map.put("type", endpointHealth.type()); + return map; } + } } diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/couchbase/CouchbaseHealthIndicatorTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/couchbase/CouchbaseHealthIndicatorTests.java index bacd129116..b3cde24ee3 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/couchbase/CouchbaseHealthIndicatorTests.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/couchbase/CouchbaseHealthIndicatorTests.java @@ -17,13 +17,22 @@ package org.springframework.boot.actuate.couchbase; import java.net.InetAddress; +import java.net.InetSocketAddress; import java.net.UnknownHostException; import java.time.Duration; +import java.util.Arrays; import java.util.Collections; +import java.util.List; +import java.util.Map; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; +import com.couchbase.client.core.message.internal.DiagnosticsReport; +import com.couchbase.client.core.message.internal.EndpointHealth; +import com.couchbase.client.core.service.ServiceType; +import com.couchbase.client.core.state.LifecycleState; import com.couchbase.client.java.Bucket; +import com.couchbase.client.java.Cluster; import com.couchbase.client.java.bucket.BucketInfo; import com.couchbase.client.java.bucket.BucketManager; import com.couchbase.client.java.cluster.ClusterInfo; @@ -49,7 +58,7 @@ import static org.mockito.Mockito.verify; public class CouchbaseHealthIndicatorTests { @Test - public void couchbaseIsUp() throws UnknownHostException { + public void couchbaseOperationsIsUp() throws UnknownHostException { BucketInfo bucketInfo = mock(BucketInfo.class); given(bucketInfo.nodeList()).willReturn( Collections.singletonList(InetAddress.getByName("127.0.0.1"))); @@ -63,6 +72,7 @@ public class CouchbaseHealthIndicatorTests { CouchbaseOperations couchbaseOperations = mock(CouchbaseOperations.class); given(couchbaseOperations.getCouchbaseBucket()).willReturn(bucket); given(couchbaseOperations.getCouchbaseClusterInfo()).willReturn(clusterInfo); + @SuppressWarnings("deprecation") CouchbaseHealthIndicator healthIndicator = new CouchbaseHealthIndicator( couchbaseOperations, Duration.ofSeconds(2)); Health health = healthIndicator.health(); @@ -74,7 +84,7 @@ public class CouchbaseHealthIndicatorTests { } @Test - public void couchbaseTimeout() { + public void couchbaseOperationsTimeout() { BucketManager bucketManager = mock(BucketManager.class); given(bucketManager.info(1500, TimeUnit.MILLISECONDS)).willThrow( new RuntimeException(new TimeoutException("timeout, expected"))); @@ -82,6 +92,7 @@ public class CouchbaseHealthIndicatorTests { given(bucket.bucketManager()).willReturn(bucketManager); CouchbaseOperations couchbaseOperations = mock(CouchbaseOperations.class); given(couchbaseOperations.getCouchbaseBucket()).willReturn(bucket); + @SuppressWarnings("deprecation") CouchbaseHealthIndicator healthIndicator = new CouchbaseHealthIndicator( couchbaseOperations, Duration.ofMillis(1500)); Health health = healthIndicator.health(); @@ -90,10 +101,11 @@ public class CouchbaseHealthIndicatorTests { } @Test - public void couchbaseIsDown() { + public void couchbaseOperationsIsDown() { CouchbaseOperations couchbaseOperations = mock(CouchbaseOperations.class); given(couchbaseOperations.getCouchbaseClusterInfo()) .willThrow(new IllegalStateException("test, expected")); + @SuppressWarnings("deprecation") CouchbaseHealthIndicator healthIndicator = new CouchbaseHealthIndicator( couchbaseOperations, Duration.ofSeconds(1)); Health health = healthIndicator.health(); @@ -102,4 +114,48 @@ public class CouchbaseHealthIndicatorTests { verify(couchbaseOperations).getCouchbaseClusterInfo(); } + @Test + @SuppressWarnings("unchecked") + public void couchbaseClusterIsUp() { + Cluster cluster = mock(Cluster.class); + CouchbaseHealthIndicator healthIndicator = new CouchbaseHealthIndicator(cluster); + List endpoints = Arrays.asList(new EndpointHealth( + ServiceType.BINARY, LifecycleState.CONNECTED, new InetSocketAddress(0), + new InetSocketAddress(0), 1234, "endpoint-1")); + DiagnosticsReport diagnostics = new DiagnosticsReport(endpoints, "test-sdk", + "test-id"); + given(cluster.diagnostics()).willReturn(diagnostics); + Health health = healthIndicator.health(); + assertThat(health.getStatus()).isEqualTo(Status.UP); + assertThat(health.getDetails()).containsEntry("sdk", "test-sdk"); + assertThat(health.getDetails()).containsKey("endpoints"); + assertThat((List>) health.getDetails().get("endpoints")) + .hasSize(1); + verify(cluster).diagnostics(); + } + + @Test + @SuppressWarnings("unchecked") + public void couchbaseClusterIsDown() { + Cluster cluster = mock(Cluster.class); + CouchbaseHealthIndicator healthIndicator = new CouchbaseHealthIndicator(cluster); + List endpoints = Arrays.asList( + new EndpointHealth(ServiceType.BINARY, LifecycleState.CONNECTED, + new InetSocketAddress(0), new InetSocketAddress(0), 1234, + "endpoint-1"), + new EndpointHealth(ServiceType.BINARY, LifecycleState.CONNECTING, + new InetSocketAddress(0), new InetSocketAddress(0), 1234, + "endpoint-2")); + DiagnosticsReport diagnostics = new DiagnosticsReport(endpoints, "test-sdk", + "test-id"); + given(cluster.diagnostics()).willReturn(diagnostics); + Health health = healthIndicator.health(); + assertThat(health.getStatus()).isEqualTo(Status.DOWN); + assertThat(health.getDetails()).containsEntry("sdk", "test-sdk"); + assertThat(health.getDetails()).containsKey("endpoints"); + assertThat((List>) health.getDetails().get("endpoints")) + .hasSize(2); + verify(cluster).diagnostics(); + } + } diff --git a/spring-boot-project/spring-boot-docs/src/main/asciidoc/appendix-application-properties.adoc b/spring-boot-project/spring-boot-docs/src/main/asciidoc/appendix-application-properties.adoc index ffeb75bfa5..eebf1a9c43 100644 --- a/spring-boot-project/spring-boot-docs/src/main/asciidoc/appendix-application-properties.adoc +++ b/spring-boot-project/spring-boot-docs/src/main/asciidoc/appendix-application-properties.adoc @@ -1284,7 +1284,6 @@ content into your application. Rather, pick only the properties that you need. management.health.db.enabled=true # Whether to enable database health check. management.health.cassandra.enabled=true # Whether to enable Cassandra health check. management.health.couchbase.enabled=true # Whether to enable Couchbase health check. - management.health.couchbase.timeout=1000ms # Timeout for getting the Bucket information from the server. management.health.defaults.enabled=true # Whether to enable default health indicators. management.health.diskspace.enabled=true # Whether to enable disk space health check. management.health.diskspace.path= # Path used to compute the available disk space.