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

ArC dev mode - fix business method invocations monitoring #17354

Merged
merged 1 commit into from
May 19, 2021
Merged

ArC dev mode - fix business method invocations monitoring #17354

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 4 additions & 5 deletions extensions/arc/runtime/src/main/java/io/quarkus/arc/runtime/devconsole/Invocation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import java.time.ZoneId;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.TimeUnit;

import io.quarkus.arc.InjectableBean;
Expand Down Expand Up @@ -108,7 +109,8 @@ static class Builder {
private long start;
private long duration;
private Method method;
private List<Builder> children;
// If async processing and the request context is not propagated a new child can be added when/after the builder is built
private final List<Builder> children = new CopyOnWriteArrayList<>();
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't it make more sense to have a volatile boolean called built or similar that is set to true at the very beginning of build() and exit from addChild() if it's true?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that volatile would not be enough in this case, we would have to synchronize the access anyway. CopyOnWriteArrayList should be safe although not so efficient, but that should not be a big problem in the dev mode.

private Builder parent;
private Kind kind;
private String message;
Expand Down Expand Up @@ -159,17 +161,14 @@ Builder setMessage(String message) {
}

boolean addChild(Builder child) {
if (children == null) {
children = new ArrayList<>();
}
child.setParent(this);
return children.add(child);
}

Invocation build() {
List<Invocation> invocations = null;
if (children != null) {
invocations = new ArrayList<>(children.size());
invocations = new ArrayList<>();
for (Builder builder : children) {
invocations.add(builder.build());
}
Expand Down
1 change: 0 additions & 1 deletion ...ns/arc/runtime/src/main/java/io/quarkus/arc/runtime/devconsole/InvocationInterceptor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ public Object monitor(InvocationContext context) throws Exception {
Object proceed(Invocation.Builder builder, InvocationContext context, ManagedContext requestContext, InvocationTree tree)
throws Exception {
long nanoTime = System.nanoTime();
// Object result;
try {
return context.proceed();
} catch (Exception e) {
Expand Down