From 8a4002d288c315753cb2f6a2fd180e73d0ba77e3 Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Tue, 21 Jan 2020 16:44:08 +0800 Subject: [PATCH 1/4] Cleanups of build including upgrade of Brave (#1530) In efforts to dig deep into benchmarks, I noticed we were a bit out of date on Brave. Then, noticed some other artifacts could be bumped safely. Finally, a find/replace on http/https was over zealous and tripped up XML. One change to test expectations and Brave is explained here: https://github.com/openzipkin/brave/blob/master/instrumentation/RATIONALE.md#calling-spanfinish-while-the-context-is-in-scope --- README.adoc | 2 -- benchmarks/pom.xml | 20 ++++++------- docs/pom.xml | 5 +--- pom.xml | 23 +++++---------- spring-cloud-sleuth-core/pom.xml | 3 +- .../client/feign/TracingFeignClientTests.java | 4 +-- spring-cloud-sleuth-dependencies/pom.xml | 4 +-- spring-cloud-sleuth-samples/pom.xml | 4 +-- .../spring-cloud-sleuth-sample-feign/pom.xml | 2 +- .../pom.xml | 3 +- .../spring-cloud-sleuth-sample-ribbon/pom.xml | 2 +- .../pom.xml | 4 +-- .../pom.xml | 3 +- .../spring-cloud-sleuth-sample-zipkin/pom.xml | 4 +-- .../spring-cloud-sleuth-sample/pom.xml | 2 +- spring-cloud-sleuth-zipkin/pom.xml | 3 +- spring-cloud-starter-sleuth/pom.xml | 2 +- spring-cloud-starter-zipkin/pom.xml | 2 +- tests/pom.xml | 2 +- .../pom.xml | 2 +- .../pom.xml | 2 +- .../pom.xml | 4 +-- .../pom.xml | 2 +- .../pom.xml | 2 +- .../pom.xml | 2 +- .../JmsTracingConfigurationTest.java | 28 ++----------------- .../pom.xml | 2 +- .../pom.xml | 2 +- .../pom.xml | 2 +- .../pom.xml | 2 +- .../pom.xml | 2 +- .../pom.xml | 2 +- .../pom.xml | 2 +- 33 files changed, 51 insertions(+), 99 deletions(-) diff --git a/README.adoc b/README.adoc index 23df48ae1..71493d83d 100644 --- a/README.adoc +++ b/README.adoc @@ -888,7 +888,6 @@ Checkstyle rules are *disabled by default*. To add checkstyle to your project ju spring-javaformat-maven-plugin <5> - org.apache.maven.plugins maven-checkstyle-plugin @@ -896,7 +895,6 @@ Checkstyle rules are *disabled by default*. To add checkstyle to your project ju <5> - org.apache.maven.plugins maven-checkstyle-plugin diff --git a/benchmarks/pom.xml b/benchmarks/pom.xml index 9ee010576..c5f8035a3 100644 --- a/benchmarks/pom.xml +++ b/benchmarks/pom.xml @@ -14,7 +14,7 @@ ~ See the License for the specific language governing permissions and ~ limitations under the License. --> - 4.0.0 @@ -27,15 +27,14 @@ ${project.basedir}/.. - 1.16 - 2.4.3 - 2.5.2 + 1.22 + 3.2.1 true 1.8 1.8 2.3.0.BUILD-SNAPSHOT - 5.8.0 - 3.11.0 + 5.9.1 + 3.14.6 @@ -92,7 +91,7 @@ org.assertj assertj-core - 3.5.2 + 3.14.0 compile @@ -134,9 +133,8 @@ - org.apache.maven.plugins maven-compiler-plugin - 3.1 + 3.8.1 ${maven.compiler.source} ${maven.compiler.target} @@ -151,7 +149,6 @@ maven-install-plugin - ${maven-install-plugin.version} true @@ -369,9 +366,8 @@ - org.apache.maven.plugins maven-failsafe-plugin - 2.19.1 + 2.22.2 diff --git a/docs/pom.xml b/docs/pom.xml index 893f74894..45c220e0d 100644 --- a/docs/pom.xml +++ b/docs/pom.xml @@ -15,7 +15,7 @@ ~ limitations under the License. --> - 4.0.0 @@ -56,11 +56,9 @@ git-commit-id-plugin - org.apache.maven.plugins maven-dependency-plugin - org.apache.maven.plugins maven-resources-plugin @@ -72,7 +70,6 @@ asciidoctor-maven-plugin - org.apache.maven.plugins maven-antrun-plugin diff --git a/pom.xml b/pom.xml index 759e967b4..032413095 100644 --- a/pom.xml +++ b/pom.xml @@ -15,7 +15,7 @@ ~ limitations under the License. --> - 4.0.0 @@ -59,9 +59,8 @@ - org.apache.maven.plugins maven-compiler-plugin - 3.1 + 3.8.1 default-compile @@ -88,7 +87,6 @@ - org.apache.maven.plugins maven-enforcer-plugin 1.3.1 @@ -108,7 +106,6 @@ - org.apache.maven.plugins maven-deploy-plugin 2.8.2 @@ -120,7 +117,6 @@ spring-javaformat-maven-plugin - org.apache.maven.plugins maven-checkstyle-plugin @@ -129,7 +125,6 @@ - org.apache.maven.plugins maven-checkstyle-plugin @@ -244,7 +239,7 @@ org.assertj assertj-core - 3.8.0 + 3.14.0 test @@ -262,13 +257,12 @@ Horsham.SR1 2.2.2.BUILD-SNAPSHOT 2.2.2.BUILD-SNAPSHOT - 5.8.0 - 2.1.7.RELEASE - + 5.9.1 + 2.1.7.RELEASE 2.2.2.BUILD-SNAPSHOT false - 3.10.0 - 3.10.0 + 3.14.6 + 3.14.6 20.0 1.7.1 3.3.0 @@ -357,9 +351,7 @@ - org.apache.maven.plugins maven-compiler-plugin - 3.1 ${maven.compiler.testSource} ${maven.compiler.testTarget} @@ -411,7 +403,6 @@ - org.apache.maven.plugins maven-surefire-plugin diff --git a/spring-cloud-sleuth-core/pom.xml b/spring-cloud-sleuth-core/pom.xml index 4189ba781..9d7157d31 100644 --- a/spring-cloud-sleuth-core/pom.xml +++ b/spring-cloud-sleuth-core/pom.xml @@ -15,7 +15,7 @@ ~ limitations under the License. --> - 4.0.0 @@ -383,7 +383,6 @@ - org.apache.maven.plugins maven-surefire-plugin 4 diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TracingFeignClientTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TracingFeignClientTests.java index 3f16f999b..d195b7bd0 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TracingFeignClientTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TracingFeignClientTests.java @@ -89,7 +89,7 @@ public class TracingFeignClientTests { } then(this.reporter.getSpans().get(0)).extracting("kind.ordinal") - .contains(Span.Kind.CLIENT.ordinal()); + .isEqualTo(Span.Kind.CLIENT.ordinal()); } @Test @@ -113,7 +113,7 @@ public class TracingFeignClientTests { } then(this.reporter.getSpans().get(0)).extracting("kind.ordinal") - .contains(Span.Kind.CLIENT.ordinal()); + .isEqualTo(Span.Kind.CLIENT.ordinal()); then(this.reporter.getSpans().get(0).tags()).containsEntry("error", "exception has occurred"); } diff --git a/spring-cloud-sleuth-dependencies/pom.xml b/spring-cloud-sleuth-dependencies/pom.xml index 113a45f45..193ee300a 100644 --- a/spring-cloud-sleuth-dependencies/pom.xml +++ b/spring-cloud-sleuth-dependencies/pom.xml @@ -15,7 +15,7 @@ ~ limitations under the License. --> - 4.0.0 @@ -31,7 +31,7 @@ spring-cloud-sleuth-dependencies Spring Cloud Sleuth Dependencies - 5.9.0 + 5.9.1 0.35.0 3.4.1 diff --git a/spring-cloud-sleuth-samples/pom.xml b/spring-cloud-sleuth-samples/pom.xml index 58ddbfda0..12c863448 100644 --- a/spring-cloud-sleuth-samples/pom.xml +++ b/spring-cloud-sleuth-samples/pom.xml @@ -15,7 +15,7 @@ ~ limitations under the License. --> - 4.0.0 @@ -73,7 +73,7 @@ io.zipkin.zipkin2 zipkin - 2.17.0 + 2.19.2 diff --git a/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-feign/pom.xml b/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-feign/pom.xml index 146e4d636..f9b6f6a2a 100644 --- a/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-feign/pom.xml +++ b/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-feign/pom.xml @@ -15,7 +15,7 @@ ~ limitations under the License. --> - 4.0.0 diff --git a/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-messaging/pom.xml b/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-messaging/pom.xml index a72da4d86..6a83d881c 100644 --- a/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-messaging/pom.xml +++ b/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-messaging/pom.xml @@ -15,7 +15,7 @@ ~ limitations under the License. --> - 4.0.0 @@ -57,7 +57,6 @@ - org.apache.maven.plugins maven-compiler-plugin 1.8 diff --git a/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-ribbon/pom.xml b/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-ribbon/pom.xml index fbd1f652f..428307d5f 100644 --- a/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-ribbon/pom.xml +++ b/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-ribbon/pom.xml @@ -15,7 +15,7 @@ ~ limitations under the License. --> - 4.0.0 diff --git a/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-test-core/pom.xml b/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-test-core/pom.xml index c787fd1f1..9469ac2c4 100644 --- a/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-test-core/pom.xml +++ b/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-test-core/pom.xml @@ -15,7 +15,7 @@ ~ limitations under the License. --> - 4.0.0 @@ -55,7 +55,7 @@ org.codehaus.mojo animal-sniffer-maven-plugin - 1.16 + 1.18 true diff --git a/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-websocket/pom.xml b/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-websocket/pom.xml index a5ba5bc5c..d4f15e8ae 100644 --- a/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-websocket/pom.xml +++ b/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-websocket/pom.xml @@ -15,7 +15,7 @@ ~ limitations under the License. --> - 4.0.0 @@ -57,7 +57,6 @@ - org.apache.maven.plugins maven-compiler-plugin 1.8 diff --git a/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-zipkin/pom.xml b/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-zipkin/pom.xml index cdc1a8747..a71e7e406 100644 --- a/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-zipkin/pom.xml +++ b/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-zipkin/pom.xml @@ -15,7 +15,7 @@ ~ limitations under the License. --> - 4.0.0 @@ -57,7 +57,6 @@ - org.apache.maven.plugins maven-compiler-plugin 1.8 @@ -101,7 +100,6 @@ com.squareup.okhttp3 mockwebserver - 3.9.0 test diff --git a/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample/pom.xml b/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample/pom.xml index 07cd5e4c0..0867149df 100644 --- a/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample/pom.xml +++ b/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample/pom.xml @@ -15,7 +15,7 @@ ~ limitations under the License. --> - 4.0.0 diff --git a/spring-cloud-sleuth-zipkin/pom.xml b/spring-cloud-sleuth-zipkin/pom.xml index a53d2c5ff..5412d198f 100644 --- a/spring-cloud-sleuth-zipkin/pom.xml +++ b/spring-cloud-sleuth-zipkin/pom.xml @@ -15,7 +15,7 @@ ~ limitations under the License. --> - 4.0.0 @@ -149,7 +149,6 @@ com.squareup.okhttp3 mockwebserver - 3.10.0 test diff --git a/spring-cloud-starter-sleuth/pom.xml b/spring-cloud-starter-sleuth/pom.xml index 3d977ba3f..a19c15fd5 100644 --- a/spring-cloud-starter-sleuth/pom.xml +++ b/spring-cloud-starter-sleuth/pom.xml @@ -15,7 +15,7 @@ ~ limitations under the License. --> - 4.0.0 diff --git a/spring-cloud-starter-zipkin/pom.xml b/spring-cloud-starter-zipkin/pom.xml index 57c0b8635..22f67d6c4 100644 --- a/spring-cloud-starter-zipkin/pom.xml +++ b/spring-cloud-starter-zipkin/pom.xml @@ -15,7 +15,7 @@ ~ limitations under the License. --> - 4.0.0 diff --git a/tests/pom.xml b/tests/pom.xml index 52d450753..d08eba732 100644 --- a/tests/pom.xml +++ b/tests/pom.xml @@ -17,7 +17,7 @@ ~ --> - 4.0.0 diff --git a/tests/spring-cloud-sleuth-instrumentation-async-tests/pom.xml b/tests/spring-cloud-sleuth-instrumentation-async-tests/pom.xml index 2647e89c6..83b5c30cb 100644 --- a/tests/spring-cloud-sleuth-instrumentation-async-tests/pom.xml +++ b/tests/spring-cloud-sleuth-instrumentation-async-tests/pom.xml @@ -17,7 +17,7 @@ ~ --> - 4.0.0 diff --git a/tests/spring-cloud-sleuth-instrumentation-feign-tests/pom.xml b/tests/spring-cloud-sleuth-instrumentation-feign-tests/pom.xml index 4b599ebad..17cf6232b 100644 --- a/tests/spring-cloud-sleuth-instrumentation-feign-tests/pom.xml +++ b/tests/spring-cloud-sleuth-instrumentation-feign-tests/pom.xml @@ -17,7 +17,7 @@ ~ --> - 4.0.0 diff --git a/tests/spring-cloud-sleuth-instrumentation-grpc-tests/pom.xml b/tests/spring-cloud-sleuth-instrumentation-grpc-tests/pom.xml index 9981074d3..df04dc0fb 100644 --- a/tests/spring-cloud-sleuth-instrumentation-grpc-tests/pom.xml +++ b/tests/spring-cloud-sleuth-instrumentation-grpc-tests/pom.xml @@ -17,7 +17,7 @@ ~ --> - 4.0.0 @@ -73,7 +73,7 @@ com.google.guava guava - 20.0 + ${guava.version} test diff --git a/tests/spring-cloud-sleuth-instrumentation-hystrix-tests/pom.xml b/tests/spring-cloud-sleuth-instrumentation-hystrix-tests/pom.xml index e73b1a05a..a6d6c31da 100644 --- a/tests/spring-cloud-sleuth-instrumentation-hystrix-tests/pom.xml +++ b/tests/spring-cloud-sleuth-instrumentation-hystrix-tests/pom.xml @@ -17,7 +17,7 @@ ~ --> - 4.0.0 diff --git a/tests/spring-cloud-sleuth-instrumentation-lettuce-tests/pom.xml b/tests/spring-cloud-sleuth-instrumentation-lettuce-tests/pom.xml index 1fd7ec41f..0efc8054a 100644 --- a/tests/spring-cloud-sleuth-instrumentation-lettuce-tests/pom.xml +++ b/tests/spring-cloud-sleuth-instrumentation-lettuce-tests/pom.xml @@ -17,7 +17,7 @@ ~ --> - 4.0.0 diff --git a/tests/spring-cloud-sleuth-instrumentation-messaging-tests/pom.xml b/tests/spring-cloud-sleuth-instrumentation-messaging-tests/pom.xml index e2ad53798..7560caebb 100644 --- a/tests/spring-cloud-sleuth-instrumentation-messaging-tests/pom.xml +++ b/tests/spring-cloud-sleuth-instrumentation-messaging-tests/pom.xml @@ -17,7 +17,7 @@ ~ --> - diff --git a/tests/spring-cloud-sleuth-instrumentation-messaging-tests/src/test/java/org/springframework/cloud/sleuth/instrument/messaging/JmsTracingConfigurationTest.java b/tests/spring-cloud-sleuth-instrumentation-messaging-tests/src/test/java/org/springframework/cloud/sleuth/instrument/messaging/JmsTracingConfigurationTest.java index 6bb90facb..4af69b17d 100644 --- a/tests/spring-cloud-sleuth-instrumentation-messaging-tests/src/test/java/org/springframework/cloud/sleuth/instrument/messaging/JmsTracingConfigurationTest.java +++ b/tests/spring-cloud-sleuth-instrumentation-messaging-tests/src/test/java/org/springframework/cloud/sleuth/instrument/messaging/JmsTracingConfigurationTest.java @@ -34,7 +34,6 @@ import javax.jms.XAConnectionFactory; import javax.resource.spi.ResourceAdapter; import brave.Tracing; -import brave.internal.HexCodec; import brave.propagation.CurrentTraceContext; import brave.propagation.TraceContext; import org.apache.activemq.ra.ActiveMQActivationSpec; @@ -43,7 +42,6 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.junit.Ignore; import org.junit.Test; -import zipkin2.Annotation; import zipkin2.Span; import org.springframework.beans.factory.annotation.Autowired; @@ -317,8 +315,6 @@ public class JmsTracingConfigurationTest { @EnableAutoConfiguration(exclude = KafkaAutoConfiguration.class) class JmsTestTracingConfiguration { - static final String CONTEXT_LEAK = "context.leak"; - /** * When testing servers or asynchronous clients, spans are reported on a worker * thread. In order to read them on the main thread, we use a concurrent queue. As @@ -340,34 +336,14 @@ class JmsTestTracingConfiguration { return () -> { Span result = this.spans.poll(3, TimeUnit.SECONDS); assertThat(result).withFailMessage("Span was not reported").isNotNull(); - assertThat(result.annotations()).extracting(Annotation::value) - .doesNotContain(CONTEXT_LEAK); return result; }; } @Bean Tracing tracing(CurrentTraceContext currentTraceContext) { - return Tracing.newBuilder().spanReporter(s -> { - // make sure the context was cleared prior to finish.. no leaks! - TraceContext current = currentTraceContext.get(); - boolean contextLeak = false; - if (current != null) { - // add annotation in addition to throwing, in case we are off the main - // thread - if (HexCodec.toLowerHex(current.spanId()).equals(s.id())) { - s = s.toBuilder().addAnnotation(s.timestampAsLong(), CONTEXT_LEAK) - .build(); - contextLeak = true; - } - } - this.spans.add(s); - // throw so that we can see the path to the code that leaked the context - if (contextLeak) { - throw new AssertionError( - CONTEXT_LEAK + " on " + Thread.currentThread().getName()); - } - }).currentTraceContext(currentTraceContext).build(); + return Tracing.newBuilder().spanReporter(spans::add) + .currentTraceContext(currentTraceContext).build(); } } diff --git a/tests/spring-cloud-sleuth-instrumentation-mvc-tests/pom.xml b/tests/spring-cloud-sleuth-instrumentation-mvc-tests/pom.xml index 55384a160..8e2163a5d 100644 --- a/tests/spring-cloud-sleuth-instrumentation-mvc-tests/pom.xml +++ b/tests/spring-cloud-sleuth-instrumentation-mvc-tests/pom.xml @@ -17,7 +17,7 @@ ~ --> - 4.0.0 diff --git a/tests/spring-cloud-sleuth-instrumentation-reactor-tests/pom.xml b/tests/spring-cloud-sleuth-instrumentation-reactor-tests/pom.xml index 1dded5f9e..2e07b1f31 100644 --- a/tests/spring-cloud-sleuth-instrumentation-reactor-tests/pom.xml +++ b/tests/spring-cloud-sleuth-instrumentation-reactor-tests/pom.xml @@ -17,7 +17,7 @@ ~ --> - 4.0.0 diff --git a/tests/spring-cloud-sleuth-instrumentation-rpc-tests/pom.xml b/tests/spring-cloud-sleuth-instrumentation-rpc-tests/pom.xml index 5261047bf..60c648ac3 100644 --- a/tests/spring-cloud-sleuth-instrumentation-rpc-tests/pom.xml +++ b/tests/spring-cloud-sleuth-instrumentation-rpc-tests/pom.xml @@ -17,7 +17,7 @@ ~ --> - 4.0.0 diff --git a/tests/spring-cloud-sleuth-instrumentation-rxjava-tests/pom.xml b/tests/spring-cloud-sleuth-instrumentation-rxjava-tests/pom.xml index 5895238de..1835a004a 100644 --- a/tests/spring-cloud-sleuth-instrumentation-rxjava-tests/pom.xml +++ b/tests/spring-cloud-sleuth-instrumentation-rxjava-tests/pom.xml @@ -17,7 +17,7 @@ ~ --> - 4.0.0 diff --git a/tests/spring-cloud-sleuth-instrumentation-scheduling-tests/pom.xml b/tests/spring-cloud-sleuth-instrumentation-scheduling-tests/pom.xml index c73181d29..edb74b25f 100644 --- a/tests/spring-cloud-sleuth-instrumentation-scheduling-tests/pom.xml +++ b/tests/spring-cloud-sleuth-instrumentation-scheduling-tests/pom.xml @@ -17,7 +17,7 @@ ~ --> - 4.0.0 diff --git a/tests/spring-cloud-sleuth-instrumentation-webflux-tests/pom.xml b/tests/spring-cloud-sleuth-instrumentation-webflux-tests/pom.xml index 8b8443145..2d4c79a35 100644 --- a/tests/spring-cloud-sleuth-instrumentation-webflux-tests/pom.xml +++ b/tests/spring-cloud-sleuth-instrumentation-webflux-tests/pom.xml @@ -17,7 +17,7 @@ ~ --> - 4.0.0 diff --git a/tests/spring-cloud-sleuth-instrumentation-zuul-tests/pom.xml b/tests/spring-cloud-sleuth-instrumentation-zuul-tests/pom.xml index b5c6393c7..192aff8d3 100644 --- a/tests/spring-cloud-sleuth-instrumentation-zuul-tests/pom.xml +++ b/tests/spring-cloud-sleuth-instrumentation-zuul-tests/pom.xml @@ -17,7 +17,7 @@ ~ --> - 4.0.0 From e5227ba0905b0f6a2208fc62b728eb00ee9cf40f Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Wed, 22 Jan 2020 21:23:47 +0800 Subject: [PATCH 2/4] Moves client instrumentation off deprecated Brave code (#1532) --- .../client/HttpClientBeanPostProcessor.java | 127 ++++++++------- .../client/TraceRequestHttpHeadersFilter.java | 148 +++++++++--------- .../TraceFeignBlockingLoadBalancerClient.java | 6 +- .../feign/TraceLoadBalancerFeignClient.java | 6 +- .../web/client/feign/TracingFeignClient.java | 136 +++++++++++----- 5 files changed, 240 insertions(+), 183 deletions(-) diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/HttpClientBeanPostProcessor.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/HttpClientBeanPostProcessor.java index dab4d2d81..dcf21694e 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/HttpClientBeanPostProcessor.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/HttpClientBeanPostProcessor.java @@ -16,6 +16,7 @@ package org.springframework.cloud.sleuth.instrument.web.client; +import java.util.List; import java.util.concurrent.atomic.AtomicReference; import java.util.function.BiConsumer; import java.util.function.BiFunction; @@ -24,10 +25,7 @@ import brave.Span; import brave.Tracer; import brave.http.HttpClientHandler; import brave.http.HttpTracing; -import brave.propagation.Propagation; -import brave.propagation.TraceContext; import io.netty.bootstrap.Bootstrap; -import io.netty.handler.codec.http.HttpHeaders; import reactor.core.publisher.Mono; import reactor.netty.Connection; import reactor.netty.http.client.HttpClient; @@ -89,31 +87,13 @@ class HttpClientBeanPostProcessor implements BeanPostProcessor { private static class TracingDoOnRequest implements BiConsumer { - static final Propagation.Setter SETTER = new Propagation.Setter() { - @Override - public void put(HttpHeaders carrier, String key, String value) { - if (!carrier.contains(key)) { - carrier.add(key, value); - } - } - - @Override - public String toString() { - return "HttpHeaders::add"; - } - }; - final BeanFactory beanFactory; HttpTracing httpTracing; - Tracer tracer; + List propagationKeys; - HttpClientHandler handler; - - TraceContext.Injector injector; - - Propagation propagation; + HttpClientHandler handler; TracingDoOnRequest(BeanFactory beanFactory) { this.beanFactory = beanFactory; @@ -130,23 +110,16 @@ class HttpClientBeanPostProcessor implements BeanPostProcessor { return this.httpTracing; } - private Propagation propagation() { - if (this.propagation == null) { - this.propagation = httpTracing().tracing().propagation(); + private List propagationKeys() { + if (this.propagationKeys == null) { + this.propagationKeys = httpTracing().tracing().propagation().keys(); } - return this.propagation; + return this.propagationKeys; } - private TraceContext.Injector injector() { - if (this.injector == null) { - this.injector = propagation().injector(SETTER); - } - return this.injector; - } - - private HttpClientHandler handler() { + private HttpClientHandler handler() { if (this.handler == null) { - this.handler = HttpClientHandler.create(httpTracing(), new HttpAdapter()); + this.handler = HttpClientHandler.create(httpTracing()); } return this.handler; } @@ -154,16 +127,18 @@ class HttpClientBeanPostProcessor implements BeanPostProcessor { @Override public void accept(HttpClientRequest req, Connection connection) { // request already instrumented - for (String key : propagation().keys()) { + // TODO: consider another, cheaper way, like flagging a context + // property. If not, comment why. + for (String key : propagationKeys()) { if (req.requestHeaders().contains(key)) { return; } } - AtomicReference reference = req.currentContext() - .getOrDefault(AtomicReference.class, new AtomicReference()); - Span span = handler().handleSend(injector(), req.requestHeaders(), req, - reference.get() == null ? handler().nextSpan(req) - : (Span) reference.get()); + AtomicReference reference = req.currentContext() + .getOrDefault(AtomicReference.class, new AtomicReference<>()); + WrappedHttpClientRequest request = new WrappedHttpClientRequest(req); + Span span = reference.get() == null ? handler().handleSend(request) + : handler().handleSend(request, reference.get()); reference.set(span); } @@ -229,7 +204,7 @@ class HttpClientBeanPostProcessor implements BeanPostProcessor { HttpTracing httpTracing; - HttpClientHandler handler; + HttpClientHandler handler; AbstractTracingDoOnHandler(BeanFactory beanFactory) { this.beanFactory = beanFactory; @@ -242,9 +217,9 @@ class HttpClientBeanPostProcessor implements BeanPostProcessor { return this.httpTracing; } - private HttpClientHandler handler() { + private HttpClientHandler handler() { if (this.handler == null) { - this.handler = HttpClientHandler.create(httpTracing(), new HttpAdapter()); + this.handler = HttpClientHandler.create(httpTracing()); } return this.handler; } @@ -259,34 +234,68 @@ class HttpClientBeanPostProcessor implements BeanPostProcessor { if (reference == null || reference.get() == null) { return; } - handler().handleReceive(httpClientResponse, throwable, - (Span) reference.get()); + handler().handleReceive(new WrappedHttpClientResponse(httpClientResponse), + throwable, (Span) reference.get()); } } - private static class HttpAdapter - extends brave.http.HttpClientAdapter { + static final class WrappedHttpClientRequest extends brave.http.HttpClientRequest { - @Override - public String method(HttpClientRequest request) { - return request.method().name(); + final HttpClientRequest delegate; + + WrappedHttpClientRequest(HttpClientRequest delegate) { + this.delegate = delegate; } @Override - public String url(HttpClientRequest request) { - return request.uri(); + public Object unwrap() { + return delegate; } @Override - public String requestHeader(HttpClientRequest request, String name) { - Object result = request.requestHeaders().get(name); - return result != null ? result.toString() : ""; + public String method() { + return delegate.method().name(); } @Override - public Integer statusCode(HttpClientResponse response) { - return response.status().code(); + public String path() { + return delegate.path(); + } + + @Override + public String url() { + return delegate.uri(); + } + + @Override + public String header(String name) { + return delegate.requestHeaders().get(name); + } + + @Override + public void header(String name, String value) { + delegate.header(name, value); + } + + } + + static final class WrappedHttpClientResponse extends brave.http.HttpClientResponse { + + final HttpClientResponse delegate; + + WrappedHttpClientResponse(HttpClientResponse delegate) { + this.delegate = delegate; + } + + @Override + public Object unwrap() { + return delegate; + } + + @Override + public int statusCode() { + return delegate.status().code(); } } diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceRequestHttpHeadersFilter.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceRequestHttpHeadersFilter.java index 447b515f4..5bb4c487d 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceRequestHttpHeadersFilter.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceRequestHttpHeadersFilter.java @@ -23,8 +23,7 @@ import brave.Span; import brave.Tracer; import brave.http.HttpClientHandler; import brave.http.HttpTracing; -import brave.propagation.Propagation; -import brave.propagation.TraceContext; +import brave.propagation.TraceContext.Extractor; import brave.propagation.TraceContextOrSamplingFlags; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -33,7 +32,6 @@ 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 { @@ -54,18 +52,18 @@ final class TraceRequestHttpHeadersFilter extends AbstractHttpHeadersFilter { log.debug("Will instrument the HTTP request headers [" + exchange.getRequest().getHeaders() + "]"); } - TraceCarrier carrier = new TraceCarrier(exchange.getRequest(), input); - Span currentSpan = currentSpan(carrier); - Span span = injectedSpan(carrier, currentSpan); + HttpClientRequest request = new HttpClientRequest(exchange.getRequest(), input); + Span currentSpan = currentSpan(request); + Span span = injectedSpan(request, currentSpan); if (log.isDebugEnabled()) { log.debug( "Client span " + span + " created for the request. New headers are " - + carrier.filteredHeaders.toSingleValueMap()); + + request.filteredHeaders.toSingleValueMap()); } exchange.getAttributes().put(SPAN_ATTRIBUTE, span); HttpHeaders headersWithInput = new HttpHeaders(); headersWithInput.addAll(input); - addHeadersWithInput(carrier.filteredHeaders, headersWithInput); + addHeadersWithInput(request.filteredHeaders, headersWithInput); if (headersWithInput.containsKey("b3") || headersWithInput.containsKey("B3")) { headersWithInput.keySet().remove("b3"); headersWithInput.keySet().remove("B3"); @@ -73,22 +71,24 @@ final class TraceRequestHttpHeadersFilter extends AbstractHttpHeadersFilter { return headersWithInput; } - private Span currentSpan(TraceCarrier carrier) { + private Span currentSpan(HttpClientRequest request) { Span currentSpan = this.tracer.currentSpan(); if (currentSpan != null) { return currentSpan; } - TraceContextOrSamplingFlags contextOrFlags = this.extractor.extract(carrier); + // Usually, an HTTP client would not attempt to resume a trace from headers, as a + // server would always place its span in scope. However, in commit 848442e, + // this behavior was added in support of gateway. + TraceContextOrSamplingFlags contextOrFlags = this.extractor.extract(request); return this.tracer.nextSpan(contextOrFlags); } - private Span injectedSpan(TraceCarrier carrier, Span currentSpan) { + private Span injectedSpan(HttpClientRequest request, Span currentSpan) { if (currentSpan == null) { - return this.handler.handleSend(this.injector, carrier); + return this.handler.handleSend(request); } - Span clientSpan = this.tracer - .nextSpan(TraceContextOrSamplingFlags.create(currentSpan.context())); - return this.handler.handleSend(this.injector, carrier, clientSpan); + Span clientSpan = this.tracer.newChild(currentSpan.context()); + return this.handler.handleSend(request, clientSpan); } private void addHeadersWithInput(HttpHeaders filteredHeaders, @@ -107,20 +107,6 @@ 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 @@ -143,7 +129,8 @@ final class TraceResponseHttpHeadersFilter extends AbstractHttpHeadersFilter { if (log.isDebugEnabled()) { log.debug("Will instrument the response"); } - this.handler.handleReceive(exchange.getResponse(), null, (Span) storedSpan); + HttpClientResponse response = new HttpClientResponse(exchange.getResponse()); + this.handler.handleReceive(response, null, (Span) storedSpan); if (log.isDebugEnabled()) { log.debug("The response was handled for span " + storedSpan); } @@ -161,71 +148,82 @@ abstract class AbstractHttpHeadersFilter implements HttpHeadersFilter { static final String SPAN_ATTRIBUTE = Span.class.getName(); - private static final Propagation.Setter SETTER = new Propagation.Setter() { - @Override - public void put(TraceCarrier carrier, String key, String value) { - carrier.filteredHeaders.set(key, value); - } - - @Override - public String toString() { - return "TraceCarrier::httpHeaders::set"; - } - }; - - private static final Propagation.Getter GETTER = new Propagation.Getter() { - @Override - public String get(TraceCarrier carrier, String key) { - return carrier.filteredHeaders.getFirst(key); - } - - @Override - public String toString() { - return "TraceCarrier::httpHeaders::getFirst"; - } - }; - final Tracer tracer; - final HttpClientHandler handler; - - final TraceContext.Injector injector; - - final TraceContext.Extractor extractor; + final HttpClientHandler handler; final HttpTracing httpTracing; + final Extractor extractor; + AbstractHttpHeadersFilter(HttpTracing httpTracing) { this.tracer = httpTracing.tracing().tracer(); - this.handler = HttpClientHandler.create(httpTracing, new ServerHttpAdapter()); - this.injector = httpTracing.tracing().propagation().injector(SETTER); - this.extractor = httpTracing.tracing().propagation().extractor(GETTER); + this.extractor = httpTracing.tracing().propagation() + .extractor(HttpClientRequest::header); + this.handler = HttpClientHandler.create(httpTracing); this.httpTracing = httpTracing; } - private static class ServerHttpAdapter - extends brave.http.HttpClientAdapter { + static final class HttpClientRequest extends brave.http.HttpClientRequest { - @Override - public String method(TraceCarrier request) { - return request.originalRequest.getMethodValue(); + final ServerHttpRequest delegate; + + final HttpHeaders filteredHeaders; + + HttpClientRequest(ServerHttpRequest delegate, HttpHeaders filteredHeaders) { + this.delegate = delegate; + this.filteredHeaders = filteredHeaders; } @Override - public String url(TraceCarrier request) { - return request.originalRequest.getURI().toString(); + public Object unwrap() { + return delegate; } @Override - public String requestHeader(TraceCarrier request, String name) { - Object result = request.filteredHeaders.get(name); - return result != null ? result.toString() : ""; + public String method() { + return delegate.getMethodValue(); } @Override - public Integer statusCode(ServerHttpResponse response) { - return response.getStatusCode() != null ? response.getStatusCode().value() - : null; + public String path() { + return delegate.getURI().getPath(); + } + + @Override + public String url() { + return delegate.getURI().toString(); + } + + @Override + public String header(String name) { + return filteredHeaders.getFirst(name); + } + + @Override + public void header(String name, String value) { + filteredHeaders.set(name, value); + } + + } + + static final class HttpClientResponse extends brave.http.HttpClientResponse { + + final ServerHttpResponse delegate; + + HttpClientResponse(ServerHttpResponse delegate) { + this.delegate = delegate; + } + + @Override + public Object unwrap() { + return delegate; + } + + @Override + public int statusCode() { + return delegate.getStatusCode() != null ? delegate.getStatusCode().value() + : 0; } } diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignBlockingLoadBalancerClient.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignBlockingLoadBalancerClient.java index 29ad789c2..b65643a6b 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignBlockingLoadBalancerClient.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignBlockingLoadBalancerClient.java @@ -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; @@ -89,9 +88,8 @@ public class TraceFeignBlockingLoadBalancerClient 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; } diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceLoadBalancerFeignClient.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceLoadBalancerFeignClient.java index 9a459b7bc..d27656fb3 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceLoadBalancerFeignClient.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceLoadBalancerFeignClient.java @@ -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; } diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TracingFeignClient.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TracingFeignClient.java index 1ca7adee4..662314df6 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TracingFeignClient.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TracingFeignClient.java @@ -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 handler; - - final TraceContext.Injector>> injector; + final HttpClientHandler 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,131 @@ final class TracingFeignClient implements Client { } @Override - public Response execute(Request request, Request.Options options) throws IOException { - Map> 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) { // possibly null on bad implementation or mocks + response = new HttpClientResponse(res); + } + 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> 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> 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 { + Map> 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 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 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(); } } From 8431588db7f3667e7182f5d8197f55636f16fd67 Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Fri, 24 Jan 2020 08:42:05 +0800 Subject: [PATCH 3/4] Removes deprecations from server code (#1534) --- .../sleuth/instrument/web/TraceWebFilter.java | 147 +++++++++--------- .../instrument/zuul/TracePostZuulFilter.java | 52 ++++++- 2 files changed, 118 insertions(+), 81 deletions(-) diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceWebFilter.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceWebFilter.java index 4d197ba21..d92d8683f 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceWebFilter.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceWebFilter.java @@ -16,13 +16,15 @@ package org.springframework.cloud.sleuth.instrument.web; +import java.net.InetSocketAddress; import java.util.concurrent.atomic.AtomicBoolean; import brave.Span; import brave.Tracer; import brave.http.HttpServerHandler; +import brave.http.HttpServerRequest; +import brave.http.HttpServerResponse; import brave.http.HttpTracing; -import brave.propagation.Propagation; import brave.propagation.TraceContext; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -35,10 +37,8 @@ import reactor.util.context.Context; import org.springframework.beans.factory.BeanFactory; import org.springframework.core.Ordered; -import org.springframework.http.HttpHeaders; import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.http.server.reactive.ServerHttpResponse; -import org.springframework.http.server.reactive.ServerHttpResponseDecorator; import org.springframework.web.method.HandlerMethod; import org.springframework.web.reactive.HandlerMapping; import org.springframework.web.server.ServerWebExchange; @@ -65,18 +65,6 @@ public final class TraceWebFilter implements WebFilter, Ordered { + ".TRACE"; static final String MVC_CONTROLLER_CLASS_KEY = "mvc.controller.class"; static final String MVC_CONTROLLER_METHOD_KEY = "mvc.controller.method"; - static final Propagation.Getter GETTER = new Propagation.Getter() { - - @Override - public String get(HttpHeaders carrier, String key) { - return carrier.getFirst(key); - } - - @Override - public String toString() { - return "HttpHeaders::getFirst"; - } - }; private static final Log log = LogFactory.getLog(TraceWebFilter.class); @@ -89,9 +77,7 @@ public final class TraceWebFilter implements WebFilter, Ordered { Tracer tracer; - HttpServerHandler handler; - - TraceContext.Extractor extractor; + HttpServerHandler handler; SleuthWebProperties webProperties; @@ -104,11 +90,10 @@ public final class TraceWebFilter implements WebFilter, Ordered { } @SuppressWarnings("unchecked") - HttpServerHandler handler() { + HttpServerHandler handler() { if (this.handler == null) { - this.handler = HttpServerHandler.create( - this.beanFactory.getBean(HttpTracing.class), - new TraceWebFilter.HttpAdapter()); + this.handler = HttpServerHandler + .create(this.beanFactory.getBean(HttpTracing.class)); } return this.handler; } @@ -120,14 +105,6 @@ public final class TraceWebFilter implements WebFilter, Ordered { return this.tracer; } - TraceContext.Extractor extractor() { - if (this.extractor == null) { - this.extractor = this.beanFactory.getBean(HttpTracing.class).tracing() - .propagation().extractor(GETTER); - } - return this.extractor; - } - SleuthWebProperties sleuthWebProperties() { if (this.webProperties == null) { this.webProperties = this.beanFactory.getBean(SleuthWebProperties.class); @@ -163,9 +140,7 @@ public final class TraceWebFilter implements WebFilter, Ordered { final Span attrSpan; - final HttpServerHandler handler; - - final TraceContext.Extractor extractor; + final HttpServerHandler handler; final AtomicBoolean initialSpanAlreadyRemoved = new AtomicBoolean(); @@ -175,7 +150,6 @@ public final class TraceWebFilter implements WebFilter, Ordered { boolean initialTracePresent, TraceWebFilter parent) { super(source); this.tracer = parent.tracer(); - this.extractor = parent.extractor(); this.handler = parent.handler(); this.exchange = exchange; this.attrSpan = exchange.getAttribute(TRACE_REQUEST_ATTR); @@ -214,9 +188,8 @@ public final class TraceWebFilter implements WebFilter, Ordered { } } else { - span = this.handler.handleReceive(this.extractor, - this.exchange.getRequest().getHeaders(), - this.exchange.getRequest()); + span = this.handler.handleReceive( + new WrappedRequest(this.exchange.getRequest())); if (log.isDebugEnabled()) { log.debug("Handled receive of span " + span); } @@ -236,7 +209,7 @@ public final class TraceWebFilter implements WebFilter, Ordered { final ServerWebExchange exchange; - final HttpServerHandler handler; + final HttpServerHandler handler; WebFilterTraceSubscriber(CoreSubscriber actual, Context context, Span span, MonoWebFilterTrace parent) { @@ -284,10 +257,10 @@ public final class TraceWebFilter implements WebFilter, Ordered { String httpRoute = pattern != null ? pattern.toString() : ""; addResponseTagsForSpanWithoutParent(this.exchange, this.exchange.getResponse(), this.span); - DecoratedServerHttpResponse delegate = new DecoratedServerHttpResponse( + WrappedResponse response = new WrappedResponse( this.exchange.getResponse(), this.exchange.getRequest().getMethodValue(), httpRoute); - this.handler.handleSend(delegate, t, this.span); + this.handler.handleSend(response, t, this.span); if (log.isDebugEnabled()) { log.debug("Handled send of " + this.span); } @@ -339,60 +312,84 @@ public final class TraceWebFilter implements WebFilter, Ordered { } - static final class DecoratedServerHttpResponse extends ServerHttpResponseDecorator { + static final class WrappedRequest extends HttpServerRequest { + + final ServerHttpRequest delegate; + + WrappedRequest(ServerHttpRequest delegate) { + this.delegate = delegate; + } + + @Override + public ServerHttpRequest unwrap() { + return delegate; + } + + @Override + public boolean parseClientIpAndPort(Span span) { + InetSocketAddress addr = delegate.getRemoteAddress(); + if (addr == null) { + return false; + } + return span.remoteIpAndPort(addr.getAddress().getHostAddress(), + addr.getPort()); + } + + @Override + public String method() { + return delegate.getMethodValue(); + } + + @Override + public String path() { + return delegate.getPath().toString(); + } + + @Override + public String url() { + return delegate.getURI().toString(); + } + + @Override + public String header(String name) { + return delegate.getHeaders().getFirst(name); + } + + } + + static final class WrappedResponse extends HttpServerResponse { + + final ServerHttpResponse delegate; final String method; final String httpRoute; - DecoratedServerHttpResponse(ServerHttpResponse delegate, String method, - String httpRoute) { - super(delegate); + WrappedResponse(ServerHttpResponse resp, String method, String httpRoute) { + this.delegate = resp; this.method = method; this.httpRoute = httpRoute; } - } - - static final class HttpAdapter - extends brave.http.HttpServerAdapter { - @Override - public String method(ServerHttpRequest request) { - return request.getMethodValue(); + public String method() { + return method; } @Override - public String url(ServerHttpRequest request) { - return request.getURI().toString(); + public String route() { + return httpRoute; } @Override - public String requestHeader(ServerHttpRequest request, String name) { - Object result = request.getHeaders().getFirst(name); - return result != null ? result.toString() : null; + public ServerHttpResponse unwrap() { + return delegate; } @Override - public Integer statusCode(ServerHttpResponse response) { - return response.getStatusCode() != null ? response.getStatusCode().value() - : null; - } - - @Override - public String methodFromResponse(ServerHttpResponse response) { - if (response instanceof DecoratedServerHttpResponse) { - return ((DecoratedServerHttpResponse) response).method; - } - return null; - } - - @Override - public String route(ServerHttpResponse response) { - if (response instanceof DecoratedServerHttpResponse) { - return ((DecoratedServerHttpResponse) response).httpRoute; - } - return null; + public int statusCode() { + return delegate.getStatusCode() != null ? delegate.getStatusCode().value() + : 0; } } diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TracePostZuulFilter.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TracePostZuulFilter.java index afb225075..0bd4e30e2 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TracePostZuulFilter.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TracePostZuulFilter.java @@ -16,13 +16,13 @@ package org.springframework.cloud.sleuth.instrument.zuul; +import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import brave.Span; import brave.Tracer; import brave.http.HttpServerHandler; import brave.http.HttpTracing; -import brave.servlet.HttpServletAdapter; import com.netflix.zuul.ZuulFilter; import com.netflix.zuul.context.RequestContext; import org.apache.commons.logging.Log; @@ -40,12 +40,12 @@ class TracePostZuulFilter extends ZuulFilter { private static final Log log = LogFactory.getLog(TracePostZuulFilter.class); - private final HttpServerHandler handler; + final HttpServerHandler handler; - private final Tracer tracer; + final Tracer tracer; TracePostZuulFilter(HttpTracing httpTracing) { - this.handler = HttpServerHandler.create(httpTracing, new HttpServletAdapter()); + this.handler = HttpServerHandler.create(httpTracing); this.tracer = httpTracing.tracing().tracer(); } @@ -69,10 +69,13 @@ class TracePostZuulFilter extends ZuulFilter { if (log.isDebugEnabled()) { log.debug("Marking current span as handled"); } - HttpServletResponse response = RequestContext.getCurrentContext().getResponse(); + HttpServletRequest req = RequestContext.getCurrentContext().getRequest(); + HttpServletResponse resp = RequestContext.getCurrentContext().getResponse(); + HttpServerResponse request = resp != null ? new HttpServerResponse(req, resp) + : null; Throwable exception = RequestContext.getCurrentContext().getThrowable(); Span currentSpan = this.tracer.currentSpan(); - this.handler.handleSend(response, exception, currentSpan); + this.handler.handleSend(request, exception, currentSpan); if (log.isDebugEnabled()) { log.debug("Handled send of " + currentSpan); } @@ -89,4 +92,41 @@ class TracePostZuulFilter extends ZuulFilter { return 0; } + // copy/paste for now https://github.com/openzipkin/brave/issues/1064 + static final class HttpServerResponse extends brave.http.HttpServerResponse { + + final HttpServletResponse delegate; + + final String method; + + final String httpRoute; + + HttpServerResponse(HttpServletRequest req, HttpServletResponse resp) { + this.delegate = resp; + this.method = req.getMethod(); + this.httpRoute = (String) req.getAttribute("http.route"); + } + + @Override + public String method() { + return method; + } + + @Override + public String route() { + return httpRoute; + } + + @Override + public HttpServletResponse unwrap() { + return delegate; + } + + @Override + public int statusCode() { + return delegate.getStatus(); + } + + } + } From 8615056527884a981bfa7e4ede7a18ab4e2e5d4b Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Tue, 28 Jan 2020 09:05:35 +0100 Subject: [PATCH 4/4] Removed the unnused modules; fixes gh-1538 --- spring-cloud-sleuth-dependencies/pom.xml | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/spring-cloud-sleuth-dependencies/pom.xml b/spring-cloud-sleuth-dependencies/pom.xml index 193ee300a..a019df6fc 100644 --- a/spring-cloud-sleuth-dependencies/pom.xml +++ b/spring-cloud-sleuth-dependencies/pom.xml @@ -42,26 +42,11 @@ spring-cloud-sleuth-core ${project.version} - - org.springframework.cloud - spring-cloud-sleuth-zipkin-legacy - ${project.version} - org.springframework.cloud spring-cloud-sleuth-zipkin ${project.version} - - org.springframework.cloud - spring-cloud-sleuth-stream - ${project.version} - - - org.springframework.cloud - spring-cloud-sleuth-zipkin-stream - ${project.version} - org.springframework.cloud spring-cloud-starter-zipkin