Merge branch '2.1.x'

This commit is contained in:
Marcin Grzejszczak
2019-10-22 13:41:46 +02:00
4 changed files with 123 additions and 82 deletions

View File

@@ -10,7 +10,6 @@ jobs:
branches:
ignore:
- gh-pages # list of branches to ignore
resource_class: large
steps:
- checkout
- restore_cache:

View File

@@ -32,6 +32,7 @@ import org.springframework.cloud.gateway.filter.headers.HttpHeadersFilter;
import org.springframework.http.HttpHeaders;
import org.springframework.http.server.reactive.ServerHttpRequest;
import org.springframework.http.server.reactive.ServerHttpResponse;
import org.springframework.lang.NonNull;
import org.springframework.web.server.ServerWebExchange;
final class TraceRequestHttpHeadersFilter extends AbstractHttpHeadersFilter {
@@ -51,21 +52,20 @@ final class TraceRequestHttpHeadersFilter extends AbstractHttpHeadersFilter {
if (log.isDebugEnabled()) {
log.debug("Will instrument the HTTP request headers");
}
ServerHttpRequest.Builder builder = exchange.getRequest().mutate();
Span span = this.handler.handleSend(this.injector, builder);
TraceCarrier carrier = new TraceCarrier(exchange.getRequest(), input);
Span span = this.handler.handleSend(this.injector, carrier);
if (log.isDebugEnabled()) {
log.debug(
"Client span " + span + " created for the request. New headers are "
+ builder.build().getHeaders().toSingleValueMap());
log.debug("Client span " + span + " created for the request. New headers are "
+ carrier.filteredHeaders.toSingleValueMap());
}
exchange.getAttributes().put(SPAN_ATTRIBUTE, span);
HttpHeaders headersWithInput = new HttpHeaders();
headersWithInput.addAll(input);
addHeadersWithInput(builder, headersWithInput);
addHeadersWithInput(carrier.filteredHeaders, headersWithInput);
return headersWithInput;
}
private void addHeadersWithInput(ServerHttpRequest.Builder builder,
private void addHeadersWithInput(HttpHeaders filteredHeaders,
HttpHeaders headersWithInput) {
for (Map.Entry<String, List<String>> entry : builder.build().getHeaders()
.entrySet()) {
@@ -82,10 +82,22 @@ final class TraceRequestHttpHeadersFilter extends AbstractHttpHeadersFilter {
}
class TraceCarrier {
final ServerHttpRequest originalRequest;
final HttpHeaders filteredHeaders;
TraceCarrier(@NonNull ServerHttpRequest originalRequest, @NonNull HttpHeaders filteredHeaders) {
this.originalRequest = originalRequest;
this.filteredHeaders = filteredHeaders;
}
}
final class TraceResponseHttpHeadersFilter extends AbstractHttpHeadersFilter {
private static final Log log = LogFactory
.getLog(TraceResponseHttpHeadersFilter.class);
private static final Log log = LogFactory.getLog(TraceResponseHttpHeadersFilter.class);
private TraceResponseHttpHeadersFilter(HttpTracing httpTracing) {
super(httpTracing);
@@ -122,23 +134,23 @@ abstract class AbstractHttpHeadersFilter implements HttpHeadersFilter {
static final String SPAN_ATTRIBUTE = Span.class.getName();
private static final Propagation.Setter<ServerHttpRequest.Builder, String> SETTER = new Propagation.Setter<ServerHttpRequest.Builder, String>() {
private static final Propagation.Setter<TraceCarrier, String> SETTER = new Propagation.Setter<TraceCarrier, String>() {
@Override
public void put(ServerHttpRequest.Builder carrier, String key, String value) {
carrier.headers(httpHeaders -> httpHeaders.set(key, value));
public void put(TraceCarrier carrier, String key, String value) {
carrier.filteredHeaders.set(key, value);
}
@Override
public String toString() {
return "ServerHttpRequest.Builder::header";
return "TraceCarrier::httpHeaders::set";
}
};
final Tracer tracer;
final HttpClientHandler<ServerHttpRequest.Builder, ServerHttpResponse> handler;
final HttpClientHandler<TraceCarrier, ServerHttpResponse> handler;
final TraceContext.Injector<ServerHttpRequest.Builder> injector;
final TraceContext.Injector<TraceCarrier> injector;
final HttpTracing httpTracing;
@@ -149,29 +161,27 @@ abstract class AbstractHttpHeadersFilter implements HttpHeadersFilter {
this.httpTracing = httpTracing;
}
private static class ServerHttpAdapter extends
brave.http.HttpClientAdapter<ServerHttpRequest.Builder, ServerHttpResponse> {
private static class ServerHttpAdapter extends brave.http.HttpClientAdapter<TraceCarrier, ServerHttpResponse> {
@Override
public String method(ServerHttpRequest.Builder request) {
return request.build().getMethodValue();
public String method(TraceCarrier request) {
return request.originalRequest.getMethodValue();
}
@Override
public String url(ServerHttpRequest.Builder request) {
return request.build().getURI().toString();
public String url(TraceCarrier request) {
return request.originalRequest.getURI().toString();
}
@Override
public String requestHeader(ServerHttpRequest.Builder request, String name) {
Object result = request.build().getHeaders().get(name);
public String requestHeader(TraceCarrier request, String name) {
Object result = request.filteredHeaders.get(name);
return result != null ? result.toString() : "";
}
@Override
public Integer statusCode(ServerHttpResponse response) {
return response.getStatusCode() != null ? response.getStatusCode().value()
: null;
return response.getStatusCode() != null ? response.getStatusCode().value() : null;
}
}

View File

@@ -33,6 +33,7 @@ import java.util.concurrent.atomic.AtomicBoolean;
import brave.Tracing;
import org.aopalliance.aop.Advice;
import org.assertj.core.api.BDDAssertions;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
@@ -72,8 +73,7 @@ public class ExecutorBeanPostProcessorTests {
@Before
public void setup() {
this.sleuthAsyncProperties = new SleuthAsyncProperties();
Mockito.when(this.beanFactory.getBean(SleuthAsyncProperties.class))
.thenReturn(this.sleuthAsyncProperties);
Mockito.when(this.beanFactory.getBean(SleuthAsyncProperties.class)).thenReturn(this.sleuthAsyncProperties);
}
@After
@@ -83,8 +83,7 @@ public class ExecutorBeanPostProcessorTests {
@Test
public void should_create_a_cglib_proxy_by_default() throws Exception {
Object o = new ExecutorBeanPostProcessor(this.beanFactory)
.postProcessAfterInitialization(new Foo(), "foo");
Object o = new ExecutorBeanPostProcessor(this.beanFactory).postProcessAfterInitialization(new Foo(), "foo");
then(o).isInstanceOf(Foo.class);
then(AopUtils.isCglibProxy(o)).isTrue();
@@ -107,8 +106,7 @@ public class ExecutorBeanPostProcessorTests {
throws Exception {
ScheduledExecutorService service = Executors.newSingleThreadScheduledExecutor();
Object o = new ExecutorBeanPostProcessor(this.beanFactory)
.postProcessAfterInitialization(service, "foo");
Object o = new ExecutorBeanPostProcessor(this.beanFactory).postProcessAfterInitialization(service, "foo");
then(o).isInstanceOf(TraceableScheduledExecutorService.class);
service.shutdown();
@@ -174,8 +172,7 @@ public class ExecutorBeanPostProcessorTests {
}
@Test
public void should_fallback_to_default_implementation_when_exception_thrown()
throws Exception {
public void should_fallback_to_default_implementation_when_exception_thrown() throws Exception {
ScheduledExecutorService service = Executors.newSingleThreadScheduledExecutor();
ExecutorBeanPostProcessor bpp = new ExecutorBeanPostProcessor(this.beanFactory) {
@@ -193,8 +190,7 @@ public class ExecutorBeanPostProcessorTests {
}
@Test
public void should_create_a_cglib_proxy_by_default_for_ThreadPoolTaskExecutor()
throws Exception {
public void should_create_a_cglib_proxy_by_default_for_ThreadPoolTaskExecutor() throws Exception {
Object o = new ExecutorBeanPostProcessor(this.beanFactory)
.postProcessAfterInitialization(new FooThreadPoolTaskExecutor(), "foo");
@@ -208,8 +204,7 @@ public class ExecutorBeanPostProcessorTests {
ThreadPoolTaskExecutor taskExecutor = new ThreadPoolTaskExecutor();
ExecutorBeanPostProcessor bpp = new ExecutorBeanPostProcessor(this.beanFactory) {
@Override
Object createThreadPoolTaskExecutorProxy(Object bean, boolean cglibProxy,
ThreadPoolTaskExecutor executor) {
Object createThreadPoolTaskExecutorProxy(Object bean, boolean cglibProxy, ThreadPoolTaskExecutor executor) {
throw new AopConfigException("foo");
}
};
@@ -239,8 +234,7 @@ public class ExecutorBeanPostProcessorTests {
ExecutorService service = exceptionThrowingExecutorService();
ExecutorBeanPostProcessor bpp = new ExecutorBeanPostProcessor(this.beanFactory);
ExecutorService o = (ExecutorService) bpp.postProcessAfterInitialization(service,
"foo");
ExecutorService o = (ExecutorService) bpp.postProcessAfterInitialization(service, "foo");
thenThrownBy(() -> o.submit((Callable<Object>) () -> "hello")).hasMessage("foo")
.isInstanceOf(IllegalStateException.class);
@@ -274,8 +268,7 @@ public class ExecutorBeanPostProcessorTests {
}
@Override
public boolean awaitTermination(long timeout, TimeUnit unit)
throws InterruptedException {
public boolean awaitTermination(long timeout, TimeUnit unit) throws InterruptedException {
return false;
}
@@ -295,14 +288,13 @@ public class ExecutorBeanPostProcessorTests {
}
@Override
public <T> List<Future<T>> invokeAll(Collection<? extends Callable<T>> tasks)
throws InterruptedException {
public <T> List<Future<T>> invokeAll(Collection<? extends Callable<T>> tasks) throws InterruptedException {
return null;
}
@Override
public <T> List<Future<T>> invokeAll(Collection<? extends Callable<T>> tasks,
long timeout, TimeUnit unit) throws InterruptedException {
public <T> List<Future<T>> invokeAll(Collection<? extends Callable<T>> tasks, long timeout, TimeUnit unit)
throws InterruptedException {
return null;
}
@@ -313,8 +305,7 @@ public class ExecutorBeanPostProcessorTests {
}
@Override
public <T> T invokeAny(Collection<? extends Callable<T>> tasks, long timeout,
TimeUnit unit)
public <T> T invokeAny(Collection<? extends Callable<T>> tasks, long timeout, TimeUnit unit)
throws InterruptedException, ExecutionException, TimeoutException {
return null;
}
@@ -323,11 +314,9 @@ public class ExecutorBeanPostProcessorTests {
@Test
public void should_use_jdk_proxy_when_executor_has_final_methods() {
ExecutorBeanPostProcessor beanPostProcessor = new ExecutorBeanPostProcessor(
this.beanFactory);
ExecutorBeanPostProcessor beanPostProcessor = new ExecutorBeanPostProcessor(this.beanFactory);
Executor executor = Runnable::run;
Executor wrappedExecutor = (Executor) beanPostProcessor
.postProcessAfterInitialization(executor, "executor");
Executor wrappedExecutor = (Executor) beanPostProcessor.postProcessAfterInitialization(executor, "executor");
then(AopUtils.isJdkDynamicProxy(wrappedExecutor)).isTrue();
then(AopUtils.isCglibProxy(wrappedExecutor)).isFalse();
@@ -340,10 +329,8 @@ public class ExecutorBeanPostProcessorTests {
}
@Test
public void should_use_jdk_proxy_when_executor_service_has_final_methods()
throws Exception {
ExecutorBeanPostProcessor beanPostProcessor = new ExecutorBeanPostProcessor(
this.beanFactory);
public void should_use_jdk_proxy_when_executor_service_has_final_methods() throws Exception {
ExecutorBeanPostProcessor beanPostProcessor = new ExecutorBeanPostProcessor(this.beanFactory);
ExecutorService executorService = new DelegatingSecurityContextExecutorService(
Executors.newSingleThreadExecutor());
ExecutorService wrappedExecutor = (ExecutorService) beanPostProcessor
@@ -356,10 +343,8 @@ public class ExecutorBeanPostProcessorTests {
}
@Test
public void should_use_jdk_proxy_when_async_task_executor_has_final_methods()
throws Exception {
ExecutorBeanPostProcessor beanPostProcessor = new ExecutorBeanPostProcessor(
this.beanFactory);
public void should_use_jdk_proxy_when_async_task_executor_has_final_methods() throws Exception {
ExecutorBeanPostProcessor beanPostProcessor = new ExecutorBeanPostProcessor(this.beanFactory);
AsyncTaskExecutor wrappedExecutor = (AsyncTaskExecutor) beanPostProcessor
.postProcessAfterInitialization(new DirectTaskExecutor(), "taskExecutor");
@@ -371,13 +356,11 @@ public class ExecutorBeanPostProcessorTests {
@Test
public void should_fallback_to_sleuth_impl_when_thread_pool_task_executor_has_final_methods() {
ExecutorBeanPostProcessor postProcessor = new ExecutorBeanPostProcessor(
this.beanFactory);
ExecutorBeanPostProcessor postProcessor = new ExecutorBeanPostProcessor(this.beanFactory);
ThreadPoolTaskExecutor threadPoolTaskExecutor = new PoolTaskExecutor();
ThreadPoolTaskExecutor wrappedTaskExecutor = (ThreadPoolTaskExecutor) postProcessor
.postProcessAfterInitialization(threadPoolTaskExecutor,
"threadPoolTaskExecutor");
.postProcessAfterInitialization(threadPoolTaskExecutor, "threadPoolTaskExecutor");
then(wrappedTaskExecutor).isInstanceOf(LazyTraceThreadPoolTaskExecutor.class);
then(AopUtils.isCglibProxy(wrappedTaskExecutor)).isFalse();
@@ -387,31 +370,26 @@ public class ExecutorBeanPostProcessorTests {
@Test
public void proxy_is_not_needed() throws Exception {
this.sleuthAsyncProperties
.setIgnoredBeans(Collections.singletonList("fooExecutor"));
this.sleuthAsyncProperties.setIgnoredBeans(Collections.singletonList("fooExecutor"));
boolean isProxyNeeded = new ExecutorBeanPostProcessor(this.beanFactory)
.isProxyNeeded("fooExecutor");
boolean isProxyNeeded = new ExecutorBeanPostProcessor(this.beanFactory).isProxyNeeded("fooExecutor");
then(isProxyNeeded).isFalse();
}
@Test
public void proxy_is_needed() throws Exception {
boolean isProxyNeeded = new ExecutorBeanPostProcessor(this.beanFactory)
.isProxyNeeded("fooExecutor");
boolean isProxyNeeded = new ExecutorBeanPostProcessor(this.beanFactory).isProxyNeeded("fooExecutor");
then(isProxyNeeded).isTrue();
}
@Test
public void should_not_create_proxy() throws Exception {
this.sleuthAsyncProperties
.setIgnoredBeans(Collections.singletonList("fooExecutor"));
this.sleuthAsyncProperties.setIgnoredBeans(Collections.singletonList("fooExecutor"));
Object o = new ExecutorBeanPostProcessor(this.beanFactory)
.postProcessAfterInitialization(new ThreadPoolTaskExecutor(),
"fooExecutor");
.postProcessAfterInitialization(new ThreadPoolTaskExecutor(), "fooExecutor");
then(o).isInstanceOf(ThreadPoolTaskExecutor.class);
then(AopUtils.isCglibProxy(o)).isFalse();
@@ -420,8 +398,7 @@ public class ExecutorBeanPostProcessorTests {
@Test
public void should_throw_real_exception_when_using_proxy() throws Exception {
Object o = new ExecutorBeanPostProcessor(this.beanFactory)
.postProcessAfterInitialization(new RejectedExecutionExecutor(),
"fooExecutor");
.postProcessAfterInitialization(new RejectedExecutionExecutor(), "fooExecutor");
then(o).isInstanceOf(RejectedExecutionExecutor.class);
then(AopUtils.isCglibProxy(o)).isTrue();
@@ -429,6 +406,32 @@ public class ExecutorBeanPostProcessorTests {
})).isInstanceOf(RejectedExecutionException.class).hasMessage("rejected");
}
// #1463
@Test
public void should_not_double_instrument_traced_executors() throws Exception {
LazyTraceThreadPoolTaskExecutor lazyTraceThreadPoolTaskExecutor = BDDMockito
.mock(LazyTraceThreadPoolTaskExecutor.class);
Object o = new ExecutorBeanPostProcessor(this.beanFactory)
.postProcessAfterInitialization(lazyTraceThreadPoolTaskExecutor, "executor");
BDDAssertions.then(o).isSameAs(lazyTraceThreadPoolTaskExecutor);
TraceableExecutorService traceableExecutorService = BDDMockito.mock(TraceableExecutorService.class);
o = new ExecutorBeanPostProcessor(this.beanFactory).postProcessAfterInitialization(traceableExecutorService,
"executor");
BDDAssertions.then(o).isSameAs(traceableExecutorService);
LazyTraceAsyncTaskExecutor lazyTraceAsyncTaskExecutor = BDDMockito.mock(LazyTraceAsyncTaskExecutor.class);
o = new ExecutorBeanPostProcessor(this.beanFactory).postProcessAfterInitialization(lazyTraceAsyncTaskExecutor,
"executor");
BDDAssertions.then(o).isSameAs(lazyTraceAsyncTaskExecutor);
LazyTraceExecutor lazyTraceExecutor = BDDMockito.mock(LazyTraceExecutor.class);
o = new ExecutorBeanPostProcessor(this.beanFactory).postProcessAfterInitialization(lazyTraceExecutor,
"executor");
BDDAssertions.then(o).isSameAs(lazyTraceExecutor);
}
class Foo implements Executor {
@Override

View File

@@ -48,14 +48,13 @@ public class TraceRequestHttpHeadersFilterTests {
HttpHeadersFilter filter = TraceRequestHttpHeadersFilter.create(this.httpTracing);
HttpHeaders httpHeaders = new HttpHeaders();
httpHeaders.set("X-Hello", "World");
httpHeaders.set("X-Auth-User", "aaaa");
httpHeaders.set("X-B3-TraceId", "52f112af7472aff0");
httpHeaders.set("X-B3-SpanId", "53e6ab6fc5dfee58");
MockServerHttpRequest request = MockServerHttpRequest.post("foo/bar")
.headers(httpHeaders).build();
MockServerWebExchange exchange = MockServerWebExchange.builder(request).build();
HttpHeaders filteredHeaders = filter.filter(requestHeaders(), exchange);
HttpHeaders filteredHeaders = filter.filter(requestHeaders(httpHeaders), exchange);
BDDAssertions.then(filteredHeaders.get("X-B3-TraceId"))
.isNotEqualTo(httpHeaders.get("X-B3-TraceId"));
@@ -81,7 +80,7 @@ public class TraceRequestHttpHeadersFilterTests {
.headers(httpHeaders).build();
MockServerWebExchange exchange = MockServerWebExchange.builder(request).build();
HttpHeaders filteredHeaders = filter.filter(requestHeaders(), exchange);
HttpHeaders filteredHeaders = filter.filter(requestHeaders(httpHeaders), exchange);
BDDAssertions.then(filteredHeaders.get("X-B3-TraceId")).isNotEmpty();
BDDAssertions.then(filteredHeaders.get("X-B3-SpanId")).isNotEmpty();
@@ -95,18 +94,40 @@ public class TraceRequestHttpHeadersFilterTests {
.isNotNull();
}
// #1352
// #1469
@Test
public void should_set_tracing_headers_with_multiple_values() {
public void should_reuse_headers_only_from_input_since_exchange_may_contain_already_ignored_headers() {
HttpHeadersFilter filter = TraceRequestHttpHeadersFilter.create(this.httpTracing);
HttpHeaders httpHeaders = new HttpHeaders();
httpHeaders.addAll("X-Hello", Arrays.asList("World1", "World2"));
httpHeaders.set("X-Hello", "World");
MockServerHttpRequest request = MockServerHttpRequest.post("foo/bar")
.headers(httpHeaders).build();
MockServerWebExchange exchange = MockServerWebExchange.builder(request).build();
HttpHeaders filteredHeaders = filter.filter(requestHeaders(), exchange);
BDDAssertions.then(filteredHeaders.get("X-B3-TraceId")).isNotEmpty();
BDDAssertions.then(filteredHeaders.get("X-B3-SpanId")).isNotEmpty();
BDDAssertions.then(filteredHeaders.get("X-Hello")).isNullOrEmpty();
BDDAssertions
.then((Object) exchange
.getAttribute(TraceRequestHttpHeadersFilter.SPAN_ATTRIBUTE))
.isNotNull();
}
// #1352
@Test
public void should_set_tracing_headers_with_multiple_values() {
HttpHeadersFilter filter = TraceRequestHttpHeadersFilter.create(this.httpTracing);
HttpHeaders httpHeaders = new HttpHeaders();
httpHeaders.add("X-Hello-Request", "Request World");
httpHeaders.addAll("X-Hello", Arrays.asList("World1", "World2"));
MockServerHttpRequest request = MockServerHttpRequest.post("foo/bar")
.headers(httpHeaders).build();
MockServerWebExchange exchange = MockServerWebExchange.builder(request).build();
HttpHeaders filteredHeaders = filter.filter(httpHeaders, exchange);
BDDAssertions.then(filteredHeaders.get("X-B3-TraceId")).isNotEmpty();
BDDAssertions.then(filteredHeaders.get("X-B3-SpanId")).isNotEmpty();
BDDAssertions.then(filteredHeaders.get("X-Hello"))
@@ -126,4 +147,12 @@ public class TraceRequestHttpHeadersFilterTests {
return headers;
}
private HttpHeaders requestHeaders(HttpHeaders originalHeaders) {
HttpHeaders headers = new HttpHeaders();
headers.putAll(originalHeaders);
headers.add("X-Hello-Request", "Request World");
headers.add("X-Auth-User", "aaaa");
return headers;
}
}