opencensus-java icon indicating copy to clipboard operation
opencensus-java copied to clipboard

Future tracing with asynchronous CompletionStage calls not supported

Open ruchirsachdeva opened this issue 5 years ago • 3 comments

I want to implement Opencensus tracing for JDK's CompletionStage's default asynchronous execution facility which uses an executor internally and doe not expose it. Opencensus provides a way(Context.wrap) to pass the context across the threads by wrapping the executor of Runnable code with the parent span and thus using the wrapped executor to run the new thread. Since I do not have access to the executor so I would be happy to see Opencensus library to provide a way to propagate the context to each successive CompletionStage such that parent span is maintained across asynchronous future executions.

ruchirsachdeva avatar Jul 12 '19 08:07 ruchirsachdeva

Just bumped into the same issue. Quite annoying, spent a nice amount of time fixing dependency hell in our monorepo to get tracing working, only to find this 🙄 .

dietervdw-spotify avatar Mar 04 '20 17:03 dietervdw-spotify

It's not so trivial to implement; which context should be propagated in the runnable of runAfterEither from two completable futures for example.. I have given it a go though in my own context propagation library, which is Apache licensed, so feel free to use (if only as inspiration) or contribute.. Here's my ContextAwareCompletableFuture implementation from https://github.com/talsma-ict/context-propagation.

sjoerdtalsma avatar Mar 04 '20 22:03 sjoerdtalsma

I've found it a lot easier to simply pass the Context instance as a method parameter to code that returns a CompletionStage or to methods that run in a thenApply/thenCompose/etc block, rather than relying on the Executor to capture the context when execute() is called and "restore" it when the task is run.

I've ran into cases when using nl.talsmasoftware.context.executors.ContextAwareExecutorService where the "wrong" Span is set as the parent of any Span I create in thenApplyAsync/thenComposeAsnyc blocks when used with ContextAwareExecutorService. I could not sort out if I was simply using it wrong, or if the approach is doomed to fail in some cases. My theory is that the context was captured when another thread completes the source CompletableFuture, when what I really wanted in my code was for the context/Span to be capture was the one that was in scope when I called .thenApplyAsync.

Passing the Context as a method parameter feels funny but it is conventional in Go.

With the help of a Tracer class (accidentally used the same name as the opencensus class) that centralizes the repeated steps of a) retrieve span from input Context b) call opencensus Tracer.spanBuilderWithExplicitParent c) create a new Context with the new Span, d) attach the new Context, any code that returns or chains together CompletionStages can add Spans by wrapping the CompletionStage-returning method in something like:

  private CompletionStage<String> doFooBar(
      Context context, String spanName, String input) {
    return tracer.trace(
        context,
        spanName,
        // ctx is the new Context to use for any calls that happen as a part of this span
        ctx ->
            databaseClient
                .lookupData(ctx, input)
                .thenCompose(response -> serviceClient.sendRequest(ctx, response)));
  }

In contrast to using context-aware Executors, this also has the benefit IMHO of making it easy to test the tracing aspects of the code being traced: to test that it creates a new span (by injecting a mock Tracer), test that a new Context (with a new Span) is passed when calling databaseClient::lookupData or serviceClient::sendRequest, etc.

mattnworb avatar Jun 29 '20 20:06 mattnworb