Add probe method to DiscoveryClient (#789)

Add probe method to DiscoveryClient and optionally use in DiscoveryClientHealthIndicator.

Fixes gh-785

Co-authored-by: Spencer Gibb <sgibb@pivotal.io>

docs/src/main/asciidoc/_configprops.adoc

@@ -10,6 +10,10 @@
 |spring.cloud.discovery.client.health-indicator.enabled | true | 
 |spring.cloud.discovery.client.health-indicator.include-description | false | 
 |spring.cloud.discovery.client.simple.instances |  | 
+|spring.cloud.discovery.client.simple.local.instance-id |  | The unique identifier or name for the service instance.
+|spring.cloud.discovery.client.simple.local.metadata |  | Metadata for the service instance. Can be used by discovery clients to modify their behaviour per instance, e.g. when load balancing.
+|spring.cloud.discovery.client.simple.local.service-id |  | The identifier or name for the service. Multiple instances might share the same service ID.
+|spring.cloud.discovery.client.simple.local.uri |  | The URI of the service instance. Will be parsed to extract the scheme, host, and port.
 |spring.cloud.discovery.client.simple.order |  | 
 |spring.cloud.discovery.enabled | true | Enables discovery client health indicators.
 |spring.cloud.features.enabled | true | Enables the features endpoint.

docs/src/main/asciidoc/spring-cloud-commons.adoc

@@ -258,14 +258,23 @@ This behavior can be disabled by setting `autoRegister=false` in `@EnableDiscove
 NOTE: `@EnableDiscoveryClient` is no longer required.
 You can put a `DiscoveryClient` implementation on the classpath to cause the Spring Boot application to register with the service discovery server.
 
-==== Health Indicator
+==== Health Indicators
 
-Commons creates a Spring Boot `HealthIndicator` that `DiscoveryClient` implementations can participate in by implementing `DiscoveryHealthIndicator`.
-To disable the composite `HealthIndicator`, set `spring.cloud.discovery.client.composite-indicator.enabled=false`.
-A generic `HealthIndicator` based on `DiscoveryClient` is auto-configured (`DiscoveryClientHealthIndicator`).
-To disable it, set `spring.cloud.discovery.client.health-indicator.enabled=false`.
-To disable the description field of the `DiscoveryClientHealthIndicator`, set `spring.cloud.discovery.client.health-indicator.include-description=false`.
+Commons auto-configures the following Spring Boot health indicators.
+
+===== DiscoveryClientHealthIndicator
+This health indicator is based on the currently registered `DiscoveryClient` implementation.
+
+* To disable entirely, set `spring.cloud.discovery.client.health-indicator.enabled=false`.
+* To disable the description field, set `spring.cloud.discovery.client.health-indicator.include-description=false`.
 Otherwise, it can bubble up as the `description` of the rolled up `HealthIndicator`.
+* To disable service retrieval, set `spring.cloud.discovery.client.health-indicator.use-services-query=false`.
+By default, the indicator invokes the client's `getServices` method. In deployments with many registered services it may too
+costly to retrieve all services during every check. This will skip the service retrieval and instead use the client's `probe` method.
+
+===== DiscoveryCompositeHealthContributor
+This composite health indicator is based on all registered `DiscoveryHealthIndicator` beans. To disable,
+set `spring.cloud.discovery.client.composite-indicator.enabled=false`.
 
 ==== Ordering `DiscoveryClient` instances
 `DiscoveryClient` interface extends `Ordered`. This is useful when using multiple discovery

spring-cloud-commons/src/main/java/org/springframework/cloud/client/discovery/DiscoveryClient.java

@@ -27,6 +27,7 @@ import org.springframework.core.Ordered;
  *
  * @author Spencer Gibb
  * @author Olga Maciaszek-Sharma
+ * @author Chris Bono
  */
 public interface DiscoveryClient extends Ordered {
 
@@ -53,6 +54,19 @@ public interface DiscoveryClient extends Ordered {
 	 */
 	List<String> getServices();
 
+	/**
+	 * Can be used to verify the client is valid and able to make calls.
+	 * <p>
+	 * A successful invocation with no exception thrown implies the client is able to make
+	 * calls.
+	 * <p>
+	 * The default implementation simply calls {@link #getServices()} - client
+	 * implementations can override with a lighter weight operation if they choose to.
+	 */
+	default void probe() {
+		getServices();
+	}
+
 	/**
 	 * Default implementation for getting order of discovery clients.
 	 * @return order

spring-cloud-commons/src/main/java/org/springframework/cloud/client/discovery/ReactiveDiscoveryClient.java

@@ -52,6 +52,19 @@ public interface ReactiveDiscoveryClient extends Ordered {
 	 */
 	Flux<String> getServices();
 
+	/**
+	 * Can be used to verify the client is still valid and able to make calls.
+	 * <p>
+	 * A successful invocation with no exception thrown implies the client is able to make
+	 * calls.
+	 * <p>
+	 * The default implementation simply calls {@link #getServices()} - client
+	 * implementations can override with a lighter weight operation if they choose to.
+	 */
+	default void probe() {
+		getServices();
+	}
+
 	/**
 	 * Default implementation for getting order of discovery clients.
 	 * @return order

spring-cloud-commons/src/main/java/org/springframework/cloud/client/discovery/health/DiscoveryClientHealthIndicator.java

@@ -32,6 +32,7 @@ import org.springframework.core.Ordered;
 
 /**
  * @author Spencer Gibb
+ * @author Chris Bono
  */
 public class DiscoveryClientHealthIndicator implements DiscoveryHealthIndicator, Ordered,
 		ApplicationListener<InstanceRegisteredEvent<?>> {
@@ -66,11 +67,18 @@ public class DiscoveryClientHealthIndicator implements DiscoveryHealthIndicator,
 		if (this.discoveryInitialized.get()) {
 			try {
 				DiscoveryClient client = this.discoveryClient.getIfAvailable();
-				List<String> services = client.getServices();
 				String description = (this.properties.isIncludeDescription())
 						? client.description() : "";
-				builder.status(new Status("UP", description)).withDetail("services",
-						services);
+
+				if (properties.isUseServicesQuery()) {
+					List<String> services = client.getServices();
+					builder.status(new Status("UP", description)).withDetail("services",
+							services);
+				}
+				else {
+					client.probe();
+					builder.status(new Status("UP", description));
+				}
 			}
 			catch (Exception e) {
 				this.log.error("Error", e);

spring-cloud-commons/src/main/java/org/springframework/cloud/client/discovery/health/DiscoveryClientHealthIndicatorProperties.java

@@ -17,6 +17,7 @@
 package org.springframework.cloud.client.discovery.health;
 
 import org.springframework.boot.context.properties.ConfigurationProperties;
+import org.springframework.cloud.client.discovery.DiscoveryClient;
 
 /**
  * @author Spencer Gibb
@@ -28,6 +29,14 @@ public class DiscoveryClientHealthIndicatorProperties {
 
 	private boolean includeDescription = false;
 
+	/**
+	 * Whether or not the indicator should use {@link DiscoveryClient#getServices} to
+	 * check its health. When set to {@code false} the indicator instead uses the lighter
+	 * {@link DiscoveryClient#probe()}. This can be helpful in large deployments where the
+	 * number of services returned makes the operation unnecessarily heavy.
+	 */
+	private boolean useServicesQuery = true;
+
 	public boolean isEnabled() {
 		return this.enabled;
 	}
@@ -44,12 +53,21 @@ public class DiscoveryClientHealthIndicatorProperties {
 		this.includeDescription = includeDescription;
 	}
 
+	public boolean isUseServicesQuery() {
+		return useServicesQuery;
+	}
+
+	public void setUseServicesQuery(boolean useServicesQuery) {
+		this.useServicesQuery = useServicesQuery;
+	}
+
 	@Override
 	public String toString() {
 		final StringBuffer sb = new StringBuffer(
 				"DiscoveryClientHealthIndicatorProperties{");
 		sb.append("enabled=").append(this.enabled);
 		sb.append(", includeDescription=").append(this.includeDescription);
+		sb.append(", useServicesQuery=").append(this.useServicesQuery);
 		sb.append('}');
 		return sb.toString();
 	}

spring-cloud-commons/src/main/java/org/springframework/cloud/client/discovery/health/reactive/ReactiveDiscoveryClientHealthIndicator.java

@@ -37,6 +37,7 @@ import static java.util.Collections.emptyList;
  * initialized.
  *
  * @author Tim Ysewyn
+ * @author Chris Bono
  */
 public class ReactiveDiscoveryClientHealthIndicator
 		implements ReactiveDiscoveryHealthIndicator, Ordered,
@@ -78,21 +79,41 @@ public class ReactiveDiscoveryClientHealthIndicator
 	}
 
 	private Mono<Health> doHealthCheck() {
+		// @formatter:off
+		return Mono.just(this.properties.isUseServicesQuery())
+				.flatMap(useServices -> useServices ? doHealthCheckWithServices() : doHealthCheckWithProbe())
+				.onErrorResume(exception -> {
+					this.log.error("Error", exception);
+					return Mono.just(Health.down().withException(exception).build());
+				});
+		// @formatter:on
+	}
+
+	private Mono<Health> doHealthCheckWithProbe() {
+		// @formatter:off
+		return Mono.justOrEmpty(this.discoveryClient)
+				.flatMap(client -> {
+					client.probe();
+					return Mono.just(client);
+				})
+				.map(client -> {
+					String description = (this.properties.isIncludeDescription()) ? client.description() : "";
+					return Health.status(new Status("UP", description)).build();
+				});
+		// @formatter:on
+	}
+
+	private Mono<Health> doHealthCheckWithServices() {
 		// @formatter:off
 		return Mono.justOrEmpty(this.discoveryClient)
 				.flatMapMany(ReactiveDiscoveryClient::getServices)
 				.collectList()
 				.defaultIfEmpty(emptyList())
 				.map(services -> {
-					ReactiveDiscoveryClient client = this.discoveryClient;
-					String description = (this.properties.isIncludeDescription())
-							? client.description() : "";
+					String description = (this.properties.isIncludeDescription()) ?
+						this.discoveryClient.description() : "";
 					return Health.status(new Status("UP", description))
-							.withDetail("services", services).build();
-				})
-				.onErrorResume(exception -> {
-					this.log.error("Error", exception);
-					return Mono.just(Health.down().withException(exception).build());
+						.withDetail("services", services).build();
 				});
 		// @formatter:on
 	}

spring-cloud-commons/src/test/java/org/springframework/cloud/client/discovery/health/DiscoveryClientHealthIndicatorUnitTests.java

@@ -0,0 +1,144 @@
+/*
+ * Copyright 2012-2020 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.cloud.client.discovery.health;
+
+import java.util.Collections;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+import org.springframework.beans.factory.ObjectProvider;
+import org.springframework.boot.actuate.health.Health;
+import org.springframework.boot.actuate.health.Status;
+import org.springframework.cloud.client.discovery.DiscoveryClient;
+import org.springframework.cloud.client.discovery.event.InstanceRegisteredEvent;
+
+import static java.util.Collections.emptyList;
+import static java.util.Collections.singletonList;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.lenient;
+import static org.mockito.Mockito.when;
+
+/**
+ * Unit tests for {@link DiscoveryClientHealthIndicator}.
+ *
+ * @author Chris Bono
+ */
+@ExtendWith(MockitoExtension.class)
+class DiscoveryClientHealthIndicatorUnitTests {
+
+	@Mock
+	private ObjectProvider<DiscoveryClient> discoveryClientProvider;
+
+	@Mock
+	private DiscoveryClient discoveryClient;
+
+	@Mock
+	private DiscoveryClientHealthIndicatorProperties properties;
+
+	@InjectMocks
+	private DiscoveryClientHealthIndicator indicator;
+
+	@BeforeEach
+	public void prepareMocks() {
+		lenient().when(discoveryClientProvider.getIfAvailable())
+				.thenReturn(discoveryClient);
+	}
+
+	@Test
+	public void shouldReturnUnknownStatusWhenNotInitialized() {
+		Health expectedHealth = Health.status(
+				new Status(Status.UNKNOWN.getCode(), "Discovery Client not initialized"))
+				.build();
+		Health health = indicator.health();
+		assertThat(health).isEqualTo(expectedHealth);
+	}
+
+	@Test
+	public void shouldReturnUpStatusWhenNotUsingServicesQueryAndProbeSucceeds() {
+		when(properties.isUseServicesQuery()).thenReturn(false);
+		Health expectedHealth = Health.status(new Status(Status.UP.getCode(), ""))
+				.build();
+
+		indicator.onApplicationEvent(new InstanceRegisteredEvent<>(this, null));
+		Health health = indicator.health();
+
+		assertThat(health).isEqualTo(expectedHealth);
+	}
+
+	@Test
+	public void shouldReturnDownStatusWhenNotUsingServicesQueryAndProbeFails() {
+		when(properties.isUseServicesQuery()).thenReturn(false);
+		RuntimeException ex = new RuntimeException("something went wrong");
+		doThrow(ex).when(discoveryClient).probe();
+		Health expectedHealth = Health.down(ex).build();
+
+		indicator.onApplicationEvent(new InstanceRegisteredEvent<>(this, null));
+		Health health = indicator.health();
+
+		assertThat(health).isEqualTo(expectedHealth);
+	}
+
+	@Test
+	public void shouldReturnUpStatusWhenUsingServicesQueryAndNoServicesReturned() {
+		when(properties.isUseServicesQuery()).thenReturn(true);
+		when(discoveryClient.getServices()).thenReturn(Collections.emptyList());
+		Health expectedHealth = Health.status(new Status(Status.UP.getCode(), ""))
+				.withDetail("services", emptyList()).build();
+
+		indicator.onApplicationEvent(new InstanceRegisteredEvent<>(this, null));
+		Health health = indicator.health();
+
+		assertThat(health).isEqualTo(expectedHealth);
+	}
+
+	@Test
+	public void shouldReturnUpStatusWhenUsingServicesQueryAndServicesReturned() {
+		when(properties.isUseServicesQuery()).thenReturn(true);
+		when(properties.isIncludeDescription()).thenReturn(true);
+		when(discoveryClient.description()).thenReturn("Mocked Service Discovery Client");
+		when(discoveryClient.getServices()).thenReturn(singletonList("service"));
+		Health expectedHealth = Health
+				.status(new Status(Status.UP.getCode(),
+						"Mocked Service Discovery Client"))
+				.withDetail("services", singletonList("service")).build();
+
+		indicator.onApplicationEvent(new InstanceRegisteredEvent<>(this, null));
+		Health health = indicator.health();
+
+		assertThat(health).isEqualTo(expectedHealth);
+	}
+
+	@Test
+	public void shouldReturnDownStatusWhenUsingServicesQueryAndCallFails() {
+		when(properties.isUseServicesQuery()).thenReturn(true);
+		RuntimeException ex = new RuntimeException("something went wrong");
+		when(discoveryClient.getServices()).thenThrow(ex);
+		Health expectedHealth = Health.down(ex).build();
+
+		indicator.onApplicationEvent(new InstanceRegisteredEvent<>(this, null));
+		Health health = indicator.health();
+
+		assertThat(health).isEqualTo(expectedHealth);
+	}
+
+}

spring-cloud-commons/src/test/java/org/springframework/cloud/client/discovery/health/reactive/ReactiveDiscoveryClientHealthIndicatorTests.java

@@ -35,10 +35,12 @@ import org.springframework.core.Ordered;
 import static java.util.Collections.emptyList;
 import static java.util.Collections.singletonList;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.when;
 
 /**
  * @author Tim Ysewyn
+ * @author Chris Bono
  */
 @ExtendWith(MockitoExtension.class)
 class ReactiveDiscoveryClientHealthIndicatorTests {
@@ -59,6 +61,12 @@ class ReactiveDiscoveryClientHealthIndicatorTests {
 		assertThat(indicator.getOrder()).isEqualTo(0);
 	}
 
+	@Test
+	public void shouldUseClientDescriptionForIndicatorName() {
+		when(discoveryClient.description()).thenReturn("Mocked Service Discovery Client");
+		assertThat(indicator.getName()).isEqualTo("Mocked Service Discovery Client");
+	}
+
 	@Test
 	public void shouldReturnUnknownStatusWhenNotInitialized() {
 		Health expectedHealth = Health.status(
@@ -69,8 +77,33 @@ class ReactiveDiscoveryClientHealthIndicatorTests {
 	}
 
 	@Test
-	public void shouldReturnUpStatusWithoutServices() {
-		when(discoveryClient.description()).thenReturn("Mocked Service Discovery Client");
+	public void shouldReturnUpStatusWhenNotUsingServicesQueryAndProbeSucceeds() {
+		when(properties.isUseServicesQuery()).thenReturn(false);
+		Health expectedHealth = Health.status(new Status(Status.UP.getCode(), ""))
+				.build();
+
+		indicator.onApplicationEvent(new InstanceRegisteredEvent<>(this, null));
+		Mono<Health> health = indicator.health();
+
+		StepVerifier.create(health).expectNext(expectedHealth).expectComplete().verify();
+	}
+
+	@Test
+	public void shouldReturnDownStatusWhenNotUsingServicesQueryAndProbeFails() {
+		when(properties.isUseServicesQuery()).thenReturn(false);
+		RuntimeException ex = new RuntimeException("something went wrong");
+		doThrow(ex).when(discoveryClient).probe();
+		Health expectedHealth = Health.down(ex).build();
+
+		indicator.onApplicationEvent(new InstanceRegisteredEvent<>(this, null));
+		Mono<Health> health = indicator.health();
+
+		StepVerifier.create(health).expectNext(expectedHealth).expectComplete().verify();
+	}
+
+	@Test
+	public void shouldReturnUpStatusWhenUsingServicesQueryAndNoServicesReturned() {
+		when(properties.isUseServicesQuery()).thenReturn(true);
 		when(discoveryClient.getServices()).thenReturn(Flux.empty());
 		Health expectedHealth = Health.status(new Status(Status.UP.getCode(), ""))
 				.withDetail("services", emptyList()).build();
@@ -78,14 +111,14 @@ class ReactiveDiscoveryClientHealthIndicatorTests {
 		indicator.onApplicationEvent(new InstanceRegisteredEvent<>(this, null));
 		Mono<Health> health = indicator.health();
 
-		assertThat(indicator.getName()).isEqualTo("Mocked Service Discovery Client");
 		StepVerifier.create(health).expectNext(expectedHealth).expectComplete().verify();
 	}
 
 	@Test
-	public void shouldReturnUpStatusWithServices() {
-		when(discoveryClient.getServices()).thenReturn(Flux.just("service"));
+	public void shouldReturnUpStatusWhenUsingServicesQueryAndServicesReturned() {
+		when(properties.isUseServicesQuery()).thenReturn(true);
 		when(properties.isIncludeDescription()).thenReturn(true);
+		when(discoveryClient.getServices()).thenReturn(Flux.just("service"));
 		when(discoveryClient.description()).thenReturn("Mocked Service Discovery Client");
 		Health expectedHealth = Health
 				.status(new Status(Status.UP.getCode(),
@@ -95,21 +128,20 @@ class ReactiveDiscoveryClientHealthIndicatorTests {
 		indicator.onApplicationEvent(new InstanceRegisteredEvent<>(this, null));
 		Mono<Health> health = indicator.health();
 
-		assertThat(indicator.getName()).isEqualTo("Mocked Service Discovery Client");
 		StepVerifier.create(health).expectNext(expectedHealth).expectComplete().verify();
 	}
 
 	@Test
-	public void shouldReturnDownStatusWhenServicesCouldNotBeRetrieved() {
+	public void shouldReturnDownStatusWhenUsingServicesQueryAndCallFails() {
+		when(properties.isUseServicesQuery()).thenReturn(true);
 		RuntimeException ex = new RuntimeException("something went wrong");
-		Health expectedHealth = Health.down(ex).build();
 		when(discoveryClient.getServices()).thenReturn(Flux.error(ex));
+		Health expectedHealth = Health.down(ex).build();
 
 		indicator.onApplicationEvent(new InstanceRegisteredEvent<>(this, null));
 		Mono<Health> health = indicator.health();
 
-		StepVerifier.create(health).expectNext(expectedHealth).expectComplete()
-				.verifyThenAssertThat();
+		StepVerifier.create(health).expectNext(expectedHealth).expectComplete().verify();
 	}
 
 }