Merge pull request #762 from rafaelweingartner/cglibThrowableRenderer

Proposal for an improved CglibThrowableRendererFollowing our discussions on mailing list, here is a PR with a proposal for an improvement on CglibThrowableRenderer. We would still have to define a number to limit the stack traces of causes (I randomly chose 3, just to get the code running). The test case is also not good in my opinion, but I ran out of ideas (waiting for suggestions on that).

* pr/762:
  Changed the test case to test each of the log traces in the array list.
  Proposal for an improved CglibThrowableRenderer

Signed-off-by: Rajani Karuturi <rajani.karuturi@citrix.com>

utils/src/main/java/com/cloud/utils/log/CglibThrowableRenderer.java

@@ -19,16 +19,17 @@
 
 package com.cloud.utils.log;
 
-import java.io.PrintWriter;
 import java.util.ArrayList;
+import java.util.List;
 
+import org.apache.commons.lang.StringUtils;
 import org.apache.log4j.spi.ThrowableRenderer;
 
 /**
- * This renderer removes all the Cglib generated methods from the call
+ * This renderer removes all the CGLib generated methods from the call
  *
- * Unfortunately, I had to copy out the EnhancedThrowableRenderer from
+ * Unfortunately, I had to copy out there-write the EnhancedThrowableRenderer from
- * the apach libraries because EnhancedThrowableRenderer is a final class.
+ * the Apache libraries because EnhancedThrowableRenderer is a final class.
  * simply override doRender. Not sure what the developers are thinking there
  * making it final.
  *
@@ -37,48 +38,45 @@ import org.apache.log4j.spi.ThrowableRenderer;
  *
  */
 public class CglibThrowableRenderer implements ThrowableRenderer {
-    /**
+
-     * Construct new instance.
+    private final static int MAX_NUMBER_OF_STACK_TRACES_ON_LOG_FOR_CAUSE = 3;
-     */
+    @Override
-    public CglibThrowableRenderer() {
+    public String[] doRender(Throwable th) {
-        super();
+        List<String> lines = new ArrayList<String>();
+        lines.add(th.toString());
+        addStackTraceToList(th, lines, 0);
+        do {
+            th = th.getCause();
+            if (th != null) {
+                lines.add("Caused by: " + th.toString());
+                addStackTraceToList(th, lines, MAX_NUMBER_OF_STACK_TRACES_ON_LOG_FOR_CAUSE);
+            }
+        } while (th != null);
+        return lines.toArray(new String[lines.size()]);
     }
 
-    @Override
+    /**
-    public String[] doRender(final Throwable th) {
+     * This method adds the stack traces retrieved from {@link Throwable#getStackTrace()}
-        try {
+     * The maxNumberOfStack attribute indicates the number of stacks that will be added,
-            ArrayList<String> lines = new ArrayList<String>();
+     * if that value is 0, then all of the stack traces will be added, otherwise the stack traces will be limited to that number
-            Throwable throwable = th;
+     * @param th
-            lines.add(throwable.toString());
+     * @param lines
-            int start = 0;
+     * @param maxNumberOfStack
-            do {
+     */
-                StackTraceElement[] elements = throwable.getStackTrace();
+    private void addStackTraceToList(Throwable th, List<String> lines, int maxNumberOfStack) {
-                for (int i = 0; i < elements.length - start; i++) {
+        StackTraceElement[] elements = th.getStackTrace();
-                    StackTraceElement element = elements[i];
+        if (maxNumberOfStack == 0 || maxNumberOfStack > elements.length) {
-                    String filename = element.getFileName();
+            maxNumberOfStack = elements.length;
-                    String method = element.getMethodName();
+        }
-                    if ((filename != null && filename.equals("<generated>")) || (method != null && method.equals("invokeSuper"))) {
+        for (int i = 0; i < maxNumberOfStack; i++) {
-                        continue;
+            StackTraceElement element = elements[i];
-                    }
+            if (StringUtils.contains(element.getClassName(), "net.sf.cglib.proxy")) {
-                    lines.add("\tat " + element.toString());
+                continue;
-                }
+            }
-                if (start != 0) {
+            lines.add("\tat " + element.toString());
-                    lines.add("\t... " + start + " more");
+        }
-                }
+        if (maxNumberOfStack < elements.length) {
-                throwable = throwable.getCause();
+            lines.add("\t... " + (elements.length - maxNumberOfStack) + " more");
-                if (throwable != null) {
-                    lines.add("Caused by: " + throwable.toString());
-                    start = elements.length - 1;
-                }
-            } while (throwable != null);
-            return lines.toArray(new String[lines.size()]);
-        } catch (Exception ex) {
-            PrintWriter pw = new PrintWriter(System.err);
-            ex.printStackTrace(pw);
-            pw = new PrintWriter(System.out);
-            ex.printStackTrace(pw);
-            ex.printStackTrace();
-            return null;
         }
     }
 }

utils/src/test/java/com/cloud/utils/log/CglibThrowableRendererTest.java

@@ -0,0 +1,85 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package com.cloud.utils.log;
+
+import java.lang.reflect.Method;
+
+import org.apache.commons.lang.StringUtils;
+import org.junit.Assert;
+import org.junit.Test;
+
+import net.sf.cglib.proxy.Enhancer;
+import net.sf.cglib.proxy.MethodInterceptor;
+import net.sf.cglib.proxy.MethodProxy;
+
+public class CglibThrowableRendererTest {
+
+    CglibThrowableRenderer cglibThrowableRenderer = new CglibThrowableRenderer();
+
+    @Test
+    public void testDoRendere() {
+        SampleClass sampleClass = (SampleClass)Enhancer.create(SampleClass.class, new MyInvocationHandler());
+        try {
+            sampleClass.theFirstMethodThatCapturesAnException();
+        } catch (Exception e) {
+            String[] exceptions = cglibThrowableRenderer.doRender(e);
+            assertThatTheTraceListDoesNotContainsCgLibLogs(exceptions);
+        }
+    }
+
+    private void assertThatTheTraceListDoesNotContainsCgLibLogs(String[] exceptions) {
+        for (String s : exceptions) {
+            Assert.assertEquals(false, isCgLibLogTrace(s));
+        }
+    }
+
+    private boolean isCgLibLogTrace(String s) {
+        return StringUtils.contains(s, "net.sf.cglib.proxy");
+    }
+
+    static class SampleClass {
+        public void theFirstMethodThatCapturesAnException() {
+            try {
+                methodThatCapturesAndThrowsException();
+            } catch (Exception e) {
+                throw new RuntimeException(e);
+            }
+        }
+
+        private void methodThatCapturesAndThrowsException() throws Exception {
+            try {
+                methodThatThrowsAnError();
+            } catch (Error e) {
+                throw new Exception("Throws an exception", e);
+            }
+        }
+
+        private void methodThatThrowsAnError() {
+            throw new Error("Exception to test the CglibThrowableRenderer.");
+        }
+    }
+
+    static class MyInvocationHandler implements MethodInterceptor {
+        @Override
+        public Object intercept(Object obj, Method method, Object[] args, MethodProxy proxy) throws Throwable {
+            return proxy.invoke(new SampleClass(), args);
+        }
+    }
+}