Skip to content

Commit

Permalink
Rever adding spans to all wrapped executor tasks (#89)
Browse files Browse the repository at this point in the history
## Before this PR
In #71, all wrapped executor tasks had spans added by default. This was pretty noisy for tasks that were already doing their own wrapping.

## After this PR
If wrapping is not explicitly requested, it is not done
  • Loading branch information
diogoholanda authored and bulldozer-bot[bot] committed Mar 7, 2019
1 parent c37d9fe commit de00d8d
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 11 deletions.
15 changes: 12 additions & 3 deletions tracing/src/main/java/com/palantir/tracing/Tracers.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ public final class Tracers {
/** The key under which trace ids are inserted into SLF4J {@link org.slf4j.MDC MDCs}. */
public static final String TRACE_ID_KEY = "traceId";
private static final String DEFAULT_ROOT_SPAN_OPERATION = "root";
private static final String DEFAULT_EXECUTOR_SPAN_OPERATION = "executor";
private static final char[] HEX_DIGITS =
{'0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'a', 'b', 'c', 'd', 'e', 'f'};

Expand Down Expand Up @@ -69,7 +68,12 @@ static String longToPaddedHex(long number) {
* #wrap wrapped} in order to be trace-aware.
*/
public static ExecutorService wrap(ExecutorService executorService) {
return wrap(DEFAULT_EXECUTOR_SPAN_OPERATION, executorService);
return new WrappingExecutorService(executorService) {
@Override
protected <T> Callable<T> wrapTask(Callable<T> callable) {
return wrap(callable);
}
};
}

/**
Expand All @@ -92,7 +96,12 @@ protected <T> Callable<T> wrapTask(Callable<T> callable) {
* trace will be generated for each execution, effectively bypassing the intent of this method.
*/
public static ScheduledExecutorService wrap(ScheduledExecutorService executorService) {
return wrap(DEFAULT_EXECUTOR_SPAN_OPERATION, executorService);
return new WrappingScheduledExecutorService(executorService) {
@Override
protected <T> Callable<T> wrapTask(Callable<T> callable) {
return wrap(callable);
}
};
}

/**
Expand Down
16 changes: 8 additions & 8 deletions tracing/src/test/java/com/palantir/tracing/TracersTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -62,15 +62,15 @@ public void testWrapExecutorService() throws Exception {
Tracers.wrap(Executors.newSingleThreadExecutor());

// Empty trace
wrappedService.submit(traceExpectingCallableWithSpan("executor")).get();
wrappedService.submit(traceExpectingRunnableWithSpan("executor")).get();
wrappedService.submit(traceExpectingCallable()).get();
wrappedService.submit(traceExpectingCallable()).get();

// Non-empty trace
Tracer.startSpan("foo");
Tracer.startSpan("bar");
Tracer.startSpan("baz");
wrappedService.submit(traceExpectingCallableWithSpan("executor")).get();
wrappedService.submit(traceExpectingRunnableWithSpan("executor")).get();
wrappedService.submit(traceExpectingCallable()).get();
wrappedService.submit(traceExpectingCallable()).get();
Tracer.fastCompleteSpan();
Tracer.fastCompleteSpan();
Tracer.fastCompleteSpan();
Expand Down Expand Up @@ -102,15 +102,15 @@ public void testWrapScheduledExecutorService() throws Exception {
Tracers.wrap(Executors.newSingleThreadScheduledExecutor());

// Empty trace
wrappedService.schedule(traceExpectingCallableWithSpan("executor"), 0, TimeUnit.SECONDS).get();
wrappedService.schedule(traceExpectingRunnableWithSpan("executor"), 0, TimeUnit.SECONDS).get();
wrappedService.schedule(traceExpectingCallable(), 0, TimeUnit.SECONDS).get();
wrappedService.schedule(traceExpectingCallable(), 0, TimeUnit.SECONDS).get();

// Non-empty trace
Tracer.startSpan("foo");
Tracer.startSpan("bar");
Tracer.startSpan("baz");
wrappedService.schedule(traceExpectingCallableWithSpan("executor"), 0, TimeUnit.SECONDS).get();
wrappedService.schedule(traceExpectingRunnableWithSpan("executor"), 0, TimeUnit.SECONDS).get();
wrappedService.schedule(traceExpectingCallable(), 0, TimeUnit.SECONDS).get();
wrappedService.schedule(traceExpectingCallable(), 0, TimeUnit.SECONDS).get();
Tracer.fastCompleteSpan();
Tracer.fastCompleteSpan();
Tracer.fastCompleteSpan();
Expand Down

0 comments on commit de00d8d

Please sign in to comment.