Return-Path: X-Original-To: apmail-accumulo-commits-archive@www.apache.org Delivered-To: apmail-accumulo-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 348D210AB1 for ; Mon, 24 Feb 2014 17:33:03 +0000 (UTC) Received: (qmail 62277 invoked by uid 500); 24 Feb 2014 17:32:50 -0000 Delivered-To: apmail-accumulo-commits-archive@accumulo.apache.org Received: (qmail 62139 invoked by uid 500); 24 Feb 2014 17:32:49 -0000 Mailing-List: contact commits-help@accumulo.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@accumulo.apache.org Delivered-To: mailing list commits@accumulo.apache.org Received: (qmail 62024 invoked by uid 99); 24 Feb 2014 17:32:47 -0000 Received: from tyr.zones.apache.org (HELO tyr.zones.apache.org) (140.211.11.114) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 24 Feb 2014 17:32:47 +0000 Received: by tyr.zones.apache.org (Postfix, from userid 65534) id 179C890377B; Mon, 24 Feb 2014 17:32:47 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: mdrob@apache.org To: commits@accumulo.apache.org Date: Mon, 24 Feb 2014 17:32:47 -0000 Message-Id: X-Mailer: ASF-Git Admin Mailer Subject: [01/10] git commit: ACCUMULO-2390 InvocationTargetEx in TraceProxy Repository: accumulo Updated Branches: refs/heads/1.4.5-SNAPSHOT 7059c767b -> 282942661 refs/heads/1.5.1-SNAPSHOT a38b004f6 -> 41ce56a51 refs/heads/1.6.0-SNAPSHOT 812df5898 -> 14d6b0bb6 refs/heads/master e9c23faed -> 0f78d23d0 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/1.4.5-SNAPSHOT Commit: 2829426618b6e7d1487a4c88dd7b09186b9898d5 Parents: 7059c76 Author: Mike Drob Authored: Fri Feb 21 13:42:01 2014 -0500 Committer: Mike Drob 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 trace(T instance) { return trace(instance, ALWAYS); } - + @SuppressWarnings("unchecked") public static 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 callable; + + @Before + public void setup() { + callable = new Callable() { + @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(); + } }