From 6c66ff78c0d04c2bc30e9b8c338c8b7e3faf1046 Mon Sep 17 00:00:00 2001 From: Christian Dupuis Date: Tue, 28 Oct 2014 12:46:14 +0100 Subject: [PATCH] Fix health status aggregation bug --- .../HealthIndicatorAutoConfiguration.java | 12 +++--- ...hIndicatorAutoConfigurationProperties.java | 42 +++++++++++++++++++ .../health/OrderedHealthAggregator.java | 14 +++++-- .../actuate/endpoint/HealthEndpointTests.java | 4 +- .../health/OrderedHealthAggregatorTests.java | 3 +- 5 files changed, 62 insertions(+), 13 deletions(-) create mode 100644 spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/HealthIndicatorAutoConfigurationProperties.java diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/HealthIndicatorAutoConfiguration.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/HealthIndicatorAutoConfiguration.java index 51d4f59725..a80546a902 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/HealthIndicatorAutoConfiguration.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/HealthIndicatorAutoConfiguration.java @@ -18,7 +18,6 @@ package org.springframework.boot.actuate.autoconfigure; import java.util.Collection; import java.util.Collections; -import java.util.List; import java.util.Map; import javax.sql.DataSource; @@ -26,7 +25,6 @@ import javax.sql.DataSource; import org.apache.solr.client.solrj.SolrServer; import org.springframework.amqp.rabbit.core.RabbitTemplate; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.beans.factory.annotation.Value; import org.springframework.boot.actuate.health.ApplicationHealthIndicator; import org.springframework.boot.actuate.health.CompositeHealthIndicator; import org.springframework.boot.actuate.health.DataSourceHealthIndicator; @@ -54,6 +52,7 @@ import org.springframework.boot.autoconfigure.mongo.MongoAutoConfiguration; import org.springframework.boot.autoconfigure.mongo.MongoDataAutoConfiguration; import org.springframework.boot.autoconfigure.redis.RedisAutoConfiguration; import org.springframework.boot.autoconfigure.solr.SolrAutoConfiguration; +import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.data.mongodb.core.MongoTemplate; @@ -72,17 +71,18 @@ import org.springframework.data.redis.connection.RedisConnectionFactory; @AutoConfigureAfter({ DataSourceAutoConfiguration.class, MongoAutoConfiguration.class, MongoDataAutoConfiguration.class, RedisAutoConfiguration.class, RabbitAutoConfiguration.class, SolrAutoConfiguration.class }) +@EnableConfigurationProperties({ HealthIndicatorAutoConfigurationProperties.class }) public class HealthIndicatorAutoConfiguration { - @Value("${health.status.order:}") - private List statusOrder = null; + @Autowired + private HealthIndicatorAutoConfigurationProperties configurationProperties = new HealthIndicatorAutoConfigurationProperties(); @Bean @ConditionalOnMissingBean public HealthAggregator healthAggregator() { OrderedHealthAggregator healthAggregator = new OrderedHealthAggregator(); - if (this.statusOrder != null) { - healthAggregator.setStatusOrder(this.statusOrder); + if (this.configurationProperties.getOrder() != null) { + healthAggregator.setStatusOrder(this.configurationProperties.getOrder()); } return healthAggregator; } diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/HealthIndicatorAutoConfigurationProperties.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/HealthIndicatorAutoConfigurationProperties.java new file mode 100644 index 0000000000..4590328279 --- /dev/null +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/HealthIndicatorAutoConfigurationProperties.java @@ -0,0 +1,42 @@ +/* + * Copyright 2012-2014 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 + * + * http://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.autoconfigure; + +import java.util.List; + +import org.springframework.boot.context.properties.ConfigurationProperties; + +/** + * Configuration properties for some health properties + * @author Christian Dupuis + */ +@ConfigurationProperties("health.status") +public class HealthIndicatorAutoConfigurationProperties { + + private List order = null; + + public List getOrder() { + return this.order; + } + + public void setOrder(List statusOrder) { + if (statusOrder != null && statusOrder.size() > 0) { + this.order = statusOrder; + } + } + +} diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/OrderedHealthAggregator.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/OrderedHealthAggregator.java index 92a3a377c1..4b2cbe5bdb 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/OrderedHealthAggregator.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/OrderedHealthAggregator.java @@ -16,6 +16,7 @@ package org.springframework.boot.actuate.health; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.Comparator; @@ -67,13 +68,20 @@ public class OrderedHealthAggregator extends AbstractHealthAggregator { @Override protected Status aggregateStatus(List candidates) { + // Only sort those status instances that we know about + List filteredCandidates = new ArrayList(); + for (Status candidate : candidates) { + if (this.statusOrder.contains(candidate.getCode())) { + filteredCandidates.add(candidate); + } + } // If no status is given return UNKNOWN - if (candidates.size() == 0) { + if (filteredCandidates.size() == 0) { return Status.UNKNOWN; } // Sort given Status instances by configured order - Collections.sort(candidates, new StatusComparator(this.statusOrder)); - return candidates.get(0); + Collections.sort(filteredCandidates, new StatusComparator(this.statusOrder)); + return filteredCandidates.get(0); } /** diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/HealthEndpointTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/HealthEndpointTests.java index 608927412b..eb4e8225ab 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/HealthEndpointTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/HealthEndpointTests.java @@ -45,8 +45,8 @@ public class HealthEndpointTests extends AbstractEndpointTests { @Test public void invoke() throws Exception { - Status result = new Status("FINE"); - assertThat(getEndpointBean().invoke().getStatus(), equalTo(result)); + // As FINE isn't configured in the order we get UNKOWN + assertThat(getEndpointBean().invoke().getStatus(), equalTo(Status.UNKNOWN)); } @Configuration diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/health/OrderedHealthAggregatorTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/health/OrderedHealthAggregatorTests.java index 59bfb6958a..5516e028c7 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/health/OrderedHealthAggregatorTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/health/OrderedHealthAggregatorTests.java @@ -69,8 +69,7 @@ public class OrderedHealthAggregatorTests { healths.put("h3", new Health.Builder().status(Status.UNKNOWN).build()); healths.put("h4", new Health.Builder().status(Status.OUT_OF_SERVICE).build()); healths.put("h5", new Health.Builder().status(new Status("CUSTOM")).build()); - assertEquals(new Status("CUSTOM"), this.healthAggregator.aggregate(healths) - .getStatus()); + assertEquals(Status.DOWN, this.healthAggregator.aggregate(healths).getStatus()); } @Test