From 2e902a7fecdc32609c3ef402c474646e37b5db51 Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Fri, 26 Feb 2021 13:53:52 +0100 Subject: [PATCH] Makes the new queueInstrumentation opt-in --- .../cloud/sleuth/benchmarks/jmh/Pair.java | 4 ++-- .../jmh/stream/MicroBenchmarkStreamTests.java | 18 ++++++++++-------- .../jmh/webflux/MicroBenchmarkHttpTests.java | 16 ++++++++++------ .../src/main/asciidoc/spring-cloud-sleuth.adoc | 2 +- .../reactor/TraceReactorAutoConfiguration.java | 2 +- .../web/TraceNoWebEnvironmentTests.java | 3 +-- .../web/client/integration/WebClientTests.java | 2 +- .../reactor/sample/FlatMapTests.java | 1 + 8 files changed, 27 insertions(+), 21 deletions(-) diff --git a/benchmarks/src/test/java/org/springframework/cloud/sleuth/benchmarks/jmh/Pair.java b/benchmarks/src/test/java/org/springframework/cloud/sleuth/benchmarks/jmh/Pair.java index 8799e4ba8..797a7b57e 100644 --- a/benchmarks/src/test/java/org/springframework/cloud/sleuth/benchmarks/jmh/Pair.java +++ b/benchmarks/src/test/java/org/springframework/cloud/sleuth/benchmarks/jmh/Pair.java @@ -33,8 +33,8 @@ public class Pair { return new Pair(key, value); } - public static Pair noHook() { - return new Pair("spring.sleuth.reactor.decorate-queues", "false"); + public static Pair onHook() { + return new Pair("spring.sleuth.reactor.decorate-queues", "true"); } public static Pair noSleuth() { diff --git a/benchmarks/src/test/java/org/springframework/cloud/sleuth/benchmarks/jmh/stream/MicroBenchmarkStreamTests.java b/benchmarks/src/test/java/org/springframework/cloud/sleuth/benchmarks/jmh/stream/MicroBenchmarkStreamTests.java index d58dee940..10fc85687 100644 --- a/benchmarks/src/test/java/org/springframework/cloud/sleuth/benchmarks/jmh/stream/MicroBenchmarkStreamTests.java +++ b/benchmarks/src/test/java/org/springframework/cloud/sleuth/benchmarks/jmh/stream/MicroBenchmarkStreamTests.java @@ -22,6 +22,7 @@ import java.util.List; import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; +import brave.Tracer; import brave.Tracing; import jmh.mbr.junit5.Microbenchmark; import org.junit.platform.commons.annotation.Testable; @@ -134,6 +135,7 @@ public class MicroBenchmarkStreamTests { } else { assertThat(b3).startsWith("4883117762eb9420"); } + assertThat(this.applicationContext.getBean(Tracer.class).currentSpan()).isNull(); } } @@ -155,15 +157,15 @@ public class MicroBenchmarkStreamTests { // @formatter:off noSleuthSimple(Pair.noSleuth(), function("simple")), - sleuthSimpleOnHooks(function("simple")), - sleuthSimpleOnEach(function("simple"), Pair.noHook(), Pair.onEach()), - sleuthSimpleOnLast(function("simple"), Pair.noHook(), Pair.onLast()), - sleuthSimpleWithAroundOnHooks(function("simple_function_with_around")), - sleuthSimpleWithAroundOnEach(function("simple_function_with_around"), Pair.noHook(), Pair.onEach()), - sleuthSimpleWithAroundOnLast(function("simple_function_with_around"), Pair.noHook(), Pair.onLast()), + sleuthSimpleOnHooks(function("simple"), Pair.onHook()), + sleuthSimpleOnEach(function("simple"), Pair.onEach()), + sleuthSimpleOnLast(function("simple"), Pair.onLast()), + sleuthSimpleWithAroundOnHooks(function("simple_function_with_around"), Pair.onHook()), + sleuthSimpleWithAroundOnEach(function("simple_function_with_around"), Pair.onEach()), + sleuthSimpleWithAroundOnLast(function("simple_function_with_around"), Pair.onLast()), noSleuthReactiveSimple(function("reactive_simple"), Pair.noSleuth()), - sleuthReactiveSimpleOnHooks(function("DECORATE_ON_EACH")), - sleuthReactiveSimpleOnEach(function("DECORATE_ON_EACH"), Pair.noHook(), Pair.onEach(), integrationEnabled()); + sleuthReactiveSimpleOnHooks(function("DECORATE_ON_EACH"), Pair.onHook()), + sleuthReactiveSimpleOnEach(function("DECORATE_ON_EACH"), Pair.onEach(), integrationEnabled()); // @formatter:on private List pairs; diff --git a/benchmarks/src/test/java/org/springframework/cloud/sleuth/benchmarks/jmh/webflux/MicroBenchmarkHttpTests.java b/benchmarks/src/test/java/org/springframework/cloud/sleuth/benchmarks/jmh/webflux/MicroBenchmarkHttpTests.java index 2b622e136..8bf4d362b 100644 --- a/benchmarks/src/test/java/org/springframework/cloud/sleuth/benchmarks/jmh/webflux/MicroBenchmarkHttpTests.java +++ b/benchmarks/src/test/java/org/springframework/cloud/sleuth/benchmarks/jmh/webflux/MicroBenchmarkHttpTests.java @@ -22,6 +22,7 @@ import java.util.List; import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; +import brave.Tracer; import brave.Tracing; import jmh.mbr.junit5.Microbenchmark; import org.junit.platform.commons.annotation.Testable; @@ -47,6 +48,8 @@ import org.springframework.cloud.sleuth.benchmarks.jmh.TracerImplementation; import org.springframework.context.ConfigurableApplicationContext; import org.springframework.test.web.reactive.server.WebTestClient; +import static org.assertj.core.api.Assertions.assertThat; + @Measurement(iterations = 10, time = 1) @Warmup(iterations = 10, time = 1) @Fork(4) @@ -98,6 +101,7 @@ public class MicroBenchmarkHttpTests { void run() { this.webTestClient.get().uri(instrumentation.url).header("X-B3-TraceId", "4883117762eb9420") .header("X-B3-SpanId", "4883117762eb9420").exchange().expectStatus().isOk(); + assertThat(this.applicationContext.getBean(Tracer.class).currentSpan()).isNull(); } @TearDown @@ -118,13 +122,13 @@ public class MicroBenchmarkHttpTests { // @formatter:off noSleuthSimple("/simple", Pair.noSleuth()), - sleuthSimpleOnHooks("/simple"), - sleuthSimpleOnEach("/simple", Pair.noHook(), Pair.onEach()), - sleuthSimpleOnLast("/simple", Pair.noHook(), Pair.onLast()), + sleuthSimpleOnHooks("/simple", Pair.onHook()), + sleuthSimpleOnEach("/simple", Pair.onEach()), + sleuthSimpleOnLast("/simple", Pair.onLast()), noSleuthComplex("/complexNoSleuth", Pair.noSleuth()), - onHooksComplex("/complex"), - onEachComplex("/complex", Pair.noHook(), Pair.onEach()), - onLastComplex("/complex", Pair.noHook(), Pair.onLast()); + onHooksComplex("/complex", Pair.onHook()), + onEachComplex("/complex", Pair.onEach()), + onLastComplex("/complex", Pair.onLast()); // @formatter:on private String url; diff --git a/docs/src/main/asciidoc/spring-cloud-sleuth.adoc b/docs/src/main/asciidoc/spring-cloud-sleuth.adoc index c018d932d..b6698b3c6 100644 --- a/docs/src/main/asciidoc/spring-cloud-sleuth.adoc +++ b/docs/src/main/asciidoc/spring-cloud-sleuth.adoc @@ -1528,7 +1528,7 @@ To turn off this feature, set the `spring.sleuth.quartz.enabled` property to `fa ==== From Spring Cloud Sleuth 2.2.8 (inclusive) -With the new Reactor https://github.com/reactor/reactor-core/pull/2566[queue wrapping mechanism] (Reactor 3.3.14) we're instrumenting the way threads are switched by Reactor. You should observe significant improvement in performance. In order to disable this feature you have to set the `spring.sleuth.reactor.decorate-queues` option to `false`. You'll fall back to the previous instrumentation mode mechanism. +With the new Reactor https://github.com/reactor/reactor-core/pull/2566[queue wrapping mechanism] (Reactor 3.3.14) we're instrumenting the way threads are switched by Reactor. You should observe significant improvement in performance. In order to enable this feature you have to set the `spring.sleuth.reactor.decorate-queues` option to `true`. ==== To Spring Cloud Sleuth 2.2.8 (exclusive) diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/reactor/TraceReactorAutoConfiguration.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/reactor/TraceReactorAutoConfiguration.java index 52fee9b6a..0a24de6be 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/reactor/TraceReactorAutoConfiguration.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/reactor/TraceReactorAutoConfiguration.java @@ -234,7 +234,7 @@ class HookRegisteringBeanDefinitionRegistryPostProcessor "You have explicitly set the decorate hooks option but you're using an old version of Reactor. Please upgrade to the latest Boot version (at least 2.3.9.RELEASE). Will fall back to the previous reactor instrumentation mode"); } else { - decorateQueues = decorateQueues != null ? decorateQueues : Boolean.TRUE; + decorateQueues = decorateQueues != null ? decorateQueues : Boolean.FALSE; } if (wrapperOnClasspathHooksPropertyTurnedOn(decorateQueues)) { if (log.isTraceEnabled()) { diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceNoWebEnvironmentTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceNoWebEnvironmentTests.java index 5def5459f..a1ca23782 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceNoWebEnvironmentTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceNoWebEnvironmentTests.java @@ -49,8 +49,7 @@ public class TraceNoWebEnvironmentTests { } catch (Exception e) { Throwable cause = e.getCause() != null ? e.getCause() : e; - then(cause.getClass()) - .isNotEqualTo(NoSuchBeanDefinitionException.class); + then(cause.getClass()).isNotEqualTo(NoSuchBeanDefinitionException.class); } } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/integration/WebClientTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/integration/WebClientTests.java index d656c86d0..45df2a695 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/integration/WebClientTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/integration/WebClientTests.java @@ -167,7 +167,7 @@ public class WebClientTests { this.testErrorController.clear(); this.fooController.clear(); } - + @Before public void setup() { log.info("Starting test"); diff --git a/tests/spring-cloud-sleuth-instrumentation-reactor-tests/src/test/java/org/springframework/cloud/sleuth/instrument/reactor/sample/FlatMapTests.java b/tests/spring-cloud-sleuth-instrumentation-reactor-tests/src/test/java/org/springframework/cloud/sleuth/instrument/reactor/sample/FlatMapTests.java index 1dae54a11..8b81b0d4e 100644 --- a/tests/spring-cloud-sleuth-instrumentation-reactor-tests/src/test/java/org/springframework/cloud/sleuth/instrument/reactor/sample/FlatMapTests.java +++ b/tests/spring-cloud-sleuth-instrumentation-reactor-tests/src/test/java/org/springframework/cloud/sleuth/instrument/reactor/sample/FlatMapTests.java @@ -88,6 +88,7 @@ public class FlatMapTests { .web(WebApplicationType.REACTIVE) .properties("server.port=0", "spring.jmx.enabled=false", "spring.application.name=TraceWebFluxOnHooksTests", + "spring.sleuth.reactor.decorate-queues=true", "security.basic.enabled=false", "management.security.enabled=false") .run();