Fix NPE in Feign interceptor

Fixes gh-37
This commit is contained in:
Dave Syer
2015-09-07 12:20:18 +01:00
parent 61212c3c9b
commit 2c3528e35b
2 changed files with 44 additions and 16 deletions

View File

@@ -17,6 +17,7 @@
package org.springframework.cloud.sleuth.instrument.web.client;
import static java.util.Collections.singletonList;
import static org.springframework.cloud.sleuth.Trace.NOT_SAMPLED_NAME;
import static org.springframework.cloud.sleuth.Trace.PARENT_ID_NAME;
import static org.springframework.cloud.sleuth.Trace.SPAN_ID_NAME;
import static org.springframework.cloud.sleuth.Trace.TRACE_ID_NAME;
@@ -73,8 +74,7 @@ public class TraceFeignClientAutoConfiguration {
public Decoder feignDecoder() {
return new ResponseEntityDecoder(new SpringDecoder(messageConverters)) {
@Override
public Object decode(Response response, Type type) throws IOException,
FeignException {
public Object decode(Response response, Type type) throws IOException, FeignException {
return super.decode(Response.create(response.status(), response.reason(),
headersWithTraceId(response.headers()), response.body()), type);
}
@@ -86,11 +86,14 @@ public class TraceFeignClientAutoConfiguration {
return new RequestInterceptor() {
@Override
public void apply(RequestTemplate template) {
template.header(TRACE_ID_NAME, getCurrentSpan().getTraceId());
setHeader(template, TRACE_ID_NAME, getCurrentSpan().getTraceId());
setHeader(template, SPAN_ID_NAME, getCurrentSpan().getSpanId());
setHeader(template, PARENT_ID_NAME, getParentId(getCurrentSpan()));
publish(new ClientSentEvent(this, getCurrentSpan()));
Span span = getCurrentSpan();
if (span != null) {
template.header(TRACE_ID_NAME, span.getTraceId());
setHeader(template, TRACE_ID_NAME, span.getTraceId());
setHeader(template, SPAN_ID_NAME, span.getSpanId());
setHeader(template, PARENT_ID_NAME, getParentId(span));
publish(new ClientSentEvent(this, span));
}
}
};
}
@@ -102,8 +105,7 @@ public class TraceFeignClientAutoConfiguration {
}
private String getParentId(Span span) {
return span.getParents() != null && !span.getParents().isEmpty() ? span
.getParents().get(0) : null;
return span.getParents() != null && !span.getParents().isEmpty() ? span.getParents().get(0) : null;
}
public void setHeader(RequestTemplate request, String name, String value) {
@@ -112,10 +114,13 @@ public class TraceFeignClientAutoConfiguration {
}
}
private Map<String, Collection<String>> headersWithTraceId(
Map<String, Collection<String>> headers) {
private Map<String, Collection<String>> headersWithTraceId(Map<String, Collection<String>> headers) {
Map<String, Collection<String>> newHeaders = new HashMap<>();
newHeaders.putAll(headers);
if (getCurrentSpan() == null) {
setHeader(newHeaders, NOT_SAMPLED_NAME, "");
return newHeaders;
}
setHeader(newHeaders, TRACE_ID_NAME, getCurrentSpan().getTraceId());
setHeader(newHeaders, SPAN_ID_NAME, getCurrentSpan().getSpanId());
setHeader(newHeaders, PARENT_ID_NAME, getParentId(getCurrentSpan()));

View File

@@ -8,9 +8,7 @@ import static org.springframework.cloud.sleuth.Trace.TRACE_ID_NAME;
import java.util.Arrays;
import java.util.List;
import com.netflix.loadbalancer.BaseLoadBalancer;
import com.netflix.loadbalancer.ILoadBalancer;
import com.netflix.loadbalancer.Server;
import org.junit.After;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.springframework.beans.factory.annotation.Autowired;
@@ -33,6 +31,10 @@ import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestMethod;
import org.springframework.web.bind.annotation.RestController;
import com.netflix.loadbalancer.BaseLoadBalancer;
import com.netflix.loadbalancer.ILoadBalancer;
import com.netflix.loadbalancer.Server;
@RunWith(SpringJUnit4ClassRunner.class)
@SpringApplicationConfiguration(classes = { TraceWebAutoConfiguration.class,
FeignTraceTest.TestConfiguration.class })
@@ -41,6 +43,20 @@ public class FeignTraceTest {
@Autowired
TestFeignInterface testFeignInterface;
@After
public void close() {
TraceContextHolder.removeCurrentSpan();
}
@Test
public void shouldWorkWhenNotTracing() {
// when
ResponseEntity<String> response = testFeignInterface.getNoTrace();
// then
assertThat(getHeader(response, TRACE_ID_NAME)).isNull();
}
@Test
public void shouldAttachTraceIdWhenUsingFeignClient() {
@@ -62,14 +78,15 @@ public class FeignTraceTest {
private String getHeader(ResponseEntity<String> response, String name) {
List<String> headers = response.getHeaders().get(name);
assertThat(headers).asList().isNotEmpty();
return headers.get(0);
return headers==null || headers.isEmpty() ? null : headers.get(0);
}
@FeignClient("fooservice")
public interface TestFeignInterface {
@RequestMapping(method = RequestMethod.GET, value = "/traceid")
ResponseEntity<String> getTraceId();
@RequestMapping(method = RequestMethod.GET, value = "/notrace")
ResponseEntity<String> getNoTrace();
}
@Configuration
@@ -87,6 +104,12 @@ public class FeignTraceTest {
@RestController
public static class FooController {
@RequestMapping(value = "/notrace", method = RequestMethod.GET)
public String notrace(@RequestHeader(name=TRACE_ID_NAME, required=false) String traceId) {
assertThat(traceId).isNull();
return "OK";
}
@RequestMapping(value = "/traceid", method = RequestMethod.GET)
public String traceId(@RequestHeader(TRACE_ID_NAME) String traceId,
@RequestHeader(SPAN_ID_NAME) String spanId,