Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[improvement] Add methods to set span operation when wrapping with a new trace #59

Merged
merged 4 commits into from
Feb 6, 2019
Merged

[improvement] Add methods to set span operation when wrapping with a new trace #59

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c1325ba
Add methods to set span operation when wrapping with a new trace
pkoenig10 Jan 31, 2019
202b3bd
Tests
pkoenig10 Jan 31, 2019
71665e3
Typo
pkoenig10 Feb 1, 2019
b74eade
CompileTimeConstant
pkoenig10 Feb 1, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 54 additions & 17 deletions tracing/src/main/java/com/palantir/tracing/Tracers.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,52 +104,74 @@ public static Runnable wrap(Runnable delegate) {
return new TracingAwareRunnable(delegate);
}

/**
* Like {@link #wrapWithNewTrace(String, ExecutorService)}, but with a default initial span operation.
*/
public static ExecutorService wrapWithNewTrace(ExecutorService executorService) {
return wrapWithNewTrace(ROOT_SPAN_OPERATION, executorService);
}

/**
* Wraps the provided executor service to make submitted tasks traceable with a fresh {@link Trace trace}
* for each execution, see {@link #wrapWithNewTrace(ExecutorService)}. This method should not be used to
* for each execution, see {@link #wrapWithNewTrace(String, ExecutorService)}. This method should not be used to
* wrap a ScheduledExecutorService that has already been {@link #wrap(ExecutorService) wrapped}. If this is
* done, a new trace will be generated for each execution, effectively bypassing the intent of the previous
* wrapping.
*/
public static ExecutorService wrapWithNewTrace(ExecutorService executorService) {
public static ExecutorService wrapWithNewTrace(String operation, ExecutorService executorService) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will appear unredacted in Telemetry, so what do you think about another beloved @CompileTimeConstant to enforce that people don't accidentally exfil usernames or something?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would make it impossible to use thread pool name patterns as the operation when created a wrapped executor.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is the operation we'd use for thread pools, we'd need to use the DeferredTracer + overload Tracers.wrap(ExecutorService) which can be implemented in a separate PR

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use this method when building scheduled executor services with new traces.

return new WrappingExecutorService(executorService) {
@Override
protected <T> Callable<T> wrapTask(Callable<T> callable) {
return wrapWithNewTrace(callable);
return wrapWithNewTrace(operation, callable);
}
};
}

/**
* Wraps the provided scheduled executor service to make submitted tasks traceable with a fresh {@link Trace trace}
* for each execution, see {@link #wrapWithNewTrace(ScheduledExecutorService)}. This method should not be used to
* wrap a ScheduledExecutorService that has already been {@link #wrap(ScheduledExecutorService) wrapped}. If this is
* done, a new trace will be generated for each execution, effectively bypassing the intent of the previous
* wrapping.
* Like {@link #wrapWithNewTrace(String, ScheduledExecutorService)}, but with a default initial span operation.
*/
public static ScheduledExecutorService wrapWithNewTrace(ScheduledExecutorService executorService) {
return wrapWithNewTrace(ROOT_SPAN_OPERATION, executorService);
}

/**
* Wraps the provided scheduled executor service to make submitted tasks traceable with a fresh {@link Trace trace}
* for each execution, see {@link #wrapWithNewTrace(String, ScheduledExecutorService)}. This method should not be
* used to wrap a ScheduledExecutorService that has already been {@link #wrap(ScheduledExecutorService) wrapped}.
* If this is done, a new trace will be generated for each execution, effectively bypassing the intent of the
* previous wrapping.
*/
public static ScheduledExecutorService wrapWithNewTrace(String operation,
ScheduledExecutorService executorService) {
return new WrappingScheduledExecutorService(executorService) {
@Override
protected <T> Callable<T> wrapTask(Callable<T> callable) {
return wrapWithNewTrace(callable);
return wrapWithNewTrace(operation, callable);
}
};
}

/**
* Like {@link #wrapWithNewTrace(String, Callable)}, but with a default initial span operation.
*/
public static <V> Callable<V> wrapWithNewTrace(Callable<V> delegate) {
return wrapWithNewTrace(ROOT_SPAN_OPERATION, delegate);
}

