diff --git a/spring-cloud-sleuth-core/pom.xml b/spring-cloud-sleuth-core/pom.xml index a41a77fe4..7eee3b105 100644 --- a/spring-cloud-sleuth-core/pom.xml +++ b/spring-cloud-sleuth-core/pom.xml @@ -184,6 +184,16 @@ io.zipkin.brave brave + + + io.zipkin.reporter2 + * + + + io.zipkin.zipkin2 + * + + io.zipkin.brave 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 73baccf9c..7f9a6eca7 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 @@ -16,13 +16,10 @@ package org.springframework.cloud.sleuth.autoconfig; -import java.util.ArrayList; import java.util.Collections; -import java.util.Comparator; import java.util.List; import brave.CurrentSpanCustomizer; -import brave.ErrorParser; import brave.Tracer; import brave.Tracing; import brave.TracingCustomizer; @@ -32,20 +29,8 @@ import brave.propagation.CurrentTraceContextCustomizer; import brave.propagation.Propagation; import brave.propagation.ThreadLocalCurrentTraceContext; import brave.sampler.Sampler; -import io.micrometer.core.instrument.MeterRegistry; -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; -import zipkin2.Span; -import zipkin2.reporter.InMemoryReporterMetrics; -import zipkin2.reporter.Reporter; -import zipkin2.reporter.ReporterMetrics; -import zipkin2.reporter.brave.ZipkinSpanHandler; -import zipkin2.reporter.metrics.micrometer.MicrometerReporterMetrics; -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.ConditionalOnMissingClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.cloud.sleuth.LocalServiceName; @@ -85,38 +70,17 @@ public class TraceAutoConfiguration { */ public static final String DEFAULT_SERVICE_NAME = "default"; - /** - * Sort Zipkin Handlers last, so that redactions etc happen prior. - */ - static final Comparator SPAN_HANDLER_COMPARATOR = (o1, o2) -> { - if (o1 instanceof ZipkinSpanHandler) { - if (o2 instanceof ZipkinSpanHandler) { - return 0; - } - return 1; - } - else if (o2 instanceof ZipkinSpanHandler) { - return -1; - } - return 0; - }; - @Bean @ConditionalOnMissingBean // NOTE: stable bean name as might be used outside sleuth Tracing tracing(@LocalServiceName String serviceName, Propagation.Factory factory, CurrentTraceContext currentTraceContext, Sampler sampler, - ErrorParser errorParser, SleuthProperties sleuthProperties, - @Nullable List> spanReporters, - @Nullable List spanHandlers, + SleuthProperties sleuthProperties, @Nullable List spanHandlers, @Nullable List tracingCustomizers) { Tracing.Builder builder = Tracing.newBuilder().sampler(sampler) - .errorParser(errorParser) .localServiceName(StringUtils.isEmpty(serviceName) ? DEFAULT_SERVICE_NAME : serviceName) .propagationFactory(factory).currentTraceContext(currentTraceContext) - .spanReporter(new CompositeReporter( - spanReporters != null ? spanReporters : Collections.emptyList())) .traceId128Bit(sleuthProperties.isTraceId128()) .supportsJoin(sleuthProperties.isSupportsJoin()); if (spanHandlers != null) { @@ -130,20 +94,9 @@ public class TraceAutoConfiguration { } } - reorderZipkinHandlersLast(builder); return builder.build(); } - private void reorderZipkinHandlersLast(Tracing.Builder builder) { - List configuredSpanHandlers = new ArrayList<>( - builder.spanHandlers()); - configuredSpanHandlers.sort(SPAN_HANDLER_COMPARATOR); - builder.clearSpanHandlers(); - for (SpanHandler spanHandler : configuredSpanHandlers) { - builder.addSpanHandler(spanHandler); - } - } - @Bean(name = TRACER_BEAN_NAME) @ConditionalOnMissingBean Tracer tracer(Tracing tracing) { @@ -182,18 +135,6 @@ public class TraceAutoConfiguration { return ThreadLocalCurrentTraceContext.newBuilder(); } - @Bean - @ConditionalOnMissingBean - Reporter noOpSpanReporter() { - return Reporter.NOOP; - } - - @Bean - @ConditionalOnMissingBean - ErrorParser errorParser() { - return new ErrorParser(); - } - @Bean @ConditionalOnMissingBean // NOTE: stable bean name as might be used outside sleuth @@ -201,91 +142,4 @@ public class TraceAutoConfiguration { return CurrentSpanCustomizer.create(tracing); } - private static final class CompositeReporter implements Reporter { - - private static final Log log = LogFactory.getLog(CompositeReporter.class); - - private final Reporter spanReporter; - - private CompositeReporter(List> spanReporters) { - this.spanReporter = spanReporters.size() == 1 ? spanReporters.get(0) - : new ListReporter(spanReporters); - } - - @Override - public void report(Span span) { - this.spanReporter.report(span); - } - - @Override - public String toString() { - return "CompositeReporter{ spanReporters=" + this.spanReporter + '}'; - } - - private static final class ListReporter implements Reporter { - - private final List> spanReporters; - - private ListReporter(List> spanReporters) { - this.spanReporters = spanReporters; - } - - @Override - public void report(Span span) { - for (Reporter spanReporter : this.spanReporters) { - try { - spanReporter.report(span); - } - catch (Exception ex) { - log.warn("Exception occurred while trying to report the span " - + span, ex); - } - } - } - - @Override - public String toString() { - return "ListReporter{" + "spanReporters=" + this.spanReporters + '}'; - } - - } - - } - - @Configuration(proxyBeanMethods = false) - @ConditionalOnMissingClass("io.micrometer.core.instrument.MeterRegistry") - static class TraceMetricsInMemoryConfiguration { - - @Bean - @ConditionalOnMissingBean - ReporterMetrics sleuthReporterMetrics() { - return new InMemoryReporterMetrics(); - } - - } - - @Configuration(proxyBeanMethods = false) - @ConditionalOnClass(MeterRegistry.class) - static class TraceMetricsMicrometerConfiguration { - - @Configuration(proxyBeanMethods = false) - @ConditionalOnMissingBean(ReporterMetrics.class) - static class NoReporterMetricsBeanConfiguration { - - @Bean - @ConditionalOnBean(MeterRegistry.class) - ReporterMetrics sleuthMicrometerReporterMetrics(MeterRegistry meterRegistry) { - return MicrometerReporterMetrics.create(meterRegistry); - } - - @Bean - @ConditionalOnMissingBean(MeterRegistry.class) - ReporterMetrics sleuthReporterMetrics() { - return new InMemoryReporterMetrics(); - } - - } - - } - } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/annotation/SleuthSpanCreatorAspectFluxTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/annotation/SleuthSpanCreatorAspectFluxTests.java index 4fff90af9..5f4a6da24 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/annotation/SleuthSpanCreatorAspectFluxTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/annotation/SleuthSpanCreatorAspectFluxTests.java @@ -312,8 +312,8 @@ public class SleuthSpanCreatorAspectFluxTests { Awaitility.await().untilAsserted(() -> { then(this.spans).hasSize(1); then(this.spans.get(0).name()).isEqualTo("test-method12"); - then(this.spans.get(0).tags()).containsEntry("testTag12", "test") - .containsEntry("error", "test exception 12"); + then(this.spans.get(0).tags()).containsEntry("testTag12", "test"); + then(this.spans.get(0).error()).hasMessageContaining("test exception 12"); then(this.spans.get(0).finishTimestamp()).isNotZero(); then(this.tracer.currentSpan()).isNull(); }); @@ -341,7 +341,7 @@ public class SleuthSpanCreatorAspectFluxTests { Awaitility.await().untilAsserted(() -> { then(this.spans).hasSize(1); then(this.spans.get(0).name()).isEqualTo("foo"); - then(this.spans.get(0).tags()).containsEntry("error", "test exception 13"); + then(this.spans.get(0).error()).hasMessageContaining("test exception 13"); then(this.spans.get(0).annotations().stream().map(Map.Entry::getValue) .collect(Collectors.toList())).contains("testMethod13.before", "testMethod13.afterFailure", "testMethod13.after"); diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/annotation/SleuthSpanCreatorAspectMonoTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/annotation/SleuthSpanCreatorAspectMonoTests.java index 81f0d0c53..afa5b3f94 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/annotation/SleuthSpanCreatorAspectMonoTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/annotation/SleuthSpanCreatorAspectMonoTests.java @@ -325,8 +325,8 @@ public class SleuthSpanCreatorAspectMonoTests { Awaitility.await().untilAsserted(() -> { then(this.spans).hasSize(1); then(this.spans.get(0).name()).isEqualTo("test-method12"); - then(this.spans.get(0).tags()).containsEntry("testTag12", "test") - .containsEntry("error", "test exception 12"); + then(this.spans.get(0).tags()).containsEntry("testTag12", "test"); + then(this.spans.get(0).error()).hasMessageContaining("test exception 12"); then(this.spans.get(0).finishTimestamp()).isNotZero(); then(this.tracer.currentSpan()).isNull(); }); @@ -354,7 +354,7 @@ public class SleuthSpanCreatorAspectMonoTests { Awaitility.await().untilAsserted(() -> { then(this.spans).hasSize(1); then(this.spans.get(0).name()).isEqualTo("foo"); - then(this.spans.get(0).tags()).containsEntry("error", "test exception 13"); + then(this.spans.get(0).error()).hasMessageContaining("test exception 13"); then(this.spans.get(0).annotations().stream().map(Map.Entry::getValue) .collect(Collectors.toList())).contains("testMethod13.before", "testMethod13.afterFailure", "testMethod13.after"); diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/annotation/SleuthSpanCreatorAspectTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/annotation/SleuthSpanCreatorAspectTests.java index 6ed435601..d32bde101 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/annotation/SleuthSpanCreatorAspectTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/annotation/SleuthSpanCreatorAspectTests.java @@ -233,8 +233,8 @@ public class SleuthSpanCreatorAspectTests { then(this.spans).hasSize(1); then(this.spans.get(0).name()).isEqualTo("test-method12"); - then(this.spans.get(0).tags()).containsEntry("testTag12", "test") - .containsEntry("error", "test exception 12"); + then(this.spans.get(0).tags()).containsEntry("testTag12", "test"); + then(this.spans.get(0).error()).hasMessageContaining("test exception 12"); then(this.spans.get(0).finishTimestamp()).isNotZero(); then(this.tracer.currentSpan()).isNull(); } @@ -256,7 +256,7 @@ public class SleuthSpanCreatorAspectTests { then(this.spans).hasSize(1); then(this.spans.get(0).name()).isEqualTo("foo"); - then(this.spans.get(0).tags()).containsEntry("error", "test exception 13"); + then(this.spans.get(0).error()).hasMessageContaining("test exception 13"); then(this.spans.get(0).annotations().stream().map(Map.Entry::getValue) .collect(Collectors.toList())).contains("testMethod13.before", "testMethod13.afterFailure", "testMethod13.after"); diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/autoconfig/TraceAutoConfigurationTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/autoconfig/TraceAutoConfigurationTests.java index 652df1a1f..5309b8f30 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/autoconfig/TraceAutoConfigurationTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/autoconfig/TraceAutoConfigurationTests.java @@ -16,7 +16,6 @@ package org.springframework.cloud.sleuth.autoconfig; -import java.util.ArrayList; import java.util.List; import brave.Tracing; @@ -24,9 +23,11 @@ import brave.baggage.BaggageField; import brave.baggage.BaggagePropagation; import brave.baggage.BaggagePropagationConfig.SingleBaggageField; import brave.baggage.BaggagePropagationCustomizer; +import brave.handler.MutableSpan; import brave.handler.SpanHandler; import brave.propagation.B3SinglePropagation; import brave.propagation.Propagation; +import brave.propagation.TraceContext; import brave.propagation.TraceContextOrSamplingFlags; import brave.sampler.RateLimitingSampler; import brave.sampler.Sampler; @@ -34,76 +35,18 @@ import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.simple.SimpleMeterRegistry; import org.assertj.core.api.BDDAssertions; import org.junit.jupiter.api.Test; -import zipkin2.reporter.InMemoryReporterMetrics; -import zipkin2.reporter.Reporter; -import zipkin2.reporter.ReporterMetrics; -import zipkin2.reporter.brave.ZipkinSpanHandler; -import zipkin2.reporter.metrics.micrometer.MicrometerReporterMetrics; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.autoconfigure.AutoConfigurations; -import org.springframework.boot.test.context.FilteredClassLoader; import org.springframework.boot.test.context.runner.ApplicationContextRunner; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; -import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.mock; -import static org.springframework.cloud.sleuth.autoconfig.TraceAutoConfiguration.SPAN_HANDLER_COMPARATOR; - public class TraceAutoConfigurationTests { private final ApplicationContextRunner contextRunner = new ApplicationContextRunner() .withConfiguration(AutoConfigurations.of(TraceAutoConfiguration.class)); - @Test - void span_handler_comparator() { - SpanHandler handler1 = mock(SpanHandler.class); - SpanHandler handler2 = mock(SpanHandler.class); - ZipkinSpanHandler zipkin1 = mock(ZipkinSpanHandler.class); - ZipkinSpanHandler zipkin2 = mock(ZipkinSpanHandler.class); - - ArrayList spanHandlers = new ArrayList<>(); - spanHandlers.add(handler1); - spanHandlers.add(zipkin1); - spanHandlers.add(handler2); - spanHandlers.add(zipkin2); - - spanHandlers.sort(SPAN_HANDLER_COMPARATOR); - - assertThat(spanHandlers).containsExactly(handler1, handler2, zipkin1, zipkin2); - } - - @Test - void should_apply_micrometer_reporter_metrics_when_meter_registry_bean_present() { - this.contextRunner.withUserConfiguration(WithMeterRegistry.class) - .run((context) -> { - ReporterMetrics bean = context.getBean(ReporterMetrics.class); - - BDDAssertions.then(bean) - .isInstanceOf(MicrometerReporterMetrics.class); - }); - } - - @Test - void should_apply_in_memory_metrics_when_meter_registry_bean_missing() { - this.contextRunner.run((context) -> { - ReporterMetrics bean = context.getBean(ReporterMetrics.class); - - BDDAssertions.then(bean).isInstanceOf(InMemoryReporterMetrics.class); - }); - } - - @Test - void should_apply_in_memory_metrics_when_meter_registry_class_missing() { - this.contextRunner.withClassLoader(new FilteredClassLoader(MeterRegistry.class)) - .run((context) -> { - ReporterMetrics bean = context.getBean(ReporterMetrics.class); - - BDDAssertions.then(bean).isInstanceOf(InMemoryReporterMetrics.class); - }); - } - /** * Duplicates * {@link org.springframework.cloud.sleuth.sampler.SamplerAutoConfigurationTests} @@ -123,8 +66,8 @@ public class TraceAutoConfigurationTests { * intentionally, to ensure configuration condition bugs do not exist. */ @Test - void should_use_RateLimitedSampler_when_reporting() { - this.contextRunner.withUserConfiguration(WithReporter.class).run((context -> { + void should_use_RateLimitedSampler_withSpanHandler() { + this.contextRunner.withUserConfiguration(WithSpanHandler.class).run((context -> { final Sampler bean = context.getBean(Sampler.class); BDDAssertions.then(bean).isInstanceOf(RateLimitingSampler.class); })); @@ -137,11 +80,10 @@ public class TraceAutoConfigurationTests { */ @Test void should_override_sampler() { - this.contextRunner.withUserConfiguration(WithReporter.class, WithSampler.class) - .run((context -> { - final Sampler bean = context.getBean(Sampler.class); - BDDAssertions.then(bean).isSameAs(Sampler.ALWAYS_SAMPLE); - })); + this.contextRunner.withUserConfiguration(WithSampler.class).run((context -> { + final Sampler bean = context.getBean(Sampler.class); + BDDAssertions.then(bean).isSameAs(Sampler.ALWAYS_SAMPLE); + })); } @Test @@ -231,21 +173,16 @@ public class TraceAutoConfigurationTests { } @Configuration - static class WithMeterRegistry { + static class WithSpanHandler { @Bean - MeterRegistry meterRegistry() { - return new SimpleMeterRegistry(); - } - - } - - @Configuration - static class WithReporter { - - @Bean - Reporter spanReporter() { - return zipkin2.Span::toString; + SpanHandler testSpanHandler() { + return new SpanHandler() { + @Override + public boolean end(TraceContext context, MutableSpan span, Cause cause) { + return true; + } + }; } } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/discoveryexception/WebClientDiscoveryExceptionTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/discoveryexception/WebClientDiscoveryExceptionTests.java index 281e11a01..144a1d7d7 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/discoveryexception/WebClientDiscoveryExceptionTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/discoveryexception/WebClientDiscoveryExceptionTests.java @@ -97,7 +97,7 @@ public class WebClientDiscoveryExceptionTests { // hystrix commands should finish at this point Thread.sleep(200); then(this.spans.spans().stream().filter(span1 -> span1.kind() == Span.Kind.CLIENT) - .findFirst().get().tags()).containsKey("error"); + .findFirst().get().error()).isNotNull(); } @Test diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/exception/WebClientExceptionTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/exception/WebClientExceptionTests.java index 1126a191d..6ef91853a 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/exception/WebClientExceptionTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/exception/WebClientExceptionTests.java @@ -98,7 +98,7 @@ public class WebClientExceptionTests { then(this.tracer.tracer().currentSpan()).isNull(); then(this.spans).isNotEmpty(); - then(this.spans.get(0).tags()).containsKey("error"); + then(this.spans.get(0).error()).isNotNull(); } static Stream parametersForShouldCloseSpanUponException() { diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/sampler/SamplerAutoConfigurationTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/sampler/SamplerAutoConfigurationTests.java index e6518167e..e7c1875ea 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/sampler/SamplerAutoConfigurationTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/sampler/SamplerAutoConfigurationTests.java @@ -25,8 +25,6 @@ import brave.sampler.RateLimitingSampler; import brave.sampler.Sampler; import org.assertj.core.api.BDDAssertions; import org.junit.jupiter.api.Test; -import zipkin2.Span; -import zipkin2.reporter.Reporter; import org.springframework.boot.autoconfigure.AutoConfigurations; import org.springframework.boot.test.context.runner.ApplicationContextRunner; @@ -59,14 +57,6 @@ public class SamplerAutoConfigurationTests { })); } - @Test - void should_use_RateLimitedSampler_withReporter() { - this.contextRunner.withUserConfiguration(WithReporter.class).run((context -> { - final Sampler bean = context.getBean(Sampler.class); - BDDAssertions.then(bean).isInstanceOf(RateLimitingSampler.class); - })); - } - @Test void should_use_RateLimitedSampler_withTracingCustomizer() { this.contextRunner.withUserConfiguration(WithTracingCustomizer.class) @@ -76,15 +66,6 @@ public class SamplerAutoConfigurationTests { })); } - @Test - void should_override_sampler() { - this.contextRunner.withUserConfiguration(WithReporter.class, WithSampler.class) - .run((context -> { - final Sampler bean = context.getBean(Sampler.class); - BDDAssertions.then(bean).isSameAs(Sampler.ALWAYS_SAMPLE); - })); - } - @Test void samplerFromProps_probability() { SamplerProperties properties = new SamplerProperties(); @@ -151,26 +132,6 @@ public class SamplerAutoConfigurationTests { } - @Configuration - static class WithReporter { - - @Bean - Reporter spanReporter() { - return zipkin2.Span::toString; - } - - } - - @Configuration - static class WithSampler { - - @Bean - Sampler alwaysSampler() { - return Sampler.ALWAYS_SAMPLE; - } - - } - @Configuration static class WithTracingCustomizer { diff --git a/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin2/ZipkinAutoConfiguration.java b/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin2/ZipkinAutoConfiguration.java index 61a3fcfea..0ea50b8a4 100644 --- a/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin2/ZipkinAutoConfiguration.java +++ b/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin2/ZipkinAutoConfiguration.java @@ -16,24 +16,38 @@ package org.springframework.cloud.sleuth.zipkin2; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Comparator; +import java.util.LinkedHashSet; +import java.util.List; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; +import brave.Tag; +import brave.TracingCustomizer; +import brave.handler.SpanHandler; +import io.micrometer.core.instrument.MeterRegistry; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import zipkin2.CheckResult; import zipkin2.Span; import zipkin2.reporter.AsyncReporter; +import zipkin2.reporter.InMemoryReporterMetrics; import zipkin2.reporter.Reporter; import zipkin2.reporter.ReporterMetrics; import zipkin2.reporter.Sender; +import zipkin2.reporter.brave.ZipkinSpanHandler; +import zipkin2.reporter.metrics.micrometer.MicrometerReporterMetrics; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; 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.ConditionalOnMissingClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.boot.autoconfigure.web.ServerProperties; import org.springframework.boot.context.properties.EnableConfigurationProperties; @@ -45,6 +59,7 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Import; import org.springframework.core.env.Environment; +import org.springframework.lang.Nullable; import org.springframework.web.client.RestTemplate; /** @@ -74,6 +89,22 @@ public class ZipkinAutoConfiguration { private static final Log log = LogFactory.getLog(ZipkinAutoConfiguration.class); + /** + * Sort Zipkin Handlers last, so that redactions etc happen prior. + */ + static final Comparator SPAN_HANDLER_COMPARATOR = (o1, o2) -> { + if (o1 instanceof ZipkinSpanHandler) { + if (o2 instanceof ZipkinSpanHandler) { + return 0; + } + return 1; + } + else if (o2 instanceof ZipkinSpanHandler) { + return -1; + } + return 0; + }; + /** * Zipkin reporter bean name. Name of the bean matters for supporting multiple tracing * systems. @@ -143,6 +174,44 @@ public class ZipkinAutoConfiguration { } } + /** Returns one handler for as many reporters as exist. */ + @Bean + SpanHandler zipkinSpanHandler(@Nullable List> spanReporters, + @Nullable Tag errorTag) { + if (spanReporters == null) { + return SpanHandler.NOOP; + } + + LinkedHashSet> reporters = new LinkedHashSet<>(spanReporters); + reporters.remove(Reporter.NOOP); + if (spanReporters.isEmpty()) { + return SpanHandler.NOOP; + } + + Reporter spanReporter = reporters.size() == 1 ? reporters.iterator().next() + : new CompositeSpanReporter(reporters.toArray(new Reporter[0])); + + ZipkinSpanHandler.Builder builder = ZipkinSpanHandler.newBuilder(spanReporter); + if (errorTag != null) { + builder.errorTag(errorTag); + } + return builder.build(); + } + + /** This ensures Zipkin reporters end up after redaction, etc. */ + @Bean + TracingCustomizer reorderZipkinHandlersLast() { + return builder -> { + List configuredSpanHandlers = new ArrayList<>( + builder.spanHandlers()); + configuredSpanHandlers.sort(SPAN_HANDLER_COMPARATOR); + builder.clearSpanHandlers(); + for (SpanHandler spanHandler : configuredSpanHandlers) { + builder.addSpanHandler(spanHandler); + } + }; + } + @Bean @ConditionalOnMissingBean public ZipkinRestTemplateCustomizer zipkinRestTemplateCustomizer( @@ -206,4 +275,84 @@ public class ZipkinAutoConfiguration { } + @Configuration(proxyBeanMethods = false) + @ConditionalOnMissingClass("io.micrometer.core.instrument.MeterRegistry") + static class TraceMetricsInMemoryConfiguration { + + @Bean + @ConditionalOnMissingBean + ReporterMetrics sleuthReporterMetrics() { + return new InMemoryReporterMetrics(); + } + + } + + @Configuration(proxyBeanMethods = false) + @ConditionalOnClass(MeterRegistry.class) + static class TraceMetricsMicrometerConfiguration { + + @Configuration(proxyBeanMethods = false) + @ConditionalOnMissingBean(ReporterMetrics.class) + static class NoReporterMetricsBeanConfiguration { + + @Bean + @ConditionalOnBean(MeterRegistry.class) + ReporterMetrics sleuthMicrometerReporterMetrics(MeterRegistry meterRegistry) { + return MicrometerReporterMetrics.create(meterRegistry); + } + + @Bean + @ConditionalOnMissingBean(MeterRegistry.class) + ReporterMetrics sleuthReporterMetrics() { + return new InMemoryReporterMetrics(); + } + + } + + } + + // Zipkin conversion only happens once per mutable span + static final class CompositeSpanReporter implements Reporter { + + final Reporter[] reporters; + + CompositeSpanReporter(Reporter[] reporters) { + this.reporters = reporters; + } + + @Override + public void report(Span span) { + for (Reporter reporter : reporters) { + try { + reporter.report(span); + } + catch (RuntimeException ex) { + // TODO: message lifted from ListReporter: this is probably too much + // for warn level + log.warn("Exception occurred while trying to report the span " + span, + ex); + } + } + } + + @Override + public int hashCode() { + return Arrays.hashCode(reporters); + } + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof CompositeSpanReporter)) { + return false; + } + return Arrays.equals(((CompositeSpanReporter) obj).reporters, reporters); + } + + @Override + public String toString() { + return Arrays.toString(reporters); + } + + } + } diff --git a/spring-cloud-sleuth-zipkin/src/test/java/org/springframework/cloud/sleuth/zipkin2/ZipkinAutoConfigurationTests.java b/spring-cloud-sleuth-zipkin/src/test/java/org/springframework/cloud/sleuth/zipkin2/ZipkinAutoConfigurationTests.java index 9464fe051..f7233d9cb 100644 --- a/spring-cloud-sleuth-zipkin/src/test/java/org/springframework/cloud/sleuth/zipkin2/ZipkinAutoConfigurationTests.java +++ b/spring-cloud-sleuth-zipkin/src/test/java/org/springframework/cloud/sleuth/zipkin2/ZipkinAutoConfigurationTests.java @@ -17,6 +17,7 @@ package org.springframework.cloud.sleuth.zipkin2; import java.io.IOException; +import java.util.ArrayList; import java.util.List; import java.util.concurrent.TimeoutException; @@ -26,8 +27,11 @@ import brave.handler.MutableSpan; import brave.handler.SpanHandler; import brave.propagation.TraceContext; import brave.sampler.Sampler; +import io.micrometer.core.instrument.MeterRegistry; +import io.micrometer.core.instrument.simple.SimpleMeterRegistry; import okhttp3.mockwebserver.MockWebServer; import okhttp3.mockwebserver.RecordedRequest; +import org.assertj.core.api.BDDAssertions; import org.awaitility.Awaitility; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -36,17 +40,24 @@ import zipkin2.Call; import zipkin2.CheckResult; import zipkin2.codec.Encoding; import zipkin2.reporter.AsyncReporter; +import zipkin2.reporter.InMemoryReporterMetrics; import zipkin2.reporter.Reporter; +import zipkin2.reporter.ReporterMetrics; import zipkin2.reporter.Sender; import zipkin2.reporter.activemq.ActiveMQSender; import zipkin2.reporter.amqp.RabbitMQSender; +import zipkin2.reporter.brave.ZipkinSpanHandler; import zipkin2.reporter.kafka.KafkaSender; +import zipkin2.reporter.metrics.micrometer.MicrometerReporterMetrics; +import org.springframework.boot.autoconfigure.AutoConfigurations; import org.springframework.boot.autoconfigure.amqp.RabbitAutoConfiguration; import org.springframework.boot.autoconfigure.context.PropertyPlaceholderAutoConfiguration; import org.springframework.boot.autoconfigure.jms.activemq.ActiveMQAutoConfiguration; import org.springframework.boot.autoconfigure.kafka.KafkaAutoConfiguration; +import org.springframework.boot.test.context.FilteredClassLoader; import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.context.runner.ApplicationContextRunner; import org.springframework.cloud.sleuth.autoconfig.TraceAutoConfiguration; import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.context.annotation.Bean; @@ -57,6 +68,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.BDDAssertions.then; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import static org.springframework.cloud.sleuth.zipkin2.ZipkinAutoConfiguration.SPAN_HANDLER_COMPARATOR; /** * Not using {@linkplain SpringBootTest} as we need to change properties per test. @@ -65,6 +77,9 @@ import static org.mockito.Mockito.when; */ public class ZipkinAutoConfigurationTests { + private final ApplicationContextRunner contextRunner = new ApplicationContextRunner() + .withConfiguration(AutoConfigurations.of(ZipkinAutoConfiguration.class)); + public MockWebServer server = new MockWebServer(); @BeforeEach @@ -89,7 +104,55 @@ public class ZipkinAutoConfigurationTests { } @Test - public void defaultsToV2Endpoint() throws Exception { + void span_handler_comparator() { + SpanHandler handler1 = mock(SpanHandler.class); + SpanHandler handler2 = mock(SpanHandler.class); + ZipkinSpanHandler zipkin1 = mock(ZipkinSpanHandler.class); + ZipkinSpanHandler zipkin2 = mock(ZipkinSpanHandler.class); + + ArrayList spanHandlers = new ArrayList<>(); + spanHandlers.add(handler1); + spanHandlers.add(zipkin1); + spanHandlers.add(handler2); + spanHandlers.add(zipkin2); + + spanHandlers.sort(SPAN_HANDLER_COMPARATOR); + + assertThat(spanHandlers).containsExactly(handler1, handler2, zipkin1, zipkin2); + } + + @Test + void should_apply_micrometer_reporter_metrics_when_meter_registry_bean_present() { + this.contextRunner.withUserConfiguration(WithMeterRegistry.class) + .run((context) -> { + ReporterMetrics bean = context.getBean(ReporterMetrics.class); + + BDDAssertions.then(bean) + .isInstanceOf(MicrometerReporterMetrics.class); + }); + } + + @Test + void should_apply_in_memory_metrics_when_meter_registry_bean_missing() { + this.contextRunner.run((context) -> { + ReporterMetrics bean = context.getBean(ReporterMetrics.class); + + BDDAssertions.then(bean).isInstanceOf(InMemoryReporterMetrics.class); + }); + } + + @Test + void should_apply_in_memory_metrics_when_meter_registry_class_missing() { + this.contextRunner.withClassLoader(new FilteredClassLoader(MeterRegistry.class)) + .run((context) -> { + ReporterMetrics bean = context.getBean(ReporterMetrics.class); + + BDDAssertions.then(bean).isInstanceOf(InMemoryReporterMetrics.class); + }); + } + + @Test + void defaultsToV2Endpoint() throws Exception { this.context = new AnnotationConfigApplicationContext(); environment().setProperty("spring.zipkin.base-url", this.server.url("/").toString()); @@ -380,7 +443,7 @@ public class ZipkinAutoConfigurationTests { } @Configuration - protected static class HandlerHanldersConfig { + protected static class HandlersConfig { @Bean SpanHandler handlerOne() { @@ -408,6 +471,26 @@ public class ZipkinAutoConfigurationTests { } + @Configuration + static class WithMeterRegistry { + + @Bean + MeterRegistry meterRegistry() { + return new SimpleMeterRegistry(); + } + + } + + @Configuration + static class WithReporter { + + @Bean + Reporter spanReporter() { + return zipkin2.Span::toString; + } + + } + @Configuration protected static class MultipleReportersConfig { diff --git a/tests/spring-cloud-sleuth-instrumentation-mvc-tests/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterIntegrationTests.java b/tests/spring-cloud-sleuth-instrumentation-mvc-tests/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterIntegrationTests.java index 5b934084b..d5a6bf98b 100644 --- a/tests/spring-cloud-sleuth-instrumentation-mvc-tests/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterIntegrationTests.java +++ b/tests/spring-cloud-sleuth-instrumentation-mvc-tests/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterIntegrationTests.java @@ -197,7 +197,7 @@ public class TraceFilterIntegrationTests extends AbstractMvcIntegrationTest { // we need to dump the span cause it's not in TracingFilter since TF // has also error dispatch and the ErrorController would report the span then(this.spans).hasSize(1); - then(this.spans.get(0).tags()).containsEntry("error", + then(this.spans.get(0).error()).hasMessageContaining( "Request processing failed; nested exception is java.lang.RuntimeException"); } 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 8d420a8d2..10540bf6c 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 @@ -107,7 +107,7 @@ public class TraceFilterWebIntegrationTests { } then(this.currentTraceContext.get()).isNull(); - MutableSpan fromFirstTraceFilterFlow = spanHandler.takeRemoteSpanWithErrorTag( + MutableSpan fromFirstTraceFilterFlow = spanHandler.takeRemoteSpanWithErrorMessage( Kind.SERVER, "Request processing failed; nested exception is java.lang.RuntimeException: Throwing exception"); then(fromFirstTraceFilterFlow.tags()).containsEntry("http.method", "GET")