From d08e106100c1e9957926517394097047a6884ea1 Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Sat, 18 Jan 2020 06:50:32 +0800 Subject: [PATCH] Reduces overhead when using reactor by avoiding redundant context syncs (#1525) This switches to pass around and use `TraceContext` instead of `Span` in Reactor's context to take advantage of `maybeScope` which is far cheaper than `withSpanInScope` when the span is already current. This also kills some extra scope checks and any other accidental calls noticed. The performance improvement here, I can't quantify as haven't done any JMH on it. --- ...stractSleuthMethodInvocationProcessor.java | 11 ++++ ...eactorSleuthMethodInvocationProcessor.java | 31 +++++----- .../instrument/reactor/ReactorSleuth.java | 20 ++++--- .../reactor/ScopePassingSpanSubscriber.java | 34 +++++------ .../instrument/reactor/SpanSubscriber.java | 41 +++++++------ .../reactor/SpanSubscriptionProvider.java | 31 +++++----- .../sleuth/instrument/web/TraceWebFilter.java | 15 ++--- .../TraceWebClientBeanPostProcessor.java | 4 +- .../SleuthSpanCreatorAspectFluxTests.java | 5 +- .../SleuthSpanCreatorAspectMonoTests.java | 57 +++++++++---------- .../ScopePassingSpanSubscriberTests.java | 24 ++++---- 11 files changed, 141 insertions(+), 132 deletions(-) diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/annotation/AbstractSleuthMethodInvocationProcessor.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/annotation/AbstractSleuthMethodInvocationProcessor.java index 3f4e21d9d..d2bac1447 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/annotation/AbstractSleuthMethodInvocationProcessor.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/annotation/AbstractSleuthMethodInvocationProcessor.java @@ -18,6 +18,7 @@ package org.springframework.cloud.sleuth.annotation; import brave.Span; import brave.Tracer; +import brave.propagation.CurrentTraceContext; import org.aopalliance.intercept.MethodInvocation; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -47,6 +48,8 @@ abstract class AbstractSleuthMethodInvocationProcessor private Tracer tracer; + private CurrentTraceContext currentTraceContext; + private SpanTagAnnotationHandler spanTagAnnotationHandler; void before(MethodInvocation invocation, Span span, String log, boolean hasLog) { @@ -105,6 +108,14 @@ abstract class AbstractSleuthMethodInvocationProcessor return this.tracer; } + CurrentTraceContext currentTraceContext() { + if (this.currentTraceContext == null) { + this.currentTraceContext = this.beanFactory + .getBean(CurrentTraceContext.class); + } + return this.currentTraceContext; + } + NewSpanParser newSpanParser() { if (this.newSpanParser == null) { this.newSpanParser = this.beanFactory.getBean(NewSpanParser.class); diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/annotation/ReactorSleuthMethodInvocationProcessor.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/annotation/ReactorSleuthMethodInvocationProcessor.java index 814ef3e68..b084a95e8 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/annotation/ReactorSleuthMethodInvocationProcessor.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/annotation/ReactorSleuthMethodInvocationProcessor.java @@ -22,6 +22,8 @@ import brave.Span; import brave.Tracer; import brave.Tracing; import brave.propagation.CurrentTraceContext; +import brave.propagation.CurrentTraceContext.Scope; +import brave.propagation.TraceContext; import org.aopalliance.intercept.MethodInvocation; import org.reactivestreams.Publisher; import org.reactivestreams.Subscription; @@ -144,14 +146,15 @@ class ReactorSleuthMethodInvocationProcessor Span span; Tracer tracer = this.processor.tracer(); if (this.span == null) { - span = tracer.nextSpan(); + span = tracer.newTrace(); this.processor.newSpanParser().parse(this.invocation, this.newSpan, span); span.start(); } else { span = this.span; } - try (Tracer.SpanInScope ws = tracer.withSpanInScope(span)) { + try (Scope ws = this.processor.currentTraceContext() + .maybeScope(span.context())) { this.source.subscribe(new SpanSubscriber(actual, this.processor, this.invocation, this.span == null, span, this.log, this.hasLog)); } @@ -197,7 +200,8 @@ class ReactorSleuthMethodInvocationProcessor else { span = this.span; } - try (Tracer.SpanInScope ws = tracer.withSpanInScope(span)) { + try (Scope ws = this.processor.currentTraceContext() + .maybeScope(span.context())) { this.source.subscribe(new SpanSubscriber(actual, this.processor, this.invocation, this.span == null, span, this.log, this.hasLog)); } @@ -238,23 +242,22 @@ class ReactorSleuthMethodInvocationProcessor this.processor = processor; this.currentTraceContext = processor.tracing().currentTraceContext(); - this.context = actual.currentContext().put(Span.class, span); + this.context = actual.currentContext().put(TraceContext.class, + span.context()); processor.before(invocation, this.span, this.log, this.hasLog); } @Override public void request(long n) { - try (CurrentTraceContext.Scope scope = this.currentTraceContext - .maybeScope(this.span.context())) { + try (Scope scope = this.currentTraceContext.maybeScope(this.span.context())) { this.parent.request(n); } } @Override public void cancel() { - try (CurrentTraceContext.Scope scope = this.currentTraceContext - .maybeScope(this.span.context())) { + try (Scope scope = this.currentTraceContext.maybeScope(this.span.context())) { this.parent.cancel(); } finally { @@ -270,24 +273,21 @@ class ReactorSleuthMethodInvocationProcessor @Override public void onSubscribe(Subscription subscription) { this.parent = subscription; - try (CurrentTraceContext.Scope scope = this.currentTraceContext - .maybeScope(this.span.context())) { + try (Scope scope = this.currentTraceContext.maybeScope(this.span.context())) { this.actual.onSubscribe(this); } } @Override public void onNext(Object o) { - try (CurrentTraceContext.Scope scope = this.currentTraceContext - .maybeScope(this.span.context())) { + try (Scope scope = this.currentTraceContext.maybeScope(this.span.context())) { this.actual.onNext(o); } } @Override public void onError(Throwable error) { - try (CurrentTraceContext.Scope scope = this.currentTraceContext - .maybeScope(this.span.context())) { + try (Scope scope = this.currentTraceContext.maybeScope(this.span.context())) { this.processor.onFailure(this.span, this.log, this.hasLog, error); this.actual.onError(error); } @@ -298,8 +298,7 @@ class ReactorSleuthMethodInvocationProcessor @Override public void onComplete() { - try (CurrentTraceContext.Scope scope = this.currentTraceContext - .maybeScope(this.span.context())) { + try (Scope scope = this.currentTraceContext.maybeScope(this.span.context())) { this.actual.onComplete(); } finally { diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/reactor/ReactorSleuth.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/reactor/ReactorSleuth.java index e6dbaa3c1..10df1a826 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/reactor/ReactorSleuth.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/reactor/ReactorSleuth.java @@ -21,8 +21,9 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.function.BooleanSupplier; import java.util.function.Function; -import brave.Span; import brave.Tracing; +import brave.propagation.CurrentTraceContext; +import brave.propagation.TraceContext; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.reactivestreams.Publisher; @@ -105,18 +106,21 @@ public abstract class ReactorSleuth { scannable.name()); } - private static Map CACHE = new ConcurrentHashMap<>(); + private static Map CACHE = new ConcurrentHashMap<>(); static CoreSubscriber scopePassingSpanSubscription( BeanFactory beanFactory, CoreSubscriber sub) { - Tracing tracing = CACHE.computeIfAbsent(beanFactory, - beanFactory1 -> beanFactory1.getBean(Tracing.class)); + CurrentTraceContext currentTraceContext = CACHE.computeIfAbsent(beanFactory, + beanFactory1 -> beanFactory1.getBean(CurrentTraceContext.class)); Context context = sub.currentContext(); - Span root = context.hasKey(Span.class) ? context.get(Span.class) - : tracing.tracer().currentSpan(); - if (root != null) { - return new ScopePassingSpanSubscriber<>(sub, context, tracing, root); + TraceContext parent = context.getOrDefault(TraceContext.class, null); + if (parent == null) { + parent = currentTraceContext.get(); + } + if (parent != null) { + return new ScopePassingSpanSubscriber<>(sub, context, currentTraceContext, + parent); } else { return sub; // no need to trace diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/reactor/ScopePassingSpanSubscriber.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/reactor/ScopePassingSpanSubscriber.java index b69df2091..488e26632 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/reactor/ScopePassingSpanSubscriber.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/reactor/ScopePassingSpanSubscriber.java @@ -18,9 +18,8 @@ package org.springframework.cloud.sleuth.instrument.reactor; import javax.annotation.Nullable; -import brave.Span; -import brave.Tracing; import brave.propagation.CurrentTraceContext; +import brave.propagation.CurrentTraceContext.Scope; import brave.propagation.TraceContext; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -46,44 +45,40 @@ final class ScopePassingSpanSubscriber implements SpanSubscription, Scanna private final CurrentTraceContext currentTraceContext; - private final TraceContext traceContext; + private final TraceContext parent; private Subscription s; ScopePassingSpanSubscriber(Subscriber subscriber, Context ctx, - Tracing tracing, @Nullable Span root) { + CurrentTraceContext currentTraceContext, @Nullable TraceContext parent) { this.subscriber = subscriber; - this.currentTraceContext = tracing.currentTraceContext(); - - this.traceContext = root == null ? null : root.context(); - this.context = ctx != null && root != null ? ctx.put(Span.class, root) + this.currentTraceContext = currentTraceContext; + this.parent = parent; + this.context = ctx != null && parent != null ? ctx.put(TraceContext.class, parent) : ctx != null ? ctx : Context.empty(); if (log.isTraceEnabled()) { - log.trace("Root span [" + root + "], context [" + this.context + "]"); + log.trace("Parent span [" + parent + "], context [" + this.context + "]"); } } @Override public void onSubscribe(Subscription subscription) { this.s = subscription; - try (CurrentTraceContext.Scope scope = this.currentTraceContext - .maybeScope(this.traceContext)) { + try (Scope scope = this.currentTraceContext.maybeScope(this.parent)) { this.subscriber.onSubscribe(this); } } @Override public void request(long n) { - try (CurrentTraceContext.Scope scope = this.currentTraceContext - .maybeScope(this.traceContext)) { + try (Scope scope = this.currentTraceContext.maybeScope(this.parent)) { this.s.request(n); } } @Override public void cancel() { - try (CurrentTraceContext.Scope scope = this.currentTraceContext - .maybeScope(this.traceContext)) { + try (Scope scope = this.currentTraceContext.maybeScope(this.parent)) { this.s.cancel(); } @@ -91,24 +86,21 @@ final class ScopePassingSpanSubscriber implements SpanSubscription, Scanna @Override public void onNext(T o) { - try (CurrentTraceContext.Scope scope = this.currentTraceContext - .maybeScope(this.traceContext)) { + try (Scope scope = this.currentTraceContext.maybeScope(this.parent)) { this.subscriber.onNext(o); } } @Override public void onError(Throwable throwable) { - try (CurrentTraceContext.Scope scope = this.currentTraceContext - .maybeScope(this.traceContext)) { + try (Scope scope = this.currentTraceContext.maybeScope(this.parent)) { this.subscriber.onError(throwable); } } @Override public void onComplete() { - try (CurrentTraceContext.Scope scope = this.currentTraceContext - .maybeScope(this.traceContext)) { + try (Scope scope = this.currentTraceContext.maybeScope(this.parent)) { this.subscriber.onComplete(); } } diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/reactor/SpanSubscriber.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/reactor/SpanSubscriber.java index 2e51eb1e6..61ad068fc 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/reactor/SpanSubscriber.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/reactor/SpanSubscriber.java @@ -21,7 +21,9 @@ import java.util.concurrent.atomic.AtomicBoolean; import brave.Span; import brave.Tracer; import brave.Tracing; -import brave.propagation.TraceContextOrSamplingFlags; +import brave.propagation.CurrentTraceContext; +import brave.propagation.CurrentTraceContext.Scope; +import brave.propagation.TraceContext; import org.reactivestreams.Subscriber; import org.reactivestreams.Subscription; import reactor.util.Logger; @@ -44,7 +46,7 @@ final class SpanSubscriber extends AtomicBoolean implements SpanSubscription< private final Span span; - private final Span rootSpan; + private final TraceContext parent; private final Subscriber subscriber; @@ -52,27 +54,32 @@ final class SpanSubscriber extends AtomicBoolean implements SpanSubscription< private final Tracer tracer; + private final CurrentTraceContext currentTraceContext; + private Subscription s; SpanSubscriber(Subscriber subscriber, Context ctx, Tracing tracing, String name) { this.subscriber = subscriber; this.tracer = tracing.tracer(); - Span root = ctx.getOrDefault(Span.class, this.tracer.currentSpan()); - if (log.isTraceEnabled()) { - log.trace("Span from context [{}]", root); + this.currentTraceContext = tracing.currentTraceContext(); + TraceContext parent = ctx.getOrDefault(TraceContext.class, null); + if (parent == null) { + parent = currentTraceContext.get(); } - this.rootSpan = root; if (log.isTraceEnabled()) { - log.trace("Stored context root span [{}]", this.rootSpan); + log.trace("Span from context [{}]", parent); } - this.span = root != null ? this.tracer - .nextSpan(TraceContextOrSamplingFlags.create(root.context())).name(name) - : this.tracer.nextSpan().name(name); + this.parent = parent; + if (log.isTraceEnabled()) { + log.trace("Stored context parent span [{}]", this.parent); + } + this.span = parent != null ? this.tracer.newChild(parent).name(name) + : this.tracer.newTrace().name(name); if (log.isTraceEnabled()) { log.trace("Created span [{}], with name [{}]", this.span, name); } - this.context = ctx.put(Span.class, this.span); + this.context = ctx.put(TraceContext.class, this.span.context()); } @Override @@ -81,7 +88,7 @@ final class SpanSubscriber extends AtomicBoolean implements SpanSubscription< log.trace("On subscribe"); } this.s = subscription; - try (Tracer.SpanInScope ws = this.tracer.withSpanInScope(this.span)) { + try (Scope ws = this.currentTraceContext.maybeScope(this.span.context())) { if (log.isTraceEnabled()) { log.trace("On subscribe - span continued"); } @@ -94,7 +101,7 @@ final class SpanSubscriber extends AtomicBoolean implements SpanSubscription< if (log.isTraceEnabled()) { log.trace("Request"); } - try (Tracer.SpanInScope ws = this.tracer.withSpanInScope(this.span)) { + try (Scope ws = this.currentTraceContext.maybeScope(this.span.context())) { if (log.isTraceEnabled()) { log.trace("Request - continued"); } @@ -102,7 +109,7 @@ final class SpanSubscriber extends AtomicBoolean implements SpanSubscription< // no additional cleaning is required cause we operate on scopes if (log.isTraceEnabled()) { log.trace("Request after cleaning. Current span [{}]", - this.tracer.currentSpan()); + this.currentTraceContext.get()); } } } @@ -154,10 +161,10 @@ final class SpanSubscriber extends AtomicBoolean implements SpanSubscription< if (log.isTraceEnabled()) { log.trace("Span closed"); } - if (this.rootSpan != null) { - this.rootSpan.finish(); + if (this.parent != null) { + this.tracer.toSpan(parent).finish(); // TODO: why are we closing this? if (log.isTraceEnabled()) { - log.trace("Closed root span"); + log.trace("Closed parent span"); } } } diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/reactor/SpanSubscriptionProvider.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/reactor/SpanSubscriptionProvider.java index 9e1c924b6..e90b34836 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/reactor/SpanSubscriptionProvider.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/reactor/SpanSubscriptionProvider.java @@ -18,8 +18,8 @@ package org.springframework.cloud.sleuth.instrument.reactor; import java.util.function.Supplier; -import brave.Span; -import brave.Tracing; +import brave.propagation.CurrentTraceContext; +import brave.propagation.TraceContext; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.reactivestreams.Subscriber; @@ -45,7 +45,7 @@ final class SpanSubscriptionProvider implements Supplier> final String name; - private volatile Tracing tracing; + private volatile CurrentTraceContext currentTraceContext; SpanSubscriptionProvider(BeanFactory beanFactory, Subscriber subscriber, Context context, String name) { @@ -61,31 +61,32 @@ final class SpanSubscriptionProvider implements Supplier> @Override public SpanSubscription get() { - return newCoreSubscriber(tracing()); + return newCoreSubscriber(currentTraceContext()); } - SpanSubscription newCoreSubscriber(Tracing tracing) { - Span root = this.context.hasKey(Span.class) ? this.context.get(Span.class) - : tracing.tracer().currentSpan(); - return new ScopePassingSpanSubscriber<>(this.subscriber, this.context, tracing, - root); + SpanSubscription newCoreSubscriber(CurrentTraceContext currentTraceContext) { + TraceContext root = this.context.hasKey(TraceContext.class) + ? this.context.get(TraceContext.class) : currentTraceContext.get(); + return new ScopePassingSpanSubscriber<>(this.subscriber, this.context, + currentTraceContext, root); } - private Tracing tracing() { - if (this.tracing == null) { + private CurrentTraceContext currentTraceContext() { + if (this.currentTraceContext == null) { try { - this.tracing = this.beanFactory.getBean(Tracing.class); + this.currentTraceContext = this.beanFactory + .getBean(CurrentTraceContext.class); } catch (Exception ex) { if (log.isDebugEnabled()) { log.debug( - "Exception occurred while trying to get the tracing bean. Will return a default instance", + "Exception occurred while trying to get the currentTraceContext bean. Will return a default instance", ex); } - return Tracing.newBuilder().build(); + return CurrentTraceContext.Default.create(); } } - return this.tracing; + return this.currentTraceContext; } } diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceWebFilter.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceWebFilter.java index 78d2cfc08..4d197ba21 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceWebFilter.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceWebFilter.java @@ -24,7 +24,6 @@ import brave.http.HttpServerHandler; import brave.http.HttpTracing; import brave.propagation.Propagation; import brave.propagation.TraceContext; -import brave.propagation.TraceContextOrSamplingFlags; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.reactivestreams.Subscription; @@ -146,7 +145,7 @@ public final class TraceWebFilter implements WebFilter, Ordered { boolean tracePresent = tracer().currentSpan() != null; if (tracePresent) { // clear any previous trace - tracer().withSpanInScope(null); + tracer().withSpanInScope(null); // TODO: dangerous and also allocates stuff } return new MonoWebFilterTrace(source, exchange, tracePresent, this); } @@ -192,7 +191,7 @@ public final class TraceWebFilter implements WebFilter, Ordered { private Context contextWithoutInitialSpan(Context context) { if (this.initialTracePresent && !this.initialSpanAlreadyRemoved.get()) { - context = context.delete(Span.class); + context = context.delete(TraceContext.class); this.initialSpanAlreadyRemoved.set(true); } return context; @@ -200,11 +199,9 @@ public final class TraceWebFilter implements WebFilter, Ordered { private Span findOrCreateSpan(Context c) { Span span; - if (c.hasKey(Span.class)) { - Span parent = c.get(Span.class); - span = this.tracer - .nextSpan(TraceContextOrSamplingFlags.create(parent.context())) - .start(); + if (c.hasKey(TraceContext.class)) { + TraceContext parent = c.get(TraceContext.class); + span = this.tracer.newChild(parent).start(); if (log.isDebugEnabled()) { log.debug("Found span in reactor context" + span); } @@ -245,7 +242,7 @@ public final class TraceWebFilter implements WebFilter, Ordered { Span span, MonoWebFilterTrace parent) { this.actual = actual; this.span = span; - this.context = context.put(Span.class, span); + this.context = context.put(TraceContext.class, span.context()); this.exchange = parent.exchange; this.handler = parent.handler; } diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceWebClientBeanPostProcessor.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceWebClientBeanPostProcessor.java index ba251de3b..8cafff570 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceWebClientBeanPostProcessor.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceWebClientBeanPostProcessor.java @@ -268,8 +268,8 @@ final class TraceExchangeFilterFunction implements ExchangeFilterFunction { this.tracing = parent.tracing; this.scopePassingTransformer = parent.scopePassingTransformer; - if (!context.hasKey(Span.class)) { - context = context.put(Span.class, span); + if (!context.hasKey(TraceContext.class)) { + context = context.put(TraceContext.class, span.context()); if (log.isDebugEnabled()) { log.debug("Reactor Context got injected with the client span " + span); 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 e831e4f1b..67865f60a 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 @@ -25,6 +25,7 @@ import java.util.stream.Collectors; import brave.Span; import brave.Tracer; +import brave.propagation.TraceContext; import brave.sampler.Sampler; import org.apache.commons.lang3.StringUtils; import org.awaitility.Awaitility; @@ -75,8 +76,8 @@ public class SleuthSpanCreatorAspectFluxTests { } protected static Long id(Context context, Tracer tracer) { - if (context.hasKey(Span.class)) { - return context.get(Span.class).context().spanId(); + if (context.hasKey(TraceContext.class)) { + return context.get(TraceContext.class).spanId(); } return id(tracer); } 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 b94888311..b0a633bd1 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 @@ -28,7 +28,6 @@ import javax.annotation.concurrent.NotThreadSafe; import brave.Span; import brave.Tracer; import brave.sampler.Sampler; -import org.apache.commons.lang3.StringUtils; import org.awaitility.Awaitility; import org.junit.Before; import org.junit.Test; @@ -69,15 +68,11 @@ public class SleuthSpanCreatorAspectMonoTests { @Autowired ArrayListSpanReporter reporter; - private static String toHexString(long value) { - return StringUtils.leftPad(Long.toHexString(value), 16, '0'); - } - - protected static Long id(Tracer tracer) { + protected static String id(Tracer tracer) { if (tracer.currentSpan() == null) { throw new IllegalStateException("Current Span is supposed to have a value!"); } - return tracer.currentSpan().context().spanId(); + return tracer.currentSpan().context().spanIdString(); } @Before @@ -401,31 +396,31 @@ public class SleuthSpanCreatorAspectMonoTests { @Test public void shouldReturnNewSpanFromTraceContext() { - Mono mono = this.testBean.newSpanInTraceContext(); + Mono mono = this.testBean.newSpanInTraceContext(); then(this.reporter.getSpans()).isEmpty(); - Long newSpanId = mono.block(); + String newSpanId = mono.block(); Awaitility.await().untilAsserted(() -> { List spans = this.reporter.getSpans(); then(spans).hasSize(1); then(spans.get(0).name()).isEqualTo("span-in-trace-context"); - then(spans.get(0).id()).isEqualTo(toHexString(newSpanId)); + then(spans.get(0).id()).isEqualTo(newSpanId); then(this.tracer.currentSpan()).isNull(); }); } @Test public void shouldReturnNewSpanFromTraceContextOuter() { - Mono, Long>> mono = this.testBeanOuter + Mono, String>> mono = this.testBeanOuter .outerNewSpanInTraceContext(); then(this.reporter.getSpans()).isEmpty(); - Pair, Long> pair = mono.block(); - Long outerSpanIdBefore = pair.getFirst().getFirst(); - Long innerSpanId = pair.getSecond(); + Pair, String> pair = mono.block(); + String outerSpanIdBefore = pair.getFirst().getFirst(); + String innerSpanId = pair.getSecond(); then(outerSpanIdBefore).isNotEqualTo(innerSpanId); @@ -436,44 +431,44 @@ public class SleuthSpanCreatorAspectMonoTests { .findFirst().orElseThrow(() -> new AssertionError( "No span with name [outer-span-in-trace-context] found")); then(outerSpan.name()).isEqualTo("outer-span-in-trace-context"); - then(outerSpan.id()).isEqualTo(toHexString(outerSpanIdBefore)); + then(outerSpan.id()).isEqualTo(outerSpanIdBefore); zipkin2.Span innerSpan = spans.stream() .filter(span -> span.name().equals("span-in-trace-context")) .findFirst().orElseThrow(() -> new AssertionError( "No span with name [span-in-trace-context] found")); then(innerSpan.name()).isEqualTo("span-in-trace-context"); - then(innerSpan.id()).isEqualTo(toHexString(innerSpanId)); + then(innerSpan.id()).isEqualTo(innerSpanId); then(this.tracer.currentSpan()).isNull(); }); } @Test public void shouldReturnNewSpanFromSubscriberContext() { - Mono mono = this.testBean.newSpanInSubscriberContext(); + Mono mono = this.testBean.newSpanInSubscriberContext(); then(this.reporter.getSpans()).isEmpty(); - Long newSpanId = mono.block(); + String newSpanId = mono.block(); Awaitility.await().untilAsserted(() -> { List spans = this.reporter.getSpans(); then(spans).hasSize(1); then(spans.get(0).name()).isEqualTo("span-in-subscriber-context"); - then(spans.get(0).id()).isEqualTo(toHexString(newSpanId)); + then(spans.get(0).id()).isEqualTo(newSpanId); then(this.tracer.currentSpan()).isNull(); }); } @Test public void shouldReturnNewSpanFromSubscriberContextOuter() { - Mono, Long>> mono = this.testBeanOuter + Mono, String>> mono = this.testBeanOuter .outerNewSpanInSubscriberContext(); then(this.reporter.getSpans()).isEmpty(); - Pair, Long> pair = mono.block(); - Long outerSpanIdBefore = pair.getFirst().getFirst(); - Long innerSpanId = pair.getSecond(); + Pair, String> pair = mono.block(); + String outerSpanIdBefore = pair.getFirst().getFirst(); + String innerSpanId = pair.getSecond(); then(outerSpanIdBefore).isNotEqualTo(innerSpanId); @@ -484,13 +479,13 @@ public class SleuthSpanCreatorAspectMonoTests { .findFirst().orElseThrow(() -> new AssertionError( "No span with name [outer-span-in-subscriber-context] found")); then(outerSpan.name()).isEqualTo("outer-span-in-subscriber-context"); - then(outerSpan.id()).isEqualTo(toHexString(outerSpanIdBefore)); + then(outerSpan.id()).isEqualTo(outerSpanIdBefore); zipkin2.Span innerSpan = spans.stream() .filter(span -> span.name().equals("span-in-subscriber-context")) .findFirst().orElseThrow(() -> new AssertionError( "No span with name [span-in-subscriber-context] found")); then(innerSpan.name()).isEqualTo("span-in-subscriber-context"); - then(innerSpan.id()).isEqualTo(toHexString(innerSpanId)); + then(innerSpan.id()).isEqualTo(innerSpanId); then(this.tracer.currentSpan()).isNull(); }); } @@ -545,10 +540,10 @@ public class SleuthSpanCreatorAspectMonoTests { Mono testMethod13(); @NewSpan(name = "spanInTraceContext") - Mono newSpanInTraceContext(); + Mono newSpanInTraceContext(); @NewSpan(name = "spanInSubscriberContext") - Mono newSpanInSubscriberContext(); + Mono newSpanInSubscriberContext(); } @@ -645,12 +640,12 @@ public class SleuthSpanCreatorAspectMonoTests { } @Override - public Mono newSpanInTraceContext() { + public Mono newSpanInTraceContext() { return Mono.defer(() -> Mono.just(id(this.tracer))); } @Override - public Mono newSpanInSubscriberContext() { + public Mono newSpanInSubscriberContext() { return Mono.subscriberContext() .flatMap(context -> Mono.just(id(this.tracer))); } @@ -669,7 +664,7 @@ public class SleuthSpanCreatorAspectMonoTests { } @NewSpan(name = "outerSpanInTraceContext") - public Mono, Long>> outerNewSpanInTraceContext() { + public Mono, String>> outerNewSpanInTraceContext() { return Mono.defer(() -> Mono.just(id(this.tracer)) .zipWith(this.testBeanInterface.newSpanInTraceContext()) .map(pair -> Pair.of(Pair.of(pair.getT1(), id(this.tracer)), @@ -677,7 +672,7 @@ public class SleuthSpanCreatorAspectMonoTests { } @NewSpan(name = "outerSpanInSubscriberContext") - public Mono, Long>> outerNewSpanInSubscriberContext() { + public Mono, String>> outerNewSpanInSubscriberContext() { return Mono.subscriberContext() .flatMap(context -> Mono.just(id(this.tracer)) .zipWith(this.testBeanInterface.newSpanInSubscriberContext()) diff --git a/tests/spring-cloud-sleuth-instrumentation-reactor-tests/src/test/java/org/springframework/cloud/sleuth/instrument/reactor/ScopePassingSpanSubscriberTests.java b/tests/spring-cloud-sleuth-instrumentation-reactor-tests/src/test/java/org/springframework/cloud/sleuth/instrument/reactor/ScopePassingSpanSubscriberTests.java index fd110e04a..6c0bf89c6 100644 --- a/tests/spring-cloud-sleuth-instrumentation-reactor-tests/src/test/java/org/springframework/cloud/sleuth/instrument/reactor/ScopePassingSpanSubscriberTests.java +++ b/tests/spring-cloud-sleuth-instrumentation-reactor-tests/src/test/java/org/springframework/cloud/sleuth/instrument/reactor/ScopePassingSpanSubscriberTests.java @@ -16,9 +16,9 @@ package org.springframework.cloud.sleuth.instrument.reactor; -import brave.Span; -import brave.Tracer; -import brave.Tracing; +import brave.propagation.CurrentTraceContext; +import brave.propagation.CurrentTraceContext.Scope; +import brave.propagation.TraceContext; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.BDDMockito; @@ -37,12 +37,15 @@ import static org.assertj.core.api.BDDAssertions.then; @RunWith(MockitoJUnitRunner.class) public class ScopePassingSpanSubscriberTests { - Tracing tracing = Tracing.newBuilder().build(); + CurrentTraceContext currentTraceContext = CurrentTraceContext.Default.create(); + + TraceContext context = TraceContext.newBuilder().traceId(1).spanId(1).sampled(true) + .build(); @Test public void should_propagate_current_context() { ScopePassingSpanSubscriber subscriber = new ScopePassingSpanSubscriber<>(null, - Context.of("foo", "bar"), this.tracing, null); + Context.of("foo", "bar"), this.currentTraceContext, null); then((String) subscriber.currentContext().get("foo")).isEqualTo("bar"); } @@ -50,28 +53,27 @@ public class ScopePassingSpanSubscriberTests { @Test public void should_set_empty_context_when_context_is_null() { ScopePassingSpanSubscriber subscriber = new ScopePassingSpanSubscriber<>(null, - null, this.tracing, null); + null, this.currentTraceContext, null); then(subscriber.currentContext().isEmpty()).isTrue(); } @Test public void should_put_current_span_to_context() { - Span span = this.tracing.tracer().nextSpan(); - try (Tracer.SpanInScope ws = this.tracing.tracer() - .withSpanInScope(span.start())) { + try (Scope ws = this.currentTraceContext.newScope(context)) { CoreSubscriber subscriber = ReactorSleuth.scopePassingSpanSubscription( beanFactory(), new BaseSubscriber() { }); - then(subscriber.currentContext().get(Span.class)).isEqualTo(span); + then(subscriber.currentContext().get(TraceContext.class)).isEqualTo(context); } } private BeanFactory beanFactory() { BeanFactory beanFactory = BDDMockito.mock(BeanFactory.class); - BDDMockito.given(beanFactory.getBean(Tracing.class)).willReturn(this.tracing); + BDDMockito.given(beanFactory.getBean(CurrentTraceContext.class)) + .willReturn(this.currentTraceContext); return beanFactory; }