Fixed brave redis setup

fixes gh-1971

spring-cloud-sleuth-autoconfigure/src/main/java/org/springframework/cloud/sleuth/autoconfig/brave/instrument/redis/BraveRedisAutoConfiguration.java

@@ -17,17 +17,23 @@
 package org.springframework.cloud.sleuth.autoconfig.brave.instrument.redis;
 
 import brave.Tracing;
+import brave.sampler.Sampler;
 import io.lettuce.core.resource.ClientResources;
+import io.lettuce.core.resource.DefaultClientResources;
 import io.lettuce.core.tracing.BraveTracing;
 
-import org.springframework.beans.factory.BeanFactory;
+import org.springframework.beans.factory.ObjectProvider;
 import org.springframework.boot.autoconfigure.AutoConfigureAfter;
+import org.springframework.boot.autoconfigure.AutoConfigureBefore;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnBean;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
+import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
+import org.springframework.boot.autoconfigure.data.redis.RedisAutoConfiguration;
 import org.springframework.boot.context.properties.EnableConfigurationProperties;
 import org.springframework.cloud.sleuth.autoconfig.brave.BraveAutoConfiguration;
-import org.springframework.cloud.sleuth.autoconfig.instrument.redis.TraceLettuceClientResourcesBeanPostProcessor;
+import org.springframework.cloud.sleuth.brave.instrument.redis.ClientResourcesBuilderCustomizer;
+import org.springframework.cloud.sleuth.brave.instrument.redis.TraceLettuceClientResourcesBuilderCustomizer;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 
@@ -40,16 +46,37 @@ import org.springframework.context.annotation.Configuration;
  */
 @Configuration(proxyBeanMethods = false)
 @ConditionalOnProperty(value = "spring.sleuth.redis.enabled", matchIfMissing = true)
-@ConditionalOnBean({ Tracing.class, ClientResources.class })
+@ConditionalOnBean(Tracing.class)
 @AutoConfigureAfter({ BraveAutoConfiguration.class })
