From 537fc5cd87b0257bb2284763c9968d31a7ce6a4c Mon Sep 17 00:00:00 2001 From: Johnny Lim Date: Fri, 30 Aug 2019 01:35:04 +0900 Subject: [PATCH] Improve handling of non-standard status codes in RestTemplate metrics (#1564) This commit backports spring-projects/spring-boot#17991. --- .../web/client/RestTemplateExchangeTags.java | 40 ++++--- .../client/RestTemplateExchangeTagsTest.java | 111 ++++++++++++++++++ 2 files changed, 133 insertions(+), 18 deletions(-) create mode 100644 micrometer-spring-legacy/src/test/java/io/micrometer/spring/web/client/RestTemplateExchangeTagsTest.java diff --git a/micrometer-spring-legacy/src/main/java/io/micrometer/spring/web/client/RestTemplateExchangeTags.java b/micrometer-spring-legacy/src/main/java/io/micrometer/spring/web/client/RestTemplateExchangeTags.java index a46a336157..7cc37ff4a4 100644 --- a/micrometer-spring-legacy/src/main/java/io/micrometer/spring/web/client/RestTemplateExchangeTags.java +++ b/micrometer-spring-legacy/src/main/java/io/micrometer/spring/web/client/RestTemplateExchangeTags.java @@ -25,6 +25,9 @@ import java.io.IOException; import java.net.URI; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; /** * Factory methods for creating {@link Tag Tags} related to a request-response exchange @@ -47,6 +50,18 @@ public final class RestTemplateExchangeTags { private static final Tag OUTCOME_SERVER_ERROR = Tag.of("outcome", "SERVER_ERROR"); + private static final Map SERIES_OUTCOMES; + + static { + Map seriesOutcomes = new HashMap<>(); + seriesOutcomes.put(HttpStatus.Series.INFORMATIONAL, OUTCOME_INFORMATIONAL); + seriesOutcomes.put(HttpStatus.Series.SUCCESSFUL, OUTCOME_SUCCESS); + seriesOutcomes.put(HttpStatus.Series.REDIRECTION, OUTCOME_REDIRECTION); + seriesOutcomes.put(HttpStatus.Series.CLIENT_ERROR, OUTCOME_CLIENT_ERROR); + seriesOutcomes.put(HttpStatus.Series.SERVER_ERROR, OUTCOME_SERVER_ERROR); + SERIES_OUTCOMES = Collections.unmodifiableMap(seriesOutcomes); + } + private RestTemplateExchangeTags() { } @@ -141,26 +156,15 @@ public static Tag clientName(HttpRequest request) { public static Tag outcome(ClientHttpResponse response) { try { if (response != null) { - HttpStatus statusCode = response.getStatusCode(); - if (statusCode.is1xxInformational()) { - return OUTCOME_INFORMATIONAL; - } - if (statusCode.is2xxSuccessful()) { - return OUTCOME_SUCCESS; - } - if (statusCode.is3xxRedirection()) { - return OUTCOME_REDIRECTION; - } - if (statusCode.is4xxClientError()) { - return OUTCOME_CLIENT_ERROR; - } - if (statusCode.is5xxServerError()) { - return OUTCOME_SERVER_ERROR; + HttpStatus.Series series = HttpStatus.Series.valueOf(response.getRawStatusCode()); + if (series != null) { + return SERIES_OUTCOMES.getOrDefault(series, OUTCOME_UNKNOWN); } } - return OUTCOME_UNKNOWN; - } catch (IOException | IllegalArgumentException e) { - return OUTCOME_UNKNOWN; } + catch (IOException | IllegalArgumentException ex) { + // Continue + } + return OUTCOME_UNKNOWN; } } diff --git a/micrometer-spring-legacy/src/test/java/io/micrometer/spring/web/client/RestTemplateExchangeTagsTest.java b/micrometer-spring-legacy/src/test/java/io/micrometer/spring/web/client/RestTemplateExchangeTagsTest.java new file mode 100644 index 0000000000..4473407d4c --- /dev/null +++ b/micrometer-spring-legacy/src/test/java/io/micrometer/spring/web/client/RestTemplateExchangeTagsTest.java @@ -0,0 +1,111 @@ +/** + * Copyright 2019 Pivotal Software, Inc. + *

+ * 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 io.micrometer.spring.web.client; + +import io.micrometer.core.instrument.Tag; +import org.junit.Test; +import org.springframework.http.HttpStatus; +import org.springframework.http.client.ClientHttpResponse; +import org.springframework.mock.http.client.MockClientHttpResponse; + +import java.io.IOException; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; + +/** + * Tests for {@link RestTemplateExchangeTags}. + * + * @author Nishant Raut + * @author Brian Clozel + */ +public class RestTemplateExchangeTagsTest { + + @Test + public void outcomeTagIsUnknownWhenResponseIsNull() { + Tag tag = RestTemplateExchangeTags.outcome(null); + assertThat(tag.getValue()).isEqualTo("UNKNOWN"); + } + + @Test + public void outcomeTagIsInformationalWhenResponseIs1xx() { + ClientHttpResponse response = new MockClientHttpResponse("foo".getBytes(), HttpStatus.CONTINUE); + Tag tag = RestTemplateExchangeTags.outcome(response); + assertThat(tag.getValue()).isEqualTo("INFORMATIONAL"); + } + + @Test + public void outcomeTagIsSuccessWhenResponseIs2xx() { + ClientHttpResponse response = new MockClientHttpResponse("foo".getBytes(), HttpStatus.OK); + Tag tag = RestTemplateExchangeTags.outcome(response); + assertThat(tag.getValue()).isEqualTo("SUCCESS"); + } + + @Test + public void outcomeTagIsRedirectionWhenResponseIs3xx() { + ClientHttpResponse response = new MockClientHttpResponse("foo".getBytes(), HttpStatus.MOVED_PERMANENTLY); + Tag tag = RestTemplateExchangeTags.outcome(response); + assertThat(tag.getValue()).isEqualTo("REDIRECTION"); + } + + @Test + public void outcomeTagIsClientErrorWhenResponseIs4xx() { + ClientHttpResponse response = new MockClientHttpResponse("foo".getBytes(), HttpStatus.BAD_REQUEST); + Tag tag = RestTemplateExchangeTags.outcome(response); + assertThat(tag.getValue()).isEqualTo("CLIENT_ERROR"); + } + + @Test + public void outcomeTagIsServerErrorWhenResponseIs5xx() { + ClientHttpResponse response = new MockClientHttpResponse("foo".getBytes(), HttpStatus.BAD_GATEWAY); + Tag tag = RestTemplateExchangeTags.outcome(response); + assertThat(tag.getValue()).isEqualTo("SERVER_ERROR"); + } + + @Test + public void outcomeTagIsUnknownWhenResponseThrowsIOException() throws Exception { + ClientHttpResponse response = mock(ClientHttpResponse.class); + given(response.getRawStatusCode()).willThrow(IOException.class); + Tag tag = RestTemplateExchangeTags.outcome(response); + assertThat(tag.getValue()).isEqualTo("UNKNOWN"); + } + + @Test + public void outcomeTagIsUnknownForCustomResponseStatus() throws Exception { + ClientHttpResponse response = mock(ClientHttpResponse.class); + given(response.getRawStatusCode()).willThrow(IllegalArgumentException.class); + Tag tag = RestTemplateExchangeTags.outcome(response); + assertThat(tag.getValue()).isEqualTo("UNKNOWN"); + } + + @Test + public void outcomeTagIsClientErrorWhenResponseIsNonStandardInClientSeries() throws IOException { + ClientHttpResponse response = mock(ClientHttpResponse.class); + given(response.getRawStatusCode()).willReturn(490); + Tag tag = RestTemplateExchangeTags.outcome(response); + assertThat(tag.getValue()).isEqualTo("CLIENT_ERROR"); + } + + @Test + public void outcomeTagIsUnknownWhenResponseStatusIsInUnknownSeries() throws IOException { + ClientHttpResponse response = mock(ClientHttpResponse.class); + given(response.getRawStatusCode()).willReturn(701); + Tag tag = RestTemplateExchangeTags.outcome(response); + assertThat(tag.getValue()).isEqualTo("UNKNOWN"); + } + +}