accumulo-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From md...@apache.org
Subject [04/10] git commit: ACCUMULO-2390 InvocationTargetEx in TraceProxy
Date Mon, 24 Feb 2014 17:32:50 GMT
ACCUMULO-2390 InvocationTargetEx in TraceProxy

Handle InvocationTargetException specifically in TraceProxy, instead
of letting it get propogated up the call stack. In some cases this is
very bad as it turned into an UndeclaredThrowableException and made
debugging more difficult than necessary.

Added unit test to verify behaviour.


Project: http://git-wip-us.apache.org/repos/asf/accumulo/repo
Commit: http://git-wip-us.apache.org/repos/asf/accumulo/commit/28294266
Tree: http://git-wip-us.apache.org/repos/asf/accumulo/tree/28294266
Diff: http://git-wip-us.apache.org/repos/asf/accumulo/diff/28294266

Branch: refs/heads/master
Commit: 2829426618b6e7d1487a4c88dd7b09186b9898d5
Parents: 7059c76
Author: Mike Drob <mdrob@cloudera.com>
Authored: Fri Feb 21 13:42:01 2014 -0500
Committer: Mike Drob <mdrob@cloudera.com>
Committed: Mon Feb 24 08:39:03 2014 -0500

----------------------------------------------------------------------
 .../cloudtrace/instrument/TraceProxy.java       | 37 ++++++++++++------
 .../cloudtrace/instrument/TracerTest.java       | 41 ++++++++++++++++++--
 2 files changed, 63 insertions(+), 15 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/accumulo/blob/28294266/src/trace/src/main/java/org/apache/accumulo/cloudtrace/instrument/TraceProxy.java
----------------------------------------------------------------------
diff --git a/src/trace/src/main/java/org/apache/accumulo/cloudtrace/instrument/TraceProxy.java
b/src/trace/src/main/java/org/apache/accumulo/cloudtrace/instrument/TraceProxy.java
index 67c4463..6b71361 100644
--- a/src/trace/src/main/java/org/apache/accumulo/cloudtrace/instrument/TraceProxy.java
+++ b/src/trace/src/main/java/org/apache/accumulo/cloudtrace/instrument/TraceProxy.java
@@ -17,43 +17,56 @@
 package org.apache.accumulo.cloudtrace.instrument;
 
 import java.lang.reflect.InvocationHandler;
+import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.lang.reflect.Proxy;
 
+import org.apache.log4j.Logger;
+
 public class TraceProxy {
-  // private static final Logger log = Logger.getLogger(TraceProxy.class);
-  
+  private static final Logger log = Logger.getLogger(TraceProxy.class);
+
   static final Sampler ALWAYS = new Sampler() {
     @Override
     public boolean next() {
       return true;
     }
   };
-  
+
   public static <T> T trace(T instance) {
     return trace(instance, ALWAYS);
   }
-  
+
   @SuppressWarnings("unchecked")
   public static <T> T trace(final T instance, final Sampler sampler) {
     InvocationHandler handler = new InvocationHandler() {
       @Override
       public Object invoke(Object obj, Method method, Object[] args) throws Throwable {
-        if (!sampler.next()) {
-          return method.invoke(instance, args);
+        Span span = null;
+        if (sampler.next()) {
+          span = Trace.on(method.getName());
         }
-        Span span = Trace.on(method.getName());
         try {
           return method.invoke(instance, args);
-        } catch (Throwable ex) {
-          ex.printStackTrace();
-          throw ex;
+          // Can throw RuntimeException, Error, or any checked exceptions of the method.
+        } catch (InvocationTargetException ite) {
+          Throwable cause = ite.getCause();
+          if (cause == null) {
+            // This should never happen, but account for it anyway
+            log.error("Invocation exception during trace with null cause: ", ite);
+            throw new RuntimeException(ite);
+          }
+          throw cause;
+        } catch (IllegalAccessException e) {
+          throw new RuntimeException(e);
         } finally {
-          span.stop();
+          if (span != null) {
+            span.stop();
+          }
         }
       }
     };
     return (T) Proxy.newProxyInstance(instance.getClass().getClassLoader(), instance.getClass().getInterfaces(),
handler);
   }
-  
+
 }

http://git-wip-us.apache.org/repos/asf/accumulo/blob/28294266/src/trace/src/test/java/org/apache/accumulo/cloudtrace/instrument/TracerTest.java
----------------------------------------------------------------------
diff --git a/src/trace/src/test/java/org/apache/accumulo/cloudtrace/instrument/TracerTest.java
b/src/trace/src/test/java/org/apache/accumulo/cloudtrace/instrument/TracerTest.java
index 30b62f9..20fc768 100644
--- a/src/trace/src/test/java/org/apache/accumulo/cloudtrace/instrument/TracerTest.java
+++ b/src/trace/src/test/java/org/apache/accumulo/cloudtrace/instrument/TracerTest.java
@@ -21,16 +21,15 @@ import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 
+import java.io.IOException;
 import java.net.ServerSocket;
 import java.net.Socket;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.Callable;
 
-import org.apache.accumulo.cloudtrace.instrument.Span;
-import org.apache.accumulo.cloudtrace.instrument.Trace;
-import org.apache.accumulo.cloudtrace.instrument.Tracer;
 import org.apache.accumulo.cloudtrace.instrument.receivers.SpanReceiver;
 import org.apache.accumulo.cloudtrace.instrument.thrift.TraceWrap;
 import org.apache.accumulo.cloudtrace.thrift.TInfo;
@@ -42,6 +41,7 @@ import org.apache.thrift.server.TThreadPoolServer;
 import org.apache.thrift.transport.TServerSocket;
 import org.apache.thrift.transport.TSocket;
 import org.apache.thrift.transport.TTransport;
+import org.junit.Before;
 import org.junit.Test;
 
 
@@ -179,4 +179,39 @@ public class TracerTest {
     tserver.stop();
     t.join(100);
   }
+
+  Callable<Object> callable;
+
+  @Before
+  public void setup() {
+    callable = new Callable<Object>() {
+      @Override
+      public Object call() throws IOException {
+        throw new IOException();
+      }
+    };
+  }
+
+  /**
+   * Verify that exceptions propagate up through the trace wrapping with sampling enabled,
instead of seeing the reflexive exceptions.
+   */
+  @Test(expected = IOException.class)
+  public void testTracedException() throws Exception {
+    TraceProxy.trace(callable).call();
+  }
+
+  /**
+   * Verify that exceptions propagate up through the trace wrapping with sampling disabled,
instead of seeing the reflexive exceptions.
+   */
+  @Test(expected = IOException.class)
+  public void testUntracedException() throws Exception {
+    Sampler never = new Sampler() {
+      @Override
+      public boolean next() {
+        return false;
+      }
+    };
+
+    TraceProxy.trace(callable, never).call();
+  }
 }


Mime
View raw message