Wrapping LoadBalancerFeignClient in tracing representation
without this change when using SC-Netflix 1.2.0 Feign can't call an external URL fixes #393
This commit is contained in:
@@ -141,6 +141,11 @@
|
||||
<artifactId>h2</artifactId>
|
||||
<scope>test</scope>
|
||||
</dependency>
|
||||
<dependency>
|
||||
<groupId>org.springframework.cloud</groupId>
|
||||
<artifactId>spring-cloud-starter-eureka</artifactId>
|
||||
<scope>test</scope>
|
||||
</dependency>
|
||||
</dependencies>
|
||||
|
||||
</project>
|
||||
|
||||
@@ -15,18 +15,18 @@
|
||||
*/
|
||||
package org.springframework.cloud.sleuth.instrument.web;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.lang.invoke.MethodHandles;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Collections;
|
||||
import java.util.Enumeration;
|
||||
import java.util.regex.Pattern;
|
||||
import javax.servlet.FilterChain;
|
||||
import javax.servlet.ServletException;
|
||||
import javax.servlet.ServletRequest;
|
||||
import javax.servlet.ServletResponse;
|
||||
import javax.servlet.http.HttpServletRequest;
|
||||
import javax.servlet.http.HttpServletResponse;
|
||||
import java.io.IOException;
|
||||
import java.lang.invoke.MethodHandles;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Collections;
|
||||
import java.util.Enumeration;
|
||||
import java.util.regex.Pattern;
|
||||
|
||||
import org.apache.commons.logging.Log;
|
||||
import org.apache.commons.logging.LogFactory;
|
||||
@@ -126,7 +126,7 @@ public class TraceFilter extends GenericFilterBean {
|
||||
continueSpan(request, spanFromRequest);
|
||||
}
|
||||
if (log.isDebugEnabled()) {
|
||||
log.debug("Received a request to uri [" + uri + "] that should be skipped [" + skip + "]");
|
||||
log.debug("Received a request to uri [" + uri + "] that should not be sampled [" + skip + "]");
|
||||
}
|
||||
// in case of a response with exception status a exception controller will close the span
|
||||
if (!httpStatusSuccessful(response) && isSpanContinued(request)) {
|
||||
|
||||
@@ -1,6 +1,9 @@
|
||||
package org.springframework.cloud.sleuth.instrument.web.client.feign;
|
||||
|
||||
import org.springframework.beans.factory.BeanFactory;
|
||||
import org.springframework.cloud.netflix.feign.ribbon.CachingSpringLoadBalancerFactory;
|
||||
import org.springframework.cloud.netflix.feign.ribbon.LoadBalancerFeignClient;
|
||||
import org.springframework.cloud.netflix.ribbon.SpringClientFactory;
|
||||
|
||||
import feign.Client;
|
||||
|
||||
@@ -14,14 +17,39 @@ final class TraceFeignObjectWrapper {
|
||||
|
||||
private final BeanFactory beanFactory;
|
||||
|
||||
private CachingSpringLoadBalancerFactory cachingSpringLoadBalancerFactory;
|
||||
private SpringClientFactory springClientFactory;
|
||||
|
||||
TraceFeignObjectWrapper(BeanFactory beanFactory) {
|
||||
this.beanFactory = beanFactory;
|
||||
}
|
||||
|
||||
Object wrap(Object bean) {
|
||||
if (bean instanceof Client && !(bean instanceof TraceFeignClient)) {
|
||||
if (bean instanceof LoadBalancerFeignClient) {
|
||||
LoadBalancerFeignClient client = ((LoadBalancerFeignClient) bean);
|
||||
return new TraceLoadBalancerFeignClient(
|
||||
client.getDelegate(), factory(),
|
||||
clientFactory(), this.beanFactory);
|
||||
}
|
||||
return new TraceFeignClient(this.beanFactory, (Client) bean);
|
||||
}
|
||||
return bean;
|
||||
}
|
||||
|
||||
CachingSpringLoadBalancerFactory factory() {
|
||||
if (this.cachingSpringLoadBalancerFactory == null) {
|
||||
this.cachingSpringLoadBalancerFactory = this.beanFactory
|
||||
.getBean(CachingSpringLoadBalancerFactory.class);
|
||||
}
|
||||
return this.cachingSpringLoadBalancerFactory;
|
||||
}
|
||||
|
||||
SpringClientFactory clientFactory() {
|
||||
if (this.springClientFactory == null) {
|
||||
this.springClientFactory = this.beanFactory
|
||||
.getBean(SpringClientFactory.class);
|
||||
}
|
||||
return this.springClientFactory;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,28 @@
|
||||
package org.springframework.cloud.sleuth.instrument.web.client.feign;
|
||||
|
||||
import org.springframework.beans.factory.BeanFactory;
|
||||
import org.springframework.cloud.netflix.feign.ribbon.CachingSpringLoadBalancerFactory;
|
||||
import org.springframework.cloud.netflix.feign.ribbon.LoadBalancerFeignClient;
|
||||
import org.springframework.cloud.netflix.ribbon.SpringClientFactory;
|
||||
|
||||
import feign.Client;
|
||||
|
||||
/**
|
||||
* We need to wrap the {@link LoadBalancerFeignClient} into a trace representation
|
||||
* due to casts in {@link org.springframework.cloud.netflix.feign.FeignClientFactoryBean}.
|
||||
*
|
||||
* @author Marcin Grzejszczak
|
||||
* @since 1.0.7
|
||||
*/
|
||||
class TraceLoadBalancerFeignClient extends LoadBalancerFeignClient {
|
||||
|
||||
public TraceLoadBalancerFeignClient(Client delegate,
|
||||
CachingSpringLoadBalancerFactory lbClientFactory,
|
||||
SpringClientFactory clientFactory, BeanFactory beanFactory) {
|
||||
super(wrap(delegate, beanFactory), lbClientFactory, clientFactory);
|
||||
}
|
||||
|
||||
private static Client wrap(Client delegate, BeanFactory beanFactory) {
|
||||
return (Client) new TraceFeignObjectWrapper(beanFactory).wrap(delegate);
|
||||
}
|
||||
}
|
||||
@@ -28,6 +28,7 @@ import org.springframework.boot.test.TestRestTemplate;
|
||||
import org.springframework.boot.test.WebIntegrationTest;
|
||||
import org.springframework.cloud.netflix.feign.EnableFeignClients;
|
||||
import org.springframework.cloud.netflix.feign.FeignClient;
|
||||
import org.springframework.cloud.sleuth.Tracer;
|
||||
import org.springframework.cloud.sleuth.sampler.AlwaysSampler;
|
||||
import org.springframework.cloud.sleuth.util.ExceptionUtils;
|
||||
import org.springframework.context.annotation.Bean;
|
||||
@@ -54,6 +55,7 @@ import static org.assertj.core.api.BDDAssertions.then;
|
||||
public class Issue350Tests {
|
||||
|
||||
RestTemplate template = new TestRestTemplate();
|
||||
@Autowired Tracer tracer;
|
||||
|
||||
@Before
|
||||
public void setup() {
|
||||
@@ -64,6 +66,7 @@ public class Issue350Tests {
|
||||
public void should_successfully_work_without_hystrix() {
|
||||
this.template.getForEntity("http://localhost:9988/sleuth/test-not-ok", String.class);
|
||||
then(ExceptionUtils.getLastException()).isNull();
|
||||
then(this.tracer.getCurrentSpan()).isNull();
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -30,6 +30,7 @@ import org.springframework.boot.test.SpringApplicationConfiguration;
|
||||
import org.springframework.boot.test.WebIntegrationTest;
|
||||
import org.springframework.cloud.netflix.feign.EnableFeignClients;
|
||||
import org.springframework.cloud.netflix.feign.FeignClient;
|
||||
import org.springframework.cloud.sleuth.Tracer;
|
||||
import org.springframework.cloud.sleuth.assertions.SleuthAssertions;
|
||||
import org.springframework.cloud.sleuth.sampler.AlwaysSampler;
|
||||
import org.springframework.cloud.sleuth.util.ExceptionUtils;
|
||||
@@ -64,6 +65,7 @@ public class Issue362Tests {
|
||||
|
||||
RestTemplate template = new RestTemplate();
|
||||
@Autowired FeignComponentAsserter feignComponentAsserter;
|
||||
@Autowired Tracer tracer;
|
||||
|
||||
@Before
|
||||
public void setup() {
|
||||
@@ -77,8 +79,9 @@ public class Issue362Tests {
|
||||
|
||||
ResponseEntity<String> response = this.template.getForEntity(securedURl, String.class);
|
||||
|
||||
SleuthAssertions.then(response.getBody()).isEqualTo("I'm OK");
|
||||
then(response.getBody()).isEqualTo("I'm OK");
|
||||
then(ExceptionUtils.getLastException()).isNull();
|
||||
then(this.tracer.getCurrentSpan()).isNull();
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -93,6 +96,7 @@ public class Issue362Tests {
|
||||
then(ExceptionUtils.getLastException()).isNull();
|
||||
then(this.feignComponentAsserter.executedComponents)
|
||||
.containsEntry(ErrorDecoder.class, true);
|
||||
then(this.tracer.getCurrentSpan()).isNull();
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,154 @@
|
||||
/*
|
||||
* Copyright 2013-2016 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.sleuth.instrument.web.client.feign.issues.issue393;
|
||||
|
||||
import java.util.Date;
|
||||
import java.util.Map;
|
||||
import java.util.concurrent.ConcurrentHashMap;
|
||||
import java.util.concurrent.ExecutionException;
|
||||
|
||||
import com.fasterxml.jackson.annotation.JsonInclude;
|
||||
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
|
||||
import com.fasterxml.jackson.databind.annotation.JsonPOJOBuilder;
|
||||
|
||||
import org.junit.After;
|
||||
import org.junit.Before;
|
||||
import org.junit.BeforeClass;
|
||||
import org.junit.Test;
|
||||
import org.junit.runner.RunWith;
|
||||
import org.springframework.beans.factory.annotation.Autowired;
|
||||
import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
|
||||
import org.springframework.boot.test.SpringApplicationConfiguration;
|
||||
import org.springframework.boot.test.WebIntegrationTest;
|
||||
import org.springframework.cloud.client.discovery.EnableDiscoveryClient;
|
||||
import org.springframework.cloud.netflix.feign.EnableFeignClients;
|
||||
import org.springframework.cloud.netflix.feign.FeignClient;
|
||||
import org.springframework.cloud.sleuth.Tracer;
|
||||
import org.springframework.cloud.sleuth.assertions.SleuthAssertions;
|
||||
import org.springframework.cloud.sleuth.sampler.AlwaysSampler;
|
||||
import org.springframework.cloud.sleuth.trace.TestSpanContextHolder;
|
||||
import org.springframework.cloud.sleuth.util.ExceptionUtils;
|
||||
import org.springframework.context.annotation.Bean;
|
||||
import org.springframework.context.annotation.Configuration;
|
||||
import org.springframework.http.HttpStatus;
|
||||
import org.springframework.http.ResponseEntity;
|
||||
import org.springframework.test.context.TestPropertySource;
|
||||
import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
|
||||
import org.springframework.web.bind.annotation.PathVariable;
|
||||
import org.springframework.web.bind.annotation.RequestMapping;
|
||||
import org.springframework.web.bind.annotation.RequestMethod;
|
||||
import org.springframework.web.bind.annotation.ResponseStatus;
|
||||
import org.springframework.web.bind.annotation.RestController;
|
||||
import org.springframework.web.client.RestTemplate;
|
||||
|
||||
import feign.Logger;
|
||||
import feign.Response;
|
||||
import feign.RetryableException;
|
||||
import feign.Retryer;
|
||||
import feign.codec.ErrorDecoder;
|
||||
|
||||
import static org.assertj.core.api.Assertions.fail;
|
||||
import static org.assertj.core.api.Assertions.registerCustomDateFormat;
|
||||
import static org.assertj.core.api.Assertions.registerFormatterForType;
|
||||
import static org.assertj.core.api.BDDAssertions.then;
|
||||
|
||||
/**
|
||||
* @author Marcin Grzejszczak
|
||||
*/
|
||||
@RunWith(SpringJUnit4ClassRunner.class)
|
||||
@SpringApplicationConfiguration(Application.class)
|
||||
@WebIntegrationTest
|
||||
@TestPropertySource(properties = {"spring.application.name=demo-feign-uri",
|
||||
"server.port=9978"})
|
||||
public class Issue393Tests {
|
||||
|
||||
RestTemplate template = new RestTemplate();
|
||||
@Autowired Tracer tracer;
|
||||
|
||||
@Before
|
||||
public void open() {
|
||||
TestSpanContextHolder.removeCurrentSpan();
|
||||
ExceptionUtils.setFail(true);
|
||||
}
|
||||
|
||||
@After
|
||||
public void close() {
|
||||
TestSpanContextHolder.removeCurrentSpan();
|
||||
}
|
||||
|
||||
@Test
|
||||
public void should_successfully_work_when_service_discovery_is_on_classpath_and_feign_uses_url() {
|
||||
String url = "http://localhost:9978/hello/mikesarver";
|
||||
|
||||
ResponseEntity<String> response = this.template.getForEntity(url, String.class);
|
||||
|
||||
then(response.getBody()).isEqualTo("mikesarver foo");
|
||||
then(ExceptionUtils.getLastException()).isNull();
|
||||
then(this.tracer.getCurrentSpan()).isNull();
|
||||
}
|
||||
}
|
||||
|
||||
@Configuration
|
||||
@EnableAutoConfiguration
|
||||
@EnableFeignClients
|
||||
@EnableDiscoveryClient
|
||||
class Application {
|
||||
|
||||
@Bean
|
||||
public DemoController demoController(MyNameRemote myNameRemote) {
|
||||
return new DemoController(myNameRemote);
|
||||
}
|
||||
|
||||
@Bean
|
||||
public feign.Logger.Level feignLoggerLevel() {
|
||||
return feign.Logger.Level.BASIC;
|
||||
}
|
||||
|
||||
@Bean
|
||||
public AlwaysSampler defaultSampler() {
|
||||
return new AlwaysSampler();
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
@FeignClient(name="no-name",
|
||||
url="http://localhost:9978")
|
||||
interface MyNameRemote {
|
||||
|
||||
@RequestMapping(value = "/name/{id}", method = RequestMethod.GET)
|
||||
String getName(@PathVariable("id") String id);
|
||||
}
|
||||
|
||||
@RestController
|
||||
class DemoController {
|
||||
|
||||
private final MyNameRemote myNameRemote;
|
||||
|
||||
public DemoController(MyNameRemote myNameRemote) {
|
||||
this.myNameRemote = myNameRemote;
|
||||
}
|
||||
|
||||
@RequestMapping(value = "/hello/{name}")
|
||||
public String getHello(@PathVariable("name") String name) {
|
||||
return myNameRemote.getName(name) + " foo";
|
||||
}
|
||||
|
||||
@RequestMapping(value = "/name/{name}")
|
||||
public String getName(@PathVariable("name") String name) {
|
||||
return name;
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user