Merge pull request #18 from paulcleary/ignore-exceptions

Ignore exceptions

money-aspectj/src/main/scala/com/comcast/money/aspectj/TraceAspect.scala

@@ -49,7 +49,7 @@ class TraceAspect extends Reflections with TraceLogging {
       joinPoint.proceed
     } catch {
       case t: Throwable =>
-        result = Result.failed
+        result = if (exceptionMatches(t, traceAnnotation.ignoredExceptions())) Result.success else Result.failed
         logException(t)
         throw t
     } finally {

money-aspectj/src/test/scala/com/comcast/money/aspectj/TraceAspectSpec.scala

@@ -55,22 +55,47 @@ class TraceAspectSpec extends TestKit(ActorSystem("money", Money.config.getConfi
   }
 
   @Traced("methodWithArgumentsPropagated")
-  def methodWithArgumentsPropagated(@TracedData(value = "PROPAGATE", propagate = true) foo: String, @TracedData("CUSTOM_NAME") bar: String) = {
+  def methodWithArgumentsPropagated(
+    @TracedData(value = "PROPAGATE", propagate = true) foo: String,
+    @TracedData("CUSTOM_NAME") bar: String
+  ) = {
     Thread.sleep(50)
     methodWithoutArguments()
   }
 
+  @Traced(
+    value = "methodWithIgnoredException",
+    ignoredExceptions = Array(classOf[IllegalArgumentException])
+  )
+  def methodWithIgnoredException() = {
+    throw new IllegalArgumentException("ignored")
+  }
+
+  @Traced(
+    value = "methodWithNonMatchingIgnoredException",
+    ignoredExceptions = Array(classOf[IllegalArgumentException])
+  )
+  def methodWithNonMatchingIgnoredException() = {
+    throw new RuntimeException("not-ignored")
+  }
+
   @Timed("methodWithTiming")
   def methodWithTiming() = {
     Thread.sleep(50)
   }
 
   def expectLogMessageContaining(contains: String, wait: FiniteDuration = 2.seconds) {
-    awaitCond(LogRecord.contains("log")(_.contains(contains)), wait, 100 milliseconds, s"Expected log message containing string $contains not found after $wait")
+    awaitCond(
+      LogRecord.contains("log")(_.contains(contains)), wait, 100 milliseconds,
+      s"Expected log message containing string $contains not found after $wait"
+    )
   }
 
   def expectLogMessageContainingStrings(strings: Seq[String], wait: FiniteDuration = 2.seconds) {
-    awaitCond(LogRecord.contains("log")(s => strings.forall(s.contains)), wait, 100 milliseconds, s"Expected log message containing $strings not found after $wait")
+    awaitCond(
+      LogRecord.contains("log")(s => strings.forall(s.contains)), wait, 100 milliseconds,
+      s"Expected log message containing $strings not found after $wait"
+    )
   }
 
   val mockMdcSupport = mock[MDCSupport]
@@ -106,6 +131,34 @@ class TraceAspectSpec extends TestKit(ActorSystem("money", Money.config.getConfi
         Then("the method execution is logged")
         expectLogMessageContaining("methodThrowingException")
 
+        And("a span-success is logged with a value of true")
+        expectLogMessageContaining("span-success=true")
+      }
+      "complete the trace with success for methods that throw ignored exceptions" in {
+        Given("a method that throws an ignored exception")
+
+        When("the method is invoked")
+        an[IllegalArgumentException] should be thrownBy {
+          methodWithIgnoredException()
+        }
+
+        Then("the method execution is logged")
+        expectLogMessageContaining("methodWithIgnoredException")
+
+        And("a span-success is logged with a value of true")
+        expectLogMessageContaining("span-success=true")
+      }
+      "complete the trace with failure for methods that throw exceptions that are not in ignored list" in {
+        Given("a method that throws an ignored exception")
+
+        When("the method is invoked")
+        a[RuntimeException] should be thrownBy {
+          methodWithNonMatchingIgnoredException()
+        }
+
+        Then("the method execution is logged")
+        expectLogMessageContaining("methodWithNonMatchingIgnoredException")
+
         And("a span-success is logged with a value of false")
         expectLogMessageContaining("span-success=false")
       }
@@ -123,7 +176,9 @@ class TraceAspectSpec extends TestKit(ActorSystem("money", Money.config.getConfi
         And("the values of the arguments that have the TracedData annotation are logged")
         expectLogMessageContaining("hello")
 
-        And("the values of the arguments that have a custom name for the TracedData annotation log using the custom name")
+        And(
+          "the values of the arguments that have a custom name for the TracedData annotation log using the custom name"
+        )
         expectLogMessageContaining("CUSTOM_NAME=bob")
       }
       "record parameters whose value is null" in {

money-core/src/main/java/com/comcast/money/annotations/Traced.java

@@ -63,4 +63,21 @@
      * @return The value that is to be the measurement key for the trace
      */
     String value();
+
+    /**
+     * Allows the developer to specify an array of exception types to ignore.  If any of these exceptions
+     * are encountered, then the span will close with a result of success=true.  Any exceptions not matching
+     * the exceptions in this array will still close the exception with success=false
+     * <pre>
+     *     {@code
+     *     {@literal @}Traced(value="my.custom.trace.key", ignoredExceptions={ IllegalArgumentException.class })
+     *      public void measureMePlease() {
+     *         // do something useful here
+     *      }
+     *     }
+     * </pre>
+     *
+     * @return The array of exception classes that will be ignored
+     */
+    Class[] ignoredExceptions() default {};
 }

money-core/src/main/scala/com/comcast/money/emitters/LogEmitter.scala

@@ -16,7 +16,7 @@
 
 package com.comcast.money.emitters
 
-import akka.actor.{Actor, ActorLogging, Props}
+import akka.actor.{ Actor, ActorLogging, Props }
 import akka.event.Logging
 import akka.event.Logging.LogLevel
 import com.comcast.money.core._
@@ -38,7 +38,6 @@ object LogEmitter {
       case _ => Props(classOf[LogEmitter], conf)
     }.get
 
-
   def buildMessage(t: Span): String = {
     implicit val builder = new StringBuilder()
     builder.append("Span: ")
@@ -59,7 +58,7 @@ object LogEmitter {
     builder.toString()
   }
 
-  def logLevel(conf:Config): LogLevel =
+  def logLevel(conf: Config): LogLevel =
     if (conf.hasPath("log-level"))
       Logging.levelFor(conf.getString("log-level")).getOrElse(Logging.WarningLevel)
     else

money-core/src/main/scala/com/comcast/money/reflect/Reflections.scala

@@ -19,7 +19,7 @@ package com.comcast.money.reflect
 import java.lang.annotation.Annotation
 import java.lang.reflect.Method
 
-import com.comcast.money.annotations.TracedData
+import com.comcast.money.annotations.{ Traced, TracedData }
 import com.comcast.money.core._
 
 trait Reflections {
@@ -84,6 +84,12 @@ trait Reflections {
       tracer.record(tdTuple._1, tdTuple._2)
     }
 
+  def exceptionMatches(t: Throwable, exceptionList: Array[Class[_]]) =
+    exceptionList.exists(isInstance(t))
+
+  private def isInstance[T](t: Throwable): Class[_] => Boolean =
+    clazz => clazz.isInstance(t)
+
   private def isBoolean(clazz: Class[_]) = clazz == classOf[Boolean] || clazz == classOf[java.lang.Boolean]
 
   private def isDouble(clazz: Class[_]) = clazz == classOf[Double] || clazz == classOf[java.lang.Double]

money-core/src/test/scala/com/comcast/money/reflect/ReflectionsSpec.scala

@@ -16,7 +16,7 @@
 
 package com.comcast.money.reflect
 
-import com.comcast.money.annotations.TracedData
+import com.comcast.money.annotations.{ Traced, TracedData }
 import com.comcast.money.core._
 import com.sun.istack.internal.NotNull
 import org.mockito.ArgumentCaptor
@@ -42,6 +42,18 @@ class ReflectionsSpec extends WordSpec with Matchers with MockitoSugar with OneI
   val methodWithNoTracedDataArguments = clazz.getMethod("methodWithNoTracedDataArguments", classOf[String])
   val methodWithTracedDataPropagate = clazz.getMethod("methodWithTracedDataPropagate", classOf[String])
 
+  "Caclulating result on failure" should {
+    "return success if an exception matches a type in the ignored list" in {
+      val matchingException = new IllegalArgumentException("ignored")
+      val result = testReflections.exceptionMatches(matchingException, Array(classOf[IllegalArgumentException]))
+      result shouldBe true
+    }
+    "return failure if an exception does not match a type in the ignored list" in {
+      val matchingException = new RuntimeException("not-ignored")
+      val result = testReflections.exceptionMatches(matchingException, Array(classOf[IllegalArgumentException]))
+      result shouldBe false
+    }
+  }
   "Extracting Traced Data Annotations" should {
     "return an empty array if there are no arguments" in {
       val anns = testReflections.extractTracedDataAnnotations(methodWithoutArguments)

money-spring3/src/main/scala/com/comcast/money/spring3/TracedMethodInterceptor.scala

@@ -53,7 +53,7 @@ class TracedMethodInterceptor @Autowired() (@Qualifier("springTracer") val trace
       } catch {
         case t: Throwable =>
           logException(t)
-          result = Result.failed
+          result = if (exceptionMatches(t, annotation.ignoredExceptions())) Result.success else Result.failed
           throw t
       } finally {
         mdcSupport.setSpanNameMDC(oldSpanName)

money-spring3/src/test/java/com/comcast/money/spring3/SampleTraceBean.java

@@ -51,6 +51,16 @@ public void doSomethingWithTracedParams(
         return;
     }
 
+    @Traced(
+        value="SampleTrace",
+        ignoredExceptions = { IllegalArgumentException.class }
+    )
+    public void doSomethingButIgnoreException() {
+
+        springTracer.record("foo", "bar", false);
+        throw new IllegalArgumentException("fail");
+    }
+
     public void doSomethingNotTraced() {
     }
 }

money-spring3/src/test/java/com/comcast/money/spring3/TracedMethodInterceptorSpec.java

@@ -146,6 +146,12 @@ public void testTracingDoesNotTraceMethodsWithoutAnnotation() {
         verifyZeroInteractions(springTracer);
     }
 
+    @Test(expected = IllegalArgumentException.class)
+    public void testTracingIgnoresException() {
+        sampleTraceBean.doSomethingButIgnoreException();
+        verifySpanResultsIn(true);
+    }
+
     private void verifySpanResultsIn(Boolean result) {
 
         verify(springTracer).stopSpan(spanResultCaptor.capture());

samples/samples-springmvc/src/main/java/com/comcast/money/samples/springmvc/controllers/SampleController.java

@@ -1,5 +1,7 @@
 package com.comcast.money.samples.springmvc.controllers;
 
+import java.lang.IllegalArgumentException;
+
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -22,11 +24,15 @@ public class SampleController {
     @Autowired
     private AsyncRootService rootService;
 
-    @Traced("SAMPLE_CONTROLLER")
+    @Traced(value="SAMPLE_CONTROLLER", ignoredExceptions={IllegalArgumentException.class})
     @RequestMapping(method = RequestMethod.GET, value = "/{name}")
     public String hello(@TracedData(value="CONTROLLER_INPUT", propagate=true) @PathVariable("name") String name) throws Exception {
 
         logger.warn("Call to sample controller with name " + name);
+
+        if ("ignore".equals(name)) {
+            throw new IllegalArgumentException("this should be ignored, check the log file for span-success=true")
+        }
         return rootService.doSomething(name).get();
     }
 }