/**
* Wraps the given {@link Callable} such that it creates a fresh {@link Trace tracing state} for its execution.
* That is, the trace during its {@link Callable#call() execution} is entirely separate from the trace at
* construction or any trace already set on the thread used to execute the callable. Each execution of the callable
* will have a fresh trace.
* will have a fresh trace. The given {@link String operation} is used to create the initial span.
*/
public static <V> Callable<V> wrapWithNewTrace(Callable<V> delegate) {
public static <V> Callable<V> wrapWithNewTrace(String operation, Callable<V> delegate) {
return () -> {
// clear the existing trace and keep it around for restoration when we're done
Optional<Trace> originalTrace = Tracer.getAndClearTraceIfPresent();

try {
Tracer.initTrace(Optional.empty(), Tracers.randomId());
Tracer.startSpan(ROOT_SPAN_OPERATION);
Tracer.startSpan(operation);
return delegate.call();
} finally {
Tracer.fastCompleteSpan();
Expand All @@ -159,16 +181,23 @@ public static <V> Callable<V> wrapWithNewTrace(Callable<V> delegate) {
}

/**
* Like {@link #wrapWithNewTrace(Callable)}, but for Runnables.
* Like {@link #wrapWithAlternateTraceId(String, Runnable)}, but with a default initial span operation.
*/
public static Runnable wrapWithNewTrace(Runnable delegate) {
return wrapWithNewTrace(ROOT_SPAN_OPERATION, delegate);
}

/**
* Like {@link #wrapWithNewTrace(String, Callable)}, but for Runnables.
*/
public static Runnable wrapWithNewTrace(String operation, Runnable delegate) {
return () -> {
// clear the existing trace and keep it around for restoration when we're done
Optional<Trace> originalTrace = Tracer.getAndClearTraceIfPresent();

try {
Tracer.initTrace(Optional.empty(), Tracers.randomId());
Tracer.startSpan(ROOT_SPAN_OPERATION);
Tracer.startSpan(operation);
delegate.run();
} finally {
Tracer.fastCompleteSpan();
Expand All @@ -177,20 +206,28 @@ public static Runnable wrapWithNewTrace(Runnable delegate) {
};
}

/**
* Like {@link #wrapWithAlternateTraceId(String, String, Runnable)}, but with a default initial span operation.
*/
public static Runnable wrapWithAlternateTraceId(String traceId, Runnable delegate) {
return wrapWithAlternateTraceId(traceId, ROOT_SPAN_OPERATION, delegate);
}

/**
* Wraps the given {@link Runnable} such that it creates a fresh {@link Trace tracing state with the given traceId}
* for its execution. That is, the trace during its {@link Runnable#run() execution} will use the traceId provided
* instead of any trace already set on the thread used to execute the runnable. Each execution of the runnable
* will use a new {@link Trace tracing state} with the same given traceId.
* will use a new {@link Trace tracing state} with the same given traceId. The given {@link String operation} is
* used to create the initial span.
*/
public static Runnable wrapWithAlternateTraceId(String traceId, Runnable delegate) {
public static Runnable wrapWithAlternateTraceId(String traceId, String operation, Runnable delegate) {
return () -> {
// clear the existing trace and keep it around for restoration when we're done
Optional<Trace> originalTrace = Tracer.getAndClearTraceIfPresent();

try {
Tracer.initTrace(Optional.empty(), traceId);
Tracer.startSpan(ROOT_SPAN_OPERATION);
Tracer.startSpan(operation);
delegate.run();
} finally {
Tracer.fastCompleteSpan();
Expand Down
88 changes: 76 additions & 12 deletions tracing/src/test/java/com/palantir/tracing/TracersTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,11 +95,12 @@ public void testScheduledExecutorServiceWrapsCallables() throws Exception {

@Test
public void testScheduledExecutorServiceWrapsCallablesWithNewTraces() throws Exception {
String someOperation = "operationToUse";
ScheduledExecutorService wrappedService =
Tracers.wrapWithNewTrace(Executors.newSingleThreadScheduledExecutor());
Tracers.wrapWithNewTrace(someOperation, Executors.newSingleThreadScheduledExecutor());

Callable<Void> callable = newTraceExpectingCallable();
Runnable runnable = newTraceExpectingRunnable();
Callable<Void> callable = newTraceExpectingCallable(someOperation);
Runnable runnable = newTraceExpectingRunnable(someOperation);

// Empty trace
wrappedService.schedule(callable, 0, TimeUnit.SECONDS).get();
Expand All @@ -121,11 +122,12 @@ public void testScheduledExecutorServiceWrapsCallablesWithNewTraces() throws Exc

@Test
public void testExecutorServiceWrapsCallablesWithNewTraces() throws Exception {
String someOperation = "operationToUse";
ExecutorService wrappedService =
Tracers.wrapWithNewTrace(Executors.newSingleThreadExecutor());
Tracers.wrapWithNewTrace(someOperation, Executors.newSingleThreadExecutor());

Callable<Void> callable = newTraceExpectingCallable();
Runnable runnable = newTraceExpectingRunnable();
Callable<Void> callable = newTraceExpectingCallable(someOperation);
Runnable runnable = newTraceExpectingRunnable(someOperation);

// Empty trace
wrappedService.submit(callable).get();
Expand Down Expand Up @@ -241,6 +243,23 @@ public void testWrapCallableWithNewTrace_traceStateInsideCallableHasSpan() throw
assertThat(span.getParentSpanId()).isEmpty();
}

@Test
public void testWrapCallableWithNewTrace_traceStateInsideCallableHasGivenSpan() throws Exception {
String operationToUse = "someOperation";
Callable<List<OpenSpan>> wrappedCallable = Tracers.wrapWithNewTrace(operationToUse, () -> {
return getCurrentFullTrace();
});

List<OpenSpan> spans = wrappedCallable.call();

assertThat(spans).hasSize(1);

OpenSpan span = spans.get(0);

assertThat(span.getOperation()).isEqualTo(operationToUse);
assertThat(span.getParentSpanId()).isEmpty();
}

@Test
public void testWrapCallableWithNewTrace_traceStateRestoredWhenThrows() throws Exception {
String traceIdBeforeConstruction = Tracer.getTraceId();
Expand Down Expand Up @@ -312,7 +331,26 @@ public void testWrapRunnableWithNewTrace_traceStateInsideRunnableHasSpan() throw
}

@Test
public void testWrapRunnableWithNewTrace_traceStateRestoredWhenThrows() throws Exception {
public void testWrapRunnableWithNewTrace_traceStateInsideRunnableHasGivenSpan() throws Exception {
List<List<OpenSpan>> spans = Lists.newArrayList();

String operationToUse = "someOperation";
Runnable wrappedRunnable = Tracers.wrapWithNewTrace(operationToUse, () -> {
spans.add(getCurrentFullTrace());
});

wrappedRunnable.run();

assertThat(spans.get(0)).hasSize(1);

OpenSpan span = spans.get(0).get(0);

assertThat(span.getOperation()).isEqualTo(operationToUse);
assertThat(span.getParentSpanId()).isEmpty();
}

@Test
public void testWrapRunnableWithNewTrace_traceStateRestoredWhenThrows() {
String traceIdBeforeConstruction = Tracer.getTraceId();

Runnable rawRunnable = () -> {
Expand Down Expand Up @@ -357,7 +395,7 @@ public void testWrapRunnableWithAlternateTraceId_traceStateInsideRunnableUsesGiv
}

@Test
public void testWrapRunnableWithAlternateTraceId_traceStateInsideRunnableHasSpan() throws Exception {
public void testWrapRunnableWithAlternateTraceId_traceStateInsideRunnableHasSpan() {
List<List<OpenSpan>> spans = Lists.newArrayList();

String traceIdToUse = "someTraceId";
Expand All @@ -375,6 +413,26 @@ public void testWrapRunnableWithAlternateTraceId_traceStateInsideRunnableHasSpan
assertThat(span.getParentSpanId()).isEmpty();
}

@Test
public void testWrapRunnableWithAlternateTraceId_traceStateInsideRunnableHasGivenSpan() {
List<List<OpenSpan>> spans = Lists.newArrayList();

String traceIdToUse = "someTraceId";
String operationToUse = "someOperation";
Runnable wrappedRunnable = Tracers.wrapWithAlternateTraceId(traceIdToUse, operationToUse, () -> {
spans.add(getCurrentFullTrace());
});

wrappedRunnable.run();

assertThat(spans.get(0)).hasSize(1);

OpenSpan span = spans.get(0).get(0);

assertThat(span.getOperation()).isEqualTo(operationToUse);
assertThat(span.getParentSpanId()).isEmpty();
}

@Test
public void testWrapRunnableWithAlternateTraceId_traceStateRestoredWhenThrows() {
String traceIdBeforeConstruction = Tracer.getTraceId();
Expand Down Expand Up @@ -406,36 +464,42 @@ public void testTraceIdGeneration() throws Exception {
assertThat(Tracers.longToPaddedHex(123456789L)).isEqualTo("00000000075bcd15");
}

private static Callable<Void> newTraceExpectingCallable() {
private static Callable<Void> newTraceExpectingCallable(String expectedOperation) {
final Set<String> seenTraceIds = new HashSet<>();
seenTraceIds.add(Tracer.getTraceId());

return new Callable<Void>() {
@Override
public Void call() throws Exception {
String newTraceId = Tracer.getTraceId();
List<OpenSpan> spans = getCurrentFullTrace();

assertThat(MDC.get(Tracers.TRACE_ID_KEY)).isEqualTo(newTraceId);
assertThat(seenTraceIds).doesNotContain(newTraceId);
assertThat(getCurrentFullTrace()).hasSize(1);
assertThat(spans).hasSize(1);
assertThat(spans.get(0).getOperation()).isEqualTo(expectedOperation);
assertThat(spans.get(0).getParentSpanId()).isEmpty();
seenTraceIds.add(newTraceId);
return null;
}
};
}

private static Runnable newTraceExpectingRunnable() {
private static Runnable newTraceExpectingRunnable(String expectedOperation) {
final Set<String> seenTraceIds = new HashSet<>();
seenTraceIds.add(Tracer.getTraceId());

return new Runnable() {
@Override
public void run() {
String newTraceId = Tracer.getTraceId();
List<OpenSpan> spans = getCurrentFullTrace();

assertThat(MDC.get(Tracers.TRACE_ID_KEY)).isEqualTo(newTraceId);
assertThat(seenTraceIds).doesNotContain(newTraceId);
assertThat(getCurrentFullTrace()).hasSize(1);
assertThat(spans).hasSize(1);
assertThat(spans.get(0).getOperation()).isEqualTo(expectedOperation);
assertThat(spans.get(0).getParentSpanId()).isEmpty();
seenTraceIds.add(newTraceId);
}
};
Expand Down