ctubbsii commented on code in PR #5398:
URL: https://github.com/apache/accumulo/pull/5398#discussion_r2045804921
##########
core/src/main/java/org/apache/accumulo/core/trace/TraceUtil.java:
##########
@@ -191,22 +250,18 @@ public static <T> Callable<T> unwrap(Callable<T> c) {
public static <T> T wrapService(final T instance) {
InvocationHandler handler = (obj, method, args) -> {
- if (args == null || args.length < 1 || args[0] == null || !(args[0]
instanceof TInfo)) {
- try {
- return method.invoke(instance, args);
- } catch (InvocationTargetException e) {
- throw e.getCause();
- }
- }
- Span span = startServerRpcSpan(instance.getClass(), method.getName(),
(TInfo) args[0]);
- try (Scope scope = span.makeCurrent()) {
+ // start a new inner span specific to this method call
+ Span childSpan = TraceUtil.startClientRpcSpan(instance.getClass(),
method.getName());
+ try (Scope scope = childSpan.makeCurrent()) {
return method.invoke(instance, args);
} catch (Exception e) {
Throwable t = e instanceof InvocationTargetException ? e.getCause() :
e;
- setException(span, t, true);
+ setException(childSpan, t, true);
throw t;
} finally {
- span.end();
+ if (childSpan != null) {
+ childSpan.end();
+ }
Review Comment:
I don't think this can be null, unless the line 255 threw an NPE. The
previous code didn't check for this and did not have an error. I think that
perhaps this isn't possible.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
