From 7a61a55e88b4f334edf12ed0bef369e3d53dd724 Mon Sep 17 00:00:00 2001 From: Sjoerd Talsma Date: Wed, 22 Feb 2017 16:26:51 +0100 Subject: [PATCH 01/23] Added Format.Builtin.toString() as they tend to show up in exception. (#87) * Added Format.Builtin.toString() as they tend to show up in exception messages. * Make Format.Builtin constructor private. * Change "Builtin" literal to Builtin.class.getSimpleName(). --- .../io/opentracing/propagation/Format.java | 22 +++++++++-- .../propagation/BuiltinFormatTest.java | 37 +++++++++++++++++++ 2 files changed, 55 insertions(+), 4 deletions(-) create mode 100644 opentracing-api/src/test/java/io/opentracing/propagation/BuiltinFormatTest.java diff --git a/opentracing-api/src/main/java/io/opentracing/propagation/Format.java b/opentracing-api/src/main/java/io/opentracing/propagation/Format.java index 29d71d6c..894ed906 100644 --- a/opentracing-api/src/main/java/io/opentracing/propagation/Format.java +++ b/opentracing-api/src/main/java/io/opentracing/propagation/Format.java @@ -1,5 +1,5 @@ /** - * Copyright 2016 The OpenTracing Authors + * Copyright 2016-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 @@ -35,6 +35,12 @@ */ public interface Format { final class Builtin implements Format { + private final String name; + + private Builtin(String name) { + this.name = name; + } + /** * The TEXT_MAP format allows for arbitrary String->String map encoding of SpanContext state for Tracer.inject * and Tracer.extract. @@ -46,7 +52,7 @@ final class Builtin implements Format { * @see Format * @see Builtin#HTTP_HEADERS */ - public final static Format TEXT_MAP = new Builtin(); + public final static Format TEXT_MAP = new Builtin("TEXT_MAP"); /** * The HTTP_HEADERS format allows for HTTP-header-compatible String->String map encoding of SpanContext state @@ -60,7 +66,7 @@ final class Builtin implements Format { * @see Format * @see Builtin#TEXT_MAP */ - public final static Format HTTP_HEADERS = new Builtin(); + public final static Format HTTP_HEADERS = new Builtin("HTTP_HEADERS"); /** * The BINARY format allows for unconstrained binary encoding of SpanContext state for Tracer.inject and @@ -70,6 +76,14 @@ final class Builtin implements Format { * @see io.opentracing.Tracer#extract(Format, Object) * @see Format */ - public final static Format BINARY = new Builtin(); + public final static Format BINARY = new Builtin("BINARY"); + + /** + * @return Short name for built-in formats as they tend to show up in exception messages. + */ + @Override + public String toString() { + return Builtin.class.getSimpleName() + "." + name; + } } } diff --git a/opentracing-api/src/test/java/io/opentracing/propagation/BuiltinFormatTest.java b/opentracing-api/src/test/java/io/opentracing/propagation/BuiltinFormatTest.java new file mode 100644 index 00000000..200ff196 --- /dev/null +++ b/opentracing-api/src/test/java/io/opentracing/propagation/BuiltinFormatTest.java @@ -0,0 +1,37 @@ +/** + * Copyright 2016-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. + */ +package io.opentracing.propagation; + +import org.junit.Test; + +import static org.junit.Assert.assertEquals; + +public class BuiltinFormatTest { + + @Test + public void test_HTTP_HEADERS_toString() { + assertEquals("Builtin.HTTP_HEADERS", Format.Builtin.HTTP_HEADERS.toString()); + } + + @Test + public void test_TEXT_MAP_toString() { + assertEquals("Builtin.TEXT_MAP", Format.Builtin.TEXT_MAP.toString()); + } + + @Test + public void test_BINARY_toString() { + assertEquals("Builtin.BINARY", Format.Builtin.BINARY.toString()); + } + +} From 612d16df4210fdf4e83b8445fd37ef5a06f3ef75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=90=B4=E6=99=9F=20Wu=20Sheng?= Date: Thu, 23 Feb 2017 09:49:06 +0800 Subject: [PATCH 02/23] Add new tags based on OT spec update. (#103) --- .../main/java/io/opentracing/tag/Tags.java | 52 ++++++++++++++----- 1 file changed, 38 insertions(+), 14 deletions(-) diff --git a/opentracing-api/src/main/java/io/opentracing/tag/Tags.java b/opentracing-api/src/main/java/io/opentracing/tag/Tags.java index 9da1118a..aa18777d 100644 --- a/opentracing-api/src/main/java/io/opentracing/tag/Tags.java +++ b/opentracing-api/src/main/java/io/opentracing/tag/Tags.java @@ -22,11 +22,12 @@ */ public final class Tags { - private Tags(){} + private Tags() { + } /** - * A constant for setting the span kind to indicate that it represents a server span. - */ + * A constant for setting the span kind to indicate that it represents a server span. + */ public static final String SPAN_KIND_SERVER = "server"; /** @@ -35,32 +36,32 @@ private Tags(){} public static final String SPAN_KIND_CLIENT = "client"; /** - * HTTP_URL records the url of the incoming request. + * HTTP_URL records the url of the incoming request. */ public static final StringTag HTTP_URL = new StringTag("http.url"); /** - * HTTP_STATUS records the http status code of the response. + * HTTP_STATUS records the http status code of the response. */ public static final IntTag HTTP_STATUS = new IntTag("http.status_code"); /** - * HTTP_METHOD records the http method. Case-insensitive. + * HTTP_METHOD records the http method. Case-insensitive. */ public static final StringTag HTTP_METHOD = new StringTag("http.method"); /** - * PEER_HOST_IPV4 records IPv4 host address of the peer. + * PEER_HOST_IPV4 records IPv4 host address of the peer. */ public static final IntTag PEER_HOST_IPV4 = new IntTag("peer.ipv4"); /** - * PEER_HOST_IPV6 records the IPv6 host address of the peer. + * PEER_HOST_IPV6 records the IPv6 host address of the peer. */ public static final StringTag PEER_HOST_IPV6 = new StringTag("peer.ipv6"); /** - * PEER_SERVICE records the service name of the peer. + * PEER_SERVICE records the service name of the peer. */ public static final StringTag PEER_SERVICE = new StringTag("peer.service"); @@ -70,27 +71,50 @@ private Tags(){} public static final StringTag PEER_HOSTNAME = new StringTag("peer.hostname"); /** - * PEER_PORT records the port number of the peer. + * PEER_PORT records the port number of the peer. */ public static final ShortTag PEER_PORT = new ShortTag("peer.port"); /** - * SAMPLING_PRIORITY determines the priority of sampling this Span. + * SAMPLING_PRIORITY determines the priority of sampling this Span. */ public static final ShortTag SAMPLING_PRIORITY = new ShortTag("sampling.priority"); /** - * SPAN_KIND hints at the relationship between spans, e.g. client/server. + * SPAN_KIND hints at the relationship between spans, e.g. client/server. */ public static final StringTag SPAN_KIND = new StringTag("span.kind"); /** - * COMPONENT is a low-cardinality identifier of the module, library, or package that is instrumented. + * COMPONENT is a low-cardinality identifier of the module, library, or package that is instrumented. */ - public static final StringTag COMPONENT = new StringTag("component"); + public static final StringTag COMPONENT = new StringTag("component"); /** * ERROR indicates whether a Span ended in an error state. */ public static final BooleanTag ERROR = new BooleanTag("error"); + + /** + * DB_TYPE indicates the type of Database. + * For any SQL database, "sql". For others, the lower-case database category, e.g. "cassandra", "hbase", or "redis" + */ + public static final StringTag DB_TYPE = new StringTag("db.type"); + + /** + * DB_INSTANCE indicates the instance name of Database. + * If the jdbc.url="jdbc:mysql://127.0.0.1:3306/customers", instance name is "customers". + */ + public static final StringTag DB_INSTANCE = new StringTag("db.instance"); + + /** + * DB_USER indicates the user name of Database, e.g. "readonly_user" or "reporting_user" + */ + public static final StringTag DB_USER = new StringTag("db.user"); + + /** + * DB_STATEMENT records a database statement for the given database type. + * For db.type="SQL", "SELECT * FROM wuser_table". For db.type="redis", "SET mykey "WuValue". + */ + public static final StringTag DB_STATEMENT = new StringTag("db.statement"); } From 16035b200062d636bfdee7d317b3758c22d5a192 Mon Sep 17 00:00:00 2001 From: Pavol Loffay Date: Sat, 4 Mar 2017 23:36:01 +0100 Subject: [PATCH 03/23] MockSpanBuilder return its type not Tracer.SpanBuilder (#101) --- .../java/io/opentracing/mock/MockTracer.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) 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..f41bebbd 100644 --- a/opentracing-mock/src/main/java/io/opentracing/mock/MockTracer.java +++ b/opentracing-mock/src/main/java/io/opentracing/mock/MockTracer.java @@ -145,7 +145,7 @@ public MockSpan.MockContext extract(Format format, C carrier) { } @Override - public Tracer.SpanBuilder buildSpan(String operationName) { + public SpanBuilder buildSpan(String operationName) { return new SpanBuilder(operationName); } @@ -164,7 +164,7 @@ synchronized void appendFinishedSpan(MockSpan mockSpan) { this.onSpanFinished(mockSpan); } - final class SpanBuilder implements Tracer.SpanBuilder { + public final class SpanBuilder implements Tracer.SpanBuilder { private final String operationName; private long startMicros; private MockSpan.MockContext firstParent; @@ -174,17 +174,17 @@ final class SpanBuilder implements Tracer.SpanBuilder { this.operationName = operationName; } @Override - public Tracer.SpanBuilder asChildOf(SpanContext parent) { + public SpanBuilder asChildOf(SpanContext parent) { return addReference(References.CHILD_OF, parent); } @Override - public Tracer.SpanBuilder asChildOf(Span parent) { + public SpanBuilder asChildOf(Span parent) { return addReference(References.CHILD_OF, parent.context()); } @Override - public Tracer.SpanBuilder addReference(String referenceType, SpanContext referencedContext) { + public SpanBuilder addReference(String referenceType, SpanContext referencedContext) { if (firstParent == null && ( referenceType.equals(References.CHILD_OF) || referenceType.equals(References.FOLLOWS_FROM))) { this.firstParent = (MockSpan.MockContext)referencedContext; @@ -193,31 +193,31 @@ public Tracer.SpanBuilder addReference(String referenceType, SpanContext referen } @Override - public Tracer.SpanBuilder withTag(String key, String value) { + public SpanBuilder withTag(String key, String value) { this.initialTags.put(key, value); return this; } @Override - public Tracer.SpanBuilder withTag(String key, boolean value) { + public SpanBuilder withTag(String key, boolean value) { this.initialTags.put(key, value); return this; } @Override - public Tracer.SpanBuilder withTag(String key, Number value) { + public SpanBuilder withTag(String key, Number value) { this.initialTags.put(key, value); return this; } @Override - public Tracer.SpanBuilder withStartTimestamp(long microseconds) { + public SpanBuilder withStartTimestamp(long microseconds) { this.startMicros = microseconds; return this; } @Override - public Span start() { + public MockSpan start() { if (this.startMicros == 0) { this.startMicros = MockSpan.nowMicros(); } From 000632149d7ccaf45470025221fa45bde63851e6 Mon Sep 17 00:00:00 2001 From: tedsuo Date: Thu, 16 Feb 2017 23:10:25 -0800 Subject: [PATCH 04/23] 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 f41bebbd..3ece4223 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 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 047c24c3..132c335b 100644 --- a/pom.xml +++ b/pom.xml @@ -238,7 +238,7 @@ true - + maven-release-plugin ${maven-release-plugin.version} From 715d52ddd09381fd420d7d562997c8328a2ad79d Mon Sep 17 00:00:00 2001 From: Ben Sigelman Date: Sun, 19 Feb 2017 17:38:17 +0100 Subject: [PATCH 05/23] 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 | 16 +++ .../java/io/opentracing/mock/MockTracer.java | 36 ++++-- .../io/opentracing/mock/MockTracerTest.java | 29 +++++ .../java/io/opentracing/NoopSpanSnapshot.java | 2 +- 13 files changed, 236 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 f747157f..defd930c 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 c432090d..a32418ae 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; private final List errors = new ArrayList<>(); @@ -106,11 +108,20 @@ public void finish() { @Override public synchronized void finish(long finishMicros) { finishedCheck("Finishing already finished span"); + + if (this.mockTracer.getActiveSpanManager() != null) { + this.mockTracer.getActiveSpanManager().deactivate(this.snapshot); + } this.finishMicros = finishMicros; this.mockTracer.appendFinishedSpan(this); this.finished = true; } + @Override + public synchronized boolean isFinished() { + return finished; + } + @Override public void close() { this.finish(); @@ -266,6 +277,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 3ece4223..53165eab 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 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 MockSpan 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 77b261899d12528666b5de0a7261c75f6f8478e3 Mon Sep 17 00:00:00 2001 From: Ben Sigelman Date: Mon, 20 Feb 2017 06:55:15 +0100 Subject: [PATCH 06/23] 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 53165eab..73b47de2 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 5c3b5634da9fe542958534861db44edd0649619f Mon Sep 17 00:00:00 2001 From: Ben Sigelman Date: Mon, 20 Feb 2017 07:17:27 +0100 Subject: [PATCH 07/23] Tweaks --- .../io/opentracing/ActiveSpanManager.java | 25 ++++----------- .../io/opentracing/MDCActiveSpanManager.java | 32 ------------------- .../java/io/opentracing/mock/MockSpan.java | 4 +-- .../java/io/opentracing/mock/MockTracer.java | 5 --- 4 files changed, 9 insertions(+), 57 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 a32418ae..563b6c7a 100644 --- a/opentracing-mock/src/main/java/io/opentracing/mock/MockSpan.java +++ b/opentracing-mock/src/main/java/io/opentracing/mock/MockSpan.java @@ -278,9 +278,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 73b47de2..86fa0221 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); From 5fcab6e9cc3e8ca52659abd08972b2991c03e5a2 Mon Sep 17 00:00:00 2001 From: Ben Sigelman Date: Mon, 20 Feb 2017 17:00:28 +0100 Subject: [PATCH 08/23] Add ref/deref --- .../src/main/java/io/opentracing/Span.java | 5 +++++ .../main/java/io/opentracing/impl/AbstractSpan.java | 13 +++++++++++++ .../src/main/java/io/opentracing/mock/MockSpan.java | 13 +++++++++++++ .../src/main/java/io/opentracing/NoopSpan.java | 6 ++++++ 4 files changed, 37 insertions(+) diff --git a/opentracing-api/src/main/java/io/opentracing/Span.java b/opentracing-api/src/main/java/io/opentracing/Span.java index 1394d28e..04c1691e 100644 --- a/opentracing-api/src/main/java/io/opentracing/Span.java +++ b/opentracing-api/src/main/java/io/opentracing/Span.java @@ -31,6 +31,11 @@ public interface Span extends Closeable { */ SpanContext context(); + // XXX: comment + void incRef(); + // if decRef moves refcount to 0, must call finish() + void decRef(); + /** * Sets the end timestamp to now and records the span. * 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 b3f08120..d99083da 100644 --- a/opentracing-impl/src/main/java/io/opentracing/impl/AbstractSpan.java +++ b/opentracing-impl/src/main/java/io/opentracing/impl/AbstractSpan.java @@ -23,6 +23,7 @@ import java.util.List; import java.util.Map; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicLong; abstract class AbstractSpan implements Span, SpanContext { @@ -34,6 +35,7 @@ abstract class AbstractSpan implements Span, SpanContext { private Duration duration; private final Map tags = new HashMap<>(); private final List logs = new ArrayList<>(); + private final AtomicLong refCount = new AtomicLong(0); // XXX AbstractSpan(String operationName ) { this(operationName, Instant.now()); @@ -44,6 +46,17 @@ abstract class AbstractSpan implements Span, SpanContext { this.start = start; } + @Override + public void incRef() { + refCount.incrementAndGet(); + } + @Override + public void decRef() { + if (refCount.decrementAndGet() == 0) { + this.finish(); + } + } + @Override public final SpanContext context() { return this; 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 563b6c7a..507bc2fb 100644 --- a/opentracing-mock/src/main/java/io/opentracing/mock/MockSpan.java +++ b/opentracing-mock/src/main/java/io/opentracing/mock/MockSpan.java @@ -40,6 +40,7 @@ public final class MockSpan implements Span { private final List logEntries = new ArrayList<>(); private String operationName; private ActiveSpanManager.Snapshot snapshot; + private static AtomicLong refCount = new AtomicLong(0); private final List errors = new ArrayList<>(); @@ -100,6 +101,18 @@ public synchronized MockContext context() { return this.context; } + @Override + public void incRef() { + refCount.incrementAndGet(); + } + + @Override + public void decRef() { + if (refCount.decrementAndGet() == 0) { + this.finish(); + }; + } + @Override public void finish() { this.finish(nowMicros()); diff --git a/opentracing-noop/src/main/java/io/opentracing/NoopSpan.java b/opentracing-noop/src/main/java/io/opentracing/NoopSpan.java index 5fe3804d..35c28451 100644 --- a/opentracing-noop/src/main/java/io/opentracing/NoopSpan.java +++ b/opentracing-noop/src/main/java/io/opentracing/NoopSpan.java @@ -24,6 +24,12 @@ final class NoopSpanImpl implements NoopSpan { @Override public SpanContext context() { return NoopSpanContextImpl.INSTANCE; } + @Override + public void incRef() { } + + @Override + public void decRef() { } + @Override public void finish() {} From 35471fa761cbd5a6505acb943bdafe0d51200ceb Mon Sep 17 00:00:00 2001 From: Ben Sigelman Date: Wed, 22 Feb 2017 03:53:33 -0800 Subject: [PATCH 09/23] Rough spike that works with incRef/decRef --- .../io/opentracing/MDCActiveSpanManager.java | 13 +-- .../concurrent/TracedCallable.java | 1 + .../concurrent/TracedRunnable.java | 1 + opentracing-mock/pom.xml | 5 ++ .../java/io/opentracing/mock/MockSpan.java | 10 +-- .../java/io/opentracing/mock/MockTracer.java | 3 + .../io/opentracing/mock/MockTracerTest.java | 83 +++++++++++++++++-- 7 files changed, 98 insertions(+), 18 deletions(-) diff --git a/opentracing-api/src/main/java/io/opentracing/MDCActiveSpanManager.java b/opentracing-api/src/main/java/io/opentracing/MDCActiveSpanManager.java index 03819eda..313ac04e 100644 --- a/opentracing-api/src/main/java/io/opentracing/MDCActiveSpanManager.java +++ b/opentracing-api/src/main/java/io/opentracing/MDCActiveSpanManager.java @@ -34,6 +34,11 @@ MDCSnapshot toRestore() { } } + @Override + public MDCSnapshot snapshot(Span span) { + return new MDCSnapshot(span); + } + @Override public Span active() { MDCSnapshot snapshot = tlsSnapshot.get(); @@ -43,11 +48,6 @@ public Span active() { return snapshot.span(); } - @Override - public MDCSnapshot snapshot(Span span) { - return new MDCSnapshot(span); - } - @Override public Span activate(Snapshot snapshot) { if (!(snapshot instanceof MDCSnapshot)) { @@ -63,6 +63,9 @@ public void deactivate(Snapshot snapshot) { if (!(snapshot instanceof MDCSnapshot)) { throw new IllegalArgumentException("deactivate() expected MDCSnapshot"); } + if (snapshot.span() != null) { + snapshot.span().decRef(); + } if (tlsSnapshot.get() != snapshot) { // do nothing 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 e2a5fdd5..5a2e06b2 100644 --- a/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedCallable.java +++ b/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedCallable.java @@ -25,6 +25,7 @@ public TracedCallable(Callable callable, Span span, ActiveSpanManager manager this.callable= callable; this.manager = manager; this.snapshot = manager.snapshot(span); + span.incRef(); } public T call() throws Exception { 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 068e8626..cbd16d7e 100644 --- a/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedRunnable.java +++ b/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedRunnable.java @@ -28,6 +28,7 @@ public TracedRunnable(Runnable runnable, Span span, ActiveSpanManager manager) { this.runnable = runnable; this.manager = manager; this.snapshot = manager.snapshot(span); + span.incRef(); } @Override diff --git a/opentracing-mock/pom.xml b/opentracing-mock/pom.xml index 7c3dbffa..86964e41 100644 --- a/opentracing-mock/pom.xml +++ b/opentracing-mock/pom.xml @@ -36,6 +36,11 @@ ${project.groupId} opentracing-api + + io.opentracing + opentracing-impl + test + 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 507bc2fb..4c99cb06 100644 --- a/opentracing-mock/src/main/java/io/opentracing/mock/MockSpan.java +++ b/opentracing-mock/src/main/java/io/opentracing/mock/MockSpan.java @@ -39,8 +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; - private static AtomicLong refCount = new AtomicLong(0); + private final AtomicLong refCount = new AtomicLong(0); private final List errors = new ArrayList<>(); @@ -103,11 +102,13 @@ public synchronized MockContext context() { @Override public void incRef() { + System.out.println("incRef: " + refCount.get() + " :: " + this.operationName); refCount.incrementAndGet(); } @Override public void decRef() { + System.out.println("decRef: " + refCount.get() + " :: " + this.operationName); if (refCount.decrementAndGet() == 0) { this.finish(); }; @@ -290,11 +291,6 @@ public long timestampMicros() { this.context = new MockContext(parent.traceId, nextId(), parent.baggage); this.parentId = parent.spanId; } - - if (tracer.activeSpanManager() != null) { - // XXX weird/awkward - this.snapshot = tracer.activeSpanManager().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 86fa0221..78fc9034 100644 --- a/opentracing-mock/src/main/java/io/opentracing/mock/MockTracer.java +++ b/opentracing-mock/src/main/java/io/opentracing/mock/MockTracer.java @@ -237,10 +237,13 @@ public MockSpan start() { this.startMicros = MockSpan.nowMicros(); } 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)); } + */ + rval.incRef(); return rval; } 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 e956306a..6bd5f7b1 100644 --- a/opentracing-mock/src/test/java/io/opentracing/mock/MockTracerTest.java +++ b/opentracing-mock/src/test/java/io/opentracing/mock/MockTracerTest.java @@ -16,17 +16,19 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotEquals; +import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.*; -import io.opentracing.MDCActiveSpanManager; +import io.opentracing.*; +import io.opentracing.concurrent.TracedCallable; +import io.opentracing.concurrent.TracedExecutorService; +import io.opentracing.concurrent.TracedRunnable; import org.junit.Assert; import org.junit.Test; -import io.opentracing.Span; -import io.opentracing.Tracer; -import io.opentracing.SpanContext; import io.opentracing.propagation.Format; import io.opentracing.propagation.TextMapExtractAdapter; import io.opentracing.propagation.TextMapInjectAdapter; @@ -38,16 +40,17 @@ public class MockTracerTest { @Test public void testMDCHack() { org.apache.log4j.BasicConfigurator.configure(); - Logger logger = org.slf4j.LoggerFactory.getLogger("hack"); + final 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(); + final MockTracer tracer = new MockTracer(); tracer.setActiveSpanManager(new MDCActiveSpanManager()); + /* Span par = tracer.buildSpan("parent").start(); Span childA = tracer.buildSpan("childA").start(); childA.finish(); @@ -58,6 +61,74 @@ public void testMDCHack() { for (MockSpan span : finishedSpans) { logger.info("finished Span. {} :: {} ({})", span.context().traceId(), span.context().spanId(), span.parentId()); } + tracer.reset(); + */ + + ExecutorService realExecutor = Executors.newFixedThreadPool(500); + final ExecutorService otExecutor = new TracedExecutorService(realExecutor, tracer.activeSpanManager()); + Span parent = tracer.buildSpan("parent").start(); + tracer.activeSpanManager().activate(tracer.activeSpanManager().snapshot(parent)); + parent.incRef(); + final List> futures = new ArrayList<>(); + final List> subfutures = new ArrayList<>(); + for (int i = 0; i < 10; i++) { + final int j = i; + futures.add(otExecutor.submit(new Runnable() { + @Override + public void run() { + final Span child = tracer.buildSpan("child_" + j).start(); + ActiveSpanManager.Snapshot childSnapshot = tracer.activeSpanManager().snapshot(child); + tracer.activeSpanManager().activate(childSnapshot); + try { + Thread.currentThread().sleep(1000); + } catch (InterruptedException e) { + e.printStackTrace(); + } + child.log("awoke"); + logger.info("defining"); + Runnable r = new Runnable() { + @Override + public void run() { + Span active = tracer.activeSpanManager().active(); + active.log("awoke again"); + Span grandchild = tracer.buildSpan("grandchild").start(); + grandchild.finish(); + } + }; + logger.info("submitting"); + subfutures.add(otExecutor.submit(r)); + logger.info("deactivating"); + tracer.activeSpanManager().deactivate(childSnapshot); + } + })); + } + try { + for (Future f : futures) { + f.get(); + } + for (Future f : subfutures) { + f.get(); + } + } catch (InterruptedException e) { + e.printStackTrace(); + } catch (ExecutionException e) { + e.printStackTrace(); + } + + otExecutor.shutdown(); + try { + otExecutor.awaitTermination(3, TimeUnit.SECONDS); + } catch (InterruptedException e) { + e.printStackTrace(); + } + parent.finish(); + + List finishedSpans = tracer.finishedSpans(); + + logger.info("DONE SLEEPING"); + for (MockSpan span : finishedSpans) { + logger.info("finished Span '{}'. trace={}, span={}, parent={}", span.operationName(), span.context().traceId(), span.context().spanId(), span.parentId()); + } } @Test From bec490f8ad0ce3608c84312620eba592fa96ac5d Mon Sep 17 00:00:00 2001 From: Ben Sigelman Date: Wed, 22 Feb 2017 07:56:05 -0800 Subject: [PATCH 10/23] Make Snapshot more of a thing --- .../io/opentracing/ActiveSpanManager.java | 12 +--- .../io/opentracing/MDCActiveSpanManager.java | 62 +++++++------------ .../concurrent/TracedCallable.java | 4 +- .../concurrent/TracedRunnable.java | 4 +- .../io/opentracing/mock/MockTracerTest.java | 9 +-- 5 files changed, 33 insertions(+), 58 deletions(-) diff --git a/opentracing-api/src/main/java/io/opentracing/ActiveSpanManager.java b/opentracing-api/src/main/java/io/opentracing/ActiveSpanManager.java index a351be36..1731fd1f 100644 --- a/opentracing-api/src/main/java/io/opentracing/ActiveSpanManager.java +++ b/opentracing-api/src/main/java/io/opentracing/ActiveSpanManager.java @@ -3,7 +3,8 @@ public interface ActiveSpanManager { // Basically a marker interface public interface Snapshot { - Span span(); + Span activate(); + void deactivate(); } // Get the currently active Span (perhaps for this thread, etc) @@ -11,13 +12,4 @@ public interface Snapshot { // Create a Snapshot encapsulating both the given Span and any state needed to activate/deactivate (see below) Snapshot snapshot(Span span); - - // Make the Snapshot and the Span it contains "active" per active(). - // - // *Must* be paired with a subsequent call to deactivate(). - Span activate(Snapshot snapshot); - - // 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 313ac04e..5352a0e0 100644 --- a/opentracing-api/src/main/java/io/opentracing/MDCActiveSpanManager.java +++ b/opentracing-api/src/main/java/io/opentracing/MDCActiveSpanManager.java @@ -21,16 +21,31 @@ class MDCSnapshot implements ActiveSpanManager.Snapshot { this.span = span; } - public Span span() { + @Override + public Span activate() { + toRestore = tlsSnapshot.get(); + tlsSnapshot.set(this); return span; } - void setToRestore(MDCSnapshot toRestore) { - this.toRestore = toRestore; - } + @Override + public void deactivate() { + if (span != null) { + span.decRef(); + } - MDCSnapshot toRestore() { - return toRestore; + if (tlsSnapshot.get() != this) { + // do nothing + return; + } + MDCSnapshot nextActiveSnapshot = toRestore; + while (nextActiveSnapshot != null) { + if (!nextActiveSnapshot.span.isFinished()) { + break; + } + nextActiveSnapshot = nextActiveSnapshot.toRestore; + } + tlsSnapshot.set(nextActiveSnapshot); } } @@ -45,39 +60,6 @@ public Span active() { if (snapshot == null) { return null; } - return snapshot.span(); - } - - @Override - 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 - public void deactivate(Snapshot snapshot) { - if (!(snapshot instanceof MDCSnapshot)) { - throw new IllegalArgumentException("deactivate() expected MDCSnapshot"); - } - if (snapshot.span() != null) { - snapshot.span().decRef(); - } - - 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); + return snapshot.span; } } 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 5a2e06b2..298654a9 100644 --- a/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedCallable.java +++ b/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedCallable.java @@ -29,11 +29,11 @@ public TracedCallable(Callable callable, Span span, ActiveSpanManager manager } public T call() throws Exception { - final Span span = manager.activate(snapshot); + final Span span = snapshot.activate(); try { return callable.call(); } finally { - manager.deactivate(snapshot); + snapshot.deactivate(); } } } 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 cbd16d7e..8e984aac 100644 --- a/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedRunnable.java +++ b/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedRunnable.java @@ -33,11 +33,11 @@ public TracedRunnable(Runnable runnable, Span span, ActiveSpanManager manager) { @Override public void run() { - final Span span = manager.activate(this.snapshot); + final Span span = this.snapshot.activate(); try { runnable.run(); } finally { - manager.deactivate(this.snapshot); + this.snapshot.deactivate(); } } } 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 6bd5f7b1..4efe1830 100644 --- a/opentracing-mock/src/test/java/io/opentracing/mock/MockTracerTest.java +++ b/opentracing-mock/src/test/java/io/opentracing/mock/MockTracerTest.java @@ -67,7 +67,8 @@ public void testMDCHack() { ExecutorService realExecutor = Executors.newFixedThreadPool(500); final ExecutorService otExecutor = new TracedExecutorService(realExecutor, tracer.activeSpanManager()); Span parent = tracer.buildSpan("parent").start(); - tracer.activeSpanManager().activate(tracer.activeSpanManager().snapshot(parent)); + ActiveSpanManager.Snapshot parentSnapshot = tracer.activeSpanManager().snapshot(parent); + parentSnapshot.activate(); parent.incRef(); final List> futures = new ArrayList<>(); final List> subfutures = new ArrayList<>(); @@ -78,7 +79,7 @@ public void testMDCHack() { public void run() { final Span child = tracer.buildSpan("child_" + j).start(); ActiveSpanManager.Snapshot childSnapshot = tracer.activeSpanManager().snapshot(child); - tracer.activeSpanManager().activate(childSnapshot); + childSnapshot.activate(); try { Thread.currentThread().sleep(1000); } catch (InterruptedException e) { @@ -98,7 +99,7 @@ public void run() { logger.info("submitting"); subfutures.add(otExecutor.submit(r)); logger.info("deactivating"); - tracer.activeSpanManager().deactivate(childSnapshot); + childSnapshot.deactivate(); } })); } @@ -121,7 +122,7 @@ public void run() { } catch (InterruptedException e) { e.printStackTrace(); } - parent.finish(); + parentSnapshot.deactivate(); List finishedSpans = tracer.finishedSpans(); From 401f9b77df448b3c47f6f3eb66f32b023f693421 Mon Sep 17 00:00:00 2001 From: Ben Sigelman Date: Sun, 26 Feb 2017 16:24:43 -0800 Subject: [PATCH 11/23] Build something arount incRef/decRef Checkpointing here, as the next idea is to change the return type for SpanBuilder.start() (to be a SpanClosure rather than a Span). --- .../io/opentracing/ActiveSpanManager.java | 15 ----- ...veSpanManager.java => MDCSpanManager.java} | 11 +-- .../main/java/io/opentracing/SpanManager.java | 67 +++++++++++++++++++ .../src/main/java/io/opentracing/Tracer.java | 7 +- .../concurrent/TracedCallable.java | 17 +++-- .../concurrent/TracedExecutorService.java | 10 ++- .../concurrent/TracedRunnable.java | 17 +++-- .../io/opentracing/impl/AbstractTracer.java | 10 ++- .../io/opentracing/impl/GlobalTracer.java | 7 +- .../java/io/opentracing/impl/NoopTracer.java | 4 +- .../java/io/opentracing/mock/MockSpan.java | 1 - .../java/io/opentracing/mock/MockTracer.java | 21 +++--- .../io/opentracing/mock/MockTracerTest.java | 16 ++--- .../main/java/io/opentracing/NoopTracer.java | 5 +- 14 files changed, 126 insertions(+), 82 deletions(-) delete mode 100644 opentracing-api/src/main/java/io/opentracing/ActiveSpanManager.java rename opentracing-api/src/main/java/io/opentracing/{MDCActiveSpanManager.java => MDCSpanManager.java} (85%) create mode 100644 opentracing-api/src/main/java/io/opentracing/SpanManager.java diff --git a/opentracing-api/src/main/java/io/opentracing/ActiveSpanManager.java b/opentracing-api/src/main/java/io/opentracing/ActiveSpanManager.java deleted file mode 100644 index 1731fd1f..00000000 --- a/opentracing-api/src/main/java/io/opentracing/ActiveSpanManager.java +++ /dev/null @@ -1,15 +0,0 @@ -package io.opentracing; - -public interface ActiveSpanManager { - // Basically a marker interface - public interface Snapshot { - Span activate(); - void deactivate(); - } - - // Get the currently active Span (perhaps for this thread, etc) - Span active(); - - // Create a Snapshot encapsulating both the given Span and any state needed to activate/deactivate (see below) - Snapshot snapshot(Span span); -} diff --git a/opentracing-api/src/main/java/io/opentracing/MDCActiveSpanManager.java b/opentracing-api/src/main/java/io/opentracing/MDCSpanManager.java similarity index 85% rename from opentracing-api/src/main/java/io/opentracing/MDCActiveSpanManager.java rename to opentracing-api/src/main/java/io/opentracing/MDCSpanManager.java index 5352a0e0..cde67e5e 100644 --- a/opentracing-api/src/main/java/io/opentracing/MDCActiveSpanManager.java +++ b/opentracing-api/src/main/java/io/opentracing/MDCSpanManager.java @@ -1,17 +1,16 @@ 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 { +public class MDCSpanManager implements SpanManager { private final ThreadLocal tlsSnapshot = new ThreadLocal(); - class MDCSnapshot implements ActiveSpanManager.Snapshot { + class MDCSnapshot implements SpanClosure { private final Map mdcContext; private final Span span; private MDCSnapshot toRestore = null; @@ -50,7 +49,11 @@ public void deactivate() { } @Override - public MDCSnapshot snapshot(Span span) { + public MDCSnapshot captureActive() { + return new MDCSnapshot(active()); + } + @Override + public MDCSnapshot captureWithSpan(Span span) { return new MDCSnapshot(span); } diff --git a/opentracing-api/src/main/java/io/opentracing/SpanManager.java b/opentracing-api/src/main/java/io/opentracing/SpanManager.java new file mode 100644 index 00000000..b3250ef6 --- /dev/null +++ b/opentracing-api/src/main/java/io/opentracing/SpanManager.java @@ -0,0 +1,67 @@ +package io.opentracing; + +public interface SpanManager { + + /** + * A SpanClosure can be used *once* to make a Span active within a SpanManager, then deactivate it once the + * "closure" (or period of Span activity) has finished. + * + * Most users do not directly interact with SpanClosure, activate(), or deactivate(), but rather use + * SpanManager-aware Runnables/Callables/Executors. + * + * @see Span#incRef() + * @see Span#decRef() + */ + interface SpanClosure { + + /** + * Make the Span encapsulated by this SpanClosure active and return it. + * + * NOTE: activate() should not affect a Span's reference count (though deactivate() and + * SpanManager.captureActive() do). + * + * @see SpanManager#captureActive() + * @return the newly-activated Span + */ + Span activate(); + + /** + * End this active period for the Span previously returned by activate() and decrement its reference count. + * + * @see Span#decRef() + */ + void deactivate(); + } + + /** + * @return the currently active Span for this SpanManager, or null if no such Span could be found + */ + Span active(); + + /** + * Capture the active() Span (even if null) via captureWithSpan(). + * + * @see SpanManager#active() + * @see SpanManager#captureWithSpan(Span) + */ + SpanClosure captureActive(); + /** + * Capture any SpanManager-specific context (e.g., MDC context) along with the given Span (even if null) and + * encapsulate it in a SpanClosure for activation in the future, perhaps in a different thread or on a different + * executor. + * + * If the provided Span is not null, the implementation must increment the reference count on the encapsulated + * (active) Span. + * + * If the active Span is null, the implementation must still return a valid SpanClosure; when the closure activates, + * it will clear any active Span. + * + * @see SpanManager.SpanClosure + * @see Span#incRef() + * + * @param span the Span to associate with any SpanManager-specific active context + * @return a SpanClosure that represents the active Span and any other SpanManager-specific context, even if the + * active Span is null. + */ + SpanClosure captureWithSpan(Span span); +} diff --git a/opentracing-api/src/main/java/io/opentracing/Tracer.java b/opentracing-api/src/main/java/io/opentracing/Tracer.java index e26d6347..82842f22 100644 --- a/opentracing-api/src/main/java/io/opentracing/Tracer.java +++ b/opentracing-api/src/main/java/io/opentracing/Tracer.java @@ -41,8 +41,8 @@ public interface Tracer { */ SpanBuilder buildSpan(String operationName); - void setActiveSpanManager(ActiveSpanManager mgr); - ActiveSpanManager activeSpanManager(); + void setSpanManager(SpanManager mgr); + SpanManager activeSpanManager(); /** * Inject a SpanContext into a `carrier` of a given type, presumably for propagation across process boundaries. @@ -92,7 +92,7 @@ public interface Tracer { SpanContext extract(Format format, C carrier); // 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. + // to the SpanManager's active Span's SpanContext. interface SpanBuilder extends SpanContext { /** @@ -130,6 +130,7 @@ interface SpanBuilder extends SpanContext { SpanBuilder withStartTimestamp(long microseconds); /** Returns the started Span. */ + // XXX: should this return a SpanClosure? Span start(); } 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 298654a9..c7f265cc 100644 --- a/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedCallable.java +++ b/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedCallable.java @@ -1,39 +1,38 @@ package io.opentracing.concurrent; -import io.opentracing.ActiveSpanManager; +import io.opentracing.SpanManager; import io.opentracing.Span; -import io.opentracing.Tracer; import io.opentracing.impl.GlobalTracer; import java.util.concurrent.Callable; public class TracedCallable implements Callable { - private ActiveSpanManager.Snapshot snapshot; - private ActiveSpanManager manager; + private SpanManager.SpanClosure spanClosure; + private SpanManager manager; private Callable callable; public TracedCallable(Callable callable) { this(callable, GlobalTracer.get().activeSpanManager()); } - public TracedCallable(Callable callable, ActiveSpanManager manager) { + public TracedCallable(Callable callable, SpanManager manager) { this(callable, manager.active(), manager); } - public TracedCallable(Callable callable, Span span, ActiveSpanManager manager) { + public TracedCallable(Callable callable, Span span, SpanManager manager) { if (callable == null) throw new NullPointerException("Callable is ."); this.callable= callable; this.manager = manager; - this.snapshot = manager.snapshot(span); + this.spanClosure = manager.captureActive(); span.incRef(); } public T call() throws Exception { - final Span span = snapshot.activate(); + final Span span = spanClosure.activate(); try { return callable.call(); } finally { - snapshot.deactivate(); + spanClosure.deactivate(); } } } 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 726f763a..d6500114 100644 --- a/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedExecutorService.java +++ b/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedExecutorService.java @@ -1,8 +1,6 @@ package io.opentracing.concurrent; -import io.opentracing.ActiveSpanManager; -import io.opentracing.Span; -import io.opentracing.Tracer; +import io.opentracing.SpanManager; import io.opentracing.impl.GlobalTracer; import java.util.ArrayList; @@ -12,15 +10,15 @@ public class TracedExecutorService implements ExecutorService { private ExecutorService executor; - private ActiveSpanManager manager; + private SpanManager manager; public TracedExecutorService(ExecutorService executor){ this(executor, GlobalTracer.get().activeSpanManager()); } - public TracedExecutorService(ExecutorService executor, ActiveSpanManager manager) { + public TracedExecutorService(ExecutorService executor, SpanManager manager) { if (executor == null) throw new NullPointerException("Executor is ."); - if (manager == null) throw new NullPointerException("ActiveSpanManager is ."); + if (manager == null) throw new NullPointerException("SpanManager is ."); this.executor = executor; this.manager = manager; } 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 8e984aac..ebea077e 100644 --- a/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedRunnable.java +++ b/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedRunnable.java @@ -1,15 +1,14 @@ package io.opentracing.concurrent; -import io.opentracing.ActiveSpanManager; +import io.opentracing.SpanManager; import io.opentracing.Span; -import io.opentracing.Tracer; import io.opentracing.impl.GlobalTracer; public class TracedRunnable implements Runnable { private Runnable runnable; - private ActiveSpanManager manager; - private ActiveSpanManager.Snapshot snapshot; + private SpanManager manager; + private SpanManager.SpanClosure spanClosure; public TracedRunnable(Runnable runnable) { this(runnable, GlobalTracer.get().activeSpanManager()); @@ -19,25 +18,25 @@ public TracedRunnable(Runnable runnable, Span span) { this(runnable, span, GlobalTracer.get().activeSpanManager()); } - public TracedRunnable(Runnable runnable, ActiveSpanManager manager) { + public TracedRunnable(Runnable runnable, SpanManager manager) { this(runnable, manager.active(), manager); } - public TracedRunnable(Runnable runnable, Span span, ActiveSpanManager manager) { + public TracedRunnable(Runnable runnable, Span span, SpanManager manager) { if (runnable == null) throw new NullPointerException("Runnable is ."); this.runnable = runnable; this.manager = manager; - this.snapshot = manager.snapshot(span); + this.spanClosure = manager.captureActive(); span.incRef(); } @Override public void run() { - final Span span = this.snapshot.activate(); + final Span span = this.spanClosure.activate(); try { runnable.run(); } finally { - this.snapshot.deactivate(); + this.spanClosure.deactivate(); } } } 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 a3dca44e..52f7f79d 100644 --- a/opentracing-impl/src/main/java/io/opentracing/impl/AbstractTracer.java +++ b/opentracing-impl/src/main/java/io/opentracing/impl/AbstractTracer.java @@ -13,8 +13,7 @@ */ package io.opentracing.impl; -import io.opentracing.ActiveSpanManager; -import io.opentracing.Span; +import io.opentracing.SpanManager; import io.opentracing.SpanContext; import io.opentracing.Tracer; import io.opentracing.propagation.Extractor; @@ -30,7 +29,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; + private SpanManager manager; protected AbstractTracer() { registry.register(Format.Builtin.TEXT_MAP, new TextMapInjectorImpl(this)); @@ -39,13 +38,12 @@ protected AbstractTracer() { abstract AbstractSpanBuilder createSpanBuilder(String operationName); - @Override - public void setActiveSpanManager(ActiveSpanManager manager) { + public void setSpanManager(SpanManager manager) { this.manager = manager; } @Override - public ActiveSpanManager activeSpanManager() { + public SpanManager activeSpanManager() { return this.manager; } 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 6d7ac692..718e0df3 100644 --- a/opentracing-impl/src/main/java/io/opentracing/impl/GlobalTracer.java +++ b/opentracing-impl/src/main/java/io/opentracing/impl/GlobalTracer.java @@ -116,13 +116,12 @@ public SpanBuilder buildSpan(String operationName) { return lazyTracer().buildSpan(operationName); } - @Override - public void setActiveSpanManager(ActiveSpanManager mgr) { - lazyTracer().setActiveSpanManager(mgr); + public void setSpanManager(SpanManager mgr) { + lazyTracer().setSpanManager(mgr); } @Override - public ActiveSpanManager activeSpanManager() { + public SpanManager activeSpanManager() { return lazyTracer().activeSpanManager(); } 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 859bdf24..3bae3ac0 100644 --- a/opentracing-impl/src/main/java/io/opentracing/impl/NoopTracer.java +++ b/opentracing-impl/src/main/java/io/opentracing/impl/NoopTracer.java @@ -23,10 +23,10 @@ final class NoopTracer extends AbstractTracer implements io.opentracing.NoopTrac private static final NoopTracer INSTANCE = new NoopTracer(); @Override - public void setActiveSpanManager(ActiveSpanManager mgr) {} + public void setSpanManager(SpanManager mgr) {} @Override - public ActiveSpanManager activeSpanManager() { return null; } + public SpanManager activeSpanManager() { return null; } @Override public void inject(SpanContext spanContext, Format format, C carrier) {} 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 4c99cb06..c638f1ec 100644 --- a/opentracing-mock/src/main/java/io/opentracing/mock/MockSpan.java +++ b/opentracing-mock/src/main/java/io/opentracing/mock/MockSpan.java @@ -16,7 +16,6 @@ import java.util.*; import java.util.concurrent.atomic.AtomicLong; -import io.opentracing.ActiveSpanManager; import io.opentracing.Span; import io.opentracing.SpanContext; 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 78fc9034..a5be2515 100644 --- a/opentracing-mock/src/main/java/io/opentracing/mock/MockTracer.java +++ b/opentracing-mock/src/main/java/io/opentracing/mock/MockTracer.java @@ -35,7 +35,7 @@ public class MockTracer implements Tracer { private List finishedSpans = new ArrayList<>(); private final Propagator propagator; - private ActiveSpanManager activeSpanManager; + private SpanManager spanManager; public MockTracer() { this(Propagator.PRINTER); @@ -145,8 +145,8 @@ public MockSpan.MockContext extract(Format format, C carrier) { @Override public SpanBuilder buildSpan(String operationName) { SpanBuilder sb = new SpanBuilder(operationName); - if (this.activeSpanManager != null) { - Span active = this.activeSpanManager.active(); + if (this.spanManager != null) { + Span active = this.spanManager.active(); if (active != null) { sb.asChildOf(active.context()); } @@ -154,14 +154,13 @@ public SpanBuilder buildSpan(String operationName) { return sb; } - @Override - public void setActiveSpanManager(ActiveSpanManager mgr) { - this.activeSpanManager = mgr; + public void setSpanManager(SpanManager mgr) { + this.spanManager = mgr; } @Override - public ActiveSpanManager activeSpanManager() { - return activeSpanManager; + public SpanManager activeSpanManager() { + return spanManager; } @Override @@ -238,9 +237,9 @@ public MockSpan start() { } 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)); + if (MockTracer.this.spanManager != null) { + MockTracer.this.spanManager.activate( + MockTracer.this.spanManager.captureActive(rval)); } */ rval.incRef(); 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 4efe1830..9fecb7b1 100644 --- a/opentracing-mock/src/test/java/io/opentracing/mock/MockTracerTest.java +++ b/opentracing-mock/src/test/java/io/opentracing/mock/MockTracerTest.java @@ -23,9 +23,7 @@ import java.util.concurrent.*; import io.opentracing.*; -import io.opentracing.concurrent.TracedCallable; import io.opentracing.concurrent.TracedExecutorService; -import io.opentracing.concurrent.TracedRunnable; import org.junit.Assert; import org.junit.Test; @@ -48,7 +46,7 @@ public void testMDCHack() { logger.info("testing: {}", MDC.getCopyOfContextMap().toString()); final MockTracer tracer = new MockTracer(); - tracer.setActiveSpanManager(new MDCActiveSpanManager()); + tracer.setSpanManager(new MDCSpanManager()); /* Span par = tracer.buildSpan("parent").start(); @@ -67,8 +65,8 @@ public void testMDCHack() { ExecutorService realExecutor = Executors.newFixedThreadPool(500); final ExecutorService otExecutor = new TracedExecutorService(realExecutor, tracer.activeSpanManager()); Span parent = tracer.buildSpan("parent").start(); - ActiveSpanManager.Snapshot parentSnapshot = tracer.activeSpanManager().snapshot(parent); - parentSnapshot.activate(); + SpanManager.SpanClosure parentSpanClosure = tracer.activeSpanManager().captureWithSpan(parent); + parentSpanClosure.activate(); parent.incRef(); final List> futures = new ArrayList<>(); final List> subfutures = new ArrayList<>(); @@ -78,8 +76,8 @@ public void testMDCHack() { @Override public void run() { final Span child = tracer.buildSpan("child_" + j).start(); - ActiveSpanManager.Snapshot childSnapshot = tracer.activeSpanManager().snapshot(child); - childSnapshot.activate(); + SpanManager.SpanClosure childSpanClosure = tracer.activeSpanManager().captureWithSpan(child); + childSpanClosure.activate(); try { Thread.currentThread().sleep(1000); } catch (InterruptedException e) { @@ -99,7 +97,7 @@ public void run() { logger.info("submitting"); subfutures.add(otExecutor.submit(r)); logger.info("deactivating"); - childSnapshot.deactivate(); + childSpanClosure.deactivate(); } })); } @@ -122,7 +120,7 @@ public void run() { } catch (InterruptedException e) { e.printStackTrace(); } - parentSnapshot.deactivate(); + parentSpanClosure.deactivate(); List finishedSpans = tracer.finishedSpans(); diff --git a/opentracing-noop/src/main/java/io/opentracing/NoopTracer.java b/opentracing-noop/src/main/java/io/opentracing/NoopTracer.java index dbe02a61..11a9aec6 100644 --- a/opentracing-noop/src/main/java/io/opentracing/NoopTracer.java +++ b/opentracing-noop/src/main/java/io/opentracing/NoopTracer.java @@ -24,11 +24,10 @@ final class NoopTracerImpl implements NoopTracer { @Override public SpanBuilder buildSpan(String operationName) { return NoopSpanBuilderImpl.INSTANCE; } - @Override - public void setActiveSpanManager(ActiveSpanManager mgr) {} + public void setSpanManager(SpanManager mgr) {} @Override - public ActiveSpanManager activeSpanManager() { return null; } + public SpanManager activeSpanManager() { return null; } @Override public void inject(SpanContext spanContext, Format format, C carrier) {} From 8fdac03e8afa3ebab86bfd7e868ffde922450c14 Mon Sep 17 00:00:00 2001 From: Ben Sigelman Date: Sun, 5 Mar 2017 15:51:00 -0800 Subject: [PATCH 12/23] Remove refcounting from core API --- .../main/java/io/opentracing/MDCSpanManager.java | 6 +++--- .../src/main/java/io/opentracing/Span.java | 5 ----- .../main/java/io/opentracing/SpanManager.java | 16 ++-------------- .../opentracing/concurrent/TracedCallable.java | 5 ++--- .../opentracing/concurrent/TracedRunnable.java | 3 +-- .../java/io/opentracing/impl/AbstractSpan.java | 11 ----------- .../main/java/io/opentracing/mock/MockSpan.java | 14 -------------- .../java/io/opentracing/mock/MockTracer.java | 1 - .../java/io/opentracing/mock/MockTracerTest.java | 5 ++--- .../src/main/java/io/opentracing/NoopSpan.java | 6 ------ 10 files changed, 10 insertions(+), 62 deletions(-) diff --git a/opentracing-api/src/main/java/io/opentracing/MDCSpanManager.java b/opentracing-api/src/main/java/io/opentracing/MDCSpanManager.java index cde67e5e..37831019 100644 --- a/opentracing-api/src/main/java/io/opentracing/MDCSpanManager.java +++ b/opentracing-api/src/main/java/io/opentracing/MDCSpanManager.java @@ -28,9 +28,9 @@ public Span activate() { } @Override - public void deactivate() { - if (span != null) { - span.decRef(); + public void deactivate(boolean finishSpan) { + if (span != null && finishSpan) { + span.finish(); } if (tlsSnapshot.get() != this) { diff --git a/opentracing-api/src/main/java/io/opentracing/Span.java b/opentracing-api/src/main/java/io/opentracing/Span.java index 04c1691e..1394d28e 100644 --- a/opentracing-api/src/main/java/io/opentracing/Span.java +++ b/opentracing-api/src/main/java/io/opentracing/Span.java @@ -31,11 +31,6 @@ public interface Span extends Closeable { */ SpanContext context(); - // XXX: comment - void incRef(); - // if decRef moves refcount to 0, must call finish() - void decRef(); - /** * Sets the end timestamp to now and records the span. * diff --git a/opentracing-api/src/main/java/io/opentracing/SpanManager.java b/opentracing-api/src/main/java/io/opentracing/SpanManager.java index b3250ef6..c864fda7 100644 --- a/opentracing-api/src/main/java/io/opentracing/SpanManager.java +++ b/opentracing-api/src/main/java/io/opentracing/SpanManager.java @@ -8,29 +8,21 @@ public interface SpanManager { * * Most users do not directly interact with SpanClosure, activate(), or deactivate(), but rather use * SpanManager-aware Runnables/Callables/Executors. - * - * @see Span#incRef() - * @see Span#decRef() */ interface SpanClosure { /** * Make the Span encapsulated by this SpanClosure active and return it. * - * NOTE: activate() should not affect a Span's reference count (though deactivate() and - * SpanManager.captureActive() do). - * * @see SpanManager#captureActive() * @return the newly-activated Span */ Span activate(); /** - * End this active period for the Span previously returned by activate() and decrement its reference count. - * - * @see Span#decRef() + * End this active period for the Span previously returned by activate(). Finish the span iff finish=true. */ - void deactivate(); + void deactivate(boolean finishSpan); } /** @@ -50,14 +42,10 @@ interface SpanClosure { * encapsulate it in a SpanClosure for activation in the future, perhaps in a different thread or on a different * executor. * - * If the provided Span is not null, the implementation must increment the reference count on the encapsulated - * (active) Span. - * * If the active Span is null, the implementation must still return a valid SpanClosure; when the closure activates, * it will clear any active Span. * * @see SpanManager.SpanClosure - * @see Span#incRef() * * @param span the Span to associate with any SpanManager-specific active context * @return a SpanClosure that represents the active Span and any other SpanManager-specific context, even if the 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 c7f265cc..145ac98a 100644 --- a/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedCallable.java +++ b/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedCallable.java @@ -21,10 +21,9 @@ public TracedCallable(Callable callable, SpanManager manager) { public TracedCallable(Callable callable, Span span, SpanManager manager) { if (callable == null) throw new NullPointerException("Callable is ."); - this.callable= callable; + this.callable = callable; this.manager = manager; this.spanClosure = manager.captureActive(); - span.incRef(); } public T call() throws Exception { @@ -32,7 +31,7 @@ public T call() throws Exception { try { return callable.call(); } finally { - spanClosure.deactivate(); + spanClosure.deactivate(false); } } } 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 ebea077e..8de6cefb 100644 --- a/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedRunnable.java +++ b/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedRunnable.java @@ -27,7 +27,6 @@ public TracedRunnable(Runnable runnable, Span span, SpanManager manager) { this.runnable = runnable; this.manager = manager; this.spanClosure = manager.captureActive(); - span.incRef(); } @Override @@ -36,7 +35,7 @@ public void run() { try { runnable.run(); } finally { - this.spanClosure.deactivate(); + this.spanClosure.deactivate(false); } } } 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 d99083da..a3524b38 100644 --- a/opentracing-impl/src/main/java/io/opentracing/impl/AbstractSpan.java +++ b/opentracing-impl/src/main/java/io/opentracing/impl/AbstractSpan.java @@ -46,17 +46,6 @@ abstract class AbstractSpan implements Span, SpanContext { this.start = start; } - @Override - public void incRef() { - refCount.incrementAndGet(); - } - @Override - public void decRef() { - if (refCount.decrementAndGet() == 0) { - this.finish(); - } - } - @Override public final SpanContext context() { return this; 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 c638f1ec..e6f66f44 100644 --- a/opentracing-mock/src/main/java/io/opentracing/mock/MockSpan.java +++ b/opentracing-mock/src/main/java/io/opentracing/mock/MockSpan.java @@ -99,20 +99,6 @@ public synchronized MockContext context() { return this.context; } - @Override - public void incRef() { - System.out.println("incRef: " + refCount.get() + " :: " + this.operationName); - refCount.incrementAndGet(); - } - - @Override - public void decRef() { - System.out.println("decRef: " + refCount.get() + " :: " + this.operationName); - if (refCount.decrementAndGet() == 0) { - this.finish(); - }; - } - @Override public void finish() { this.finish(nowMicros()); 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 a5be2515..3fde059c 100644 --- a/opentracing-mock/src/main/java/io/opentracing/mock/MockTracer.java +++ b/opentracing-mock/src/main/java/io/opentracing/mock/MockTracer.java @@ -242,7 +242,6 @@ public MockSpan start() { MockTracer.this.spanManager.captureActive(rval)); } */ - rval.incRef(); return rval; } 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 9fecb7b1..1d60c67c 100644 --- a/opentracing-mock/src/test/java/io/opentracing/mock/MockTracerTest.java +++ b/opentracing-mock/src/test/java/io/opentracing/mock/MockTracerTest.java @@ -67,7 +67,6 @@ public void testMDCHack() { Span parent = tracer.buildSpan("parent").start(); SpanManager.SpanClosure parentSpanClosure = tracer.activeSpanManager().captureWithSpan(parent); parentSpanClosure.activate(); - parent.incRef(); final List> futures = new ArrayList<>(); final List> subfutures = new ArrayList<>(); for (int i = 0; i < 10; i++) { @@ -97,7 +96,7 @@ public void run() { logger.info("submitting"); subfutures.add(otExecutor.submit(r)); logger.info("deactivating"); - childSpanClosure.deactivate(); + childSpanClosure.deactivate(true); } })); } @@ -120,7 +119,7 @@ public void run() { } catch (InterruptedException e) { e.printStackTrace(); } - parentSpanClosure.deactivate(); + parentSpanClosure.deactivate(true); List finishedSpans = tracer.finishedSpans(); diff --git a/opentracing-noop/src/main/java/io/opentracing/NoopSpan.java b/opentracing-noop/src/main/java/io/opentracing/NoopSpan.java index 35c28451..5fe3804d 100644 --- a/opentracing-noop/src/main/java/io/opentracing/NoopSpan.java +++ b/opentracing-noop/src/main/java/io/opentracing/NoopSpan.java @@ -24,12 +24,6 @@ final class NoopSpanImpl implements NoopSpan { @Override public SpanContext context() { return NoopSpanContextImpl.INSTANCE; } - @Override - public void incRef() { } - - @Override - public void decRef() { } - @Override public void finish() {} From 1bd949da536654bb7b9fcab087f37f8279f6661e Mon Sep 17 00:00:00 2001 From: Ben Sigelman Date: Sun, 5 Mar 2017 15:51:08 -0800 Subject: [PATCH 13/23] groundwork for MDCDemo --- opentracing-mdc-demo/pom.xml | 45 +++++++++++++++++++ .../java/io/opentracing/mdcdemo/MDCDemo.java | 7 +++ 2 files changed, 52 insertions(+) create mode 100644 opentracing-mdc-demo/pom.xml create mode 100644 opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/MDCDemo.java diff --git a/opentracing-mdc-demo/pom.xml b/opentracing-mdc-demo/pom.xml new file mode 100644 index 00000000..3b5ccd53 --- /dev/null +++ b/opentracing-mdc-demo/pom.xml @@ -0,0 +1,45 @@ + + + + 4.0.0 + + + io.opentracing + parent + 0.20.8-SNAPSHOT + + + opentracing-mdc-demo + OpenTracing-mdc-demo + OpenTracing MDC Demo + + + ${project.basedir}/.. + + + + + ${project.groupId} + opentracing-api + + + ${project.groupId} + opentracing-mock + + + + diff --git a/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/MDCDemo.java b/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/MDCDemo.java new file mode 100644 index 00000000..86c0dd82 --- /dev/null +++ b/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/MDCDemo.java @@ -0,0 +1,7 @@ +package io.opentracing.mdcdemo; + +public class MDCDemo { + public static void main(String[] args) { + System.out.println("Hi"); + } +} From 1cc804bc795425d52bad2498b9df110470959279 Mon Sep 17 00:00:00 2001 From: Ben Sigelman Date: Sun, 5 Mar 2017 16:00:44 -0800 Subject: [PATCH 14/23] Clarify what actually needs to be in OT-java ... by moving things around a bit more. --- .../java/io/opentracing/mdcdemo/MDCDemo.java | 108 +++++++++++++++++- .../opentracing/mdcdemo}/MDCSpanManager.java | 4 +- .../opentracing/mdcdemo}/TracedCallable.java | 2 +- .../mdcdemo}/TracedExecutorService.java | 2 +- .../opentracing/mdcdemo}/TracedRunnable.java | 2 +- .../io/opentracing/mock/MockTracerTest.java | 96 +--------------- 6 files changed, 114 insertions(+), 100 deletions(-) rename {opentracing-api/src/main/java/io/opentracing => opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo}/MDCSpanManager.java (94%) rename {opentracing-impl/src/main/java/io/opentracing/concurrent => opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo}/TracedCallable.java (96%) rename {opentracing-impl/src/main/java/io/opentracing/concurrent => opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo}/TracedExecutorService.java (98%) rename {opentracing-impl/src/main/java/io/opentracing/concurrent => opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo}/TracedRunnable.java (97%) diff --git a/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/MDCDemo.java b/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/MDCDemo.java index 86c0dd82..27c0a58e 100644 --- a/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/MDCDemo.java +++ b/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/MDCDemo.java @@ -1,7 +1,113 @@ package io.opentracing.mdcdemo; +import io.opentracing.Span; +import io.opentracing.SpanManager; +import io.opentracing.mock.MockSpan; +import io.opentracing.mock.MockTracer; +import org.slf4j.Logger; +import org.slf4j.MDC; + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.concurrent.*; + public class MDCDemo { public static void main(String[] args) { - System.out.println("Hi"); + doEverything(); + } + + public static void doEverything() { + org.apache.log4j.BasicConfigurator.configure(); + final 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()); + + final MockTracer tracer = new MockTracer(); + tracer.setSpanManager(new MDCSpanManager()); + + /* + 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()); + } + tracer.reset(); + */ + + ExecutorService realExecutor = Executors.newFixedThreadPool(500); + final ExecutorService otExecutor = new TracedExecutorService(realExecutor, tracer.activeSpanManager()); + Span parent = tracer.buildSpan("parent").start(); + SpanManager.SpanClosure parentSpanClosure = tracer.activeSpanManager().captureWithSpan(parent); + parentSpanClosure.activate(); + final List> futures = new ArrayList<>(); + final List> subfutures = new ArrayList<>(); + for (int i = 0; i < 10; i++) { + final int j = i; + futures.add(otExecutor.submit(new Runnable() { + @Override + public void run() { + final Span child = tracer.buildSpan("child_" + j).start(); + SpanManager.SpanClosure childSpanClosure = tracer.activeSpanManager().captureWithSpan(child); + childSpanClosure.activate(); + try { + Thread.currentThread().sleep(1000); + } catch (InterruptedException e) { + e.printStackTrace(); + } + child.log("awoke"); + logger.info("defining"); + Runnable r = new Runnable() { + @Override + public void run() { + Span active = tracer.activeSpanManager().active(); + active.log("awoke again"); + Span grandchild = tracer.buildSpan("grandchild").start(); + grandchild.finish(); + } + }; + logger.info("submitting"); + subfutures.add(otExecutor.submit(r)); + logger.info("deactivating"); + childSpanClosure.deactivate(true); + } + })); + } + try { + for (Future f : futures) { + f.get(); + } + for (Future f : subfutures) { + f.get(); + } + } catch (InterruptedException e) { + e.printStackTrace(); + } catch (ExecutionException e) { + e.printStackTrace(); + } + + otExecutor.shutdown(); + try { + otExecutor.awaitTermination(3, TimeUnit.SECONDS); + } catch (InterruptedException e) { + e.printStackTrace(); + } + parentSpanClosure.deactivate(true); + + List finishedSpans = tracer.finishedSpans(); + + logger.info("DONE SLEEPING"); + for (MockSpan span : finishedSpans) { + logger.info("finished Span '{}'. trace={}, span={}, parent={}", span.operationName(), span.context().traceId(), span.context().spanId(), span.parentId()); + } + } } diff --git a/opentracing-api/src/main/java/io/opentracing/MDCSpanManager.java b/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/MDCSpanManager.java similarity index 94% rename from opentracing-api/src/main/java/io/opentracing/MDCSpanManager.java rename to opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/MDCSpanManager.java index 37831019..79d6fe23 100644 --- a/opentracing-api/src/main/java/io/opentracing/MDCSpanManager.java +++ b/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/MDCSpanManager.java @@ -1,5 +1,7 @@ -package io.opentracing; +package io.opentracing.mdcdemo; +import io.opentracing.Span; +import io.opentracing.SpanManager; import org.slf4j.MDC; import java.util.Map; diff --git a/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedCallable.java b/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/TracedCallable.java similarity index 96% rename from opentracing-impl/src/main/java/io/opentracing/concurrent/TracedCallable.java rename to opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/TracedCallable.java index 145ac98a..beae2b2a 100644 --- a/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedCallable.java +++ b/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/TracedCallable.java @@ -1,4 +1,4 @@ -package io.opentracing.concurrent; +package io.opentracing.mdcdemo; import io.opentracing.SpanManager; import io.opentracing.Span; diff --git a/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedExecutorService.java b/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/TracedExecutorService.java similarity index 98% rename from opentracing-impl/src/main/java/io/opentracing/concurrent/TracedExecutorService.java rename to opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/TracedExecutorService.java index d6500114..4f7168b6 100644 --- a/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedExecutorService.java +++ b/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/TracedExecutorService.java @@ -1,4 +1,4 @@ -package io.opentracing.concurrent; +package io.opentracing.mdcdemo; import io.opentracing.SpanManager; import io.opentracing.impl.GlobalTracer; diff --git a/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedRunnable.java b/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/TracedRunnable.java similarity index 97% rename from opentracing-impl/src/main/java/io/opentracing/concurrent/TracedRunnable.java rename to opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/TracedRunnable.java index 8de6cefb..fae446ea 100644 --- a/opentracing-impl/src/main/java/io/opentracing/concurrent/TracedRunnable.java +++ b/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/TracedRunnable.java @@ -1,4 +1,4 @@ -package io.opentracing.concurrent; +package io.opentracing.mdcdemo; import io.opentracing.SpanManager; import io.opentracing.Span; 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 1d60c67c..562a42e5 100644 --- a/opentracing-mock/src/test/java/io/opentracing/mock/MockTracerTest.java +++ b/opentracing-mock/src/test/java/io/opentracing/mock/MockTracerTest.java @@ -23,7 +23,7 @@ import java.util.concurrent.*; import io.opentracing.*; -import io.opentracing.concurrent.TracedExecutorService; +import io.opentracing.mock.MockTracer; import org.junit.Assert; import org.junit.Test; @@ -35,100 +35,6 @@ import org.slf4j.MDC; public class MockTracerTest { - @Test - public void testMDCHack() { - org.apache.log4j.BasicConfigurator.configure(); - final 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()); - - final MockTracer tracer = new MockTracer(); - tracer.setSpanManager(new MDCSpanManager()); - - /* - 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()); - } - tracer.reset(); - */ - - ExecutorService realExecutor = Executors.newFixedThreadPool(500); - final ExecutorService otExecutor = new TracedExecutorService(realExecutor, tracer.activeSpanManager()); - Span parent = tracer.buildSpan("parent").start(); - SpanManager.SpanClosure parentSpanClosure = tracer.activeSpanManager().captureWithSpan(parent); - parentSpanClosure.activate(); - final List> futures = new ArrayList<>(); - final List> subfutures = new ArrayList<>(); - for (int i = 0; i < 10; i++) { - final int j = i; - futures.add(otExecutor.submit(new Runnable() { - @Override - public void run() { - final Span child = tracer.buildSpan("child_" + j).start(); - SpanManager.SpanClosure childSpanClosure = tracer.activeSpanManager().captureWithSpan(child); - childSpanClosure.activate(); - try { - Thread.currentThread().sleep(1000); - } catch (InterruptedException e) { - e.printStackTrace(); - } - child.log("awoke"); - logger.info("defining"); - Runnable r = new Runnable() { - @Override - public void run() { - Span active = tracer.activeSpanManager().active(); - active.log("awoke again"); - Span grandchild = tracer.buildSpan("grandchild").start(); - grandchild.finish(); - } - }; - logger.info("submitting"); - subfutures.add(otExecutor.submit(r)); - logger.info("deactivating"); - childSpanClosure.deactivate(true); - } - })); - } - try { - for (Future f : futures) { - f.get(); - } - for (Future f : subfutures) { - f.get(); - } - } catch (InterruptedException e) { - e.printStackTrace(); - } catch (ExecutionException e) { - e.printStackTrace(); - } - - otExecutor.shutdown(); - try { - otExecutor.awaitTermination(3, TimeUnit.SECONDS); - } catch (InterruptedException e) { - e.printStackTrace(); - } - parentSpanClosure.deactivate(true); - - List finishedSpans = tracer.finishedSpans(); - - logger.info("DONE SLEEPING"); - for (MockSpan span : finishedSpans) { - logger.info("finished Span '{}'. trace={}, span={}, parent={}", span.operationName(), span.context().traceId(), span.context().spanId(), span.parentId()); - } - } - @Test public void testRootSpan() { // Create and finish a root Span. From f45ffbb40b5ab1405c25d2171527fac8e8aaec2f Mon Sep 17 00:00:00 2001 From: Ben Sigelman Date: Tue, 7 Mar 2017 10:14:19 -0800 Subject: [PATCH 15/23] Cleanup --- .../java/io/opentracing/mdcdemo/MDCDemo.java | 24 +------------------ 1 file changed, 1 insertion(+), 23 deletions(-) diff --git a/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/MDCDemo.java b/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/MDCDemo.java index 27c0a58e..d08aa649 100644 --- a/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/MDCDemo.java +++ b/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/MDCDemo.java @@ -14,35 +14,13 @@ public class MDCDemo { public static void main(String[] args) { - doEverything(); - } - - public static void doEverything() { org.apache.log4j.BasicConfigurator.configure(); final 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()); + MDC.put("mdcKey", "mdcVal"); final MockTracer tracer = new MockTracer(); tracer.setSpanManager(new MDCSpanManager()); - /* - 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()); - } - tracer.reset(); - */ - ExecutorService realExecutor = Executors.newFixedThreadPool(500); final ExecutorService otExecutor = new TracedExecutorService(realExecutor, tracer.activeSpanManager()); Span parent = tracer.buildSpan("parent").start(); From d2c18248d9137fb183ae05a24927445091a37957 Mon Sep 17 00:00:00 2001 From: Ben Sigelman Date: Tue, 7 Mar 2017 15:43:49 -0800 Subject: [PATCH 16/23] Make start()'ed Spans active in MockTracer Still need to figure out how to mandate this: via comments or compilers? --- .../main/java/io/opentracing/SpanManager.java | 19 ++++++++----------- .../java/io/opentracing/mdcdemo/MDCDemo.java | 4 ++-- .../opentracing/mdcdemo/MDCSpanManager.java | 3 ++- .../java/io/opentracing/mock/MockTracer.java | 5 +---- 4 files changed, 13 insertions(+), 18 deletions(-) diff --git a/opentracing-api/src/main/java/io/opentracing/SpanManager.java b/opentracing-api/src/main/java/io/opentracing/SpanManager.java index c864fda7..11143a98 100644 --- a/opentracing-api/src/main/java/io/opentracing/SpanManager.java +++ b/opentracing-api/src/main/java/io/opentracing/SpanManager.java @@ -14,6 +14,8 @@ interface SpanClosure { /** * Make the Span encapsulated by this SpanClosure active and return it. * + * XXX: handle the case where the Span is null. + * * @see SpanManager#captureActive() * @return the newly-activated Span */ @@ -30,15 +32,8 @@ interface SpanClosure { */ Span active(); - /** - * Capture the active() Span (even if null) via captureWithSpan(). - * - * @see SpanManager#active() - * @see SpanManager#captureWithSpan(Span) - */ - SpanClosure captureActive(); - /** - * Capture any SpanManager-specific context (e.g., MDC context) along with the given Span (even if null) and + /** + * Capture any SpanManager-specific context (e.g., MDC context) along with the active Span (even if null) and * encapsulate it in a SpanClosure for activation in the future, perhaps in a different thread or on a different * executor. * @@ -47,9 +42,11 @@ interface SpanClosure { * * @see SpanManager.SpanClosure * - * @param span the Span to associate with any SpanManager-specific active context * @return a SpanClosure that represents the active Span and any other SpanManager-specific context, even if the * active Span is null. */ - SpanClosure captureWithSpan(Span span); + SpanClosure captureActive(); + + // XXX: comment + SpanClosure capture(Span span); } diff --git a/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/MDCDemo.java b/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/MDCDemo.java index d08aa649..9e8378eb 100644 --- a/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/MDCDemo.java +++ b/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/MDCDemo.java @@ -24,7 +24,7 @@ public static void main(String[] args) { ExecutorService realExecutor = Executors.newFixedThreadPool(500); final ExecutorService otExecutor = new TracedExecutorService(realExecutor, tracer.activeSpanManager()); Span parent = tracer.buildSpan("parent").start(); - SpanManager.SpanClosure parentSpanClosure = tracer.activeSpanManager().captureWithSpan(parent); + SpanManager.SpanClosure parentSpanClosure = tracer.activeSpanManager().captureActive(); parentSpanClosure.activate(); final List> futures = new ArrayList<>(); final List> subfutures = new ArrayList<>(); @@ -34,7 +34,7 @@ public static void main(String[] args) { @Override public void run() { final Span child = tracer.buildSpan("child_" + j).start(); - SpanManager.SpanClosure childSpanClosure = tracer.activeSpanManager().captureWithSpan(child); + SpanManager.SpanClosure childSpanClosure = tracer.activeSpanManager().captureActive(); childSpanClosure.activate(); try { Thread.currentThread().sleep(1000); diff --git a/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/MDCSpanManager.java b/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/MDCSpanManager.java index 79d6fe23..db12d829 100644 --- a/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/MDCSpanManager.java +++ b/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/MDCSpanManager.java @@ -54,8 +54,9 @@ public void deactivate(boolean finishSpan) { public MDCSnapshot captureActive() { return new MDCSnapshot(active()); } + @Override - public MDCSnapshot captureWithSpan(Span span) { + public SpanClosure capture(Span span) { return new MDCSnapshot(span); } 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 3fde059c..0706de0f 100644 --- a/opentracing-mock/src/main/java/io/opentracing/mock/MockTracer.java +++ b/opentracing-mock/src/main/java/io/opentracing/mock/MockTracer.java @@ -236,12 +236,9 @@ public MockSpan start() { this.startMicros = MockSpan.nowMicros(); } Span rval = new MockSpan(MockTracer.this, this.operationName, this.startMicros, initialTags, this.firstParent); - /* if (MockTracer.this.spanManager != null) { - MockTracer.this.spanManager.activate( - MockTracer.this.spanManager.captureActive(rval)); + MockTracer.this.spanManager.capture(rval).activate(); } - */ return rval; } From e13dbe4608d85e81ecedacb77a0bed59b863597d Mon Sep 17 00:00:00 2001 From: Ben Sigelman Date: Wed, 8 Mar 2017 22:11:49 -0800 Subject: [PATCH 17/23] Minor cleanups --- opentracing-api/src/main/java/io/opentracing/Tracer.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/opentracing-api/src/main/java/io/opentracing/Tracer.java b/opentracing-api/src/main/java/io/opentracing/Tracer.java index 82842f22..6b667495 100644 --- a/opentracing-api/src/main/java/io/opentracing/Tracer.java +++ b/opentracing-api/src/main/java/io/opentracing/Tracer.java @@ -23,6 +23,9 @@ public interface Tracer { /** * Return a new SpanBuilder for a Span with the given `operationName`. * + *

If there is an active Span according to the activeSpanManager(), + * buildSpan will automatically have an asChildOf() reference to same. + * *

You can override the operationName later via {@link Span#setOperationName(String)}. * *

A contrived example: @@ -41,7 +44,6 @@ public interface Tracer { */ SpanBuilder buildSpan(String operationName); - void setSpanManager(SpanManager mgr); SpanManager activeSpanManager(); /** @@ -91,8 +93,7 @@ public interface Tracer { */ SpanContext extract(Format format, C carrier); - // XXX(bhs): could make this an abstract class. In any case, by default a SpanBuilder will have an asChildOf pointer - // to the SpanManager's active Span's SpanContext. + interface SpanBuilder extends SpanContext { /** @@ -130,7 +131,6 @@ interface SpanBuilder extends SpanContext { SpanBuilder withStartTimestamp(long microseconds); /** Returns the started Span. */ - // XXX: should this return a SpanClosure? Span start(); } From f5d0451b40e8aa075e73ed3a1f8dc89dfd69d5b7 Mon Sep 17 00:00:00 2001 From: Ben Sigelman Date: Wed, 8 Mar 2017 23:18:14 -0800 Subject: [PATCH 18/23] Replace isFinished with onFinished Also, clean up some copy --- .../src/main/java/io/opentracing/Span.java | 3 -- .../main/java/io/opentracing/SpanManager.java | 2 + .../io/opentracing/impl/AbstractSpan.java | 5 -- .../io/opentracing/impl/AbstractTracer.java | 10 ++-- .../io/opentracing/impl/GlobalTracer.java | 4 -- .../java/io/opentracing/impl/NoopSpan.java | 5 -- .../java/io/opentracing/impl/NoopTracer.java | 3 -- .../io/opentracing/impl/TestSpanImpl.java | 6 --- .../java/io/opentracing/mdcdemo/MDCDemo.java | 48 ++++++++++++++----- .../opentracing/mdcdemo/MDCSpanManager.java | 33 +++++++++---- .../java/io/opentracing/mock/MockSpan.java | 11 ++--- .../java/io/opentracing/mock/MockTracer.java | 11 +++-- .../main/java/io/opentracing/NoopSpan.java | 3 -- 13 files changed, 77 insertions(+), 67 deletions(-) diff --git a/opentracing-api/src/main/java/io/opentracing/Span.java b/opentracing-api/src/main/java/io/opentracing/Span.java index 1394d28e..7a670c46 100644 --- a/opentracing-api/src/main/java/io/opentracing/Span.java +++ b/opentracing-api/src/main/java/io/opentracing/Span.java @@ -41,9 +41,6 @@ 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/SpanManager.java b/opentracing-api/src/main/java/io/opentracing/SpanManager.java index 11143a98..18c430bc 100644 --- a/opentracing-api/src/main/java/io/opentracing/SpanManager.java +++ b/opentracing-api/src/main/java/io/opentracing/SpanManager.java @@ -49,4 +49,6 @@ interface SpanClosure { // XXX: comment SpanClosure capture(Span span); + + void onFinish(Span span); } 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 a3524b38..49fc5e59 100644 --- a/opentracing-impl/src/main/java/io/opentracing/impl/AbstractSpan.java +++ b/opentracing-impl/src/main/java/io/opentracing/impl/AbstractSpan.java @@ -57,11 +57,6 @@ 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 52f7f79d..0a5aa59b 100644 --- a/opentracing-impl/src/main/java/io/opentracing/impl/AbstractTracer.java +++ b/opentracing-impl/src/main/java/io/opentracing/impl/AbstractTracer.java @@ -31,17 +31,19 @@ abstract class AbstractTracer implements Tracer { private final PropagationRegistry registry = new PropagationRegistry(); private SpanManager manager; + protected AbstractTracer() { + this(null); // SpanManager is optional for this spike + } + + protected AbstractTracer(SpanManager manager) { + this.manager = manager; registry.register(Format.Builtin.TEXT_MAP, new TextMapInjectorImpl(this)); registry.register(Format.Builtin.TEXT_MAP, new TextMapExtractorImpl(this)); } abstract AbstractSpanBuilder createSpanBuilder(String operationName); - public void setSpanManager(SpanManager manager) { - this.manager = manager; - } - @Override public SpanManager activeSpanManager() { return this.manager; 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 718e0df3..bbcdcf56 100644 --- a/opentracing-impl/src/main/java/io/opentracing/impl/GlobalTracer.java +++ b/opentracing-impl/src/main/java/io/opentracing/impl/GlobalTracer.java @@ -116,10 +116,6 @@ public SpanBuilder buildSpan(String operationName) { return lazyTracer().buildSpan(operationName); } - public void setSpanManager(SpanManager mgr) { - lazyTracer().setSpanManager(mgr); - } - @Override public SpanManager activeSpanManager() { return lazyTracer().activeSpanManager(); 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 be8b5d7d..2820dc0b 100644 --- a/opentracing-impl/src/main/java/io/opentracing/impl/NoopSpan.java +++ b/opentracing-impl/src/main/java/io/opentracing/impl/NoopSpan.java @@ -27,11 +27,6 @@ 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/NoopTracer.java b/opentracing-impl/src/main/java/io/opentracing/impl/NoopTracer.java index 3bae3ac0..95012645 100644 --- a/opentracing-impl/src/main/java/io/opentracing/impl/NoopTracer.java +++ b/opentracing-impl/src/main/java/io/opentracing/impl/NoopTracer.java @@ -22,9 +22,6 @@ final class NoopTracer extends AbstractTracer implements io.opentracing.NoopTrac private static final NoopTracer INSTANCE = new NoopTracer(); - @Override - public void setSpanManager(SpanManager mgr) {} - @Override public SpanManager activeSpanManager() { return null; } 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 0ef62481..0db93dc8 100644 --- a/opentracing-impl/src/test/java/io/opentracing/impl/TestSpanImpl.java +++ b/opentracing-impl/src/test/java/io/opentracing/impl/TestSpanImpl.java @@ -16,13 +16,7 @@ import io.opentracing.impl.AbstractSpan; public class TestSpanImpl extends AbstractSpan { - TestSpanImpl(String operationName) { super(operationName); } - - @Override - public boolean isFinished() { - return true; - } } diff --git a/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/MDCDemo.java b/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/MDCDemo.java index 9e8378eb..8e7c442a 100644 --- a/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/MDCDemo.java +++ b/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/MDCDemo.java @@ -2,6 +2,7 @@ import io.opentracing.Span; import io.opentracing.SpanManager; +import io.opentracing.Tracer; import io.opentracing.mock.MockSpan; import io.opentracing.mock.MockTracer; import org.slf4j.Logger; @@ -13,13 +14,27 @@ import java.util.concurrent.*; public class MDCDemo { - public static void main(String[] args) { - org.apache.log4j.BasicConfigurator.configure(); - final Logger logger = org.slf4j.LoggerFactory.getLogger("hack"); - MDC.put("mdcKey", "mdcVal"); + Tracer tracer; + + private MDCDemo(Tracer tracer) { + this.tracer = tracer; + } + + public void trivialSpan() { + Span span = this.tracer.buildSpan("trivial").start(); + span.finish(); + } + + public void trivialChild() { + Span parent = this.tracer.buildSpan("trivialParent").start(); + // The child will automatically know about the parent. + Span child = this.tracer.buildSpan("trivialChild").start(); + child.finish(); + parent.finish(); + } - final MockTracer tracer = new MockTracer(); - tracer.setSpanManager(new MDCSpanManager()); + public void asyncSpans() { + final Tracer tracer = this.tracer; // save typing ExecutorService realExecutor = Executors.newFixedThreadPool(500); final ExecutorService otExecutor = new TracedExecutorService(realExecutor, tracer.activeSpanManager()); @@ -42,20 +57,18 @@ public void run() { e.printStackTrace(); } child.log("awoke"); - logger.info("defining"); Runnable r = new Runnable() { @Override public void run() { Span active = tracer.activeSpanManager().active(); active.log("awoke again"); - Span grandchild = tracer.buildSpan("grandchild").start(); + Span grandchild = tracer.buildSpan("grandchild_" + j).start(); grandchild.finish(); + active.finish(); } }; - logger.info("submitting"); subfutures.add(otExecutor.submit(r)); - logger.info("deactivating"); - childSpanClosure.deactivate(true); + childSpanClosure.deactivate(false); } })); } @@ -79,6 +92,19 @@ public void run() { e.printStackTrace(); } parentSpanClosure.deactivate(true); + } + + public static void main(String[] args) { + org.apache.log4j.BasicConfigurator.configure(); + final Logger logger = org.slf4j.LoggerFactory.getLogger("hack"); + MDC.put("mdcKey", "mdcVal"); + + final MockTracer tracer = new MockTracer(new MDCSpanManager()); + + MDCDemo demo = new MDCDemo(tracer); + demo.trivialSpan(); + demo.trivialChild(); + demo.asyncSpans(); List finishedSpans = tracer.finishedSpans(); diff --git a/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/MDCSpanManager.java b/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/MDCSpanManager.java index db12d829..a3c15f6b 100644 --- a/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/MDCSpanManager.java +++ b/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/MDCSpanManager.java @@ -7,7 +7,8 @@ import java.util.Map; /** - * XXX: comment + * MDCSpanManager illustrates the core SpanManager concepts and capabilities to a first approximation. Not + * production-quality code. */ public class MDCSpanManager implements SpanManager { private final ThreadLocal tlsSnapshot = new ThreadLocal(); @@ -36,17 +37,12 @@ public void deactivate(boolean finishSpan) { } if (tlsSnapshot.get() != this) { - // do nothing + // This probably shouldn't happen. + // + // XXX: log or throw something here? return; } - MDCSnapshot nextActiveSnapshot = toRestore; - while (nextActiveSnapshot != null) { - if (!nextActiveSnapshot.span.isFinished()) { - break; - } - nextActiveSnapshot = nextActiveSnapshot.toRestore; - } - tlsSnapshot.set(nextActiveSnapshot); + tlsSnapshot.set(toRestore); } } @@ -68,4 +64,21 @@ public Span active() { } return snapshot.span; } + + @Override + public void onFinish(Span span) { + MDCSnapshot snapshot = tlsSnapshot.get(); + MDCSnapshot prevSnapshot = null; + while (snapshot != null) { + if (snapshot.span == span) { + if (prevSnapshot == null) { + tlsSnapshot.set(snapshot.toRestore); + } else { + prevSnapshot.toRestore = snapshot.toRestore; + } + } + prevSnapshot = snapshot; + snapshot = snapshot.toRestore; + } + } } 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 e6f66f44..bec56dd5 100644 --- a/opentracing-mock/src/main/java/io/opentracing/mock/MockSpan.java +++ b/opentracing-mock/src/main/java/io/opentracing/mock/MockSpan.java @@ -108,17 +108,12 @@ public void finish() { public synchronized void finish(long finishMicros) { finishedCheck("Finishing already finished span"); - if (this.mockTracer.getActiveSpanManager() != null) { - this.mockTracer.getActiveSpanManager().deactivate(this.snapshot); - } this.finishMicros = finishMicros; this.mockTracer.appendFinishedSpan(this); this.finished = true; - } - - @Override - public synchronized boolean isFinished() { - return finished; + if (this.mockTracer.activeSpanManager() != null) { + this.mockTracer.activeSpanManager().onFinish(this); + } } @Override 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 0706de0f..8c7c43ce 100644 --- a/opentracing-mock/src/main/java/io/opentracing/mock/MockTracer.java +++ b/opentracing-mock/src/main/java/io/opentracing/mock/MockTracer.java @@ -41,6 +41,11 @@ public MockTracer() { this(Propagator.PRINTER); } + public MockTracer(SpanManager manager) { + this(Propagator.PRINTER); + this.spanManager = manager; + } + /** * Create a new MockTracer that passes through any calls to inject() and/or extract(). */ @@ -154,10 +159,6 @@ public SpanBuilder buildSpan(String operationName) { return sb; } - public void setSpanManager(SpanManager mgr) { - this.spanManager = mgr; - } - @Override public SpanManager activeSpanManager() { return spanManager; @@ -235,7 +236,7 @@ public MockSpan start() { if (this.startMicros == 0) { this.startMicros = MockSpan.nowMicros(); } - Span rval = new MockSpan(MockTracer.this, this.operationName, this.startMicros, initialTags, this.firstParent); + MockSpan rval = new MockSpan(MockTracer.this, this.operationName, this.startMicros, initialTags, this.firstParent); if (MockTracer.this.spanManager != null) { MockTracer.this.spanManager.capture(rval).activate(); } diff --git a/opentracing-noop/src/main/java/io/opentracing/NoopSpan.java b/opentracing-noop/src/main/java/io/opentracing/NoopSpan.java index 5fe3804d..1a5e20c5 100644 --- a/opentracing-noop/src/main/java/io/opentracing/NoopSpan.java +++ b/opentracing-noop/src/main/java/io/opentracing/NoopSpan.java @@ -30,9 +30,6 @@ public void finish() {} @Override public void finish(long finishMicros) {} - @Override - public boolean isFinished() { return true; } - @Override public void close() { finish(); } From d8570be6ca76fba18bcfdd5acb5c406079dd466c Mon Sep 17 00:00:00 2001 From: Ben Sigelman Date: Wed, 8 Mar 2017 23:21:52 -0800 Subject: [PATCH 19/23] Add a few comments --- .../src/main/java/io/opentracing/SpanManager.java | 13 +++++++++++-- .../main/java/io/opentracing/mdcdemo/MDCDemo.java | 15 ++++++++------- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/opentracing-api/src/main/java/io/opentracing/SpanManager.java b/opentracing-api/src/main/java/io/opentracing/SpanManager.java index 18c430bc..c031e61a 100644 --- a/opentracing-api/src/main/java/io/opentracing/SpanManager.java +++ b/opentracing-api/src/main/java/io/opentracing/SpanManager.java @@ -14,7 +14,7 @@ interface SpanClosure { /** * Make the Span encapsulated by this SpanClosure active and return it. * - * XXX: handle the case where the Span is null. + * XXX: think harder about the returned Span being null. * * @see SpanManager#captureActive() * @return the newly-activated Span @@ -47,8 +47,17 @@ interface SpanClosure { */ SpanClosure captureActive(); - // XXX: comment + /** + * Explicitly capture the given Span and any active state (e.g., MDC state) about the current execution context. + * + * @param span + * @return a SpanClosure that represents the active Span and any other SpanManager-specific context, even if the + * active Span is null. + */ SpanClosure capture(Span span); + /** + * Tell the SpanManager that a particular Span has finished (and update any structures accordingly). + */ void onFinish(Span span); } diff --git a/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/MDCDemo.java b/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/MDCDemo.java index 8e7c442a..3f7062f3 100644 --- a/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/MDCDemo.java +++ b/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/MDCDemo.java @@ -10,7 +10,6 @@ import java.util.ArrayList; import java.util.List; -import java.util.Map; import java.util.concurrent.*; public class MDCDemo { @@ -101,14 +100,16 @@ public static void main(String[] args) { final MockTracer tracer = new MockTracer(new MDCSpanManager()); - MDCDemo demo = new MDCDemo(tracer); - demo.trivialSpan(); - demo.trivialChild(); - demo.asyncSpans(); + // Do stuff with the MockTracer. + { + MDCDemo demo = new MDCDemo(tracer); + demo.trivialSpan(); + demo.trivialChild(); + demo.asyncSpans(); + } + // Print out all mock-Spans List finishedSpans = tracer.finishedSpans(); - - logger.info("DONE SLEEPING"); for (MockSpan span : finishedSpans) { logger.info("finished Span '{}'. trace={}, span={}, parent={}", span.operationName(), span.context().traceId(), span.context().spanId(), span.parentId()); } From 0c702ff4ff858609261d8cb9307dd5a8e82d1c55 Mon Sep 17 00:00:00 2001 From: Ben Sigelman Date: Wed, 8 Mar 2017 23:26:47 -0800 Subject: [PATCH 20/23] Comment updates --- .../src/main/java/io/opentracing/SpanManager.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/opentracing-api/src/main/java/io/opentracing/SpanManager.java b/opentracing-api/src/main/java/io/opentracing/SpanManager.java index c031e61a..9eaf2f99 100644 --- a/opentracing-api/src/main/java/io/opentracing/SpanManager.java +++ b/opentracing-api/src/main/java/io/opentracing/SpanManager.java @@ -1,5 +1,9 @@ package io.opentracing; +/** + * SpanManager allows an existing (possibly thread-local-aware) execution context provider to also manage the current + * active OpenTracing span. + */ public interface SpanManager { /** @@ -7,7 +11,8 @@ public interface SpanManager { * "closure" (or period of Span activity) has finished. * * Most users do not directly interact with SpanClosure, activate(), or deactivate(), but rather use - * SpanManager-aware Runnables/Callables/Executors. + * SpanManager-aware Runnables/Callables/Executors. Those higher-level primitives need not be defined within the + * OpenTracing core API. */ interface SpanClosure { From 2e744ef769461efa650dd4de4e6eff87c1dcfe69 Mon Sep 17 00:00:00 2001 From: Ben Sigelman Date: Thu, 9 Mar 2017 22:11:51 -0800 Subject: [PATCH 21/23] Remove unused member --- opentracing-mock/src/main/java/io/opentracing/mock/MockSpan.java | 1 - 1 file changed, 1 deletion(-) 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 bec56dd5..46f2c9bd 100644 --- a/opentracing-mock/src/main/java/io/opentracing/mock/MockSpan.java +++ b/opentracing-mock/src/main/java/io/opentracing/mock/MockSpan.java @@ -38,7 +38,6 @@ public final class MockSpan implements Span { private final Map tags; private final List logEntries = new ArrayList<>(); private String operationName; - private final AtomicLong refCount = new AtomicLong(0); private final List errors = new ArrayList<>(); From 555f5ea9ee96972c9324d80dabc20adacfb4fbd6 Mon Sep 17 00:00:00 2001 From: Ben Sigelman Date: Sat, 11 Mar 2017 15:15:40 -0800 Subject: [PATCH 22/23] Add a SpanBuilder.startAndActivate method As well as a few other minor cleanups --- .../src/main/java/io/opentracing/SpanManager.java | 12 +++++++++++- .../src/main/java/io/opentracing/Tracer.java | 2 ++ .../io/opentracing/impl/AbstractSpanBuilder.java | 11 +++++++---- .../main/java/io/opentracing/mdcdemo/MDCDemo.java | 10 +++------- .../java/io/opentracing/mdcdemo/MDCSpanManager.java | 5 +++++ .../main/java/io/opentracing/mock/MockTracer.java | 6 ++++++ .../main/java/io/opentracing/NoopSpanBuilder.java | 3 +++ 7 files changed, 37 insertions(+), 12 deletions(-) diff --git a/opentracing-api/src/main/java/io/opentracing/SpanManager.java b/opentracing-api/src/main/java/io/opentracing/SpanManager.java index 9eaf2f99..ae951a1e 100644 --- a/opentracing-api/src/main/java/io/opentracing/SpanManager.java +++ b/opentracing-api/src/main/java/io/opentracing/SpanManager.java @@ -13,23 +13,33 @@ public interface SpanManager { * Most users do not directly interact with SpanClosure, activate(), or deactivate(), but rather use * SpanManager-aware Runnables/Callables/Executors. Those higher-level primitives need not be defined within the * OpenTracing core API. + * + * @see SpanManager#captureActive() */ interface SpanClosure { /** * Make the Span encapsulated by this SpanClosure active and return it. * - * XXX: think harder about the returned Span being null. + * NOTE: It is an error to call activate() more than once on a single SpanClosure instance. * * @see SpanManager#captureActive() * @return the newly-activated Span */ Span activate(); + /** + * @return the encapsulated Span, or null if there isn't one. + */ + Span span(); + /** * End this active period for the Span previously returned by activate(). Finish the span iff finish=true. + * + * NOTE: It is an error to call deactivate() more than once on a single SpanClosure instance. */ void deactivate(boolean finishSpan); + } /** diff --git a/opentracing-api/src/main/java/io/opentracing/Tracer.java b/opentracing-api/src/main/java/io/opentracing/Tracer.java index 6b667495..74206c0e 100644 --- a/opentracing-api/src/main/java/io/opentracing/Tracer.java +++ b/opentracing-api/src/main/java/io/opentracing/Tracer.java @@ -133,5 +133,7 @@ interface SpanBuilder extends SpanContext { /** Returns the started Span. */ Span start(); + SpanManager.SpanClosure startAndActivate(); + } } diff --git a/opentracing-impl/src/main/java/io/opentracing/impl/AbstractSpanBuilder.java b/opentracing-impl/src/main/java/io/opentracing/impl/AbstractSpanBuilder.java index 4c04b29a..70fcca0f 100644 --- a/opentracing-impl/src/main/java/io/opentracing/impl/AbstractSpanBuilder.java +++ b/opentracing-impl/src/main/java/io/opentracing/impl/AbstractSpanBuilder.java @@ -13,10 +13,8 @@ */ package io.opentracing.impl; -import io.opentracing.References; -import io.opentracing.Span; -import io.opentracing.SpanContext; -import io.opentracing.Tracer; +import io.opentracing.*; + import java.time.Instant; import java.util.ArrayList; import java.util.HashMap; @@ -122,6 +120,11 @@ public final Span start() { return span; } + @Override + public SpanManager.SpanClosure startAndActivate() { + return null; // XXX: not correct... we'd need access to the AbstractTracer's SpanManager. + } + private void withBaggageFrom(SpanContext from) { for (Entry baggageItem : from.baggageItems()) { this.withBaggageItem(baggageItem.getKey(), baggageItem.getValue()); diff --git a/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/MDCDemo.java b/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/MDCDemo.java index 3f7062f3..86760e0f 100644 --- a/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/MDCDemo.java +++ b/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/MDCDemo.java @@ -37,9 +37,7 @@ public void asyncSpans() { ExecutorService realExecutor = Executors.newFixedThreadPool(500); final ExecutorService otExecutor = new TracedExecutorService(realExecutor, tracer.activeSpanManager()); - Span parent = tracer.buildSpan("parent").start(); - SpanManager.SpanClosure parentSpanClosure = tracer.activeSpanManager().captureActive(); - parentSpanClosure.activate(); + SpanManager.SpanClosure parentSpanClosure = tracer.buildSpan("parent").startAndActivate(); final List> futures = new ArrayList<>(); final List> subfutures = new ArrayList<>(); for (int i = 0; i < 10; i++) { @@ -47,15 +45,13 @@ public void asyncSpans() { futures.add(otExecutor.submit(new Runnable() { @Override public void run() { - final Span child = tracer.buildSpan("child_" + j).start(); - SpanManager.SpanClosure childSpanClosure = tracer.activeSpanManager().captureActive(); - childSpanClosure.activate(); + SpanManager.SpanClosure childSpanClosure = tracer.buildSpan("child_" + j).startAndActivate(); try { Thread.currentThread().sleep(1000); } catch (InterruptedException e) { e.printStackTrace(); } - child.log("awoke"); + tracer.activeSpanManager().active().log("awoke"); Runnable r = new Runnable() { @Override public void run() { diff --git a/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/MDCSpanManager.java b/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/MDCSpanManager.java index a3c15f6b..c3880d09 100644 --- a/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/MDCSpanManager.java +++ b/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/MDCSpanManager.java @@ -30,6 +30,11 @@ public Span activate() { return span; } + @Override + public Span span() { + return span; + } + @Override public void deactivate(boolean finishSpan) { if (span != null && finishSpan) { 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 8c7c43ce..f3d9e6dc 100644 --- a/opentracing-mock/src/main/java/io/opentracing/mock/MockTracer.java +++ b/opentracing-mock/src/main/java/io/opentracing/mock/MockTracer.java @@ -243,6 +243,12 @@ public MockSpan start() { return rval; } + @Override + public SpanManager.SpanClosure startAndActivate() { + MockSpan span = this.start(); + return MockTracer.this.spanManager.capture(span); + } + @Override public Iterable> baggageItems() { if (firstParent == null) { diff --git a/opentracing-noop/src/main/java/io/opentracing/NoopSpanBuilder.java b/opentracing-noop/src/main/java/io/opentracing/NoopSpanBuilder.java index 30c5e20c..85e85979 100644 --- a/opentracing-noop/src/main/java/io/opentracing/NoopSpanBuilder.java +++ b/opentracing-noop/src/main/java/io/opentracing/NoopSpanBuilder.java @@ -62,6 +62,9 @@ public Span start() { return NoopSpanImpl.INSTANCE; } + @Override + public SpanManager.SpanClosure startAndActivate() { return null; } // XXX: not correct + @Override public Iterable> baggageItems() { return Collections.EMPTY_MAP.entrySet(); From 396abd2e085a5fcd1871146d37c38c2c5d91dec8 Mon Sep 17 00:00:00 2001 From: Ben Sigelman Date: Sat, 11 Mar 2017 15:35:54 -0800 Subject: [PATCH 23/23] Make SpanClosure autocloseable --- opentracing-api/pom.xml | 4 +- .../main/java/io/opentracing/SpanManager.java | 2 +- .../java/io/opentracing/mdcdemo/MDCDemo.java | 72 ++++++++++--------- .../opentracing/mdcdemo/MDCSpanManager.java | 5 ++ 4 files changed, 47 insertions(+), 36 deletions(-) diff --git a/opentracing-api/pom.xml b/opentracing-api/pom.xml index defd930c..0c44e104 100644 --- a/opentracing-api/pom.xml +++ b/opentracing-api/pom.xml @@ -29,8 +29,8 @@ ${project.basedir}/.. - 1.6 - java16 + 1.7 + java17 diff --git a/opentracing-api/src/main/java/io/opentracing/SpanManager.java b/opentracing-api/src/main/java/io/opentracing/SpanManager.java index ae951a1e..7bf45355 100644 --- a/opentracing-api/src/main/java/io/opentracing/SpanManager.java +++ b/opentracing-api/src/main/java/io/opentracing/SpanManager.java @@ -16,7 +16,7 @@ public interface SpanManager { * * @see SpanManager#captureActive() */ - interface SpanClosure { + interface SpanClosure extends AutoCloseable { /** * Make the Span encapsulated by this SpanClosure active and return it. diff --git a/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/MDCDemo.java b/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/MDCDemo.java index 86760e0f..59b080b3 100644 --- a/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/MDCDemo.java +++ b/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/MDCDemo.java @@ -35,38 +35,49 @@ public void trivialChild() { public void asyncSpans() { final Tracer tracer = this.tracer; // save typing + // Create an ExecutorService and wrap it in a TracedExecutorService. ExecutorService realExecutor = Executors.newFixedThreadPool(500); final ExecutorService otExecutor = new TracedExecutorService(realExecutor, tracer.activeSpanManager()); - SpanManager.SpanClosure parentSpanClosure = tracer.buildSpan("parent").startAndActivate(); + + // Hacky lists of futures we wait for before exiting asyncSpans. final List> futures = new ArrayList<>(); final List> subfutures = new ArrayList<>(); - for (int i = 0; i < 10; i++) { - final int j = i; - futures.add(otExecutor.submit(new Runnable() { - @Override - public void run() { - SpanManager.SpanClosure childSpanClosure = tracer.buildSpan("child_" + j).startAndActivate(); - try { - Thread.currentThread().sleep(1000); - } catch (InterruptedException e) { - e.printStackTrace(); + + // Create a parent SpanClosure for all of the async activity. + try (SpanManager.SpanClosure parentSpanClosure = tracer.buildSpan("parent").startAndActivate();) { + + // Create 10 async children. + for (int i = 0; i < 10; i++) { + final int j = i; + futures.add(otExecutor.submit(new Runnable() { + @Override + public void run() { + // START child body + + try (SpanManager.SpanClosure childSpanClosure = + tracer.buildSpan("child_" + j).startAndActivate();) { + Thread.currentThread().sleep(1000); + childSpanClosure.span().log("awoke"); + Runnable r = new Runnable() { + @Override + public void run() { + Span active = tracer.activeSpanManager().active(); + active.log("awoke again"); + // Create a grandchild for each child. + Span grandchild = tracer.buildSpan("grandchild_" + j).start(); + grandchild.finish(); + active.finish(); + } + }; + subfutures.add(otExecutor.submit(r)); + } catch (Exception e) { } + + // END child body } - tracer.activeSpanManager().active().log("awoke"); - Runnable r = new Runnable() { - @Override - public void run() { - Span active = tracer.activeSpanManager().active(); - active.log("awoke again"); - Span grandchild = tracer.buildSpan("grandchild_" + j).start(); - grandchild.finish(); - active.finish(); - } - }; - subfutures.add(otExecutor.submit(r)); - childSpanClosure.deactivate(false); - } - })); - } + })); + } + } catch (Exception e) { } + try { for (Future f : futures) { f.get(); @@ -74,11 +85,7 @@ public void run() { for (Future f : subfutures) { f.get(); } - } catch (InterruptedException e) { - e.printStackTrace(); - } catch (ExecutionException e) { - e.printStackTrace(); - } + } catch (Exception e) { } otExecutor.shutdown(); try { @@ -86,7 +93,6 @@ public void run() { } catch (InterruptedException e) { e.printStackTrace(); } - parentSpanClosure.deactivate(true); } public static void main(String[] args) { diff --git a/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/MDCSpanManager.java b/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/MDCSpanManager.java index c3880d09..5fff078d 100644 --- a/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/MDCSpanManager.java +++ b/opentracing-mdc-demo/src/main/java/io/opentracing/mdcdemo/MDCSpanManager.java @@ -30,6 +30,11 @@ public Span activate() { return span; } + @Override + public void close() { + this.deactivate(true); + } + @Override public Span span() { return span;