diff --git a/opentracing-api/src/main/java/io/opentracing/ScopeManager.java b/opentracing-api/src/main/java/io/opentracing/ScopeManager.java index 2bef978b..ec150e6c 100644 --- a/opentracing-api/src/main/java/io/opentracing/ScopeManager.java +++ b/opentracing-api/src/main/java/io/opentracing/ScopeManager.java @@ -40,7 +40,8 @@ public interface ScopeManager { * This {@link Scope} instance can be accessed at any time through {@link #active()}, * in case it is not possible for the user to store it (when used through middleware * or start/finish event hooks, for example). The corresponding {@link Span} can be - * accessed through {@link #activeSpan()} likewise. + * accessed through {@link #activeSpan()} likewise. You can also get ahold of the + * active {@link Span}'s {@link SpanContext} via {@link #activeSpanContext()}. * *

* Usage: @@ -58,12 +59,65 @@ public interface ScopeManager { * } * * + *

+ * Note: You may only activate spans when you own its life cycle. + * That means you must make sure that no other thread calls {@link Span#finish()} + * while the scope is still active. + * If you can't guarantee that, use {@link #activate(SpanContext)} instead. + * * @param span the {@link Span} that should become the {@link #activeSpan()} * @return a {@link Scope} instance to control the end of the active period for the {@link Span}. It is a * programming error to neglect to call {@link Scope#close()} on the returned instance. */ Scope activate(Span span); + /** + * Similar to {@link #activate(Span)} but used in cases where the thread in which the + * activation is performed does not have control over the life cycle of the span. + * + *

+ * One example of that is when performing an activation in the {@link Runnable#run()} + * method of a traced {@link Runnable} wrapper which is executed by an + * {@link java.util.concurrent.ExecutorService}. + * + *

+ * The returned {@link Scope} represents the active state for the span. + * Once its active period is due, {@link Scope#close()} ought to be called. + * To ease this operation, {@link Scope} supports try-with-resources. + * Observe the span will not be automatically finished when {@link Scope#close()} + * is called. + * + *

+ * This {@link Scope} instance can be accessed at any time through {@link #active()}, + * in case it is not possible for the user to store it (when used through middleware + * or start/finish event hooks, for example). The corresponding {@link SpanContext} can be + * accessed through {@link #activeSpanContext()} likewise. + * In contrast to {@link #activate(Span)}, {@link #activeSpan()} will return {@code null}. + * This prevents users of the {@link #activeSpan()} API to accidentally interacting with + * already {@linkplain Span#finish() finished} spans. + * + * Usage: + *


+     *     Span span = tracer.buildSpan("...").start();
+     *     try (Scope scope = tracer.scopeManager().activate(span.context())) {
+     *         span.setTag("...", "...");
+     *         ...
+     *     } catch (Exception e) {
+     *         span.log(...);
+     *     } finally {
+     *         // Optionally finish the Span if the operation it represents
+     *         // is logically completed at this point.
+     *         span.finish();
+     *     }
+     * 
+ * + * @param spanContext the {@link SpanContext} that should become the {@link #activeSpanContext()} + * @see #activate(Span) + * @return a {@link Scope} instance to control the end of the active period for the {@link Span}. It is a + * programming error to neglect to call {@link Scope#close()} on the returned instance. + */ + Scope activate(SpanContext spanContext); + /** * Return the currently active {@link Scope} which can be used to deactivate the currently active * {@link Span}. @@ -84,13 +138,25 @@ public interface ScopeManager { * Return the currently active {@link Span}. * *

- * Because both {@link #active()} and {@link #activeSpan()} reference the current - * active state, they both will be either null or non-null. + * Note that {@link #activeSpan()} can return {@code null} while {@link #active()} is non-null + * in case of a {@linkplain #activate(SpanContext) span context activation}. * * @return the {@link Span active span}, or null if none could be found. */ Span activeSpan(); + /** + * Return the currently active {@link SpanContext}, which was activated by activating either + * a {@link #activate(Span) Span} or a {@link #activate(SpanContext) SpanContext}. + * + *

+ * Because both {@link #active()} and {@link #activeSpanContext()} reference the current + * active state, they both will be either null or non-null. + * + * @return the {@link SpanContext active span context}, or null if none could be found. + */ + SpanContext activeSpanContext(); + /** * @deprecated use {@link #activate(Span)} instead. * Set the specified {@link Span} as the active instance for the current diff --git a/opentracing-api/src/main/java/io/opentracing/Tracer.java b/opentracing-api/src/main/java/io/opentracing/Tracer.java index edd263f3..4812bef8 100644 --- a/opentracing-api/src/main/java/io/opentracing/Tracer.java +++ b/opentracing-api/src/main/java/io/opentracing/Tracer.java @@ -31,6 +31,11 @@ public interface Tracer { */ Span activeSpan(); + /** + * @return the active {@link SpanContext}. This is a shorthand for {@code Tracer.scopeManager().activeSpanContext()}. + */ + SpanContext activeSpanContext(); + /** * Make a {@link Span} instance active for the current context (usually a thread). * This is a shorthand for {@code Tracer.scopeManager().activate(span)}. @@ -41,6 +46,18 @@ public interface Tracer { */ Scope activateSpan(Span span); + + /** + * Make a {@link SpanContext} instance active for the current context (usually a thread). + * This is a shorthand for {@code Tracer.scopeManager().activate(spanContext)}. + * + * @see {@link ScopeManager#activate(SpanContext)} + * @return a {@link Scope} instance to control the end of the active period for the {@link SpanContext}. It is a + * programming error to neglect to call {@link Scope#close()} on the returned instance, + * and it may lead to memory leaks as the {@link Scope} may remain in the thread-local stack. + */ + Scope activateSpanContext(SpanContext spanContext); + /** * Return a new SpanBuilder for a Span with the given `operationName`. * 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 36336314..c6f0b45a 100644 --- a/opentracing-mock/src/main/java/io/opentracing/mock/MockTracer.java +++ b/opentracing-mock/src/main/java/io/opentracing/mock/MockTracer.java @@ -30,7 +30,6 @@ import io.opentracing.Span; import io.opentracing.SpanContext; import io.opentracing.Tracer; -import io.opentracing.noop.NoopScopeManager; import io.opentracing.propagation.Binary; import io.opentracing.propagation.Format; import io.opentracing.propagation.TextMap; @@ -277,18 +276,19 @@ public Scope activateSpan(Span span) { return this.scopeManager.activate(span); } + @Override + public Scope activateSpanContext(SpanContext spanContext) { + return this.scopeManager.activate(spanContext); + } + synchronized void appendFinishedSpan(MockSpan mockSpan) { this.finishedSpans.add(mockSpan); this.onSpanFinished(mockSpan); } - private SpanContext activeSpanContext() { - Span span = activeSpan(); - if (span == null) { - return null; - } - - return span.context(); + @Override + public SpanContext activeSpanContext() { + return scopeManager.activeSpanContext(); } public final class SpanBuilder implements Tracer.SpanBuilder { 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 99fbc2fc..291d96b4 100644 --- a/opentracing-mock/src/test/java/io/opentracing/mock/MockTracerTest.java +++ b/opentracing-mock/src/test/java/io/opentracing/mock/MockTracerTest.java @@ -261,6 +261,23 @@ public void testActiveSpan() { Assert.assertTrue(mockTracer.finishedSpans().isEmpty()); } + @Test + public void testActiveSpanContext() { + MockTracer mockTracer = new MockTracer(); + Assert.assertNull(mockTracer.activeSpan()); + Assert.assertNull(mockTracer.activeSpanContext()); + + Span span = mockTracer.buildSpan("foo").start(); + try (Scope scope = mockTracer.activateSpanContext(span.context())) { + Assert.assertNull(mockTracer.activeSpan()); + Assert.assertEquals(mockTracer.scopeManager().activeSpanContext(), mockTracer.activeSpanContext()); + } + + Assert.assertNull(mockTracer.activeSpan()); + Assert.assertNull(mockTracer.activeSpanContext()); + Assert.assertTrue(mockTracer.finishedSpans().isEmpty()); + } + @Test public void testActiveSpanFinish() { MockTracer mockTracer = new MockTracer(); diff --git a/opentracing-noop/src/main/java/io/opentracing/noop/NoopScopeManager.java b/opentracing-noop/src/main/java/io/opentracing/noop/NoopScopeManager.java index 911ac5a1..f3dfce19 100644 --- a/opentracing-noop/src/main/java/io/opentracing/noop/NoopScopeManager.java +++ b/opentracing-noop/src/main/java/io/opentracing/noop/NoopScopeManager.java @@ -16,6 +16,7 @@ import io.opentracing.Scope; import io.opentracing.ScopeManager; import io.opentracing.Span; +import io.opentracing.SpanContext; public interface NoopScopeManager extends ScopeManager { NoopScopeManager INSTANCE = new NoopScopeManagerImpl(); @@ -39,6 +40,11 @@ public Scope activate(Span span) { return NoopScope.INSTANCE; } + @Override + public Scope activate(SpanContext spanContext) { + return NoopScope.INSTANCE; + } + @Override public Scope active() { return NoopScope.INSTANCE; @@ -49,6 +55,11 @@ public Span activeSpan() { return NoopSpan.INSTANCE; } + @Override + public SpanContext activeSpanContext() { + return NoopSpanContextImpl.INSTANCE; + } + static class NoopScopeImpl implements NoopScopeManager.NoopScope { @Override public void close() {} diff --git a/opentracing-noop/src/main/java/io/opentracing/noop/NoopTracer.java b/opentracing-noop/src/main/java/io/opentracing/noop/NoopTracer.java index abed3ad4..c1e26381 100644 --- a/opentracing-noop/src/main/java/io/opentracing/noop/NoopTracer.java +++ b/opentracing-noop/src/main/java/io/opentracing/noop/NoopTracer.java @@ -36,11 +36,21 @@ public Span activeSpan() { return NoopSpanImpl.INSTANCE; } + @Override + public SpanContext activeSpanContext() { + return NoopSpanContextImpl.INSTANCE; + } + @Override public Scope activateSpan(Span span) { return NoopScopeManager.NoopScope.INSTANCE; } + @Override + public Scope activateSpanContext(SpanContext spanContext) { + return NoopScopeManager.NoopScope.INSTANCE; + } + @Override public SpanBuilder buildSpan(String operationName) { return NoopSpanBuilderImpl.INSTANCE; } diff --git a/opentracing-testbed/pom.xml b/opentracing-testbed/pom.xml index 89200e51..69167325 100644 --- a/opentracing-testbed/pom.xml +++ b/opentracing-testbed/pom.xml @@ -28,6 +28,7 @@ OpenTracing Testbed + 1.8 ${project.basedir}/.. diff --git a/opentracing-testbed/src/test/java/io/opentracing/testbed/TestUtils.java b/opentracing-testbed/src/test/java/io/opentracing/testbed/TestUtils.java index ba2332fc..e28b1310 100644 --- a/opentracing-testbed/src/test/java/io/opentracing/testbed/TestUtils.java +++ b/opentracing-testbed/src/test/java/io/opentracing/testbed/TestUtils.java @@ -26,6 +26,7 @@ import java.util.concurrent.TimeUnit; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; public class TestUtils { @@ -90,9 +91,8 @@ public int compare(MockSpan o1, MockSpan o2) { public static void assertSameTrace(List spans) { for (int i = 0; i < spans.size() - 1; i++) { - assertEquals(true, spans.get(spans.size() - 1).finishMicros() >= spans.get(i).finishMicros()); + assertTrue(spans.get(spans.size() - 1).finishMicros() >= spans.get(i).finishMicros()); assertEquals(spans.get(spans.size() - 1).context().traceId(), spans.get(i).context().traceId()); - assertEquals(spans.get(spans.size() - 1).context().spanId(), spans.get(i).parentId()); } } } diff --git a/opentracing-testbed/src/test/java/io/opentracing/testbed/early_span_finish/EarlySpanFinishTest.java b/opentracing-testbed/src/test/java/io/opentracing/testbed/early_span_finish/EarlySpanFinishTest.java new file mode 100644 index 00000000..ee10c041 --- /dev/null +++ b/opentracing-testbed/src/test/java/io/opentracing/testbed/early_span_finish/EarlySpanFinishTest.java @@ -0,0 +1,119 @@ +/* + * Copyright 2016-2018 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.testbed.early_span_finish; + +import io.opentracing.Scope; +import io.opentracing.Span; +import io.opentracing.mock.MockSpan; +import io.opentracing.mock.MockTracer; +import io.opentracing.mock.MockTracer.Propagator; +import io.opentracing.util.ThreadLocalScopeManager; +import org.junit.Test; + +import java.util.List; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; + +import static io.opentracing.testbed.TestUtils.assertSameTrace; +import static io.opentracing.testbed.TestUtils.sleep; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; + +public class EarlySpanFinishTest { + + private final MockTracer tracer = new MockTracer(new ThreadLocalScopeManager(), + Propagator.TEXT_MAP); + private final ExecutorService executor = Executors.newCachedThreadPool(); + + @Test + public void test() throws Exception { + // Create a Span manually and use it as parent of a pair of subtasks + Span parentSpan = tracer.buildSpan("parent").start(); + submitTasks(parentSpan); + + // Early-finish the parent Span now + parentSpan.finish(); + + // Wait for the threadpool to be done first, instead of polling/waiting + executor.shutdown(); + executor.awaitTermination(15, TimeUnit.SECONDS); + + + List spans = tracer.finishedSpans(); + assertEquals(3, spans.size()); + assertEquals("parent", spans.get(0).operationName()); + assertEquals(1, spans.get(0).generatedErrors().size()); + assertEquals("task1", spans.get(1).operationName()); + assertEquals("task2", spans.get(2).operationName()); + assertSameTrace(spans); + + assertNull(tracer.scopeManager().active()); + } + + + /** + * Fire away a few subtasks, passing a parent Span whose lifetime + * is not tied at-all to the children + */ + private void submitTasks(final Span parentSpan) { + + executor.submit(new Runnable() { + @Override + public void run() { + // Activating the parent span is illegal as we don't have control over its life cycle. + // The caller of tracer.activeSpan() has no way of knowing whether the active span is already finished. + try (Scope scope = tracer.scopeManager().activate(parentSpan)) { + Span childSpan = tracer.buildSpan("task1").start(); + try (Scope childScope = tracer.scopeManager().activate(childSpan)) { + sleep(55); + childSpan.setTag("foo", "bar"); + } finally { + childSpan.finish(); + } + assertNotNull(tracer.activeSpan()); + // this fails as the parent span is already finished + tracer.activeSpan().setTag("foo", "bar"); + } + } + }); + + executor.submit(new Runnable() { + @Override + public void run() { + // We don't own the lifecycle of the parent span, + // therefore we must activate the span context + // tracer.activeSpan() will then always return null to make the intention clear + // that interacting with the parent span is not possible. + // This puts the burden of being aware of the lifecycle to the person writing the instrumentation + // (for example io.opentracing.contrib.concurrent.TracedRunnable) + // instead of the end users, who only want to customize spans. + try (Scope scope = tracer.scopeManager().activate(parentSpan.context())) { + Span childSpan = tracer.buildSpan("task2").start(); + try (Scope childScope = tracer.scopeManager().activate(childSpan)) { + sleep(85); + childSpan.setTag("foo", "bar"); + } finally { + childSpan.finish(); + } + assertNull(tracer.activeSpan()); + assertNotNull(tracer.activeSpanContext()); + } + } + }); + } + +} + diff --git a/opentracing-util/src/main/java/io/opentracing/util/AutoFinishScope.java b/opentracing-util/src/main/java/io/opentracing/util/AutoFinishScope.java index ad03c8cd..9ff7b790 100644 --- a/opentracing-util/src/main/java/io/opentracing/util/AutoFinishScope.java +++ b/opentracing-util/src/main/java/io/opentracing/util/AutoFinishScope.java @@ -16,6 +16,7 @@ import io.opentracing.Scope; import io.opentracing.Span; +import io.opentracing.SpanContext; import java.util.concurrent.atomic.AtomicInteger; @@ -24,12 +25,22 @@ public class AutoFinishScope implements Scope { final AtomicInteger refCount; private final Span wrapped; private final AutoFinishScope toRestore; + private final SpanContext spanContext; AutoFinishScope(AutoFinishScopeManager manager, AtomicInteger refCount, Span wrapped) { + this(manager, refCount, wrapped, null); + } + + AutoFinishScope(AutoFinishScopeManager manager, SpanContext spanContext) { + this(manager, new AtomicInteger(), null, spanContext); + } + + private AutoFinishScope(AutoFinishScopeManager manager, AtomicInteger refCount, Span wrapped, SpanContext spanContext) { this.manager = manager; this.refCount = refCount; this.wrapped = wrapped; this.toRestore = manager.tlsScope.get(); + this.spanContext = spanContext; manager.tlsScope.set(this); } @@ -53,7 +64,7 @@ public void close() { return; } - if (refCount.decrementAndGet() == 0) { + if (refCount.decrementAndGet() == 0 && wrapped != null) { wrapped.finish(); } @@ -64,4 +75,8 @@ public void close() { public Span span() { return wrapped; } + + SpanContext spanContext() { + return wrapped != null ? wrapped.context() : spanContext; + } } diff --git a/opentracing-util/src/main/java/io/opentracing/util/AutoFinishScopeManager.java b/opentracing-util/src/main/java/io/opentracing/util/AutoFinishScopeManager.java index 0a60deb3..f846d95e 100644 --- a/opentracing-util/src/main/java/io/opentracing/util/AutoFinishScopeManager.java +++ b/opentracing-util/src/main/java/io/opentracing/util/AutoFinishScopeManager.java @@ -13,8 +13,10 @@ */ package io.opentracing.util; +import io.opentracing.Scope; import io.opentracing.ScopeManager; import io.opentracing.Span; +import io.opentracing.SpanContext; import java.util.concurrent.atomic.AtomicInteger; @@ -31,6 +33,11 @@ public AutoFinishScope activate(Span span) { return new AutoFinishScope(this, new AtomicInteger(1), span); } + @Override + public Scope activate(SpanContext spanContext) { + return new AutoFinishScope(this, spanContext); + } + @Override public AutoFinishScope active() { return tlsScope.get(); @@ -41,4 +48,10 @@ public Span activeSpan() { AutoFinishScope scope = tlsScope.get(); return scope == null ? null : scope.span(); } + + @Override + public SpanContext activeSpanContext() { + AutoFinishScope scope = tlsScope.get(); + return scope == null ? null : scope.spanContext(); + } } diff --git a/opentracing-util/src/main/java/io/opentracing/util/GlobalTracer.java b/opentracing-util/src/main/java/io/opentracing/util/GlobalTracer.java index e463e2ff..a72b628d 100644 --- a/opentracing-util/src/main/java/io/opentracing/util/GlobalTracer.java +++ b/opentracing-util/src/main/java/io/opentracing/util/GlobalTracer.java @@ -179,11 +179,21 @@ public Span activeSpan() { return tracer.activeSpan(); } + @Override + public SpanContext activeSpanContext() { + return tracer.activeSpanContext(); + } + @Override public Scope activateSpan(Span span) { return tracer.activateSpan(span); } + @Override + public Scope activateSpanContext(SpanContext spanContext) { + return tracer.activateSpanContext(spanContext); + } + @Override public String toString() { return GlobalTracer.class.getSimpleName() + '{' + tracer + '}'; diff --git a/opentracing-util/src/main/java/io/opentracing/util/ThreadLocalScope.java b/opentracing-util/src/main/java/io/opentracing/util/ThreadLocalScope.java index 93d39339..dfcebecf 100644 --- a/opentracing-util/src/main/java/io/opentracing/util/ThreadLocalScope.java +++ b/opentracing-util/src/main/java/io/opentracing/util/ThreadLocalScope.java @@ -16,6 +16,7 @@ import io.opentracing.Scope; import io.opentracing.ScopeManager; import io.opentracing.Span; +import io.opentracing.SpanContext; /** * {@link ThreadLocalScope} is a simple {@link Scope} implementation that relies on Java's @@ -28,16 +29,26 @@ public class ThreadLocalScope implements Scope { private final Span wrapped; private final boolean finishOnClose; private final ThreadLocalScope toRestore; + private final SpanContext spanContext; ThreadLocalScope(ThreadLocalScopeManager scopeManager, Span wrapped) { - this(scopeManager, wrapped, false); + this(scopeManager, wrapped, null, false); + } + + ThreadLocalScope(ThreadLocalScopeManager scopeManager, SpanContext spanContext) { + this(scopeManager, null, spanContext, false); } ThreadLocalScope(ThreadLocalScopeManager scopeManager, Span wrapped, boolean finishOnClose) { + this(scopeManager, wrapped, null, finishOnClose); + } + + private ThreadLocalScope(ThreadLocalScopeManager scopeManager, Span wrapped, SpanContext spanContext, boolean finishOnClose) { this.scopeManager = scopeManager; this.wrapped = wrapped; this.finishOnClose = finishOnClose; this.toRestore = scopeManager.tlsScope.get(); + this.spanContext = spanContext; scopeManager.tlsScope.set(this); } @@ -59,4 +70,8 @@ public void close() { public Span span() { return wrapped; } + + SpanContext spanContext() { + return wrapped != null ? wrapped.context() : spanContext; + } } diff --git a/opentracing-util/src/main/java/io/opentracing/util/ThreadLocalScopeManager.java b/opentracing-util/src/main/java/io/opentracing/util/ThreadLocalScopeManager.java index 433f4bc0..d9cf7546 100644 --- a/opentracing-util/src/main/java/io/opentracing/util/ThreadLocalScopeManager.java +++ b/opentracing-util/src/main/java/io/opentracing/util/ThreadLocalScopeManager.java @@ -16,6 +16,7 @@ import io.opentracing.Scope; import io.opentracing.ScopeManager; import io.opentracing.Span; +import io.opentracing.SpanContext; /** * A simple {@link ScopeManager} implementation built on top of Java's thread-local storage primitive. @@ -35,6 +36,11 @@ public Scope activate(Span span) { return new ThreadLocalScope(this, span); } + @Override + public Scope activate(SpanContext spanContext) { + return new ThreadLocalScope(this, spanContext); + } + @Override public Scope active() { return tlsScope.get(); @@ -45,4 +51,10 @@ public Span activeSpan() { Scope scope = tlsScope.get(); return scope == null ? null : scope.span(); } + + @Override + public SpanContext activeSpanContext() { + ThreadLocalScope scope = tlsScope.get(); + return scope == null ? null : scope.spanContext(); + } }