From 6c396876037b03c858a79811c2dcf94a3267f89e Mon Sep 17 00:00:00 2001 From: tedsuo Date: Thu, 16 Feb 2017 23:10:25 -0800 Subject: [PATCH 1/4] WIP Still Missing: * MockTracer and TestTracer must be implemented * Exectors should be subclassed with traced wrapper methods https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/Executors.html * Some tests mebbe --- .../java/io/opentracing/SpanSnapshot.java | 5 + .../src/main/java/io/opentracing/Tracer.java | 23 +++ .../concurrent/TracedCallable.java | 39 ++++ .../concurrent/TracedExecutorService.java | 107 +++++++++++ .../concurrent/TracedRunnable.java | 43 +++++ .../io/opentracing/impl/AbstractTracer.java | 7 + .../io/opentracing/impl/GlobalTracer.java | 170 ++++++++++++++++++ .../java/io/opentracing/impl/NoopSpan.java | 3 +- .../java/io/opentracing/impl/NoopTracer.java | 13 +- .../opentracing/impl/AbstractTracerTest.java | 4 + .../java/io/opentracing/mock/MockTracer.java | 10 ++ .../java/io/opentracing/NoopSpanSnapshot.java | 9 + .../main/java/io/opentracing/NoopTracer.java | 12 ++ pom.xml | 4 +- 14 files changed, 444 insertions(+), 5 deletions(-) create mode 100644 opentracing-api/src/main/java/io/opentracing/SpanSnapshot.java create mode 100644 opentracing-impl/src/main/java/io/opentracing/concurrent/TracedCallable.java create mode 100644 opentracing-impl/src/main/java/io/opentracing/concurrent/TracedExecutorService.java create mode 100644 opentracing-impl/src/main/java/io/opentracing/concurrent/TracedRunnable.java create mode 100644 opentracing-impl/src/main/java/io/opentracing/impl/GlobalTracer.java create mode 100644 opentracing-noop/src/main/java/io/opentracing/NoopSpanSnapshot.java diff --git a/opentracing-api/src/main/java/io/opentracing/SpanSnapshot.java b/opentracing-api/src/main/java/io/opentracing/SpanSnapshot.java new file mode 100644 index 00000000..702a160a --- /dev/null +++ b/opentracing-api/src/main/java/io/opentracing/SpanSnapshot.java @@ -0,0 +1,5 @@ +package io.opentracing; + +public interface SpanSnapshot { + +} diff --git a/opentracing-api/src/main/java/io/opentracing/Tracer.java b/opentracing-api/src/main/java/io/opentracing/Tracer.java index 26b3d001..d1d31ea2 100644 --- a/opentracing-api/src/main/java/io/opentracing/Tracer.java +++ b/opentracing-api/src/main/java/io/opentracing/Tracer.java @@ -88,6 +88,29 @@ public interface Tracer { */ SpanContext extract(Format format, C carrier); + /** + * Returns the span associated with the current execution context. + * + */ + Span active(); + + /** + * Retrieve the associated SpanSnapshot. + * @return the SpanSnapshot that encapsulates Span state that should propagate across in-process concurrency boundaries. + */ + SpanSnapshot snapshot(Span span); + + /** + * Resusitates a span in the current execution context, and sets it to active. + * + */ + Span resume(SpanSnapshot snapshot); + + /** + * Clears the Span from the stack of active spans. + * @param span + */ + void deactivate(Span span); interface SpanBuilder extends SpanContext { diff --git a/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedCallable.java b/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedCallable.java new file mode 100644 index 00000000..a1ac6cbb --- /dev/null +++ b/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedCallable.java @@ -0,0 +1,39 @@ +package io.opentracing.concurrent; + +import io.opentracing.Span; +import io.opentracing.SpanSnapshot; +import io.opentracing.Tracer; +import io.opentracing.impl.GlobalTracer; + +import java.util.concurrent.Callable; + +public class TracedCallable implements Callable { + private SpanSnapshot snapshot; + private Tracer tracer; + private Callable callable; + + public TracedCallable(Callable callable) { + this(callable, GlobalTracer.get()); + } + + public TracedCallable(Callable callable, Tracer tracer) { + this(callable, tracer.active(), tracer); + } + + public TracedCallable(Callable callable, Span span, Tracer tracer) { + if (callable == null) throw new NullPointerException("Callable is ."); + if (tracer == null) throw new NullPointerException("Tracer is ."); + this.callable= callable; + this.snapshot = tracer.snapshot(span); + this.tracer = tracer; + } + + public T call() throws Exception { + final Span span = tracer.resume(snapshot); + try { + return callable.call(); + } finally { + tracer.deactivate(span); + } + } +} diff --git a/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedExecutorService.java b/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedExecutorService.java new file mode 100644 index 00000000..c52e1545 --- /dev/null +++ b/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedExecutorService.java @@ -0,0 +1,107 @@ +package io.opentracing.concurrent; + +import io.opentracing.Span; +import io.opentracing.Tracer; +import io.opentracing.impl.GlobalTracer; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import java.util.concurrent.*; + +public class TracedExecutorService implements ExecutorService { + private ExecutorService executor; + private Span span; + private Tracer tracer; + + public TracedExecutorService(ExecutorService executor){ + this(executor, GlobalTracer.get()); + } + + public TracedExecutorService(ExecutorService executor, Tracer tracer){ + this(executor, tracer.active(), tracer); + } + + public TracedExecutorService(ExecutorService executor, Span span, Tracer tracer){ + if (executor == null) throw new NullPointerException("Executor is ."); + if (tracer == null) throw new NullPointerException("Tracer is ."); + this.executor = executor; + this.span = span; + this.tracer = tracer; + } + + @Override + public void execute(Runnable command) { + executor.execute(new TracedRunnable(command, span, tracer)); + } + + @Override + public Future submit(Runnable task) { + return executor.submit(new TracedRunnable(task, span, tracer)); + } + + @Override + public Future submit(Runnable task, T result) { + return executor.submit(new TracedRunnable(task, span, tracer), result); + } + + @Override + public Future submit(Callable task) { + return executor.submit(new TracedCallable(task, span, tracer)); + } + + @Override + public List> invokeAll(Collection> tasks) throws InterruptedException { + return executor.invokeAll(tasksWithTracing(tasks)); + } + + @Override + public List> invokeAll(Collection> tasks, long timeout, TimeUnit unit) + throws InterruptedException { + return executor.invokeAll(tasksWithTracing(tasks), timeout, unit); + } + + @Override + public T invokeAny(Collection> tasks) throws InterruptedException, ExecutionException { + return executor.invokeAny(tasksWithTracing(tasks)); + } + + @Override + public T invokeAny(Collection> tasks, long timeout, TimeUnit unit) + throws InterruptedException, ExecutionException, TimeoutException { + return executor.invokeAny(tasksWithTracing(tasks), timeout, unit); + } + + @Override + public void shutdown() { + executor.shutdown(); + } + + @Override + public List shutdownNow() { + return executor.shutdownNow(); + } + + @Override + public boolean isShutdown() { + return executor.isShutdown(); + } + + @Override + public boolean isTerminated() { + return executor.isTerminated(); + } + + @Override + public boolean awaitTermination(long timeout, TimeUnit unit) throws InterruptedException { + return executor.awaitTermination(timeout, unit); + } + + private Collection> tasksWithTracing( + Collection> tasks) { + if (tasks == null) throw new NullPointerException("Collection of tasks is ."); + Collection> result = new ArrayList>(tasks.size()); + for (Callable task : tasks) result.add(new TracedCallable(task, span, tracer)); + return result; + } +} diff --git a/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedRunnable.java b/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedRunnable.java new file mode 100644 index 00000000..4b5696f5 --- /dev/null +++ b/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedRunnable.java @@ -0,0 +1,43 @@ +package io.opentracing.concurrent; + +import io.opentracing.Span; +import io.opentracing.SpanSnapshot; +import io.opentracing.Tracer; +import io.opentracing.impl.GlobalTracer; + + +public class TracedRunnable implements Runnable { + private Runnable runnable; + private SpanSnapshot snapshot; + private Tracer tracer; + + public TracedRunnable(Runnable runnable) { + this(runnable, GlobalTracer.get()); + } + + public TracedRunnable(Runnable runnable, Span span) { + this(runnable, span, GlobalTracer.get()); + } + + public TracedRunnable(Runnable runnable, Tracer tracer) { + this(runnable, tracer.active(), tracer); + } + + public TracedRunnable(Runnable runnable, Span span, Tracer tracer) { + if (runnable == null) throw new NullPointerException("Runnable is ."); + if (tracer == null) throw new NullPointerException("Tracer is ."); + this.runnable = runnable; + this.snapshot = tracer.snapshot(span); + this.tracer = tracer; + } + + @Override + public void run() { + final Span span = tracer.resume(snapshot); + try { + runnable.run(); + } finally { + tracer.deactivate(span); + } + } +} diff --git a/opentracing-impl/src/main/java/io/opentracing/impl/AbstractTracer.java b/opentracing-impl/src/main/java/io/opentracing/impl/AbstractTracer.java index 9b0c8169..e50bb0ea 100644 --- a/opentracing-impl/src/main/java/io/opentracing/impl/AbstractTracer.java +++ b/opentracing-impl/src/main/java/io/opentracing/impl/AbstractTracer.java @@ -13,7 +13,9 @@ */ package io.opentracing.impl; +import io.opentracing.Span; import io.opentracing.SpanContext; +import io.opentracing.SpanSnapshot; import io.opentracing.Tracer; import io.opentracing.propagation.Extractor; import io.opentracing.propagation.Format; @@ -62,6 +64,11 @@ public Extractor register(Format format, Extractor extractor) { /** @return the minimal set of properties required to propagate this span */ abstract Map getTraceState(SpanContext spanContext); + + public abstract Span active(); + + public abstract Span resume(SpanSnapshot snapshot); + private static class PropagationRegistry { private final ConcurrentMap injectors = new ConcurrentHashMap<>(); diff --git a/opentracing-impl/src/main/java/io/opentracing/impl/GlobalTracer.java b/opentracing-impl/src/main/java/io/opentracing/impl/GlobalTracer.java new file mode 100644 index 00000000..9389257e --- /dev/null +++ b/opentracing-impl/src/main/java/io/opentracing/impl/GlobalTracer.java @@ -0,0 +1,170 @@ +package io.opentracing.impl; + +/** + * Copyright 2017 The OpenTracing 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 + * + * http://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. + */ + +import io.opentracing.*; +import io.opentracing.propagation.Format; + +import java.util.Iterator; +import java.util.ServiceLoader; +import java.util.concurrent.atomic.AtomicReference; +import java.util.logging.Level; +import java.util.logging.Logger; + +/** + * Forwards all methods to another tracer that can be configured in one of two ways: + *
    + *
  1. Explicitly, calling {@link #register(Tracer)} with a configured tracer, or:
    2. + *
  2. Automatically using the Java {@link ServiceLoader} SPI mechanism to load a {@link Tracer} from the classpath.
    3. + *
+ *

+ * When the tracer is needed it is lazily looked up using the following rules: + *

    + *
  1. The last-{@link #register(Tracer) registered} tracer always takes precedence.
    2. + *
  2. If no tracer was registered, one is looked up from the {@link ServiceLoader}.
    + * The {@linkplain GlobalTracer} will not attempt to choose between implementations:
    3. + *
  3. If no single tracer service is found, the {@link io.opentracing.NoopTracer NoopTracer} will be used.
    4. + *
+ */ +public final class GlobalTracer implements Tracer { + private static final Logger LOGGER = Logger.getLogger(GlobalTracer.class.getName()); + + /** + * Singleton instance. + *

+ * Since we cannot prevent people using {@linkplain #get() GlobalTracer.get()} as a constant, + * this guarantees that references obtained before, during or after initialization + * all behave as if obtained after initialization once properly initialized.
+ * As a minor additional benefit it makes it harder to circumvent the {@link Tracer} API. + */ + private static final GlobalTracer INSTANCE = new GlobalTracer(); + + /** + * The resolved {@link Tracer} to delegate to. + *

+ * This can be either an {@link #register(Tracer) explicitly registered} or + * the {@link #loadSingleSpiImplementation() automatically resolved} Tracer + * (or null during initialization). + */ + private final AtomicReference globalTracer = new AtomicReference(); + + private GlobalTracer() { + } + + private Tracer lazyTracer() { + Tracer tracer = globalTracer.get(); + if (tracer == null) { + final Tracer resolved = loadSingleSpiImplementation(); + while (tracer == null && resolved != null) { // handle rare race condition + globalTracer.compareAndSet(null, resolved); + tracer = globalTracer.get(); + } + LOGGER.log(Level.INFO, "Using GlobalTracer: {0}.", tracer); + } + return tracer; + } + + /** + * Returns the constant {@linkplain GlobalTracer}. + *

+ * All methods are forwarded to the currently configured tracer.
+ * Until a tracer is {@link #register(Tracer) explicitly configured}, + * one is looked up from the {@link ServiceLoader}, + * falling back to the {@link io.opentracing.NoopTracer NoopTracer}.
+ * A tracer can be re-configured at any time. + * For example, the tracer used to extract a span may be different than the one that injects it. + * + * @return The global tracer constant. + * @see #register(Tracer) + */ + public static Tracer get() { + return INSTANCE; + } + + /** + * Explicitly configures a {@link Tracer} to back the behaviour of the {@link #get() global tracer}. + *

+ * The previous global tracer is returned so it can be restored later if necessary. + * + * @param tracer Tracer to use as global tracer. + * @return The previous global tracer or null if there was none. + */ + public static Tracer register(final Tracer tracer) { + if (tracer instanceof GlobalTracer) { + LOGGER.log(Level.FINE, "Attempted to register the GlobalTracer as delegate of itself."); + return INSTANCE.globalTracer.get(); // no-op, return 'previous' tracer. + } + Tracer previous = INSTANCE.globalTracer.getAndSet(tracer); + LOGGER.log(Level.INFO, "Registered GlobalTracer {0} (previously {1}).", new Object[]{tracer, previous}); + return previous; + } + + @Override + public Span active(){ + return lazyTracer().active(); + } + + @Override + public SpanSnapshot snapshot(Span span){ + return lazyTracer().snapshot(span); + } + + @Override + public void deactivate(Span span){ + lazyTracer().deactivate(span); + } + + @Override + public Span resume(SpanSnapshot snapshot){ + return lazyTracer().resume(snapshot); + } + + @Override + public SpanBuilder buildSpan(String operationName) { + return lazyTracer().buildSpan(operationName); + } + + @Override + public void inject(SpanContext spanContext, Format format, C carrier) { + lazyTracer().inject(spanContext, format, carrier); + } + + @Override + public SpanContext extract(Format format, C carrier) { + return lazyTracer().extract(format, carrier); + } + + /** + * Loads a single service implementation from {@link ServiceLoader}. + * + * @return The single service or a NoopTracer. + */ + private static Tracer loadSingleSpiImplementation() { + // Use the ServiceLoader to find the declared Tracer implementation. + Iterator spiImplementations = + ServiceLoader.load(Tracer.class, Tracer.class.getClassLoader()).iterator(); + if (spiImplementations.hasNext()) { + Tracer foundImplementation = spiImplementations.next(); + if (!spiImplementations.hasNext()) { + return foundImplementation; + } + LOGGER.log(Level.WARNING, "More than one Tracer service found. " + + "Falling back to NoopTracer implementation."); + } + return NoopTracerFactory.create(); + } + +} + diff --git a/opentracing-impl/src/main/java/io/opentracing/impl/NoopSpan.java b/opentracing-impl/src/main/java/io/opentracing/impl/NoopSpan.java index de0f832b..2820dc0b 100644 --- a/opentracing-impl/src/main/java/io/opentracing/impl/NoopSpan.java +++ b/opentracing-impl/src/main/java/io/opentracing/impl/NoopSpan.java @@ -13,8 +13,7 @@ */ package io.opentracing.impl; -import io.opentracing.NoopSpanContext; -import io.opentracing.Span; +import io.opentracing.*; final class NoopSpan extends AbstractSpan implements io.opentracing.NoopSpan, NoopSpanContext { diff --git a/opentracing-impl/src/main/java/io/opentracing/impl/NoopTracer.java b/opentracing-impl/src/main/java/io/opentracing/impl/NoopTracer.java index e990d019..13508f2d 100644 --- a/opentracing-impl/src/main/java/io/opentracing/impl/NoopTracer.java +++ b/opentracing-impl/src/main/java/io/opentracing/impl/NoopTracer.java @@ -13,7 +13,7 @@ */ package io.opentracing.impl; -import io.opentracing.SpanContext; +import io.opentracing.*; import io.opentracing.propagation.Format; import java.util.Collections; import java.util.Map; @@ -40,5 +40,16 @@ Map getTraceState(SpanContext spanContext) { return Collections.emptyMap(); } + @Override + public Span active() { return io.opentracing.NoopSpan.INSTANCE; } + + @Override + public SpanSnapshot snapshot(Span span) { return NoopSpanSnapshot.INSTANCE; } + + @Override + public Span resume(SpanSnapshot snapshot) { return io.opentracing.NoopSpan.INSTANCE; } + + @Override + public void deactivate(Span span) {} } diff --git a/opentracing-impl/src/test/java/io/opentracing/impl/AbstractTracerTest.java b/opentracing-impl/src/test/java/io/opentracing/impl/AbstractTracerTest.java index c45a2fac..12f526c4 100644 --- a/opentracing-impl/src/test/java/io/opentracing/impl/AbstractTracerTest.java +++ b/opentracing-impl/src/test/java/io/opentracing/impl/AbstractTracerTest.java @@ -133,6 +133,7 @@ public void propagatesBaggageFromSpanContext() { final class TestTracerImpl extends AbstractTracer { static final String OPERATION_NAME = "operation-name"; + Span activeSpan; @Override public AbstractSpanBuilder createSpanBuilder(String operationName) { @@ -157,6 +158,9 @@ AbstractSpanBuilder withStateItem(String key, Object value) { Map getTraceState(SpanContext spanContext) { return new HashMap<>(((AbstractSpan)spanContext).getBaggage()); } + + @Override + public Span active() { return this.activeSpan; } } } diff --git a/opentracing-mock/src/main/java/io/opentracing/mock/MockTracer.java b/opentracing-mock/src/main/java/io/opentracing/mock/MockTracer.java index 27ca15ff..39d3218a 100644 --- a/opentracing-mock/src/main/java/io/opentracing/mock/MockTracer.java +++ b/opentracing-mock/src/main/java/io/opentracing/mock/MockTracer.java @@ -38,6 +38,7 @@ public class MockTracer implements Tracer { private List finishedSpans = new ArrayList<>(); private final Propagator propagator; + private Span activeSpan; public MockTracer() { this(Propagator.PRINTER); @@ -149,6 +150,15 @@ public Tracer.SpanBuilder buildSpan(String operationName) { return new SpanBuilder(operationName); } + @Override + public Span active() { return this.activeSpan; } + + @Override + public void activate(Span span) { this.activeSpan = span; } + + @Override + public void deactivate(Span span) { this.activeSpan = null; } + @Override public void inject(SpanContext spanContext, Format format, C carrier) { this.propagator.inject((MockSpan.MockContext)spanContext, format, carrier); diff --git a/opentracing-noop/src/main/java/io/opentracing/NoopSpanSnapshot.java b/opentracing-noop/src/main/java/io/opentracing/NoopSpanSnapshot.java new file mode 100644 index 00000000..f6cd92d1 --- /dev/null +++ b/opentracing-noop/src/main/java/io/opentracing/NoopSpanSnapshot.java @@ -0,0 +1,9 @@ +package io.opentracing; + +public interface NoopSpanSnapshot extends SpanSnapshot { + static final NoopSpanSnapshotImpl INSTANCE = new NoopSpanSnapshotImpl(); +} + +final class NoopSpanSnapshotImpl implements NoopSpanSnapshot { + +} \ No newline at end of file diff --git a/opentracing-noop/src/main/java/io/opentracing/NoopTracer.java b/opentracing-noop/src/main/java/io/opentracing/NoopTracer.java index 40d17a9a..48c75d2c 100644 --- a/opentracing-noop/src/main/java/io/opentracing/NoopTracer.java +++ b/opentracing-noop/src/main/java/io/opentracing/NoopTracer.java @@ -30,5 +30,17 @@ public void inject(SpanContext spanContext, Format format, C carrier) {} @Override public SpanContext extract(Format format, C carrier) { return NoopSpanBuilderImpl.INSTANCE; } + @Override + public Span active() { return NoopSpan.INSTANCE; } + + @Override + public SpanSnapshot snapshot(Span span) { return NoopSpanSnapshot.INSTANCE; } + + @Override + public Span resume(SpanSnapshot snapshot) { return NoopSpan.INSTANCE; } + + @Override + public void deactivate(Span span) {} + } diff --git a/pom.xml b/pom.xml index 142bcd68..973f1021 100644 --- a/pom.xml +++ b/pom.xml @@ -238,7 +238,7 @@ true - + maven-release-plugin ${maven-release-plugin.version} From 09787dd03537d5dbf3b3e8dbcf19862b18bd8cd6 Mon Sep 17 00:00:00 2001 From: Ben Sigelman Date: Sun, 19 Feb 2017 17:38:17 +0100 Subject: [PATCH 2/4] Spike that captures MDC context --- opentracing-api/pom.xml | 15 +++ .../io/opentracing/ActiveSpanManager.java | 34 ++++++ .../io/opentracing/MDCActiveSpanManager.java | 111 ++++++++++++++++++ .../src/main/java/io/opentracing/Span.java | 3 + .../java/io/opentracing/SpanSnapshot.java | 5 - .../src/main/java/io/opentracing/Tracer.java | 28 +---- .../concurrent/TracedCallable.java | 1 - .../concurrent/TracedRunnable.java | 1 - .../io/opentracing/impl/AbstractTracer.java | 1 - .../java/io/opentracing/mock/MockSpan.java | 15 +++ .../java/io/opentracing/mock/MockTracer.java | 36 ++++-- .../io/opentracing/mock/MockTracerTest.java | 29 +++++ .../java/io/opentracing/NoopSpanSnapshot.java | 2 +- 13 files changed, 235 insertions(+), 46 deletions(-) create mode 100644 opentracing-api/src/main/java/io/opentracing/ActiveSpanManager.java create mode 100644 opentracing-api/src/main/java/io/opentracing/MDCActiveSpanManager.java delete mode 100644 opentracing-api/src/main/java/io/opentracing/SpanSnapshot.java diff --git a/opentracing-api/pom.xml b/opentracing-api/pom.xml index 29d0d810..dd8d8078 100644 --- a/opentracing-api/pom.xml +++ b/opentracing-api/pom.xml @@ -40,5 +40,20 @@ 1.10.19 test + + org.slf4j + slf4j-api + 1.7.23 + + + log4j + log4j + 1.2.17 + + + org.slf4j + slf4j-log4j12 + 1.7.23 + diff --git a/opentracing-api/src/main/java/io/opentracing/ActiveSpanManager.java b/opentracing-api/src/main/java/io/opentracing/ActiveSpanManager.java new file mode 100644 index 00000000..2dd9d3e4 --- /dev/null +++ b/opentracing-api/src/main/java/io/opentracing/ActiveSpanManager.java @@ -0,0 +1,34 @@ +package io.opentracing; + +/** + * XXX(bhs): comment + */ +public interface ActiveSpanManager { + // XXX(bhs): good comment. just a marker, obviously. + public interface Snapshot { + Span span(); + } + + /** + * Returns the span associated with the current execution context. + * + */ + Span active(); + + /** + * Retrieve the associated SpanSnapshot. + * @return the SpanSnapshot that encapsulates Span state that should propagate across in-process concurrency boundaries. + */ + Snapshot snapshot(Span span); + + /** + * Activates a given Snapshot (per snapshot()). + */ + void activate(Snapshot snapshot); + + /** + * XXX: comment + */ + void deactivate(Snapshot snapshot); + +} diff --git a/opentracing-api/src/main/java/io/opentracing/MDCActiveSpanManager.java b/opentracing-api/src/main/java/io/opentracing/MDCActiveSpanManager.java new file mode 100644 index 00000000..05fee002 --- /dev/null +++ b/opentracing-api/src/main/java/io/opentracing/MDCActiveSpanManager.java @@ -0,0 +1,111 @@ +package io.opentracing; + +import org.slf4j.MDC; +import sun.awt.image.ImageWatched; + +import java.util.Map; + +/** + * XXX: comment + */ +public class MDCActiveSpanManager implements io.opentracing.ActiveSpanManager { + private final ThreadLocal tlsSnapshot = new ThreadLocal(); + + class MDCSnapshot implements ActiveSpanManager.Snapshot { + private final Map mdcContext; + private final Span span; + private MDCSnapshot toRestore = null; + + MDCSnapshot(Span span) { + this.mdcContext = MDC.getCopyOfContextMap(); + this.span = span; + } + + public Span span() { + return span; + } + + void setToRestore(MDCSnapshot toRestore) { + this.toRestore = toRestore; + } + + MDCSnapshot toRestore() { + return toRestore; + } + } + + @Override + public Span active() { + MDCSnapshot snapshot = tlsSnapshot.get(); + if (snapshot == null) { + return null; + } + return snapshot.span(); + } + + @Override + public MDCSnapshot snapshot(Span span) { + return new MDCSnapshot(span); + } + + @Override + public void activate(Snapshot snapshot) { + if (!(snapshot instanceof MDCSnapshot)) { + throw new IllegalArgumentException("activate() expected MDCSnapshot"); + } + ((MDCSnapshot) snapshot).setToRestore(tlsSnapshot.get()); + tlsSnapshot.set((MDCSnapshot)snapshot); + } + + @Override + public void deactivate(Snapshot snapshot) { + if (!(snapshot instanceof MDCSnapshot)) { + throw new IllegalArgumentException("deactivate() expected MDCSnapshot"); + } + + if (tlsSnapshot.get() != snapshot) { + // do nothing + return; + } + MDCSnapshot nextActiveSnapshot = ((MDCSnapshot)snapshot).toRestore(); + while (nextActiveSnapshot != null) { + if (!nextActiveSnapshot.span().isFinished()) { + break; + } + nextActiveSnapshot = nextActiveSnapshot.toRestore(); + } + tlsSnapshot.set(nextActiveSnapshot); + } +} + +/* + +STREAM OF CONSCIOUSNESS: + +When the user starts a new Span: + - Span goes into TLS immediately + - Existing TLS Span must become a parent pointer or similar + +When a Span comes into the foreground: + - must adopt the context from the last time it was paused or created + - must retain a pointer to whatever it replaced + +When a Span goes into the background or finishes: + - restore whatever retained pointer there was + - ... and if that pointer has already been finished? + +I'm troubled by the case where multiple children are created and then the parent finishes before the children. + +[ Span P ] + [ Span A ] + [ Span B ] + [ Span C ] + +(Where P is the parent of A, B, and C) + +When A-C finish in that scenario, what should happen to the threadlocal? + +What if all "managed" activity had to happen in the context of a Closure/Runnable? Return value could indicate whether +the Span should finish or not. + + */ diff --git a/opentracing-api/src/main/java/io/opentracing/Span.java b/opentracing-api/src/main/java/io/opentracing/Span.java index 7a670c46..1394d28e 100644 --- a/opentracing-api/src/main/java/io/opentracing/Span.java +++ b/opentracing-api/src/main/java/io/opentracing/Span.java @@ -41,6 +41,9 @@ public interface Span extends Closeable { */ void finish(); + // XXX: comment + boolean isFinished(); + /** * Sets an explicit end timestamp and records the span. * diff --git a/opentracing-api/src/main/java/io/opentracing/SpanSnapshot.java b/opentracing-api/src/main/java/io/opentracing/SpanSnapshot.java deleted file mode 100644 index 702a160a..00000000 --- a/opentracing-api/src/main/java/io/opentracing/SpanSnapshot.java +++ /dev/null @@ -1,5 +0,0 @@ -package io.opentracing; - -public interface SpanSnapshot { - -} diff --git a/opentracing-api/src/main/java/io/opentracing/Tracer.java b/opentracing-api/src/main/java/io/opentracing/Tracer.java index d1d31ea2..a941c079 100644 --- a/opentracing-api/src/main/java/io/opentracing/Tracer.java +++ b/opentracing-api/src/main/java/io/opentracing/Tracer.java @@ -41,6 +41,8 @@ public interface Tracer { */ SpanBuilder buildSpan(String operationName); + void setActiveSpanManager(ActiveSpanManager mgr); + /** * Inject a SpanContext into a `carrier` of a given type, presumably for propagation across process boundaries. * @@ -88,30 +90,8 @@ public interface Tracer { */ SpanContext extract(Format format, C carrier); - /** - * Returns the span associated with the current execution context. - * - */ - Span active(); - - /** - * Retrieve the associated SpanSnapshot. - * @return the SpanSnapshot that encapsulates Span state that should propagate across in-process concurrency boundaries. - */ - SpanSnapshot snapshot(Span span); - - /** - * Resusitates a span in the current execution context, and sets it to active. - * - */ - Span resume(SpanSnapshot snapshot); - - /** - * Clears the Span from the stack of active spans. - * @param span - */ - void deactivate(Span span); - + // XXX(bhs): could make this an abstract class. In any case, by default a SpanBuilder will have an asChildOf pointer + // to the ActiveSpanManager's active Span's SpanContext. interface SpanBuilder extends SpanContext { /** diff --git a/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedCallable.java b/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedCallable.java index a1ac6cbb..b9213b88 100644 --- a/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedCallable.java +++ b/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedCallable.java @@ -1,7 +1,6 @@ package io.opentracing.concurrent; import io.opentracing.Span; -import io.opentracing.SpanSnapshot; import io.opentracing.Tracer; import io.opentracing.impl.GlobalTracer; diff --git a/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedRunnable.java b/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedRunnable.java index 4b5696f5..b8bd66cf 100644 --- a/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedRunnable.java +++ b/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedRunnable.java @@ -1,7 +1,6 @@ package io.opentracing.concurrent; import io.opentracing.Span; -import io.opentracing.SpanSnapshot; import io.opentracing.Tracer; import io.opentracing.impl.GlobalTracer; diff --git a/opentracing-impl/src/main/java/io/opentracing/impl/AbstractTracer.java b/opentracing-impl/src/main/java/io/opentracing/impl/AbstractTracer.java index e50bb0ea..6e8fc1c8 100644 --- a/opentracing-impl/src/main/java/io/opentracing/impl/AbstractTracer.java +++ b/opentracing-impl/src/main/java/io/opentracing/impl/AbstractTracer.java @@ -15,7 +15,6 @@ import io.opentracing.Span; import io.opentracing.SpanContext; -import io.opentracing.SpanSnapshot; import io.opentracing.Tracer; import io.opentracing.propagation.Extractor; import io.opentracing.propagation.Format; diff --git a/opentracing-mock/src/main/java/io/opentracing/mock/MockSpan.java b/opentracing-mock/src/main/java/io/opentracing/mock/MockSpan.java index 47a8469b..17df017b 100644 --- a/opentracing-mock/src/main/java/io/opentracing/mock/MockSpan.java +++ b/opentracing-mock/src/main/java/io/opentracing/mock/MockSpan.java @@ -16,6 +16,7 @@ import java.util.*; import java.util.concurrent.atomic.AtomicLong; +import io.opentracing.ActiveSpanManager; import io.opentracing.Span; import io.opentracing.SpanContext; @@ -38,6 +39,7 @@ public final class MockSpan implements Span { private final Map tags; private final List logEntries = new ArrayList<>(); private String operationName; + private ActiveSpanManager.Snapshot snapshot; public String operationName() { return this.operationName; @@ -96,6 +98,9 @@ public void finish() { @Override public synchronized void finish(long finishMicros) { if (!finished) { + if (this.mockTracer.getActiveSpanManager() != null) { + this.mockTracer.getActiveSpanManager().deactivate(this.snapshot); + } this.finishMicros = finishMicros; this.mockTracer.appendFinishedSpan(this); this.finished = true; @@ -104,6 +109,11 @@ public synchronized void finish(long finishMicros) { } } + @Override + public synchronized boolean isFinished() { + return finished; + } + @Override public void close() { this.finish(); @@ -253,6 +263,11 @@ public long timestampMicros() { this.context = new MockContext(parent.traceId, nextId(), parent.baggage); this.parentId = parent.spanId; } + + if (tracer.getActiveSpanManager() != null) { + // XXX weird/awkward + this.snapshot = tracer.getActiveSpanManager().snapshot(this); + } } static long nextId() { diff --git a/opentracing-mock/src/main/java/io/opentracing/mock/MockTracer.java b/opentracing-mock/src/main/java/io/opentracing/mock/MockTracer.java index 39d3218a..8a68f62c 100644 --- a/opentracing-mock/src/main/java/io/opentracing/mock/MockTracer.java +++ b/opentracing-mock/src/main/java/io/opentracing/mock/MockTracer.java @@ -20,10 +20,7 @@ import java.util.List; import java.util.Map; -import io.opentracing.References; -import io.opentracing.Span; -import io.opentracing.SpanContext; -import io.opentracing.Tracer; +import io.opentracing.*; import io.opentracing.propagation.Format; import io.opentracing.propagation.TextMap; @@ -38,7 +35,7 @@ public class MockTracer implements Tracer { private List finishedSpans = new ArrayList<>(); private final Propagator propagator; - private Span activeSpan; + private ActiveSpanManager activeSpanManager; public MockTracer() { this(Propagator.PRINTER); @@ -147,17 +144,25 @@ public MockSpan.MockContext extract(Format format, C carrier) { @Override public Tracer.SpanBuilder buildSpan(String operationName) { - return new SpanBuilder(operationName); + SpanBuilder sb = new SpanBuilder(operationName); + if (this.activeSpanManager != null) { + Span active = this.activeSpanManager.active(); + if (active != null) { + sb.asChildOf(active.context()); + } + } + return sb; } @Override - public Span active() { return this.activeSpan; } - - @Override - public void activate(Span span) { this.activeSpan = span; } + public void setActiveSpanManager(ActiveSpanManager mgr) { + this.activeSpanManager = mgr; + } - @Override - public void deactivate(Span span) { this.activeSpan = null; } + // XXX: comment + ActiveSpanManager getActiveSpanManager() { + return this.activeSpanManager; + } @Override public void inject(SpanContext spanContext, Format format, C carrier) { @@ -231,7 +236,12 @@ public Span start() { if (this.startMicros == 0) { this.startMicros = MockSpan.nowMicros(); } - return new MockSpan(MockTracer.this, this.operationName, this.startMicros, initialTags, this.firstParent); + Span rval = new MockSpan(MockTracer.this, this.operationName, this.startMicros, initialTags, this.firstParent); + if (MockTracer.this.activeSpanManager != null) { + MockTracer.this.activeSpanManager.activate( + MockTracer.this.activeSpanManager.snapshot(rval)); + } + return rval; } @Override diff --git a/opentracing-mock/src/test/java/io/opentracing/mock/MockTracerTest.java b/opentracing-mock/src/test/java/io/opentracing/mock/MockTracerTest.java index c24959c6..e956306a 100644 --- a/opentracing-mock/src/test/java/io/opentracing/mock/MockTracerTest.java +++ b/opentracing-mock/src/test/java/io/opentracing/mock/MockTracerTest.java @@ -20,6 +20,7 @@ import java.util.List; import java.util.Map; +import io.opentracing.MDCActiveSpanManager; import org.junit.Assert; import org.junit.Test; @@ -30,7 +31,35 @@ import io.opentracing.propagation.TextMapExtractAdapter; import io.opentracing.propagation.TextMapInjectAdapter; +import org.slf4j.Logger; +import org.slf4j.MDC; + public class MockTracerTest { + @Test + public void testMDCHack() { + org.apache.log4j.BasicConfigurator.configure(); + Logger logger = org.slf4j.LoggerFactory.getLogger("hack"); + MDC.put("key1", "val1"); + Map ctxMap = MDC.getCopyOfContextMap(); + MDC.put("key2", "val2"); + MDC.setContextMap(ctxMap); + logger.info("testing: {}", MDC.getCopyOfContextMap().toString()); + + MockTracer tracer = new MockTracer(); + tracer.setActiveSpanManager(new MDCActiveSpanManager()); + + Span par = tracer.buildSpan("parent").start(); + Span childA = tracer.buildSpan("childA").start(); + childA.finish(); + par.finish(); + + List finishedSpans = tracer.finishedSpans(); + + for (MockSpan span : finishedSpans) { + logger.info("finished Span. {} :: {} ({})", span.context().traceId(), span.context().spanId(), span.parentId()); + } + } + @Test public void testRootSpan() { // Create and finish a root Span. diff --git a/opentracing-noop/src/main/java/io/opentracing/NoopSpanSnapshot.java b/opentracing-noop/src/main/java/io/opentracing/NoopSpanSnapshot.java index f6cd92d1..a484a72d 100644 --- a/opentracing-noop/src/main/java/io/opentracing/NoopSpanSnapshot.java +++ b/opentracing-noop/src/main/java/io/opentracing/NoopSpanSnapshot.java @@ -1,6 +1,6 @@ package io.opentracing; -public interface NoopSpanSnapshot extends SpanSnapshot { +public interface NoopSpanSnapshot { static final NoopSpanSnapshotImpl INSTANCE = new NoopSpanSnapshotImpl(); } From 84936be53cff3d66d161366d5bc5bc1888e975ea Mon Sep 17 00:00:00 2001 From: Ben Sigelman Date: Mon, 20 Feb 2017 06:55:15 +0100 Subject: [PATCH 3/4] Get everything to compile --- .../io/opentracing/ActiveSpanManager.java | 2 +- .../io/opentracing/MDCActiveSpanManager.java | 3 ++- .../src/main/java/io/opentracing/Tracer.java | 1 + .../concurrent/TracedCallable.java | 22 +++++++-------- .../concurrent/TracedExecutorService.java | 27 ++++++++----------- .../concurrent/TracedRunnable.java | 24 ++++++++--------- .../io/opentracing/impl/AbstractSpan.java | 5 ++++ .../io/opentracing/impl/AbstractTracer.java | 17 ++++++++---- .../io/opentracing/impl/GlobalTracer.java | 22 +++++---------- .../java/io/opentracing/impl/NoopSpan.java | 5 ++++ .../io/opentracing/impl/NoopSpanBuilder.java | 2 ++ .../java/io/opentracing/impl/NoopTracer.java | 18 +++++-------- .../impl/TextMapExtractorImpl.java | 2 +- .../opentracing/impl/AbstractTracerTest.java | 3 --- .../io/opentracing/impl/TestSpanImpl.java | 5 ++++ .../java/io/opentracing/mock/MockTracer.java | 5 ++++ .../main/java/io/opentracing/NoopSpan.java | 4 ++- .../main/java/io/opentracing/NoopTracer.java | 14 +++------- 18 files changed, 92 insertions(+), 89 deletions(-) diff --git a/opentracing-api/src/main/java/io/opentracing/ActiveSpanManager.java b/opentracing-api/src/main/java/io/opentracing/ActiveSpanManager.java index 2dd9d3e4..d87077af 100644 --- a/opentracing-api/src/main/java/io/opentracing/ActiveSpanManager.java +++ b/opentracing-api/src/main/java/io/opentracing/ActiveSpanManager.java @@ -24,7 +24,7 @@ public interface Snapshot { /** * Activates a given Snapshot (per snapshot()). */ - void activate(Snapshot snapshot); + Span activate(Snapshot snapshot); /** * XXX: comment diff --git a/opentracing-api/src/main/java/io/opentracing/MDCActiveSpanManager.java b/opentracing-api/src/main/java/io/opentracing/MDCActiveSpanManager.java index 05fee002..2d020a8a 100644 --- a/opentracing-api/src/main/java/io/opentracing/MDCActiveSpanManager.java +++ b/opentracing-api/src/main/java/io/opentracing/MDCActiveSpanManager.java @@ -49,12 +49,13 @@ public MDCSnapshot snapshot(Span span) { } @Override - public void activate(Snapshot snapshot) { + public Span activate(Snapshot snapshot) { if (!(snapshot instanceof MDCSnapshot)) { throw new IllegalArgumentException("activate() expected MDCSnapshot"); } ((MDCSnapshot) snapshot).setToRestore(tlsSnapshot.get()); tlsSnapshot.set((MDCSnapshot)snapshot); + return snapshot.span(); } @Override diff --git a/opentracing-api/src/main/java/io/opentracing/Tracer.java b/opentracing-api/src/main/java/io/opentracing/Tracer.java index a941c079..e26d6347 100644 --- a/opentracing-api/src/main/java/io/opentracing/Tracer.java +++ b/opentracing-api/src/main/java/io/opentracing/Tracer.java @@ -42,6 +42,7 @@ public interface Tracer { SpanBuilder buildSpan(String operationName); void setActiveSpanManager(ActiveSpanManager mgr); + ActiveSpanManager activeSpanManager(); /** * Inject a SpanContext into a `carrier` of a given type, presumably for propagation across process boundaries. diff --git a/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedCallable.java b/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedCallable.java index b9213b88..e2a5fdd5 100644 --- a/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedCallable.java +++ b/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedCallable.java @@ -1,5 +1,6 @@ package io.opentracing.concurrent; +import io.opentracing.ActiveSpanManager; import io.opentracing.Span; import io.opentracing.Tracer; import io.opentracing.impl.GlobalTracer; @@ -7,32 +8,31 @@ import java.util.concurrent.Callable; public class TracedCallable implements Callable { - private SpanSnapshot snapshot; - private Tracer tracer; + private ActiveSpanManager.Snapshot snapshot; + private ActiveSpanManager manager; private Callable callable; public TracedCallable(Callable callable) { - this(callable, GlobalTracer.get()); + this(callable, GlobalTracer.get().activeSpanManager()); } - public TracedCallable(Callable callable, Tracer tracer) { - this(callable, tracer.active(), tracer); + public TracedCallable(Callable callable, ActiveSpanManager manager) { + this(callable, manager.active(), manager); } - public TracedCallable(Callable callable, Span span, Tracer tracer) { + public TracedCallable(Callable callable, Span span, ActiveSpanManager manager) { if (callable == null) throw new NullPointerException("Callable is ."); - if (tracer == null) throw new NullPointerException("Tracer is ."); this.callable= callable; - this.snapshot = tracer.snapshot(span); - this.tracer = tracer; + this.manager = manager; + this.snapshot = manager.snapshot(span); } public T call() throws Exception { - final Span span = tracer.resume(snapshot); + final Span span = manager.activate(snapshot); try { return callable.call(); } finally { - tracer.deactivate(span); + manager.deactivate(snapshot); } } } diff --git a/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedExecutorService.java b/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedExecutorService.java index c52e1545..726f763a 100644 --- a/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedExecutorService.java +++ b/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedExecutorService.java @@ -1,5 +1,6 @@ package io.opentracing.concurrent; +import io.opentracing.ActiveSpanManager; import io.opentracing.Span; import io.opentracing.Tracer; import io.opentracing.impl.GlobalTracer; @@ -11,43 +12,37 @@ public class TracedExecutorService implements ExecutorService { private ExecutorService executor; - private Span span; - private Tracer tracer; + private ActiveSpanManager manager; public TracedExecutorService(ExecutorService executor){ - this(executor, GlobalTracer.get()); + this(executor, GlobalTracer.get().activeSpanManager()); } - public TracedExecutorService(ExecutorService executor, Tracer tracer){ - this(executor, tracer.active(), tracer); - } - - public TracedExecutorService(ExecutorService executor, Span span, Tracer tracer){ + public TracedExecutorService(ExecutorService executor, ActiveSpanManager manager) { if (executor == null) throw new NullPointerException("Executor is ."); - if (tracer == null) throw new NullPointerException("Tracer is ."); + if (manager == null) throw new NullPointerException("ActiveSpanManager is ."); this.executor = executor; - this.span = span; - this.tracer = tracer; + this.manager = manager; } @Override public void execute(Runnable command) { - executor.execute(new TracedRunnable(command, span, tracer)); + executor.execute(new TracedRunnable(command, manager)); } @Override public Future submit(Runnable task) { - return executor.submit(new TracedRunnable(task, span, tracer)); + return executor.submit(new TracedRunnable(task, manager)); } @Override public Future submit(Runnable task, T result) { - return executor.submit(new TracedRunnable(task, span, tracer), result); + return executor.submit(new TracedRunnable(task, manager), result); } @Override public Future submit(Callable task) { - return executor.submit(new TracedCallable(task, span, tracer)); + return executor.submit(new TracedCallable(task, manager)); } @Override @@ -101,7 +96,7 @@ private Collection> tasksWithTracing( Collection> tasks) { if (tasks == null) throw new NullPointerException("Collection of tasks is ."); Collection> result = new ArrayList>(tasks.size()); - for (Callable task : tasks) result.add(new TracedCallable(task, span, tracer)); + for (Callable task : tasks) result.add(new TracedCallable(task, manager)); return result; } } diff --git a/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedRunnable.java b/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedRunnable.java index b8bd66cf..068e8626 100644 --- a/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedRunnable.java +++ b/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedRunnable.java @@ -1,5 +1,6 @@ package io.opentracing.concurrent; +import io.opentracing.ActiveSpanManager; import io.opentracing.Span; import io.opentracing.Tracer; import io.opentracing.impl.GlobalTracer; @@ -7,36 +8,35 @@ public class TracedRunnable implements Runnable { private Runnable runnable; - private SpanSnapshot snapshot; - private Tracer tracer; + private ActiveSpanManager manager; + private ActiveSpanManager.Snapshot snapshot; public TracedRunnable(Runnable runnable) { - this(runnable, GlobalTracer.get()); + this(runnable, GlobalTracer.get().activeSpanManager()); } public TracedRunnable(Runnable runnable, Span span) { - this(runnable, span, GlobalTracer.get()); + this(runnable, span, GlobalTracer.get().activeSpanManager()); } - public TracedRunnable(Runnable runnable, Tracer tracer) { - this(runnable, tracer.active(), tracer); + public TracedRunnable(Runnable runnable, ActiveSpanManager manager) { + this(runnable, manager.active(), manager); } - public TracedRunnable(Runnable runnable, Span span, Tracer tracer) { + public TracedRunnable(Runnable runnable, Span span, ActiveSpanManager manager) { if (runnable == null) throw new NullPointerException("Runnable is ."); - if (tracer == null) throw new NullPointerException("Tracer is ."); this.runnable = runnable; - this.snapshot = tracer.snapshot(span); - this.tracer = tracer; + this.manager = manager; + this.snapshot = manager.snapshot(span); } @Override public void run() { - final Span span = tracer.resume(snapshot); + final Span span = manager.activate(this.snapshot); try { runnable.run(); } finally { - tracer.deactivate(span); + manager.deactivate(this.snapshot); } } } diff --git a/opentracing-impl/src/main/java/io/opentracing/impl/AbstractSpan.java b/opentracing-impl/src/main/java/io/opentracing/impl/AbstractSpan.java index 4305684d..b3f08120 100644 --- a/opentracing-impl/src/main/java/io/opentracing/impl/AbstractSpan.java +++ b/opentracing-impl/src/main/java/io/opentracing/impl/AbstractSpan.java @@ -55,6 +55,11 @@ public void finish() { duration = Duration.between(start, Instant.now()); } + @Override + public boolean isFinished() { + return null != duration; + } + @Override public void finish(long finishMicros) { long finishEpochSeconds = TimeUnit.MICROSECONDS.toSeconds(finishMicros); diff --git a/opentracing-impl/src/main/java/io/opentracing/impl/AbstractTracer.java b/opentracing-impl/src/main/java/io/opentracing/impl/AbstractTracer.java index 6e8fc1c8..a3dca44e 100644 --- a/opentracing-impl/src/main/java/io/opentracing/impl/AbstractTracer.java +++ b/opentracing-impl/src/main/java/io/opentracing/impl/AbstractTracer.java @@ -13,6 +13,7 @@ */ package io.opentracing.impl; +import io.opentracing.ActiveSpanManager; import io.opentracing.Span; import io.opentracing.SpanContext; import io.opentracing.Tracer; @@ -29,6 +30,7 @@ abstract class AbstractTracer implements Tracer { static final boolean BAGGAGE_ENABLED = !Boolean.getBoolean("opentracing.propagation.dropBaggage"); private final PropagationRegistry registry = new PropagationRegistry(); + private ActiveSpanManager manager; protected AbstractTracer() { registry.register(Format.Builtin.TEXT_MAP, new TextMapInjectorImpl(this)); @@ -37,6 +39,16 @@ protected AbstractTracer() { abstract AbstractSpanBuilder createSpanBuilder(String operationName); + @Override + public void setActiveSpanManager(ActiveSpanManager manager) { + this.manager = manager; + } + + @Override + public ActiveSpanManager activeSpanManager() { + return this.manager; + } + @Override public SpanBuilder buildSpan(String operationName){ return createSpanBuilder(operationName); @@ -63,11 +75,6 @@ public Extractor register(Format format, Extractor extractor) { /** @return the minimal set of properties required to propagate this span */ abstract Map getTraceState(SpanContext spanContext); - - public abstract Span active(); - - public abstract Span resume(SpanSnapshot snapshot); - private static class PropagationRegistry { private final ConcurrentMap injectors = new ConcurrentHashMap<>(); diff --git a/opentracing-impl/src/main/java/io/opentracing/impl/GlobalTracer.java b/opentracing-impl/src/main/java/io/opentracing/impl/GlobalTracer.java index 9389257e..6d7ac692 100644 --- a/opentracing-impl/src/main/java/io/opentracing/impl/GlobalTracer.java +++ b/opentracing-impl/src/main/java/io/opentracing/impl/GlobalTracer.java @@ -112,28 +112,18 @@ public static Tracer register(final Tracer tracer) { } @Override - public Span active(){ - return lazyTracer().active(); - } - - @Override - public SpanSnapshot snapshot(Span span){ - return lazyTracer().snapshot(span); - } - - @Override - public void deactivate(Span span){ - lazyTracer().deactivate(span); + public SpanBuilder buildSpan(String operationName) { + return lazyTracer().buildSpan(operationName); } @Override - public Span resume(SpanSnapshot snapshot){ - return lazyTracer().resume(snapshot); + public void setActiveSpanManager(ActiveSpanManager mgr) { + lazyTracer().setActiveSpanManager(mgr); } @Override - public SpanBuilder buildSpan(String operationName) { - return lazyTracer().buildSpan(operationName); + public ActiveSpanManager activeSpanManager() { + return lazyTracer().activeSpanManager(); } @Override diff --git a/opentracing-impl/src/main/java/io/opentracing/impl/NoopSpan.java b/opentracing-impl/src/main/java/io/opentracing/impl/NoopSpan.java index 2820dc0b..be8b5d7d 100644 --- a/opentracing-impl/src/main/java/io/opentracing/impl/NoopSpan.java +++ b/opentracing-impl/src/main/java/io/opentracing/impl/NoopSpan.java @@ -27,6 +27,11 @@ public NoopSpan(String operationName) { public void finish() { } + @Override + public boolean isFinished() { + return true; + } + @Override public void finish(long finishMicros) { } diff --git a/opentracing-impl/src/main/java/io/opentracing/impl/NoopSpanBuilder.java b/opentracing-impl/src/main/java/io/opentracing/impl/NoopSpanBuilder.java index 08976fc6..77b0e8a2 100644 --- a/opentracing-impl/src/main/java/io/opentracing/impl/NoopSpanBuilder.java +++ b/opentracing-impl/src/main/java/io/opentracing/impl/NoopSpanBuilder.java @@ -15,6 +15,8 @@ import io.opentracing.NoopSpanContext; +import java.util.Map; + final class NoopSpanBuilder extends AbstractSpanBuilder implements io.opentracing.NoopSpanBuilder, NoopSpanContext { static final NoopSpanBuilder INSTANCE = new NoopSpanBuilder("noop"); diff --git a/opentracing-impl/src/main/java/io/opentracing/impl/NoopTracer.java b/opentracing-impl/src/main/java/io/opentracing/impl/NoopTracer.java index 13508f2d..859bdf24 100644 --- a/opentracing-impl/src/main/java/io/opentracing/impl/NoopTracer.java +++ b/opentracing-impl/src/main/java/io/opentracing/impl/NoopTracer.java @@ -22,6 +22,12 @@ final class NoopTracer extends AbstractTracer implements io.opentracing.NoopTrac private static final NoopTracer INSTANCE = new NoopTracer(); + @Override + public void setActiveSpanManager(ActiveSpanManager mgr) {} + + @Override + public ActiveSpanManager activeSpanManager() { return null; } + @Override public void inject(SpanContext spanContext, Format format, C carrier) {} @@ -40,16 +46,4 @@ Map getTraceState(SpanContext spanContext) { return Collections.emptyMap(); } - @Override - public Span active() { return io.opentracing.NoopSpan.INSTANCE; } - - @Override - public SpanSnapshot snapshot(Span span) { return NoopSpanSnapshot.INSTANCE; } - - @Override - public Span resume(SpanSnapshot snapshot) { return io.opentracing.NoopSpan.INSTANCE; } - - @Override - public void deactivate(Span span) {} - } diff --git a/opentracing-impl/src/main/java/io/opentracing/impl/TextMapExtractorImpl.java b/opentracing-impl/src/main/java/io/opentracing/impl/TextMapExtractorImpl.java index 4e80ab87..53b29c1a 100644 --- a/opentracing-impl/src/main/java/io/opentracing/impl/TextMapExtractorImpl.java +++ b/opentracing-impl/src/main/java/io/opentracing/impl/TextMapExtractorImpl.java @@ -41,4 +41,4 @@ public Tracer.SpanBuilder extract(TextMap carrier) { return builder; } -} \ No newline at end of file +} diff --git a/opentracing-impl/src/test/java/io/opentracing/impl/AbstractTracerTest.java b/opentracing-impl/src/test/java/io/opentracing/impl/AbstractTracerTest.java index 12f526c4..bd0aa7f3 100644 --- a/opentracing-impl/src/test/java/io/opentracing/impl/AbstractTracerTest.java +++ b/opentracing-impl/src/test/java/io/opentracing/impl/AbstractTracerTest.java @@ -158,9 +158,6 @@ AbstractSpanBuilder withStateItem(String key, Object value) { Map getTraceState(SpanContext spanContext) { return new HashMap<>(((AbstractSpan)spanContext).getBaggage()); } - - @Override - public Span active() { return this.activeSpan; } } } diff --git a/opentracing-impl/src/test/java/io/opentracing/impl/TestSpanImpl.java b/opentracing-impl/src/test/java/io/opentracing/impl/TestSpanImpl.java index 634fbb34..0ef62481 100644 --- a/opentracing-impl/src/test/java/io/opentracing/impl/TestSpanImpl.java +++ b/opentracing-impl/src/test/java/io/opentracing/impl/TestSpanImpl.java @@ -20,4 +20,9 @@ public class TestSpanImpl extends AbstractSpan { TestSpanImpl(String operationName) { super(operationName); } + + @Override + public boolean isFinished() { + return true; + } } diff --git a/opentracing-mock/src/main/java/io/opentracing/mock/MockTracer.java b/opentracing-mock/src/main/java/io/opentracing/mock/MockTracer.java index 8a68f62c..f85ea069 100644 --- a/opentracing-mock/src/main/java/io/opentracing/mock/MockTracer.java +++ b/opentracing-mock/src/main/java/io/opentracing/mock/MockTracer.java @@ -159,6 +159,11 @@ public void setActiveSpanManager(ActiveSpanManager mgr) { this.activeSpanManager = mgr; } + @Override + public ActiveSpanManager activeSpanManager() { + return activeSpanManager; + } + // XXX: comment ActiveSpanManager getActiveSpanManager() { return this.activeSpanManager; diff --git a/opentracing-noop/src/main/java/io/opentracing/NoopSpan.java b/opentracing-noop/src/main/java/io/opentracing/NoopSpan.java index 8e971804..5fe3804d 100644 --- a/opentracing-noop/src/main/java/io/opentracing/NoopSpan.java +++ b/opentracing-noop/src/main/java/io/opentracing/NoopSpan.java @@ -21,7 +21,6 @@ public interface NoopSpan extends Span { final class NoopSpanImpl implements NoopSpan { - @Override public SpanContext context() { return NoopSpanContextImpl.INSTANCE; } @@ -31,6 +30,9 @@ public void finish() {} @Override public void finish(long finishMicros) {} + @Override + public boolean isFinished() { return true; } + @Override public void close() { finish(); } diff --git a/opentracing-noop/src/main/java/io/opentracing/NoopTracer.java b/opentracing-noop/src/main/java/io/opentracing/NoopTracer.java index 48c75d2c..dbe02a61 100644 --- a/opentracing-noop/src/main/java/io/opentracing/NoopTracer.java +++ b/opentracing-noop/src/main/java/io/opentracing/NoopTracer.java @@ -25,22 +25,16 @@ final class NoopTracerImpl implements NoopTracer { public SpanBuilder buildSpan(String operationName) { return NoopSpanBuilderImpl.INSTANCE; } @Override - public void inject(SpanContext spanContext, Format format, C carrier) {} - - @Override - public SpanContext extract(Format format, C carrier) { return NoopSpanBuilderImpl.INSTANCE; } + public void setActiveSpanManager(ActiveSpanManager mgr) {} @Override - public Span active() { return NoopSpan.INSTANCE; } + public ActiveSpanManager activeSpanManager() { return null; } @Override - public SpanSnapshot snapshot(Span span) { return NoopSpanSnapshot.INSTANCE; } - - @Override - public Span resume(SpanSnapshot snapshot) { return NoopSpan.INSTANCE; } + public void inject(SpanContext spanContext, Format format, C carrier) {} @Override - public void deactivate(Span span) {} + public SpanContext extract(Format format, C carrier) { return NoopSpanBuilderImpl.INSTANCE; } } From 5964dd96e3c9129b9b518be2b0e5e988df985c1e Mon Sep 17 00:00:00 2001 From: Ben Sigelman Date: Mon, 20 Feb 2017 07:17:27 +0100 Subject: [PATCH 4/4] Tweaks --- .../io/opentracing/ActiveSpanManager.java | 25 ++++----------- .../io/opentracing/MDCActiveSpanManager.java | 32 ------------------- .../java/io/opentracing/mock/MockSpan.java | 8 ++--- .../java/io/opentracing/mock/MockTracer.java | 5 --- 4 files changed, 11 insertions(+), 59 deletions(-) diff --git a/opentracing-api/src/main/java/io/opentracing/ActiveSpanManager.java b/opentracing-api/src/main/java/io/opentracing/ActiveSpanManager.java index d87077af..a351be36 100644 --- a/opentracing-api/src/main/java/io/opentracing/ActiveSpanManager.java +++ b/opentracing-api/src/main/java/io/opentracing/ActiveSpanManager.java @@ -1,34 +1,23 @@ package io.opentracing; -/** - * XXX(bhs): comment - */ public interface ActiveSpanManager { - // XXX(bhs): good comment. just a marker, obviously. + // Basically a marker interface public interface Snapshot { Span span(); } - /** - * Returns the span associated with the current execution context. - * - */ + // Get the currently active Span (perhaps for this thread, etc) Span active(); - /** - * Retrieve the associated SpanSnapshot. - * @return the SpanSnapshot that encapsulates Span state that should propagate across in-process concurrency boundaries. - */ + // Create a Snapshot encapsulating both the given Span and any state needed to activate/deactivate (see below) Snapshot snapshot(Span span); - /** - * Activates a given Snapshot (per snapshot()). - */ + // Make the Snapshot and the Span it contains "active" per active(). + // + // *Must* be paired with a subsequent call to deactivate(). Span activate(Snapshot snapshot); - /** - * XXX: comment - */ + // See activate() above. void deactivate(Snapshot snapshot); } diff --git a/opentracing-api/src/main/java/io/opentracing/MDCActiveSpanManager.java b/opentracing-api/src/main/java/io/opentracing/MDCActiveSpanManager.java index 2d020a8a..03819eda 100644 --- a/opentracing-api/src/main/java/io/opentracing/MDCActiveSpanManager.java +++ b/opentracing-api/src/main/java/io/opentracing/MDCActiveSpanManager.java @@ -78,35 +78,3 @@ public void deactivate(Snapshot snapshot) { tlsSnapshot.set(nextActiveSnapshot); } } - -/* - -STREAM OF CONSCIOUSNESS: - -When the user starts a new Span: - - Span goes into TLS immediately - - Existing TLS Span must become a parent pointer or similar - -When a Span comes into the foreground: - - must adopt the context from the last time it was paused or created - - must retain a pointer to whatever it replaced - -When a Span goes into the background or finishes: - - restore whatever retained pointer there was - - ... and if that pointer has already been finished? - -I'm troubled by the case where multiple children are created and then the parent finishes before the children. - -[ Span P ] - [ Span A ] - [ Span B ] - [ Span C ] - -(Where P is the parent of A, B, and C) - -When A-C finish in that scenario, what should happen to the threadlocal? - -What if all "managed" activity had to happen in the context of a Closure/Runnable? Return value could indicate whether -the Span should finish or not. - - */ diff --git a/opentracing-mock/src/main/java/io/opentracing/mock/MockSpan.java b/opentracing-mock/src/main/java/io/opentracing/mock/MockSpan.java index 17df017b..69eafab9 100644 --- a/opentracing-mock/src/main/java/io/opentracing/mock/MockSpan.java +++ b/opentracing-mock/src/main/java/io/opentracing/mock/MockSpan.java @@ -98,8 +98,8 @@ public void finish() { @Override public synchronized void finish(long finishMicros) { if (!finished) { - if (this.mockTracer.getActiveSpanManager() != null) { - this.mockTracer.getActiveSpanManager().deactivate(this.snapshot); + if (this.mockTracer.activeSpanManager() != null) { + this.mockTracer.activeSpanManager().deactivate(this.snapshot); } this.finishMicros = finishMicros; this.mockTracer.appendFinishedSpan(this); @@ -264,9 +264,9 @@ public long timestampMicros() { this.parentId = parent.spanId; } - if (tracer.getActiveSpanManager() != null) { + if (tracer.activeSpanManager() != null) { // XXX weird/awkward - this.snapshot = tracer.getActiveSpanManager().snapshot(this); + this.snapshot = tracer.activeSpanManager().snapshot(this); } } diff --git a/opentracing-mock/src/main/java/io/opentracing/mock/MockTracer.java b/opentracing-mock/src/main/java/io/opentracing/mock/MockTracer.java index f85ea069..92f12991 100644 --- a/opentracing-mock/src/main/java/io/opentracing/mock/MockTracer.java +++ b/opentracing-mock/src/main/java/io/opentracing/mock/MockTracer.java @@ -164,11 +164,6 @@ public ActiveSpanManager activeSpanManager() { return activeSpanManager; } - // XXX: comment - ActiveSpanManager getActiveSpanManager() { - return this.activeSpanManager; - } - @Override public void inject(SpanContext spanContext, Format format, C carrier) { this.propagator.inject((MockSpan.MockContext)spanContext, format, carrier);