SpringCloud/spring-cloud-sleuth
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Updates feign client and tests around latest Brave

Browse Source
This commit is contained in:
Adrian Cole
2020-02-13 14:22:25 -08:00
parent aefa2a0119
commit d97f4d22de
3 changed files with 118 additions and 74 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceLoadBalancerFeignClient.java
View File

@@ -17,7 +17,6 @@
package org.springframework.cloud.sleuth.instrument.web.client.feign;
import java.io.IOException;
import java.util.HashMap;
import brave.Span;
import brave.Tracer;
@@ -86,9 +85,8 @@ public class TraceLoadBalancerFeignClient extends LoadBalancerFeignClient {
log.debug(
"General exception was thrown, so most likely the traced client wasn't called. Falling back to a manual span");
}
fallbackSpan = tracingFeignClient().handleSend(
new HashMap<>(request.headers()), request, fallbackSpan);
tracingFeignClient().handleReceive(fallbackSpan, response, e);
tracingFeignClient().handleSendAndReceive(fallbackSpan, request, response,
e);
}
throw e;
}

140
spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TracingFeignClient.java
View File

@@ -17,6 +17,7 @@
package org.springframework.cloud.sleuth.instrument.web.client.feign;
import java.io.IOException;
import java.net.URI;
import java.nio.charset.Charset;
import java.util.Collection;
import java.util.Collections;
@@ -28,7 +29,6 @@ import brave.Tracer;
import brave.http.HttpClientHandler;
import brave.http.HttpTracing;
import brave.propagation.Propagation;
import brave.propagation.TraceContext;
import feign.Client;
import feign.Request;
import feign.Response;
@@ -72,14 +72,11 @@ final class TracingFeignClient implements Client {
final Client delegate;
final HttpClientHandler<Request, Response> handler;
final TraceContext.Injector<Map<String, Collection<String>>> injector;
final HttpClientHandler<brave.http.HttpClientRequest, brave.http.HttpClientResponse> handler;
TracingFeignClient(HttpTracing httpTracing, Client delegate) {
this.tracer = httpTracing.tracing().tracer();
this.handler = HttpClientHandler.create(httpTracing, new HttpAdapter());
this.injector = httpTracing.tracing().propagation().injector(SETTER);
this.handler = HttpClientHandler.create(httpTracing);
this.delegate = delegate;
}
@@ -88,74 +85,135 @@ final class TracingFeignClient implements Client {
}
@Override
public Response execute(Request request, Request.Options options) throws IOException {
Map<String, Collection<String>> headers = new LinkedHashMap<>(request.headers());
Span span = handleSend(headers, request, null);
public Response execute(Request req, Request.Options options) throws IOException {
HttpClientRequest request = new HttpClientRequest(req);
Span span = this.handler.handleSend(request);
if (log.isDebugEnabled()) {
log.debug("Handled send of " + span);
}
Response response = null;
HttpClientResponse response = null;
Throwable error = null;
try (Tracer.SpanInScope ws = this.tracer.withSpanInScope(span)) {
response = this.delegate.execute(modifiedRequest(request, headers), options);
return response;
Response res = this.delegate.execute(request.build(), options);
if (res != null) {
response = new HttpClientResponse(res);
}
else { // possibly null on bad implementation or mocks
response = new HttpClientResponse(
Response.builder().request(req).build());
}
return res;
}
catch (IOException | RuntimeException | Error e) {
error = e;
throw e;
}
finally {
handleReceive(span, response, error);
this.handler.handleReceive(response, error, span);
if (log.isDebugEnabled()) {
log.debug("Handled receive of " + span);
}
}
}
Span handleSend(Map<String, Collection<String>> headers, Request request,
Span clientSpan) {
if (clientSpan != null) {
return this.handler.handleSend(this.injector, headers, request, clientSpan);
}
return this.handler.handleSend(this.injector, headers, request);
void handleSendAndReceive(Span span, Request request, Response response,
Throwable error) {
this.handler.handleSend(new HttpClientRequest(request), span);
this.handler.handleReceive(
response != null ? new HttpClientResponse(response) : null, error, span);
}
void handleReceive(Span span, Response response, Throwable error) {
this.handler.handleReceive(response, error, span);
}
static final class HttpClientRequest extends brave.http.HttpClientRequest {
private Request modifiedRequest(Request request,
Map<String, Collection<String>> headers) {
String method = request.method();
String url = request.url();
byte[] body = request.body();
Charset charset = request.charset();
return Request.create(method, url, headers, body, charset);
}
final Request delegate;
static final class HttpAdapter
extends brave.http.HttpClientAdapter<Request, Response> {
Map<String, Collection<String>> headers;
@Override
public String method(Request request) {
return request.method();
HttpClientRequest(Request delegate) {
this.delegate = delegate;
}
@Override
public String url(Request request) {
return request.url();
public Object unwrap() {
return delegate;
}
@Override
public String requestHeader(Request request, String name) {
Collection<String> result = request.headers().get(name);
public String method() {
return delegate.method();
}
@Override
public String path() {
String url = url();
if (url == null) {
return null;
}
return URI.create(url).getPath();
}
@Override
public String url() {
return delegate.url();
}
@Override
public String header(String name) {
Collection<String> result = delegate.headers().get(name);
return result != null && result.iterator().hasNext()
? result.iterator().next() : null;
}
@Override
public Integer statusCode(Response response) {
return response.status();
public void header(String name, String value) {
if (headers == null) {
headers = new LinkedHashMap<>(delegate.headers());
}
if (!headers.containsKey(name)) {
headers.put(name, Collections.singletonList(value));
if (log.isTraceEnabled()) {
log.trace(
"Added key [" + name + "] and header value [" + value + "]");
}
}
else {
// TODO: this is incorrect to ignore as opposed to overwrite!
if (log.isTraceEnabled()) {
log.trace("Key [" + name + "] already there in the headers");
}
}
}
Request build() {
if (headers == null) {
return delegate;
}
String method = delegate.method();
String url = delegate.url();
byte[] body = delegate.body();
Charset charset = delegate.charset();
return Request.create(method, url, headers, body, charset);
}
}
static final class HttpClientResponse extends brave.http.HttpClientResponse {
final Response delegate;
HttpClientResponse(Response delegate) {
this.delegate = delegate;
}
@Override
public Object unwrap() {
return delegate;
}
@Override
public int statusCode() {
return delegate.status();
}
}

46
spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TracingFeignClientTests.java
View File

@@ -17,8 +17,9 @@
package org.springframework.cloud.sleuth.instrument.web.client.feign;
import java.io.IOException;
import java.nio.charset.Charset;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import brave.Span;
import brave.Tracer;
@@ -36,9 +37,7 @@ import org.mockito.BDDMockito;
import org.mockito.Mock;
import org.mockito.junit.MockitoJUnitRunner;
import org.springframework.beans.factory.BeanFactory;
import org.springframework.cloud.sleuth.instrument.web.SleuthHttpParserAccessor;
import org.springframework.cloud.sleuth.util.ArrayListSpanReporter;
import static org.assertj.core.api.BDDAssertions.then;
@@ -48,15 +47,16 @@ import static org.assertj.core.api.BDDAssertions.then;
@RunWith(MockitoJUnitRunner.class)
public class TracingFeignClientTests {
ArrayListSpanReporter reporter = new ArrayListSpanReporter();
Request request = Request.create("GET", "https://foo", new HashMap<>(), null, null);
@Mock
BeanFactory beanFactory;
Request.Options options = new Request.Options();
List<zipkin2.Span> spans = new ArrayList<>();
Tracing tracing = Tracing.newBuilder()
.currentTraceContext(ThreadLocalCurrentTraceContext.newBuilder()
.addScopeDecorator(StrictScopeDecorator.create()).build())
.spanReporter(this.reporter).build();
.spanReporter(spans::add).build();
Tracer tracer = this.tracing.tracer();
@@ -78,18 +78,14 @@ public class TracingFeignClientTests {
Span span = this.tracer.nextSpan().name("foo");
try (Tracer.SpanInScope ws = this.tracer.withSpanInScope(span.start())) {
this.traceFeignClient
.execute(
Request.create("GET", "http://foo", new HashMap<>(),
"".getBytes(), Charset.defaultCharset()),
new Request.Options());
this.traceFeignClient.execute(this.request, this.options);
}
finally {
span.finish();
}
then(this.reporter.getSpans().get(0)).extracting("kind.ordinal")
.contains(Span.Kind.CLIENT.ordinal());
then(spans.get(0)).extracting("kind.ordinal")
.isEqualTo(Span.Kind.CLIENT.ordinal());
}
@Test
@@ -99,11 +95,7 @@ public class TracingFeignClientTests {
.willThrow(new RuntimeException("exception has occurred"));
try (Tracer.SpanInScope ws = this.tracer.withSpanInScope(span.start())) {
this.traceFeignClient
.execute(
Request.create("GET", "http://foo", new HashMap<>(),
"".getBytes(), Charset.defaultCharset()),
new Request.Options());
this.traceFeignClient.execute(this.request, this.options);
BDDAssertions.fail("Exception should have been thrown");
}
catch (Exception e) {
@@ -112,21 +104,17 @@ public class TracingFeignClientTests {
span.finish();
}
then(this.reporter.getSpans().get(0)).extracting("kind.ordinal")
.contains(Span.Kind.CLIENT.ordinal());
then(this.reporter.getSpans().get(0).tags()).containsEntry("error",
"exception has occurred");
then(this.spans.get(0)).extracting("kind.ordinal")
.isEqualTo(Span.Kind.CLIENT.ordinal());
then(this.spans.get(0).tags()).containsEntry("error", "exception has occurred");
}
@Test
public void should_shorten_the_span_name() throws IOException {
this.traceFeignClient
.execute(
Request.create("GET", "http://foo/" + bigName(), new HashMap<>(),
"".getBytes(), Charset.defaultCharset()),
new Request.Options());
this.traceFeignClient.execute(Request.create("GET", "https://foo/" + bigName(),
new HashMap<>(), null, null), this.options);
then(this.reporter.getSpans().get(0).name()).hasSize(50);
then(this.spans.get(0).name()).hasSize(50);
}
private String bigName() {