Skip to content
This repository has been archived by the owner on May 23, 2023. It is now read-only.

Commit

Permalink
Define only activate(s, bool)
Browse files Browse the repository at this point in the history
  • Loading branch information
pavolloffay committed Dec 1, 2017
1 parent 8cb95fb commit 6423db2
Show file tree
Hide file tree
Showing 15 changed files with 22 additions and 106 deletions.
30 changes: 7 additions & 23 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,21 +44,18 @@ if (scope != null) {
### Starting a new Span

The common case starts a `Scope` that's automatically registered for intra-process propagation via `ScopeManager`.
Note that the default behaviour of `startActive()` does not finish the span on `Scope.close()`.
This decision was made on purpose because the `try-with-resources` construct would finish the span before
the `catch` or `finally` blocks are executed, which makes logging exceptions and setting tags impossible.

This behaviour forces users to intercept all exceptional states rather than inadvertently
finishing a span before a fallback action takes place. In the most case OpenTracing API
is predominantly used in framework instrumentations where this behaviour is used most.
Auto-finishing can be enabled with convenient `startActive(finishOnClose)` which is demonstrated
later in this section.
Note that `startActive(true)` finishes the span on `Scope.close()`.
Use it carefully because the `try-with-resources` construct finishes the span before
the `catch` or `finally` blocks are executed, which makes logging exceptions and
setting tags impossible. It is recommended to start the span and activate it later in `try-with-resources`.
This makes the span available in catch and finally blocks.

```java
io.opentracing.Tracer tracer = ...;
...
Span span = tracer.buildSpan("someWork").startManual();
try (Scope scope = tracer.scopeManager().activate(span))
Span span = tracer.buildSpan("someWork").start();
try (Scope scope = tracer.scopeManager().activate(span, false))
// Do things.
} catch {
Tags.ERROR.set(scope.span(), true);
Expand All @@ -79,19 +76,6 @@ try (Scope scope = tracer.buildSpan("someWork").startActive(true)) {
}
```

To manually step around the `ScopeManager` registration, use `startManual()`, like this:

```java
io.opentracing.Tracer tracer = ...;
...
Span span = tracer.buildSpan("someWork").startManual();
try {
// (do things / record data to `span`)
} finally {
span.finish();
}
```

**If there is a `Scope`, it will act as the parent to any newly started `Span`** unless
the programmer invokes `ignoreActiveSpan()` at `buildSpan()` time or specified parent context explicitly:

Expand Down
13 changes: 2 additions & 11 deletions opentracing-api/src/main/java/io/opentracing/ScopeManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,22 +15,13 @@

/**
* The {@link ScopeManager} interface abstracts both the activation of {@link Span} instances (via
* {@link ScopeManager#activate(Span)}) and access to an active {@link Span}/{@link Scope}
* {@link ScopeManager#activate(Span, boolean)}) and access to an active {@link Span}/{@link Scope}
* (via {@link ScopeManager#active()}).
*
* @see Scope
* @see Tracer#scopeManager()
*/
public interface ScopeManager {
/**
* Make a {@link Span} instance active.
*
* @param span the {@link Span} that should become the {@link #active()}
* @return a {@link Scope} instance to control the end of the active period for the {@link Span}.
* Span will not automatically be finished when {@link Scope#close()} is called. It is a
* programming error to neglect to call {@link Scope#close()} on the returned instance.
*/
Scope activate(Span span);

/**
* Make a {@link Span} instance active.
Expand All @@ -48,7 +39,7 @@ public interface ScopeManager {
*
* <p>
* If there is an {@link Scope non-null scope}, its wrapped {@link Span} becomes an implicit parent of any
* newly-created {@link Span} at {@link Tracer.SpanBuilder#startActive()} time (rather than at
* newly-created {@link Span} at {@link Tracer.SpanBuilder#startActive(boolean)} time (rather than at
* {@link Tracer#buildSpan(String)} time).
*
* @return the {@link Scope active scope}, or null if none could be found.
Expand Down
40 changes: 3 additions & 37 deletions opentracing-api/src/main/java/io/opentracing/Tracer.java
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ interface SpanBuilder {
* <li>{@link SpanBuilder#ignoreActiveSpan()} is not invoked,
* </ul>
* ... then an inferred {@link References#CHILD_OF} reference is created to the
* {@link ScopeManager#active()} {@link SpanContext} when either {@link SpanBuilder#startActive()} or
* {@link ScopeManager#active()} {@link SpanContext} when either {@link SpanBuilder#startActive(boolean)} or
* {@link SpanBuilder#startManual} is invoked.
*
* @param referenceType the reference type, typically one of the constants defined in References
Expand Down Expand Up @@ -159,40 +159,6 @@ interface SpanBuilder {
/** Specify a timestamp of when the Span was started, represented in microseconds since epoch. */
SpanBuilder withStartTimestamp(long microseconds);

/**
* Returns a newly started and activated {@link Scope}.
*
* <p>
* The returned {@link Scope} supports try-with-resources. For example:
* <pre><code>
* Span span = tracer.buildSpan("...").startManual();
* try (Scope scope = tracer.scopeManager().activate(span))
* // (Do work)
* scope.span().setTag( ... ); // etc, etc
* } catch(Exception ex) {
* Tags.ERROR.set(span, true)'
* } finally {
* span.finish();
* }
* // Span is not automatically finished when the Scope is closed,
* // following the default behavior of ScopeManager.activate(Span)
* </code></pre>
*
* <p>
* For detailed information, see {@link SpanBuilder#startActive(boolean)}
*
* <p>
* Note: {@link SpanBuilder#startActive()} is a shorthand for
* {@code tracer.scopeManager().activate(spanBuilder.startManual())}.
*
* @return a {@link Scope}, already registered via the {@link ScopeManager}
*
* @see ScopeManager
* @see Scope
* @see SpanBuilder#startActive(boolean)
*/
Scope startActive();

/**
* Returns a newly started and activated {@link Scope}.
*
Expand Down Expand Up @@ -230,10 +196,10 @@ interface SpanBuilder {
Scope startActive(boolean finishSpanOnClose);

/**
* Like {@link #startActive()}, but the returned {@link Span} has not been registered via the
* Like {@link #startActive(boolean)}, but the returned {@link Span} has not been registered via the
* {@link ScopeManager}.
*
* @see SpanBuilder#startActive()
* @see SpanBuilder#startActive(boolean)
* @return the newly-started Span instance, which has *not* been automatically registered
* via the {@link ScopeManager}
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,6 @@
public class AutoFinishScopeManager implements ScopeManager {
final ThreadLocal<AutoFinishScope> tlsScope = new ThreadLocal<AutoFinishScope>();

@Override
public AutoFinishScope activate(Span span) {
return new AutoFinishScope(this, new AtomicInteger(1), span);
}

@Override
public AutoFinishScope activate(Span span, boolean finishOnClose) {
return new AutoFinishScope(this, new AtomicInteger(1), span);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ private Thread entryThread() {
@Override
public void run() {
logger.info("Entry thread started");
try (Scope scope = tracer.buildSpan("parent").startActive()) {
try (Scope scope = tracer.buildSpan("parent").startActive(false)) {
Runnable action = new RunnableAction((AutoFinishScope)scope);

// Action is executed at some time and we are not able to check status
Expand All @@ -108,7 +108,7 @@ private Thread entryThreadWithTwoActions() {
@Override
public void run() {
logger.info("Entry thread 2x started");
try (Scope scope = tracer.buildSpan("parent").startActive()) {
try (Scope scope = tracer.buildSpan("parent").startActive(false)) {
Runnable action = new RunnableAction((AutoFinishScope)scope);
Runnable action2 = new RunnableAction((AutoFinishScope)scope);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ private void submitTasks(final Span parentSpan) {
@Override
public void run() {
// Alternative to calling makeActive() is to pass it manually to asChildOf() for each created Span.
try (Scope scope = tracer.scopeManager().activate(parentSpan)) {
try (Scope scope = tracer.scopeManager().activate(parentSpan, false)) {
try (Scope childScope1 = tracer.buildSpan("task1").startActive(true)) {
sleep(55);
}
Expand All @@ -83,7 +83,7 @@ public void run() {
executor.submit(new Runnable() {
@Override
public void run() {
try (Scope span = tracer.scopeManager().activate(parentSpan)) {
try (Scope span = tracer.scopeManager().activate(parentSpan, false)) {
try (Scope childScope2 = tracer.buildSpan("task2").startActive(true)) {
sleep(85);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public Object call() throws Exception {
logger.info("Child thread with message '{}' started", message);

try (Scope parentScope = cont.activate()) {
try (Scope subtaskScope = tracer.buildSpan("subtask").startActive()) {
try (Scope subtaskScope = tracer.buildSpan("subtask").startActive(false)) {
// Simulate work.
sleep(milliseconds);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public class MultipleCallbacksTest {
@Test
public void test() throws Exception {
Client client = new Client(tracer);
try (Scope scope = tracer.buildSpan("parent").startActive()) {
try (Scope scope = tracer.buildSpan("parent").startActive(false)) {
client.send("task1", 300);
client.send("task2", 200);
client.send("task3", 100);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public class NestedCallbacksTest {
@Test
public void test() throws Exception {

try (Scope scope = tracer.buildSpan("one").startActive()) {
try (Scope scope = tracer.buildSpan("one").startActive(false)) {
submitCallbacks(scope.span());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -247,11 +247,6 @@ public SpanBuilder withStartTimestamp(long microseconds) {
return this;
}

@Override
public Scope startActive() {
return MockTracer.this.scopeManager().activate(this.startManual());
}

@Override
public Scope startActive(boolean finishOnClose) {
return MockTracer.this.scopeManager().activate(this.startManual(), finishOnClose);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,6 @@ interface NoopScope extends Scope {
* A noop (i.e., cheap-as-possible) implementation of an ScopeManager.
*/
class NoopScopeManagerImpl implements NoopScopeManager {
@Override
public Scope activate(Span span) {
return NoopScope.INSTANCE;
}

@Override
public Scope activate(Span span, boolean finishOnClose) {
return NoopScope.INSTANCE;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,6 @@ public Tracer.SpanBuilder withStartTimestamp(long microseconds) {
return this;
}

@Override
public Scope startActive() {
return NoopScopeManager.NoopScope.INSTANCE;
}

@Override
public Scope startActive(boolean finishOnClose) {
return NoopScopeManager.NoopScope.INSTANCE;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,6 @@
public class ThreadLocalScopeManager implements ScopeManager {
final ThreadLocal<ThreadLocalScope> tlsScope = new ThreadLocal<ThreadLocalScope>();

@Override
public Scope activate(Span span) {
return new ThreadLocalScope(this, span, false);
}

@Override
public Scope activate(Span span, boolean finishOnClose) {
return new ThreadLocalScope(this, span, finishOnClose);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public void defaultActivate() throws Exception {
Span span = mock(Span.class);

// We can't use 1.7 features like try-with-resources in this repo without meddling with pom details for tests.
Scope scope = source.activate(span);
Scope scope = source.activate(span, false);
try {
assertNotNull(scope);
Scope otherScope = source.active();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,8 @@ public void implicitSpanStack() throws Exception {
public void testDeactivateWhenDifferentSpanIsActive() {
Span span = mock(Span.class);

Scope active = scopeManager.activate(span);
scopeManager.activate(mock(Span.class));
Scope active = scopeManager.activate(span, false);
scopeManager.activate(mock(Span.class), false);
active.close();

verify(span, times(0)).finish();
Expand Down

0 comments on commit 6423db2

Please sign in to comment.