From eed5a6f10baeddbe8caa8b9189ef52cf61103031 Mon Sep 17 00:00:00 2001 From: Spencer Gibb Date: Mon, 7 Mar 2016 14:08:27 -0700 Subject: [PATCH] Test RestTemplate with ribbon when metrics are enabled. --- .../metrics/DefaultMetricsTagProvider.java | 20 +++++-- .../netflix/ribbon/RibbonHttpRequest.java | 4 +- .../ribbon/RibbonLoadBalancerClient.java | 11 ++++ .../metrics/MetricsRestTemplateTests.java | 58 +++++++++++++++++++ .../RibbonClientHttpRequestFactoryTests.java | 24 ++++---- 5 files changed, 97 insertions(+), 20 deletions(-) create mode 100644 spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/metrics/MetricsRestTemplateTests.java diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/metrics/DefaultMetricsTagProvider.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/metrics/DefaultMetricsTagProvider.java index 6d3e96ff..4e319b3a 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/metrics/DefaultMetricsTagProvider.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/metrics/DefaultMetricsTagProvider.java @@ -41,8 +41,8 @@ public class DefaultMetricsTagProvider implements MetricsTagProvider { String status; try { - status = (response == null) ? "CLIENT_ERROR" - : ((Integer) response.getRawStatusCode()).toString(); + status = (response == null) ? "CLIENT_ERROR" : ((Integer) response + .getRawStatusCode()).toString(); } catch (IOException e) { status = "IO_ERROR"; @@ -50,9 +50,13 @@ public class DefaultMetricsTagProvider implements MetricsTagProvider { String host = request.getURI().getHost(); - return ImmutableMap.of("method", request.getMethod().name(), "uri", - sanitizeUrlTemplate(urlTemplate.replaceAll("^https?://[^/]+/", "")), - "status", status, "clientName", host != null ? host : "none"); + String strippedUrlTemplate = urlTemplate.replaceAll("^https?://[^/]+/", ""); + //@formatter:off + return ImmutableMap.of("method", request.getMethod().name(), + "uri", sanitizeUrlTemplate(strippedUrlTemplate), + "status", status, "clientName", + host != null ? host : "none"); + //@formatter:on } @Override @@ -91,6 +95,10 @@ public class DefaultMetricsTagProvider implements MetricsTagProvider { * Atlas take place via query parameters */ private String sanitizeUrlTemplate(String urlTemplate) { - return urlTemplate.replaceAll("/", "_").replaceAll("[{}]", "-"); + String sanitized = urlTemplate.replaceAll("/", "_").replaceAll("[{}]", "-"); + if (!StringUtils.hasText(sanitized)) { + sanitized = "none"; + } + return sanitized; } } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonHttpRequest.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonHttpRequest.java index 121d4b49..04bbf983 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonHttpRequest.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonHttpRequest.java @@ -96,7 +96,7 @@ public class RibbonHttpRequest extends AbstractClientHttpRequest { for (String name : headers.keySet()) { // apache http RequestContent pukes if there is a body and // the dynamic headers are already present - if (!isDynamic(name) || outputStream == null) { + if (!isDynamic(name)) { List values = headers.get(name); for (String value : values) { builder.header(name, value); @@ -106,6 +106,6 @@ public class RibbonHttpRequest extends AbstractClientHttpRequest { } private boolean isDynamic(String name) { - return name.equals("Content-Length") || name.equals("Transfer-Encoding"); + return name.equalsIgnoreCase("Content-Length") || name.equalsIgnoreCase("Transfer-Encoding"); } } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancerClient.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancerClient.java index fc0f7bc8..a462e15b 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancerClient.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancerClient.java @@ -177,6 +177,17 @@ public class RibbonLoadBalancerClient implements LoadBalancerClient { public Server getServer() { return this.server; } + + @Override + public String toString() { + final StringBuffer sb = new StringBuffer("RibbonServer{"); + sb.append("serviceId='").append(serviceId).append('\''); + sb.append(", server=").append(server); + sb.append(", secure=").append(secure); + sb.append(", metadata=").append(metadata); + sb.append('}'); + return sb.toString(); + } } } diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/metrics/MetricsRestTemplateTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/metrics/MetricsRestTemplateTests.java new file mode 100644 index 00000000..c3a04308 --- /dev/null +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/metrics/MetricsRestTemplateTests.java @@ -0,0 +1,58 @@ +/* + * Copyright 2013-2015 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.cloud.netflix.metrics; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.boot.test.SpringApplicationConfiguration; +import org.springframework.boot.test.WebIntegrationTest; +import org.springframework.cloud.netflix.ribbon.RibbonClientHttpRequestFactory; +import org.springframework.cloud.netflix.ribbon.RibbonClientHttpRequestFactoryTests; +import org.springframework.http.client.ClientHttpRequestFactory; +import org.springframework.http.client.InterceptingClientHttpRequestFactory; +import org.springframework.test.annotation.DirtiesContext; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; +import org.springframework.test.util.ReflectionTestUtils; + +import static org.junit.Assert.assertTrue; + +/** + * @author Spencer Gibb + */ +@RunWith(SpringJUnit4ClassRunner.class) +@SpringApplicationConfiguration(classes = MetricsRestTemplateTests.App.class) +@WebIntegrationTest(value = { "spring.application.name=ribbonclienttest", + "spring.jmx.enabled=true" }, randomPort = true) +@DirtiesContext +public class MetricsRestTemplateTests extends RibbonClientHttpRequestFactoryTests { + + @Test + @Override + public void requestFactoryIsRibbon() { + ClientHttpRequestFactory requestFactory = this.restTemplate.getRequestFactory(); + assertTrue("wrong RequestFactory type: " + requestFactory.getClass(), + requestFactory instanceof InterceptingClientHttpRequestFactory); + + InterceptingClientHttpRequestFactory intercepting = (InterceptingClientHttpRequestFactory) requestFactory; + + Object realRequestFactory = ReflectionTestUtils.getField(intercepting, + "requestFactory"); + assertTrue("wrong RequestFactory type: " + realRequestFactory.getClass(), + realRequestFactory instanceof RibbonClientHttpRequestFactory); + } + +} diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonClientHttpRequestFactoryTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonClientHttpRequestFactoryTests.java index b7f1224e..de2732f7 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonClientHttpRequestFactoryTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonClientHttpRequestFactoryTests.java @@ -69,7 +69,7 @@ public class RibbonClientHttpRequestFactoryTests { @LoadBalanced @Autowired - private RestTemplate restTemplate; + protected RestTemplate restTemplate; @Test public void requestFactoryIsRibbon() { @@ -149,7 +149,7 @@ public class RibbonClientHttpRequestFactoryTests { @EnableAutoConfiguration @RestController @RibbonClient(value = "simple", configuration = SimpleRibbonClientConfiguration.class) - protected static class App { + public static class App { @LoadBalanced @Bean @@ -187,17 +187,17 @@ public class RibbonClientHttpRequestFactoryTests { return "hello " + param; } } -} -@Configuration -class SimpleRibbonClientConfiguration { + @Configuration + static class SimpleRibbonClientConfiguration { - @Value("${local.server.port}") - private int port = 0; + @Value("${local.server.port}") + private int port = 0; + + @Bean + public ServerList ribbonServerList() { + return new StaticServerList<>(new Server("localhost", this.port)); + } - @Bean - public ServerList ribbonServerList() { - return new StaticServerList<>(new Server("localhost", this.port)); } - -} \ No newline at end of file +}