+@AutoConfigureBefore({ RedisAutoConfiguration.class })
 @EnableConfigurationProperties(TraceRedisProperties.class)
 @ConditionalOnClass(BraveTracing.class)
 public class BraveRedisAutoConfiguration {
 
+	// TODO: The customization auto configuration should come from Spring Boot
+	@Bean(destroyMethod = "shutdown")
+	@ConditionalOnMissingBean(ClientResources.class)
+	DefaultClientResources traceLettuceClientResources(ObjectProvider<ClientResourcesBuilderCustomizer> customizer) {
+		DefaultClientResources.Builder builder = DefaultClientResources.builder();
+		customizer.stream().forEach(c -> c.customize(builder));
+		return builder.build();
+	}
+
 	@Bean
-	static TraceLettuceClientResourcesBeanPostProcessor traceLettuceClientResourcesBeanPostProcessor(
-			BeanFactory beanFactory) {
-		return new TraceLettuceClientResourcesBeanPostProcessor(beanFactory);
+	TraceLettuceClientResourcesBuilderCustomizer traceLettuceClientResourcesBuilderCustomizer(Tracing tracing,
+			TraceRedisProperties traceRedisProperties, Sampler sampler) {
+		eagerlyInitializePotentiallyRefreshScopeSampler(sampler);
+		return new TraceLettuceClientResourcesBuilderCustomizer(tracing, traceRedisProperties.getRemoteServiceName());
+	}
+
+	/**
+	 * We need to do the eager method invocation. Since this might be @RefreshScope, a
+	 * proxy is being created. Trying to resolve the proxy from a different thread than
+	 * main can lead to cross thread locking.
+	 * @param sampler potentially refresh scope sampler
+	 */
+	private void eagerlyInitializePotentiallyRefreshScopeSampler(Sampler sampler) {
+		sampler.isSampled(0L);
 	}
 
 }

spring-cloud-sleuth-autoconfigure/src/main/java/org/springframework/cloud/sleuth/autoconfig/instrument/redis/TraceLettuceClientResourcesBeanPostProcessor.java

@@ -33,6 +33,7 @@ import org.springframework.beans.BeansException;
 import org.springframework.beans.factory.BeanFactory;
 import org.springframework.beans.factory.config.BeanPostProcessor;
 import org.springframework.cloud.sleuth.autoconfig.brave.instrument.redis.TraceRedisProperties;
+import org.springframework.cloud.sleuth.brave.instrument.redis.TraceLettuceClientResourcesBuilderCustomizer;
 import org.springframework.cloud.sleuth.internal.ContextUtil;
 
 /**
@@ -40,6 +41,7 @@ import org.springframework.cloud.sleuth.internal.ContextUtil;
  *
  * @author Marcin Grzejszczak
  * @since 3.0.0
+ * @deprecated please use {@link TraceLettuceClientResourcesBuilderCustomizer}.
  */
 public class TraceLettuceClientResourcesBeanPostProcessor implements BeanPostProcessor {
 
@@ -90,26 +92,27 @@ class LazyTracing implements io.lettuce.core.tracing.Tracing {
 
 	@Override
 	public TracerProvider getTracerProvider() {
-		if (ContextUtil.isContextUnusable(this.beanFactory)) {
-			return this.noOpTracing.getTracerProvider();
-		}
-		return braveTracing().getTracerProvider();
+		return () -> {
+			if (ContextUtil.isContextUnusable(beanFactory)) {
+				return noOpTracing.getTracerProvider().getTracer();
+			}
+			return braveTracing().getTracerProvider().getTracer();
+		};
 	}
 
 	@Override
 	public TraceContextProvider initialTraceContextProvider() {
-		if (ContextUtil.isContextUnusable(this.beanFactory)) {
-			return this.noOpTracing.initialTraceContextProvider();
-		}
-		return braveTracing().initialTraceContextProvider();
+		return () -> {
+			if (ContextUtil.isContextUnusable(beanFactory)) {
+				return noOpTracing.initialTraceContextProvider().getTraceContext();
+			}
+			return braveTracing().initialTraceContextProvider().getTraceContext();
+		};
 	}
 
 	@Override
 	public boolean isEnabled() {
-		if (ContextUtil.isContextUnusable(this.beanFactory)) {
-			return this.noOpTracing.isEnabled();
-		}
-		return braveTracing().isEnabled();
+		return true;
 	}
 
 	@Override
@@ -238,3 +241,4 @@ enum NoOpTracing implements io.lettuce.core.tracing.Tracing, TraceContextProvide
 	}
 
 }
+

spring-cloud-sleuth-brave/src/main/java/org/springframework/cloud/sleuth/brave/instrument/redis/ClientResourcesBuilderCustomizer.java

@@ -0,0 +1,34 @@
+/*
+ * Copyright 2013-2021 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.cloud.sleuth.brave.instrument.redis;
+
+import io.lettuce.core.resource.ClientResources;
+
+/**
+ * Customizer for {@link ClientResources.Builder}.
+ *
+ * @author Marcin Grzejszczak
+ * @since 3.0.4
+ */
+public interface ClientResourcesBuilderCustomizer {
+
+	/**
+	 * Customizes the builder.
+	 * @param builder builder to customize
+	 */
+	void customize(ClientResources.Builder builder);
+}

spring-cloud-sleuth-brave/src/main/java/org/springframework/cloud/sleuth/brave/instrument/redis/TraceLettuceClientResourcesBuilderCustomizer.java

@@ -0,0 +1,47 @@
+/*
+ * Copyright 2013-2021 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.cloud.sleuth.brave.instrument.redis;
+
+import brave.Tracing;
+import io.lettuce.core.resource.ClientResources;
+import io.lettuce.core.tracing.BraveTracing;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+
+import org.springframework.beans.factory.config.BeanPostProcessor;
+
+/**
+ * {@link BeanPostProcessor} for wrapping Lettuce components in a tracing representation.
+ *
+ * @author Marcin Grzejszczak
+ * @since 3.0.4
+ */
+public class TraceLettuceClientResourcesBuilderCustomizer implements ClientResourcesBuilderCustomizer {
+
+	private final BraveTracing tracing;
+
+	public TraceLettuceClientResourcesBuilderCustomizer(Tracing tracing, String serviceName) {
+		this.tracing = BraveTracing.builder().tracing(tracing)
+				.excludeCommandArgsFromSpanTags()
+				.serviceName(serviceName).build();
+	}
+
+	@Override
+	public void customize(ClientResources.Builder builder) {
+		builder.tracing(this.tracing);
+	}
+}

tests/brave/spring-cloud-sleuth-instrumentation-lettuce-tests/src/test/java/org/springframework/cloud/sleuth/brave/instrument/redis/BraveRedisAutoConfigurationTests.java

@@ -17,15 +17,12 @@
 package org.springframework.cloud.sleuth.brave.instrument.redis;
 
 import io.lettuce.core.resource.ClientResources;
+import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 
-import org.springframework.beans.BeansException;
-import org.springframework.beans.factory.BeanFactory;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
 import org.springframework.boot.test.context.SpringBootTest;
-import org.springframework.cloud.sleuth.autoconfig.instrument.redis.TraceLettuceClientResourcesBeanPostProcessor;
-import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 
 import static org.assertj.core.api.BDDAssertions.then;
@@ -42,47 +39,18 @@ public class BraveRedisAutoConfigurationTests {
 	ClientResources clientResources;
 
 	@Autowired
-	TestTraceLettuceClientResourcesBeanPostProcessor traceLettuceClientResourcesBeanPostProcessor;
+	TraceLettuceClientResourcesBuilderCustomizer customizer;
 
 	@Test
 	public void tracing_should_be_set() {
-		then(this.traceLettuceClientResourcesBeanPostProcessor.tracingCalled).isTrue();
 		then(this.clientResources.tracing().isEnabled()).isTrue();
+		then(this.customizer).isNotNull();
 	}
 
 	@Configuration(proxyBeanMethods = false)
 	@EnableAutoConfiguration
 	protected static class Config {
 
-		@Bean
-		ClientResources clientResources() {
-			ClientResources clientResources = ClientResources.create();
-			then(clientResources.tracing().isEnabled()).isFalse();
-			return clientResources;
-		}
-
-		@Bean
-		static TestTraceLettuceClientResourcesBeanPostProcessor testTraceLettuceClientResourcesBeanPostProcessor(
-				BeanFactory beanFactory) {
-			return new TestTraceLettuceClientResourcesBeanPostProcessor(beanFactory);
-		}
-
-	}
-
-}
-
-class TestTraceLettuceClientResourcesBeanPostProcessor extends TraceLettuceClientResourcesBeanPostProcessor {
-
-	boolean tracingCalled = false;
-
-	TestTraceLettuceClientResourcesBeanPostProcessor(BeanFactory beanFactory) {
-		super(beanFactory);
-	}
-
-	@Override
-	public Object postProcessAfterInitialization(Object bean, String beanName) throws BeansException {
-		this.tracingCalled = true;
-		return super.postProcessAfterInitialization(bean, beanName);
 	}
 
 }