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.

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 support for tracing-aware Guava FutureCallbacks. #77

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
82 changes: 82 additions & 0 deletions tracing/src/main/java/com/palantir/tracing/Tracers.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,13 @@

package com.palantir.tracing;

import com.google.common.util.concurrent.FutureCallback;
import java.util.Optional;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ThreadLocalRandom;
import org.checkerframework.checker.nullness.compatqual.NullableDecl;

/** Utility methods for making {@link ExecutorService} and {@link Runnable} instances tracing-aware. */
public final class Tracers {
Expand Down Expand Up @@ -104,6 +106,11 @@ public static Runnable wrap(Runnable delegate) {
return new TracingAwareRunnable(delegate);
}

/** Like {@link #wrap(Callable)}, but for Guava's FutureCallback. */
public static <V> FutureCallback<V> wrap(FutureCallback<V> delegate) {
return new TracingAwareFutureCallback<>(delegate);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this take an operation name? Once #71 merges DeferredTracer can take an operation name. I'd prefer we we required an operation name for new methods, and did not provide a method using a default.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. Can wait until #71 merges and then rebase to use the DeferredTracer with the operation name.

}

/**
* Like {@link #wrapWithNewTrace(String, ExecutorService)}, but with a default initial span operation.
*/
Expand Down Expand Up @@ -206,6 +213,50 @@ public static Runnable wrapWithNewTrace(String operation, Runnable delegate) {
};
}

/**
* Like {@link #wrapWithNewTrace(Callable)}, but for Guava's FutureCallback.
*/
public static <V> FutureCallback<V> wrapWithNewTrace(FutureCallback<V> delegate) {
return wrapWithNewTrace(ROOT_SPAN_OPERATION, delegate);
}

/**
* Like {@link #wrapWithNewTrace(String, Callable)}, but for Guava's FutureCallback.
*/
public static <V> FutureCallback<V> wrapWithNewTrace(String operation, FutureCallback<V> delegate) {
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 we ever want to create a new trace around just the callback

return new FutureCallback<V>() {
@Override
public void onSuccess(@NullableDecl V result) {
// 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(operation);
delegate.onSuccess(result);
} finally {
Tracer.fastCompleteSpan();
restoreTrace(originalTrace);
}
}

@Override
public void onFailure(Throwable throwable) {
// 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(operation);
delegate.onFailure(throwable);
} finally {
Tracer.fastCompleteSpan();
restoreTrace(originalTrace);
}
}
};
}

/**
* Like {@link #wrapWithAlternateTraceId(String, String, Runnable)}, but with a default initial span operation.
*/
Expand Down Expand Up @@ -297,6 +348,37 @@ public void run() {
}
}

/**
* Wrap a given guava future callback such that its execution operated with the {@link Trace thread-local Trace} of
* the thread the constructs the {@link TracingAwareFutureCallback} instance rather than the thread that executes
* the callback.
*/
private static class TracingAwareFutureCallback<V> implements FutureCallback<V> {
private final FutureCallback<V> delegate;
private DeferredTracer deferredTracer;

TracingAwareFutureCallback(FutureCallback<V> delegate) {
this.delegate = delegate;
this.deferredTracer = new DeferredTracer();
}

@Override
public void onSuccess(@NullableDecl V result) {
deferredTracer.withTrace(() -> {
delegate.onSuccess(result);
return null;
});
}

@Override
public void onFailure(Throwable throwable) {
deferredTracer.withTrace(() -> {
delegate.onFailure(throwable);
return null;
});
}
}

public interface ThrowingCallable<T, E extends Throwable> {
T call() throws E;
}
Expand Down
Loading