diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/autoconfig/TraceAutoConfiguration.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/autoconfig/TraceAutoConfiguration.java index abf67c9c4..b7a99b743 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/autoconfig/TraceAutoConfiguration.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/autoconfig/TraceAutoConfiguration.java @@ -54,6 +54,7 @@ import org.springframework.cloud.sleuth.DefaultSpanNamer; import org.springframework.cloud.sleuth.LocalServiceName; import org.springframework.cloud.sleuth.SpanAdjuster; import org.springframework.cloud.sleuth.SpanNamer; +import org.springframework.cloud.sleuth.log.SleuthLogAutoConfiguration; import org.springframework.cloud.sleuth.sampler.SamplerAutoConfiguration; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -73,7 +74,7 @@ import org.springframework.util.StringUtils; @Configuration(proxyBeanMethods = false) @ConditionalOnProperty(value = "spring.sleuth.enabled", matchIfMissing = true) @EnableConfigurationProperties(SleuthProperties.class) -@Import(SamplerAutoConfiguration.class) +@Import({ SleuthLogAutoConfiguration.class, SamplerAutoConfiguration.class }) public class TraceAutoConfiguration { /** @@ -92,18 +93,12 @@ public class TraceAutoConfiguration { @Autowired(required = false) List finishedSpanHandlers = new ArrayList<>(); - @Autowired(required = false) - List scopeDecorators = new ArrayList<>(); - @Autowired(required = false) ExtraFieldPropagation.FactoryBuilder extraFieldPropagationFactoryBuilder; @Autowired(required = false) List tracingCustomizers = new ArrayList<>(); - @Autowired(required = false) - List currentTraceContextCustomizers = new ArrayList<>(); - @Autowired(required = false) List extraFieldCustomizers = new ArrayList<>(); @@ -186,11 +181,20 @@ public class TraceAutoConfiguration { } @Bean - CurrentTraceContext sleuthCurrentTraceContext(CurrentTraceContext.Builder builder) { - for (CurrentTraceContext.ScopeDecorator scopeDecorator : this.scopeDecorators) { + CurrentTraceContext sleuthCurrentTraceContext(CurrentTraceContext.Builder builder, + @Nullable List scopeDecorators, + @Nullable List currentTraceContextCustomizers) { + if (scopeDecorators == null) { + scopeDecorators = Collections.emptyList(); + } + if (currentTraceContextCustomizers == null) { + currentTraceContextCustomizers = Collections.emptyList(); + } + + for (CurrentTraceContext.ScopeDecorator scopeDecorator : scopeDecorators) { builder.addScopeDecorator(scopeDecorator); } - for (CurrentTraceContextCustomizer customizer : this.currentTraceContextCustomizers) { + for (CurrentTraceContextCustomizer customizer : currentTraceContextCustomizers) { customizer.customize(builder); } return builder.build(); diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/ExceptionLoggingFilter.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/ExceptionLoggingFilter.java index 3601994bd..a18a476df 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/ExceptionLoggingFilter.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/ExceptionLoggingFilter.java @@ -29,11 +29,9 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; /** - * Filter running after {@link brave.servlet.TracingFilter} that logs uncaught exceptions. - * - * @author Marcin Grzejszczak - * @since 2.0.0 + * @deprecated Since 2.2.3 this is disabled by default and will be removed in 3.0 */ +@Deprecated class ExceptionLoggingFilter implements Filter { private static final Log log = LogFactory.getLog(ExceptionLoggingFilter.class); diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceWebServletAutoConfiguration.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceWebServletAutoConfiguration.java index d39a71a69..866f51daa 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceWebServletAutoConfiguration.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceWebServletAutoConfiguration.java @@ -83,10 +83,8 @@ public class TraceWebServletAutoConfiguration { return filterRegistrationBean; } - // TODO: Rename to exception-logging-filter for 3.0 @Bean - @ConditionalOnProperty(value = "spring.sleuth.web.exception-logging-filter-enabled", - matchIfMissing = true) + @ConditionalOnProperty("spring.sleuth.web.exception-logging-filter-enabled") public FilterRegistrationBean exceptionThrowingFilter( SleuthWebProperties webProperties) { FilterRegistrationBean filterRegistrationBean = new FilterRegistrationBean( diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/log/SleuthLogAutoConfiguration.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/log/SleuthLogAutoConfiguration.java index ae1f852a5..9f91b4248 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/log/SleuthLogAutoConfiguration.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/log/SleuthLogAutoConfiguration.java @@ -19,28 +19,29 @@ package org.springframework.cloud.sleuth.log; import brave.propagation.CurrentTraceContext; import org.slf4j.MDC; -import org.springframework.boot.autoconfigure.AutoConfigureBefore; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.cloud.sleuth.autoconfig.SleuthProperties; -import org.springframework.cloud.sleuth.autoconfig.TraceAutoConfiguration; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; /** - * {@link org.springframework.boot.autoconfigure.EnableAutoConfiguration - * Auto-configuration} adds a {@link Slf4jScopeDecorator} that prints tracing information - * in the logs. + * {@link Configuration} that adds a {@link Slf4jScopeDecorator} that prints tracing + * information in the logs. *

* * @author Spencer Gibb * @author Marcin Grzejszczak * @since 2.0.0 + * @deprecated Do not use this type directly as it was removed in 3.x */ @Configuration(proxyBeanMethods = false) @ConditionalOnProperty(value = "spring.sleuth.enabled", matchIfMissing = true) -@AutoConfigureBefore(TraceAutoConfiguration.class) +// This is not auto-configuration, but it was in the past. Leaving the name as +// SleuthLogAutoConfiguration because some may have imported this directly. +// A less precise name is better than rev-locking code. +@Deprecated public class SleuthLogAutoConfiguration { /** @@ -49,7 +50,7 @@ public class SleuthLogAutoConfiguration { @Configuration(proxyBeanMethods = false) @ConditionalOnClass(MDC.class) @EnableConfigurationProperties(SleuthSlf4jProperties.class) - protected static class Slf4jConfiguration { + public static class Slf4jConfiguration { @Bean @ConditionalOnProperty(value = "spring.sleuth.log.slf4j.enabled", diff --git a/spring-cloud-sleuth-core/src/main/resources/META-INF/spring.factories b/spring-cloud-sleuth-core/src/main/resources/META-INF/spring.factories index 741ba6969..fcad7b053 100644 --- a/spring-cloud-sleuth-core/src/main/resources/META-INF/spring.factories +++ b/spring-cloud-sleuth-core/src/main/resources/META-INF/spring.factories @@ -2,7 +2,6 @@ org.springframework.boot.autoconfigure.EnableAutoConfiguration=\ org.springframework.cloud.sleuth.annotation.SleuthAnnotationAutoConfiguration,\ org.springframework.cloud.sleuth.autoconfig.TraceAutoConfiguration,\ -org.springframework.cloud.sleuth.log.SleuthLogAutoConfiguration,\ org.springframework.cloud.sleuth.propagation.SleuthTagPropagationAutoConfiguration,\ org.springframework.cloud.sleuth.instrument.web.TraceHttpAutoConfiguration,\ org.springframework.cloud.sleuth.instrument.web.TraceWebAutoConfiguration,\ diff --git a/tests/spring-cloud-sleuth-instrumentation-mvc-tests/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterWebIntegrationTests.java b/tests/spring-cloud-sleuth-instrumentation-mvc-tests/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterWebIntegrationTests.java index d66e04160..b24121300 100644 --- a/tests/spring-cloud-sleuth-instrumentation-mvc-tests/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterWebIntegrationTests.java +++ b/tests/spring-cloud-sleuth-instrumentation-mvc-tests/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterWebIntegrationTests.java @@ -22,15 +22,22 @@ import java.util.List; import java.util.regex.Pattern; import java.util.stream.Collectors; +import brave.Span.Kind; +import brave.handler.FinishedSpanHandler; +import brave.handler.MutableSpan; import brave.http.HttpRequest; import brave.http.HttpRequestParser; import brave.propagation.CurrentTraceContext; +import brave.propagation.CurrentTraceContext.Scope; +import brave.propagation.TraceContext; import brave.sampler.Sampler; import brave.sampler.SamplerFunction; import org.assertj.core.api.BDDAssertions; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import zipkin2.Span; import org.springframework.beans.factory.annotation.Autowired; @@ -64,6 +71,9 @@ import static org.assertj.core.api.BDDAssertions.then; properties = "spring.sleuth.http.legacy.enabled=true") public class TraceFilterWebIntegrationTests { + private static final Logger log = LoggerFactory + .getLogger(TraceFilterWebIntegrationTests.class); + @Autowired CurrentTraceContext currentTraceContext; @@ -92,7 +102,8 @@ public class TraceFilterWebIntegrationTests { } @Test - public void should_not_create_a_span_for_error_controller(CapturedOutput capture) { + public void exception_logging_span_handler_logs_synchronous_exceptions( + CapturedOutput capture) { try { new RestTemplate().getForObject("http://localhost:" + port() + "/", String.class); @@ -107,7 +118,7 @@ public class TraceFilterWebIntegrationTests { .containsEntry("mvc.controller.class", "ExceptionThrowingController") .containsEntry("error", "Request processing failed; nested exception is java.lang.RuntimeException: Throwing exception"); - // issue#714 + // Trace IDs in logs: issue#714 String hex = fromFirstTraceFilterFlow.traceId(); String[] split = capture.toString().split("\n"); List list = Arrays.stream(split) @@ -162,6 +173,32 @@ public class TraceFilterWebIntegrationTests { return new BlockingQueueSpanReporter(); } + @Bean + FinishedSpanHandler uncaughtExceptionThrown( + CurrentTraceContext currentTraceContext) { + return new FinishedSpanHandler() { + @Override + public boolean handle(TraceContext context, MutableSpan span) { + if (span.kind() != Kind.SERVER || span.error() == null + || !log.isErrorEnabled()) { + return true; // don't add overhead as we only log server errors + } + + // In TracingFilter, the exception is raised in scope. This is is more + // explicit to ensure it works in other tech such as WebFlux. + try (Scope scope = currentTraceContext.maybeScope(context)) { + log.error("Uncaught exception thrown", span.error()); + } + return true; + } + + @Override + public String toString() { + return "UncaughtExceptionThrown"; + } + }; + } + @Bean Sampler alwaysSampler() { return Sampler.ALWAYS_SAMPLE; diff --git a/tests/spring-cloud-sleuth-instrumentation-mvc-tests/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceWebServletAutoConfigurationTests.java b/tests/spring-cloud-sleuth-instrumentation-mvc-tests/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceWebServletAutoConfigurationTests.java index 6d74c47b1..9bf7035bb 100644 --- a/tests/spring-cloud-sleuth-instrumentation-mvc-tests/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceWebServletAutoConfigurationTests.java +++ b/tests/spring-cloud-sleuth-instrumentation-mvc-tests/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceWebServletAutoConfigurationTests.java @@ -37,9 +37,9 @@ public class TraceWebServletAutoConfigurationTests { TraceWebServletAutoConfiguration.class)); @Test - public void shouldCreateExceptionLoggingFilterBeanByDefault() { + public void shouldNotCreateExceptionLoggingFilterBeanByDefault() { this.contextRunner.run((context) -> { - assertThat(context).hasBean(EXCEPTION_LOGGING_FILTER_BEAN_NAME); + assertThat(context).doesNotHaveBean(EXCEPTION_LOGGING_FILTER_BEAN_NAME); }); }