diff --git a/money-aspectj/src/main/scala/com/comcast/money/aspectj/TraceAspect.scala b/money-aspectj/src/main/scala/com/comcast/money/aspectj/TraceAspect.scala index fcb48aa7..89c07f02 100644 --- a/money-aspectj/src/main/scala/com/comcast/money/aspectj/TraceAspect.scala +++ b/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 { diff --git a/money-aspectj/src/test/scala/com/comcast/money/aspectj/TraceAspectSpec.scala b/money-aspectj/src/test/scala/com/comcast/money/aspectj/TraceAspectSpec.scala index 47889881..ab426747 100644 --- a/money-aspectj/src/test/scala/com/comcast/money/aspectj/TraceAspectSpec.scala +++ b/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 { diff --git a/money-core/src/main/java/com/comcast/money/annotations/Traced.java b/money-core/src/main/java/com/comcast/money/annotations/Traced.java index aa76eddb..c8357403 100644 --- a/money-core/src/main/java/com/comcast/money/annotations/Traced.java +++ b/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 + *
+     *     {@code
+     *     {@literal @}Traced(value="my.custom.trace.key", ignoredExceptions={ IllegalArgumentException.class })
+     *      public void measureMePlease() {
+     *         // do something useful here
+     *      }
+     *     }
+     * 
+ * + * @return The array of exception classes that will be ignored + */ + Class[] ignoredExceptions() default {}; } diff --git a/money-core/src/main/scala/com/comcast/money/emitters/LogEmitter.scala b/money-core/src/main/scala/com/comcast/money/emitters/LogEmitter.scala index 354d1b44..d7ed070e 100644 --- a/money-core/src/main/scala/com/comcast/money/emitters/LogEmitter.scala +++ b/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 diff --git a/money-core/src/main/scala/com/comcast/money/reflect/Reflections.scala b/money-core/src/main/scala/com/comcast/money/reflect/Reflections.scala index c2f5e4cc..a8cd85f2 100644 --- a/money-core/src/main/scala/com/comcast/money/reflect/Reflections.scala +++ b/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] diff --git a/money-core/src/test/scala/com/comcast/money/reflect/ReflectionsSpec.scala b/money-core/src/test/scala/com/comcast/money/reflect/ReflectionsSpec.scala index ce800620..2cda6931 100644 --- a/money-core/src/test/scala/com/comcast/money/reflect/ReflectionsSpec.scala +++ b/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) diff --git a/money-spring3/src/main/scala/com/comcast/money/spring3/TracedMethodInterceptor.scala b/money-spring3/src/main/scala/com/comcast/money/spring3/TracedMethodInterceptor.scala index 6994289b..2a11c8c9 100644 --- a/money-spring3/src/main/scala/com/comcast/money/spring3/TracedMethodInterceptor.scala +++ b/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) diff --git a/money-spring3/src/test/java/com/comcast/money/spring3/SampleTraceBean.java b/money-spring3/src/test/java/com/comcast/money/spring3/SampleTraceBean.java index ebb8cd4c..6de71b06 100644 --- a/money-spring3/src/test/java/com/comcast/money/spring3/SampleTraceBean.java +++ b/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() { } } diff --git a/money-spring3/src/test/java/com/comcast/money/spring3/TracedMethodInterceptorSpec.java b/money-spring3/src/test/java/com/comcast/money/spring3/TracedMethodInterceptorSpec.java index e3886954..8cec5f63 100644 --- a/money-spring3/src/test/java/com/comcast/money/spring3/TracedMethodInterceptorSpec.java +++ b/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()); diff --git a/samples/samples-springmvc/src/main/java/com/comcast/money/samples/springmvc/controllers/SampleController.java b/samples/samples-springmvc/src/main/java/com/comcast/money/samples/springmvc/controllers/SampleController.java index 880f6007..abcc62b1 100644 --- a/samples/samples-springmvc/src/main/java/com/comcast/money/samples/springmvc/controllers/SampleController.java +++ b/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(); } }