From 0f458b03ee30cf83b3adb6cc264585e70f91698a Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Thu, 10 Dec 2020 11:41:48 +0100 Subject: [PATCH] Making Redis tracing lazy; fixes gh-1756 --- .../redis/TraceRedisAutoConfiguration.java | 204 +++++++++++++++--- src/checkstyle/checkstyle-suppressions.xml | 1 + .../TraceRedisAutoConfigurationTests.java | 22 +- 3 files changed, 186 insertions(+), 41 deletions(-) diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/redis/TraceRedisAutoConfiguration.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/redis/TraceRedisAutoConfiguration.java index dd42c0a59..a254e88b6 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/redis/TraceRedisAutoConfiguration.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/redis/TraceRedisAutoConfiguration.java @@ -16,9 +16,15 @@ package org.springframework.cloud.sleuth.instrument.redis; +import java.net.SocketAddress; + import brave.Tracing; import io.lettuce.core.resource.ClientResources; import io.lettuce.core.tracing.BraveTracing; +import io.lettuce.core.tracing.TraceContext; +import io.lettuce.core.tracing.TraceContextProvider; +import io.lettuce.core.tracing.Tracer; +import io.lettuce.core.tracing.TracerProvider; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -30,6 +36,7 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.cloud.sleuth.autoconfig.TraceAutoConfiguration; +import org.springframework.cloud.sleuth.internal.ContextUtil; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -50,16 +57,10 @@ import org.springframework.context.annotation.Configuration; @EnableConfigurationProperties(TraceRedisProperties.class) public class TraceRedisAutoConfiguration { - @Configuration(proxyBeanMethods = false) - static class LettuceConfig { - - @Bean - static TraceLettuceClientResourcesBeanPostProcessor traceLettuceClientResourcesBeanPostProcessor( - BeanFactory beanFactory, TraceRedisProperties traceRedisProperties) { - return new TraceLettuceClientResourcesBeanPostProcessor(beanFactory, - traceRedisProperties); - } - + @Bean + static TraceLettuceClientResourcesBeanPostProcessor traceLettuceClientResourcesBeanPostProcessor( + BeanFactory beanFactory) { + return new TraceLettuceClientResourcesBeanPostProcessor(beanFactory); } } @@ -71,14 +72,8 @@ class TraceLettuceClientResourcesBeanPostProcessor implements BeanPostProcessor private final BeanFactory beanFactory; - private final TraceRedisProperties traceRedisProperties; - - private Tracing tracing; - - TraceLettuceClientResourcesBeanPostProcessor(BeanFactory beanFactory, - TraceRedisProperties traceRedisProperties) { + TraceLettuceClientResourcesBeanPostProcessor(BeanFactory beanFactory) { this.beanFactory = beanFactory; - this.traceRedisProperties = traceRedisProperties; } @Override @@ -97,12 +92,9 @@ class TraceLettuceClientResourcesBeanPostProcessor implements BeanPostProcessor log.debug( "Lettuce ClientResources bean is auto-configured to enable tracing."); } - BraveTracing lettuceTracing = BraveTracing.builder().tracing(tracing()) - .excludeCommandArgsFromSpanTags() - .serviceName(traceRedisProperties.getRemoteServiceName()).build(); - return cr.mutate().tracing(lettuceTracing).build(); + return cr.mutate().tracing(new LazyTracing(this.beanFactory)).build(); } - if (log.isDebugEnabled()) { + else if (log.isDebugEnabled()) { log.debug( "Lettuce ClientResources bean is skipped for auto-configuration because tracing was already enabled."); } @@ -110,11 +102,171 @@ class TraceLettuceClientResourcesBeanPostProcessor implements BeanPostProcessor return bean; } - private Tracing tracing() { - if (this.tracing == null) { - this.tracing = this.beanFactory.getBean(Tracing.class); +} + +class LazyTracing implements io.lettuce.core.tracing.Tracing { + + private final BeanFactory beanFactory; + + private final io.lettuce.core.tracing.Tracing noOpTracing = NoOpTracing.INSTANCE; + + private BraveTracing braveTracing; + + LazyTracing(BeanFactory beanFactory) { + this.beanFactory = beanFactory; + } + + @Override + public TracerProvider getTracerProvider() { + if (ContextUtil.isContextUnusable(this.beanFactory)) { + return this.noOpTracing.getTracerProvider(); } - return this.tracing; + return braveTracing().getTracerProvider(); + } + + @Override + public TraceContextProvider initialTraceContextProvider() { + if (ContextUtil.isContextUnusable(this.beanFactory)) { + return this.noOpTracing.initialTraceContextProvider(); + } + return braveTracing().initialTraceContextProvider(); + } + + @Override + public boolean isEnabled() { + if (ContextUtil.isContextUnusable(this.beanFactory)) { + return this.noOpTracing.isEnabled(); + } + return braveTracing().isEnabled(); + } + + @Override + public boolean includeCommandArgsInSpanTags() { + if (ContextUtil.isContextUnusable(this.beanFactory)) { + return this.noOpTracing.includeCommandArgsInSpanTags(); + } + return braveTracing().includeCommandArgsInSpanTags(); + } + + @Override + public Endpoint createEndpoint(SocketAddress socketAddress) { + if (ContextUtil.isContextUnusable(this.beanFactory)) { + return this.noOpTracing.createEndpoint(socketAddress); + } + return braveTracing().createEndpoint(socketAddress); + } + + private BraveTracing braveTracing() { + if (this.braveTracing == null) { + this.braveTracing = BraveTracing.builder() + .tracing(this.beanFactory.getBean(Tracing.class)) + .excludeCommandArgsFromSpanTags().serviceName(this.beanFactory + .getBean(TraceRedisProperties.class).getRemoteServiceName()) + .build(); + } + return this.braveTracing; + } + +} + +enum NoOpTracing + implements io.lettuce.core.tracing.Tracing, TraceContextProvider, TracerProvider { + + INSTANCE; + + private final Endpoint NOOP_ENDPOINT = new Endpoint() { + }; + + @Override + public TraceContext getTraceContext() { + return TraceContext.EMPTY; + } + + @Override + public Tracer getTracer() { + return NoOpTracer.INSTANCE; + } + + @Override + public TracerProvider getTracerProvider() { + return this; + } + + @Override + public TraceContextProvider initialTraceContextProvider() { + return this; + } + + @Override + public boolean isEnabled() { + return false; + } + + @Override + public boolean includeCommandArgsInSpanTags() { + return false; + } + + @Override + public Endpoint createEndpoint(SocketAddress socketAddress) { + return NOOP_ENDPOINT; + } + + static class NoOpTracer extends Tracer { + + static final Tracer INSTANCE = new NoOpTracer(); + + @Override + public Span nextSpan(TraceContext traceContext) { + return NoOpSpan.INSTANCE; + } + + @Override + public Span nextSpan() { + return NoOpSpan.INSTANCE; + } + + } + + public static class NoOpSpan extends Tracer.Span { + + static final NoOpSpan INSTANCE = new NoOpSpan(); + + @Override + public Tracer.Span start() { + return this; + } + + @Override + public Tracer.Span name(String name) { + return this; + } + + @Override + public Tracer.Span annotate(String value) { + return this; + } + + @Override + public Tracer.Span tag(String key, String value) { + return this; + } + + @Override + public Tracer.Span error(Throwable throwable) { + return this; + } + + @Override + public Tracer.Span remoteEndpoint( + io.lettuce.core.tracing.Tracing.Endpoint endpoint) { + return this; + } + + @Override + public void finish() { + } + } } diff --git a/src/checkstyle/checkstyle-suppressions.xml b/src/checkstyle/checkstyle-suppressions.xml index e75174d51..88af9289a 100644 --- a/src/checkstyle/checkstyle-suppressions.xml +++ b/src/checkstyle/checkstyle-suppressions.xml @@ -32,5 +32,6 @@ + diff --git a/tests/spring-cloud-sleuth-instrumentation-lettuce-tests/src/test/java/org/springframework/cloud/sleuth/instrument/redis/TraceRedisAutoConfigurationTests.java b/tests/spring-cloud-sleuth-instrumentation-lettuce-tests/src/test/java/org/springframework/cloud/sleuth/instrument/redis/TraceRedisAutoConfigurationTests.java index 872065b7b..7e732338d 100644 --- a/tests/spring-cloud-sleuth-instrumentation-lettuce-tests/src/test/java/org/springframework/cloud/sleuth/instrument/redis/TraceRedisAutoConfigurationTests.java +++ b/tests/spring-cloud-sleuth-instrumentation-lettuce-tests/src/test/java/org/springframework/cloud/sleuth/instrument/redis/TraceRedisAutoConfigurationTests.java @@ -36,7 +36,9 @@ import static org.assertj.core.api.BDDAssertions.then; */ @RunWith(SpringRunner.class) @SpringBootTest(classes = TraceRedisAutoConfigurationTests.Config.class, - webEnvironment = SpringBootTest.WebEnvironment.NONE) + webEnvironment = SpringBootTest.WebEnvironment.NONE, + properties = { "spring.sleuth.redis.enabled=true", + "spring.sleuth.redis.remote-service-name=redis-foo" }) public class TraceRedisAutoConfigurationTests { @Autowired @@ -62,19 +64,10 @@ public class TraceRedisAutoConfigurationTests { return clientResources; } - @Bean - TraceRedisProperties traceRedisProperties() { - TraceRedisProperties traceRedisProperties = new TraceRedisProperties(); - traceRedisProperties.setEnabled(true); - traceRedisProperties.setRemoteServiceName("redis-foo"); - return traceRedisProperties; - } - @Bean TestTraceLettuceClientResourcesBeanPostProcessor testTraceLettuceClientResourcesBeanPostProcessor( - BeanFactory beanFactory, TraceRedisProperties traceRedisProperties) { - return new TestTraceLettuceClientResourcesBeanPostProcessor(beanFactory, - traceRedisProperties); + BeanFactory beanFactory) { + return new TestTraceLettuceClientResourcesBeanPostProcessor(beanFactory); } } @@ -86,9 +79,8 @@ class TestTraceLettuceClientResourcesBeanPostProcessor boolean tracingCalled = false; - TestTraceLettuceClientResourcesBeanPostProcessor(BeanFactory beanFactory, - TraceRedisProperties traceRedisProperties) { - super(beanFactory, traceRedisProperties); + TestTraceLettuceClientResourcesBeanPostProcessor(BeanFactory beanFactory) { + super(beanFactory); } @Override