-
Notifications
You must be signed in to change notification settings - Fork 882
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
Add asynchronous tracing for Java 8 CompletableFuture in WithSpanAdvice #2530
Merged
Merged
Changes from all commits
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
fd02a85
Add asynchronous tracing for Java 8 CompletableFuture in Spring WithS…
HaloFour ee201f3
Add unit tests, fix bugs
HaloFour 6b8f4f1
Placate spotless
HaloFour 7ac98d5
Move MethodSpanStrategies to instrumentation-api, enable registration…
HaloFour b63a896
Merge branch 'main' of github.com:open-telemetry/opentelemetry-java-i…
HaloFour 44a019a
Switch to withSpan
HaloFour 0f5a370
Fix unit test, refactor MethodSpanStrategy interface
HaloFour ef099f5
Refactor to instrumentation-api, add docs
HaloFour b465fbd
spotless, handle null and already done scenarios
HaloFour 7c7d7f1
Minor refactorings, add unit tests
HaloFour cbe3ec6
Placate checkstyle+spotless
HaloFour 1978160
placate codeNarc
HaloFour a5b5104
Merge branch 'main' of github.com:open-telemetry/opentelemetry-java-i…
HaloFour 850f3a8
Isolate changes to only otelannotations instrumentation
HaloFour 3c8a635
nix unit tests
HaloFour b1981de
Consolidate JDK8 strategies, remove use of context
HaloFour 272a0f7
Clarify verbiage in Javadoc
HaloFour 4a717d2
Merge branch 'main' of github.com:open-telemetry/opentelemetry-java-i…
HaloFour 9f74fea
Refactor synchronous completion and add comments, tests
HaloFour 5ad2ea8
Merge branch 'main' of github.com:open-telemetry/opentelemetry-java-i…
HaloFour 16025a4
Early return on uncompleted future
HaloFour 72594ae
Add check to Jdk8MethodStrategy to ensure return type of method is co…
HaloFour da48a91
A couple of suggestions (#1)
trask File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
75 changes: 75 additions & 0 deletions
75
.../io/opentelemetry/javaagent/instrumentation/otelannotations/async/Jdk8MethodStrategy.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,75 @@ | ||
/* | ||
* Copyright The OpenTelemetry Authors | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
package io.opentelemetry.javaagent.instrumentation.otelannotations.async; | ||
|
||
import io.opentelemetry.context.Context; | ||
import io.opentelemetry.instrumentation.api.tracer.BaseTracer; | ||
import java.util.concurrent.CompletableFuture; | ||
import java.util.concurrent.CompletionStage; | ||
|
||
enum Jdk8MethodStrategy implements MethodSpanStrategy { | ||
INSTANCE; | ||
|
||
@Override | ||
public boolean supports(Class<?> returnType) { | ||
return returnType == CompletionStage.class || returnType == CompletableFuture.class; | ||
} | ||
|
||
@Override | ||
public Object end(BaseTracer tracer, Context context, Object returnValue) { | ||
if (returnValue instanceof CompletableFuture) { | ||
CompletableFuture<?> future = (CompletableFuture<?>) returnValue; | ||
if (endSynchronously(future, tracer, context)) { | ||
return future; | ||
} | ||
return endWhenComplete(future, tracer, context); | ||
} | ||
CompletionStage<?> stage = (CompletionStage<?>) returnValue; | ||
return endWhenComplete(stage, tracer, context); | ||
} | ||
|
||
/** | ||
* Checks to see if the {@link CompletableFuture} has already been completed and if so | ||
* synchronously ends the span to avoid additional allocations and overhead registering for | ||
* notification of completion. | ||
*/ | ||
private boolean endSynchronously( | ||
CompletableFuture<?> future, BaseTracer tracer, Context context) { | ||
|
||
if (!future.isDone()) { | ||
return false; | ||
} | ||
|
||
if (future.isCompletedExceptionally()) { | ||
// If the future completed exceptionally then join to catch the exception | ||
// so that it can be recorded to the span | ||
try { | ||
future.join(); | ||
} catch (Exception exception) { | ||
tracer.endExceptionally(context, exception); | ||
return true; | ||
} | ||
} | ||
tracer.end(context); | ||
return true; | ||
} | ||
|
||
/** | ||
* Registers for notification of the completion of the {@link CompletionStage} at which time the | ||
* span will be ended. | ||
*/ | ||
private CompletionStage<?> endWhenComplete( | ||
CompletionStage<?> stage, BaseTracer tracer, Context context) { | ||
return stage.whenComplete( | ||
(result, exception) -> { | ||
if (exception != null) { | ||
tracer.endExceptionally(context, exception); | ||
} else { | ||
tracer.end(context); | ||
} | ||
}); | ||
} | ||
} |
42 changes: 42 additions & 0 deletions
42
...o/opentelemetry/javaagent/instrumentation/otelannotations/async/MethodSpanStrategies.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
/* | ||
* Copyright The OpenTelemetry Authors | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
package io.opentelemetry.javaagent.instrumentation.otelannotations.async; | ||
|
||
import java.util.List; | ||
import java.util.Objects; | ||
import java.util.concurrent.CopyOnWriteArrayList; | ||
|
||
/** | ||
* Registry of {@link MethodSpanStrategy} implementations for tracing the asynchronous operations | ||
* represented by the return type of a traced method. | ||
*/ | ||
public class MethodSpanStrategies { | ||
private static final MethodSpanStrategies instance = new MethodSpanStrategies(); | ||
|
||
public static MethodSpanStrategies getInstance() { | ||
return instance; | ||
} | ||
|
||
private final List<MethodSpanStrategy> strategies = new CopyOnWriteArrayList<>(); | ||
|
||
private MethodSpanStrategies() { | ||
strategies.add(Jdk8MethodStrategy.INSTANCE); | ||
} | ||
|
||
public void registerStrategy(MethodSpanStrategy strategy) { | ||
Objects.requireNonNull(strategy); | ||
strategies.add(strategy); | ||
} | ||
|
||
public MethodSpanStrategy resolveStrategy(Class<?> returnType) { | ||
for (MethodSpanStrategy strategy : strategies) { | ||
if (strategy.supports(returnType)) { | ||
return strategy; | ||
} | ||
} | ||
return MethodSpanStrategy.synchronous(); | ||
} | ||
} |
43 changes: 43 additions & 0 deletions
43
.../io/opentelemetry/javaagent/instrumentation/otelannotations/async/MethodSpanStrategy.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
/* | ||
* Copyright The OpenTelemetry Authors | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
package io.opentelemetry.javaagent.instrumentation.otelannotations.async; | ||
|
||
import io.opentelemetry.context.Context; | ||
import io.opentelemetry.instrumentation.api.tracer.BaseTracer; | ||
|
||
/** | ||
* Represents an implementation of a strategy for composing over the return value of a traced | ||
* method. If the return value represents the result of an asynchronous operation the implementation | ||
* can compose or register for notification of completion at which point the span representing the | ||
* invocation of the method will be ended. | ||
*/ | ||
public interface MethodSpanStrategy { | ||
boolean supports(Class<?> returnType); | ||
|
||
/** | ||
* Denotes the end of the invocation of the traced method with a successful result which will end | ||
* the span stored in the passed {@code context}. If the method returned a value representing an | ||
* asynchronous operation then the span will remain open until the asynchronous operation has | ||
* completed. | ||
* | ||
* @param tracer {@link BaseTracer} tracer to be used to end the span stored in the {@code | ||
* context}. | ||
* @param returnValue Return value from the traced method. Must be an instance of a {@code | ||
* returnType} for which {@link #supports(Class)} returned true (in particular it must not be | ||
* {@code null}). | ||
* @return Either {@code returnValue} or a value composing over {@code returnValue} for | ||
* notification of completion. | ||
*/ | ||
Object end(BaseTracer tracer, Context context, Object returnValue); | ||
|
||
/** | ||
* Returns a {@link MethodSpanStrategy} for tracing synchronous methods where the return value | ||
* does not represent the completion of an asynchronous operation. | ||
*/ | ||
static MethodSpanStrategy synchronous() { | ||
return SynchronousMethodSpanStrategy.INSTANCE; | ||
} | ||
} |
24 changes: 24 additions & 0 deletions
24
...emetry/javaagent/instrumentation/otelannotations/async/SynchronousMethodSpanStrategy.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
/* | ||
* Copyright The OpenTelemetry Authors | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
package io.opentelemetry.javaagent.instrumentation.otelannotations.async; | ||
|
||
import io.opentelemetry.context.Context; | ||
import io.opentelemetry.instrumentation.api.tracer.BaseTracer; | ||
|
||
enum SynchronousMethodSpanStrategy implements MethodSpanStrategy { | ||
INSTANCE; | ||
|
||
@Override | ||
public boolean supports(Class<?> returnType) { | ||
return true; | ||
} | ||
|
||
@Override | ||
public Object end(BaseTracer tracer, Context context, Object returnValue) { | ||
tracer.end(context); | ||
return returnValue; | ||
} | ||
} |
5 changes: 5 additions & 0 deletions
5
...n/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/package-info.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
/** | ||
* Provides implementations of strategies for tracing methods that return asynchronous and reactive | ||
* values so that the span can be ended when the asynchronous operation completes. | ||
*/ | ||
package io.opentelemetry.javaagent.instrumentation.otelannotations.async; |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the stage is already complete I think this is guaranteed to be synchronous - I guess we can remove endSynchronously?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I double checked to be sure and yes,
whenComplete
should always be synchronous, at least given how it's implemented inCompletableFuture<T>
. Checking and completing synchronously was more about optimizing away the need for the callback and the extra allocations that requires. It's not observable, but it is cheaper/faster. But if that's not worth the complexity I can remove it.