From d7852cb1768bca92226cb07c44897333ac2704cd Mon Sep 17 00:00:00 2001 From: Moritz Halbritter Date: Thu, 12 Jan 2023 10:52:04 +0100 Subject: [PATCH] Log failing calls to health indicators See gh-33774 --- .../AbstractReactiveHealthIndicator.java | 16 ++- .../boot/actuate/health/Health.java | 11 ++ .../AbstractReactiveHealthIndicatorTests.java | 118 ++++++++++++++++++ 3 files changed, 140 insertions(+), 5 deletions(-) create mode 100644 spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/health/AbstractReactiveHealthIndicatorTests.java diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/AbstractReactiveHealthIndicator.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/AbstractReactiveHealthIndicator.java index ea819301d6..80c0832178 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/AbstractReactiveHealthIndicator.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/AbstractReactiveHealthIndicator.java @@ -31,6 +31,7 @@ import org.springframework.util.StringUtils; * * @author Stephane Nicoll * @author Nikolay Rybak + * @author Moritz Halbritter * @since 2.0.0 */ public abstract class AbstractReactiveHealthIndicator implements ReactiveHealthIndicator { @@ -75,19 +76,24 @@ public abstract class AbstractReactiveHealthIndicator implements ReactiveHealthI @Override public final Mono health() { + Mono result; try { - return doHealthCheck(new Health.Builder()).onErrorResume(this::handleFailure); + result = doHealthCheck(new Health.Builder()).onErrorResume(this::handleFailure); } catch (Exception ex) { - return handleFailure(ex); + result = handleFailure(ex); } + return result.doOnNext((health) -> logExceptionIfPresent(health.getException())); } - private Mono handleFailure(Throwable ex) { - if (this.logger.isWarnEnabled()) { - String message = this.healthCheckFailedMessage.apply(ex); + private void logExceptionIfPresent(Throwable ex) { + if (ex != null && this.logger.isWarnEnabled()) { + String message = (ex instanceof Exception) ? this.healthCheckFailedMessage.apply(ex) : null; this.logger.warn(StringUtils.hasText(message) ? message : DEFAULT_MESSAGE, ex); } + } + + private Mono handleFailure(Throwable ex) { return Mono.just(new Health.Builder().down(ex).build()); } diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/Health.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/Health.java index d0f293e28d..55bc9fcd71 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/Health.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/Health.java @@ -20,6 +20,7 @@ import java.util.Collections; import java.util.LinkedHashMap; import java.util.Map; +import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.annotation.JsonInclude.Include; @@ -46,6 +47,7 @@ import org.springframework.util.Assert; * @author Christian Dupuis * @author Phillip Webb * @author Michael Pratt + * @author Moritz Halbritter * @since 1.1.0 */ @JsonInclude(Include.NON_EMPTY) @@ -55,6 +57,8 @@ public final class Health extends HealthComponent { private final Map details; + private final Throwable exception; + /** * Create a new {@link Health} instance with the specified status and details. * @param builder the Builder to use @@ -63,11 +67,13 @@ public final class Health extends HealthComponent { Assert.notNull(builder, "Builder must not be null"); this.status = builder.status; this.details = Collections.unmodifiableMap(builder.details); + this.exception = builder.exception; } Health(Status status, Map details) { this.status = status; this.details = details; + this.exception = null; } /** @@ -101,6 +107,11 @@ public final class Health extends HealthComponent { return status(getStatus()).build(); } + @JsonIgnore + Throwable getException() { + return this.exception; + } + @Override public boolean equals(Object obj) { if (obj == this) { diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/health/AbstractReactiveHealthIndicatorTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/health/AbstractReactiveHealthIndicatorTests.java new file mode 100644 index 0000000000..04d00def80 --- /dev/null +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/health/AbstractReactiveHealthIndicatorTests.java @@ -0,0 +1,118 @@ +/* + * Copyright 2012-2019 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.boot.actuate.health; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import reactor.core.publisher.Mono; + +import org.springframework.boot.actuate.health.Health.Builder; +import org.springframework.boot.test.system.CapturedOutput; +import org.springframework.boot.test.system.OutputCaptureExtension; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for {@link AbstractReactiveHealthIndicator}. + * + * @author Moritz Halbritter + */ +@ExtendWith(OutputCaptureExtension.class) +class AbstractReactiveHealthIndicatorTests { + + @Test + void healthCheckWhenUpDoesNotLogHealthCheckFailedMessage(CapturedOutput output) { + Health health = new AbstractReactiveHealthIndicator("Test message") { + @Override + protected Mono doHealthCheck(Builder builder) { + return Mono.just(builder.up().build()); + } + + }.health().block(); + assertThat(health).isNotNull(); + assertThat(health.getStatus()).isEqualTo(Status.UP); + assertThat(output).doesNotContain("Test message"); + } + + @Test + void healthCheckWhenDownWithExceptionThrownDoesNotLogHealthCheckFailedMessage(CapturedOutput output) { + Health health = new AbstractReactiveHealthIndicator("Test message") { + @Override + protected Mono doHealthCheck(Builder builder) { + throw new IllegalStateException("Test exception"); + } + }.health().block(); + assertThat(health).isNotNull(); + assertThat(health.getStatus()).isEqualTo(Status.DOWN); + assertThat(output).contains("Test message").contains("Test exception"); + } + + @Test + void healthCheckWhenDownWithExceptionConfiguredDoesNotLogHealthCheckFailedMessage(CapturedOutput output) { + Health health = new AbstractReactiveHealthIndicator("Test message") { + @Override + protected Mono doHealthCheck(Builder builder) { + return Mono.just(builder.down().withException(new IllegalStateException("Test exception")).build()); + } + }.health().block(); + assertThat(health).isNotNull(); + assertThat(health.getStatus()).isEqualTo(Status.DOWN); + assertThat(output).contains("Test message").contains("Test exception"); + } + + @Test + void healthCheckWhenDownWithExceptionConfiguredDoesNotLogHealthCheckFailedMessageTwice(CapturedOutput output) { + Health health = new AbstractReactiveHealthIndicator("Test message") { + @Override + protected Mono doHealthCheck(Builder builder) { + IllegalStateException ex = new IllegalStateException("Test exception"); + builder.down().withException(ex); + throw ex; + } + }.health().block(); + assertThat(health).isNotNull(); + assertThat(health.getStatus()).isEqualTo(Status.DOWN); + assertThat(output).contains("Test message").containsOnlyOnce("Test exception"); + } + + @Test + void healthCheckWhenDownWithExceptionAndNoFailureMessageLogsDefaultMessage(CapturedOutput output) { + Health health = new AbstractReactiveHealthIndicator() { + @Override + protected Mono doHealthCheck(Builder builder) { + return Mono.just(builder.down().withException(new IllegalStateException("Test exception")).build()); + } + }.health().block(); + assertThat(health).isNotNull(); + assertThat(health.getStatus()).isEqualTo(Status.DOWN); + assertThat(output).contains("Health check failed").contains("Test exception"); + } + + @Test + void healthCheckWhenDownWithErrorLogsDefaultMessage(CapturedOutput output) { + Health health = new AbstractReactiveHealthIndicator("Test Message") { + @Override + protected Mono doHealthCheck(Builder builder) { + return Mono.just(builder.down().withException(new Error("Test error")).build()); + } + }.health().block(); + assertThat(health).isNotNull(); + assertThat(health.getStatus()).isEqualTo(Status.DOWN); + assertThat(output).contains("Health check failed").contains("Test error"); + } + +}