From 1ec2bbf54fd761023ef6e708cbea9444ed3ab024 Mon Sep 17 00:00:00 2001 From: Filip Hrisafov Date: Tue, 11 Dec 2018 11:47:46 +0100 Subject: [PATCH 1/2] Include details in ElasticsearchRestHealthIndicator This commit adds more information to the ElasticSearch REST health indicator. When the ES instance responds with an error HTTP status, the health details now include the actual status code and reason phrase. When the ES instance returns a HTTP 200 response, the entire response map is used as health details. See gh-15366 --- .../ElasticsearchRestHealthIndicator.java | 18 ++++- .../ElasticsearchRestHealthIndicatorTest.java | 74 +++++++++++++++++-- 2 files changed, 80 insertions(+), 12 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/elasticsearch/ElasticsearchRestHealthIndicator.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/elasticsearch/ElasticsearchRestHealthIndicator.java index 963faa73bd..1024dded02 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/elasticsearch/ElasticsearchRestHealthIndicator.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/elasticsearch/ElasticsearchRestHealthIndicator.java @@ -18,8 +18,10 @@ package org.springframework.boot.actuate.elasticsearch; import java.io.InputStream; import java.nio.charset.StandardCharsets; +import java.util.Map; import org.apache.http.HttpStatus; +import org.apache.http.StatusLine; import org.elasticsearch.client.Request; import org.elasticsearch.client.Response; import org.elasticsearch.client.RestClient; @@ -36,6 +38,7 @@ import org.springframework.util.StreamUtils; * * @author Artsiom Yudovin * @author Brian Clozel + * @author Filip Hrisafov * @since 2.1.1 */ public class ElasticsearchRestHealthIndicator extends AbstractHealthIndicator { @@ -56,8 +59,11 @@ public class ElasticsearchRestHealthIndicator extends AbstractHealthIndicator { protected void doHealthCheck(Health.Builder builder) throws Exception { Response response = this.client .performRequest(new Request("GET", "/_cluster/health/")); - if (response.getStatusLine().getStatusCode() != HttpStatus.SC_OK) { + StatusLine statusLine = response.getStatusLine(); + if (statusLine.getStatusCode() != HttpStatus.SC_OK) { builder.down(); + builder.withDetail("statusCode", statusLine.getStatusCode()); + builder.withDetail("reasonPhrase", statusLine.getReasonPhrase()); return; } try (InputStream inputStream = response.getEntity().getContent()) { @@ -67,12 +73,16 @@ public class ElasticsearchRestHealthIndicator extends AbstractHealthIndicator { } private void doHealthCheck(Health.Builder builder, String json) { - String status = (String) this.jsonParser.parseMap(json).get("status"); + Map response = this.jsonParser.parseMap(json); + String status = (String) response.get("status"); if (RED_STATUS.equals(status)) { builder.outOfService(); - return; } - builder.up(); + else { + builder.up(); + } + + builder.withDetails(response); } } diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/elasticsearch/ElasticsearchRestHealthIndicatorTest.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/elasticsearch/ElasticsearchRestHealthIndicatorTest.java index 67caab025a..7f3bd7427d 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/elasticsearch/ElasticsearchRestHealthIndicatorTest.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/elasticsearch/ElasticsearchRestHealthIndicatorTest.java @@ -26,9 +26,11 @@ import org.elasticsearch.client.Response; import org.elasticsearch.client.RestClient; import org.junit.Test; +import org.springframework.boot.actuate.health.Health; import org.springframework.boot.actuate.health.Status; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.entry; import static org.mockito.ArgumentMatchers.any; import static org.mockito.BDDMockito.mock; import static org.mockito.BDDMockito.when; @@ -37,6 +39,7 @@ import static org.mockito.BDDMockito.when; * Tests for {@link ElasticsearchRestHealthIndicator}. * * @author Artsiom Yudovin + * @author Filip Hrisafov */ public class ElasticsearchRestHealthIndicatorTest { @@ -59,8 +62,48 @@ public class ElasticsearchRestHealthIndicatorTest { when(response.getEntity()).thenReturn(httpEntity); when(this.restClient.performRequest(any(Request.class))).thenReturn(response); - assertThat(this.elasticsearchRestHealthIndicator.health().getStatus()) - .isEqualTo(Status.UP); + Health health = this.elasticsearchRestHealthIndicator.health(); + assertThat(health.getStatus()).isEqualTo(Status.UP); + + assertThat(health.getDetails()).contains(entry("cluster_name", "elasticsearch"), + entry("status", "green"), entry("timed_out", false), + entry("number_of_nodes", 1), entry("number_of_data_nodes", 1), + entry("active_primary_shards", 0), entry("active_shards", 0), + entry("relocating_shards", 0), entry("initializing_shards", 0), + entry("unassigned_shards", 0), entry("delayed_unassigned_shards", 0), + entry("number_of_pending_tasks", 0), + entry("number_of_in_flight_fetch", 0), + entry("task_max_waiting_in_queue_millis", 0), + entry("active_shards_percent_as_number", 100.0)); + } + + @Test + public void elasticsearchWithYellowStatusIsUp() throws IOException { + BasicHttpEntity httpEntity = new BasicHttpEntity(); + httpEntity.setContent( + new ByteArrayInputStream(createJsonResult(200, "yellow").getBytes())); + + Response response = mock(Response.class); + StatusLine statusLine = mock(StatusLine.class); + + when(statusLine.getStatusCode()).thenReturn(200); + when(response.getStatusLine()).thenReturn(statusLine); + when(response.getEntity()).thenReturn(httpEntity); + when(this.restClient.performRequest(any(Request.class))).thenReturn(response); + + Health health = this.elasticsearchRestHealthIndicator.health(); + assertThat(health.getStatus()).isEqualTo(Status.UP); + + assertThat(health.getDetails()).contains(entry("cluster_name", "elasticsearch"), + entry("status", "yellow"), entry("timed_out", false), + entry("number_of_nodes", 1), entry("number_of_data_nodes", 1), + entry("active_primary_shards", 0), entry("active_shards", 0), + entry("relocating_shards", 0), entry("initializing_shards", 0), + entry("unassigned_shards", 0), entry("delayed_unassigned_shards", 0), + entry("number_of_pending_tasks", 0), + entry("number_of_in_flight_fetch", 0), + entry("task_max_waiting_in_queue_millis", 0), + entry("active_shards_percent_as_number", 100.0)); } @Test @@ -68,8 +111,10 @@ public class ElasticsearchRestHealthIndicatorTest { when(this.restClient.performRequest(any(Request.class))) .thenThrow(new IOException("Couldn't connect")); - assertThat(this.elasticsearchRestHealthIndicator.health().getStatus()) - .isEqualTo(Status.DOWN); + Health health = this.elasticsearchRestHealthIndicator.health(); + assertThat(health.getStatus()).isEqualTo(Status.DOWN); + assertThat(health.getDetails()) + .contains(entry("error", "java.io.IOException: Couldn't connect")); } @Test @@ -79,11 +124,14 @@ public class ElasticsearchRestHealthIndicatorTest { StatusLine statusLine = mock(StatusLine.class); when(statusLine.getStatusCode()).thenReturn(500); + when(statusLine.getReasonPhrase()).thenReturn("Internal server error"); when(response.getStatusLine()).thenReturn(statusLine); when(this.restClient.performRequest(any(Request.class))).thenReturn(response); - assertThat(this.elasticsearchRestHealthIndicator.health().getStatus()) - .isEqualTo(Status.DOWN); + Health health = this.elasticsearchRestHealthIndicator.health(); + assertThat(health.getStatus()).isEqualTo(Status.DOWN); + assertThat(health.getDetails()).contains(entry("statusCode", 500), + entry("reasonPhrase", "Internal server error")); } @Test @@ -100,8 +148,18 @@ public class ElasticsearchRestHealthIndicatorTest { when(response.getEntity()).thenReturn(httpEntity); when(this.restClient.performRequest(any(Request.class))).thenReturn(response); - assertThat(this.elasticsearchRestHealthIndicator.health().getStatus()) - .isEqualTo(Status.OUT_OF_SERVICE); + Health health = this.elasticsearchRestHealthIndicator.health(); + assertThat(health.getStatus()).isEqualTo(Status.OUT_OF_SERVICE); + assertThat(health.getDetails()).contains(entry("cluster_name", "elasticsearch"), + entry("status", "red"), entry("timed_out", false), + entry("number_of_nodes", 1), entry("number_of_data_nodes", 1), + entry("active_primary_shards", 0), entry("active_shards", 0), + entry("relocating_shards", 0), entry("initializing_shards", 0), + entry("unassigned_shards", 0), entry("delayed_unassigned_shards", 0), + entry("number_of_pending_tasks", 0), + entry("number_of_in_flight_fetch", 0), + entry("task_max_waiting_in_queue_millis", 0), + entry("active_shards_percent_as_number", 100.0)); } private String createJsonResult(int responseCode, String status) { From 036aace2cca7f33c0e96d3b222376b48947b735f Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Tue, 11 Dec 2018 11:48:07 +0100 Subject: [PATCH 2/2] Add details in Jest ElasticSearch HealthIndicator This commit polishes the previous one and aligns the Jest HealthIndicator with the changes made for the REST variant. Closes gh-15366 --- .../ElasticsearchJestHealthIndicator.java | 18 ++++++---- ...ElasticsearchJestHealthIndicatorTests.java | 28 +++++++++++++++ .../ElasticsearchRestHealthIndicatorTest.java | 35 ++++++------------- 3 files changed, 49 insertions(+), 32 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/elasticsearch/ElasticsearchJestHealthIndicator.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/elasticsearch/ElasticsearchJestHealthIndicator.java index a74786f660..796432a2fc 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/elasticsearch/ElasticsearchJestHealthIndicator.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/elasticsearch/ElasticsearchJestHealthIndicator.java @@ -16,14 +16,16 @@ package org.springframework.boot.actuate.elasticsearch; -import com.google.gson.JsonElement; -import com.google.gson.JsonParser; +import java.util.Map; + import io.searchbox.client.JestClient; import io.searchbox.client.JestResult; import org.springframework.boot.actuate.health.AbstractHealthIndicator; import org.springframework.boot.actuate.health.Health; import org.springframework.boot.actuate.health.HealthIndicator; +import org.springframework.boot.json.JsonParser; +import org.springframework.boot.json.JsonParserFactory; /** * {@link HealthIndicator} for Elasticsearch using a {@link JestClient}. @@ -37,7 +39,7 @@ public class ElasticsearchJestHealthIndicator extends AbstractHealthIndicator { private final JestClient jestClient; - private final JsonParser jsonParser = new JsonParser(); + private final JsonParser jsonParser = JsonParserFactory.getJsonParser(); public ElasticsearchJestHealthIndicator(JestClient jestClient) { super("Elasticsearch health check failed"); @@ -50,17 +52,19 @@ public class ElasticsearchJestHealthIndicator extends AbstractHealthIndicator { .execute(new io.searchbox.cluster.Health.Builder().build()); if (healthResult.getResponseCode() != 200 || !healthResult.isSucceeded()) { builder.down(); + builder.withDetail("statusCode", healthResult.getResponseCode()); } else { - JsonElement root = this.jsonParser.parse(healthResult.getJsonString()); - JsonElement status = root.getAsJsonObject().get("status"); - if (status.getAsString() - .equals(io.searchbox.cluster.Health.Status.RED.getKey())) { + Map response = this.jsonParser + .parseMap(healthResult.getJsonString()); + String status = (String) response.get("status"); + if (status.equals(io.searchbox.cluster.Health.Status.RED.getKey())) { builder.outOfService(); } else { builder.up(); } + builder.withDetails(response); } } diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/elasticsearch/ElasticsearchJestHealthIndicatorTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/elasticsearch/ElasticsearchJestHealthIndicatorTests.java index 82af2a8e18..82f2478051 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/elasticsearch/ElasticsearchJestHealthIndicatorTests.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/elasticsearch/ElasticsearchJestHealthIndicatorTests.java @@ -17,6 +17,7 @@ package org.springframework.boot.actuate.elasticsearch; import java.io.IOException; +import java.util.Map; import com.google.gson.Gson; import com.google.gson.JsonParser; @@ -31,6 +32,7 @@ import org.springframework.boot.actuate.health.Health; import org.springframework.boot.actuate.health.Status; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.entry; import static org.mockito.ArgumentMatchers.any; import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.mock; @@ -56,6 +58,16 @@ public class ElasticsearchJestHealthIndicatorTests { .willReturn(createJestResult(200, true, "green")); Health health = this.healthIndicator.health(); assertThat(health.getStatus()).isEqualTo(Status.UP); + assertHealthDetailsWithStatus(health.getDetails(), "green"); + } + + @Test + public void elasticsearchWithYellowStatusIsUp() throws IOException { + given(this.jestClient.execute(any(Action.class))) + .willReturn(createJestResult(200, true, "yellow")); + Health health = this.healthIndicator.health(); + assertThat(health.getStatus()).isEqualTo(Status.UP); + assertHealthDetailsWithStatus(health.getDetails(), "yellow"); } @SuppressWarnings("unchecked") @@ -83,6 +95,7 @@ public class ElasticsearchJestHealthIndicatorTests { .willReturn(createJestResult(500, false, "")); Health health = this.healthIndicator.health(); assertThat(health.getStatus()).isEqualTo(Status.DOWN); + assertThat(health.getDetails()).contains(entry("statusCode", 500)); } @SuppressWarnings("unchecked") @@ -92,6 +105,21 @@ public class ElasticsearchJestHealthIndicatorTests { .willReturn(createJestResult(200, true, "red")); Health health = this.healthIndicator.health(); assertThat(health.getStatus()).isEqualTo(Status.OUT_OF_SERVICE); + assertHealthDetailsWithStatus(health.getDetails(), "red"); + } + + private void assertHealthDetailsWithStatus(Map details, + String status) { + assertThat(details).contains(entry("cluster_name", "elasticsearch"), + entry("status", status), entry("timed_out", false), + entry("number_of_nodes", 1), entry("number_of_data_nodes", 1), + entry("active_primary_shards", 0), entry("active_shards", 0), + entry("relocating_shards", 0), entry("initializing_shards", 0), + entry("unassigned_shards", 0), entry("delayed_unassigned_shards", 0), + entry("number_of_pending_tasks", 0), + entry("number_of_in_flight_fetch", 0), + entry("task_max_waiting_in_queue_millis", 0), + entry("active_shards_percent_as_number", 100.0)); } private static JestResult createJestResult(int responseCode, boolean succeeded, diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/elasticsearch/ElasticsearchRestHealthIndicatorTest.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/elasticsearch/ElasticsearchRestHealthIndicatorTest.java index 7f3bd7427d..0387073731 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/elasticsearch/ElasticsearchRestHealthIndicatorTest.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/elasticsearch/ElasticsearchRestHealthIndicatorTest.java @@ -18,6 +18,7 @@ package org.springframework.boot.actuate.elasticsearch; import java.io.ByteArrayInputStream; import java.io.IOException; +import java.util.Map; import org.apache.http.StatusLine; import org.apache.http.entity.BasicHttpEntity; @@ -64,17 +65,7 @@ public class ElasticsearchRestHealthIndicatorTest { Health health = this.elasticsearchRestHealthIndicator.health(); assertThat(health.getStatus()).isEqualTo(Status.UP); - - assertThat(health.getDetails()).contains(entry("cluster_name", "elasticsearch"), - entry("status", "green"), entry("timed_out", false), - entry("number_of_nodes", 1), entry("number_of_data_nodes", 1), - entry("active_primary_shards", 0), entry("active_shards", 0), - entry("relocating_shards", 0), entry("initializing_shards", 0), - entry("unassigned_shards", 0), entry("delayed_unassigned_shards", 0), - entry("number_of_pending_tasks", 0), - entry("number_of_in_flight_fetch", 0), - entry("task_max_waiting_in_queue_millis", 0), - entry("active_shards_percent_as_number", 100.0)); + assertHealthDetailsWithStatus(health.getDetails(), "green"); } @Test @@ -93,17 +84,7 @@ public class ElasticsearchRestHealthIndicatorTest { Health health = this.elasticsearchRestHealthIndicator.health(); assertThat(health.getStatus()).isEqualTo(Status.UP); - - assertThat(health.getDetails()).contains(entry("cluster_name", "elasticsearch"), - entry("status", "yellow"), entry("timed_out", false), - entry("number_of_nodes", 1), entry("number_of_data_nodes", 1), - entry("active_primary_shards", 0), entry("active_shards", 0), - entry("relocating_shards", 0), entry("initializing_shards", 0), - entry("unassigned_shards", 0), entry("delayed_unassigned_shards", 0), - entry("number_of_pending_tasks", 0), - entry("number_of_in_flight_fetch", 0), - entry("task_max_waiting_in_queue_millis", 0), - entry("active_shards_percent_as_number", 100.0)); + assertHealthDetailsWithStatus(health.getDetails(), "yellow"); } @Test @@ -119,7 +100,6 @@ public class ElasticsearchRestHealthIndicatorTest { @Test public void elasticsearchIsDownByResponseCode() throws IOException { - Response response = mock(Response.class); StatusLine statusLine = mock(StatusLine.class); @@ -150,8 +130,13 @@ public class ElasticsearchRestHealthIndicatorTest { Health health = this.elasticsearchRestHealthIndicator.health(); assertThat(health.getStatus()).isEqualTo(Status.OUT_OF_SERVICE); - assertThat(health.getDetails()).contains(entry("cluster_name", "elasticsearch"), - entry("status", "red"), entry("timed_out", false), + assertHealthDetailsWithStatus(health.getDetails(), "red"); + } + + private void assertHealthDetailsWithStatus(Map details, + String status) { + assertThat(details).contains(entry("cluster_name", "elasticsearch"), + entry("status", status), entry("timed_out", false), entry("number_of_nodes", 1), entry("number_of_data_nodes", 1), entry("active_primary_shards", 0), entry("active_shards", 0), entry("relocating_shards", 0), entry("initializing_shards", 0),