Put FutureTracing instrumentation in place, simpler and more explicit test

6 files changed, 75 insertions, 96 deletions

diff --git a/kamon-core/src/main/resources/META-INF/aop.xml b/kamon-core/src/main/resources/META-INF/aop.xml
index af1dad7c..b3e6a290 100644
--- a/kamon-core/src/main/resources/META-INF/aop.xml
+++ b/kamon-core/src/main/resources/META-INF/aop.xml
@@ -9,7 +9,6 @@
 
         <aspect name="kamon.trace.instrumentation.EnvelopeTracingContext"/>
         <aspect name="kamon.trace.instrumentation.ActorCellInvokeInstrumentation"/>
-        <aspect name="kamon.trace.instrumentation.RunnableTracing" />
         <aspect name="kamon.instrumentation.ActorSystemInstrumentation"/>
         <aspect name="kamon.trace.instrumentation.ActorLoggingInstrumentation"/>
         <aspect name="akka.pattern.tracing.AskPatternTracing"/>
diff --git a/kamon-trace/src/main/resources/META-INF/aop.xml b/kamon-trace/src/main/resources/META-INF/aop.xml
new file mode 100644
index 00000000..fdc1c496
--- /dev/null
+++ b/kamon-trace/src/main/resources/META-INF/aop.xml
@@ -0,0 +1,13 @@
+<!DOCTYPE aspectj PUBLIC "-//AspectJ//DTD//EN" "http://www.eclipse.org/aspectj/dtd/aspectj.dtd">
+
+<aspectj>
+    <aspects>
+        <aspect name="kamon.trace.instrumentation.FutureTracing" />
+
+        <include within="scala.concurrent..*"/>
+        <include within="akka..*"/>
+        <include within="spray..*"/>
+        <include within="kamon..*"/>
+    </aspects>
+
+</aspectj>
diff --git a/kamon-trace/src/main/scala/kamon/trace/Trace.scala b/kamon-trace/src/main/scala/kamon/trace/Trace.scala
index 0ef438d4..fbd4ea8b 100644
--- a/kamon-trace/src/main/scala/kamon/trace/Trace.scala
+++ b/kamon-trace/src/main/scala/kamon/trace/Trace.scala
@@ -35,6 +35,8 @@ object Trace extends ExtensionId[TraceExtension] with ExtensionIdProvider {
     ctx
   }
 
+  def withValue[T](ctx: Option[TraceContext])(thunk: => T): T = traceContext.withValue(ctx)(thunk)
+
   def transformContext(f: TraceContext => TraceContext): Unit = {
     context.map(f).foreach(set(_))
   }
diff --git a/kamon-trace/src/main/scala/kamon/trace/instrumentation/RunnableTracing.scala b/kamon-trace/src/main/scala/kamon/trace/instrumentation/FutureTracing.scala
index 5c16a648..10daa2da 100644
--- a/kamon-trace/src/main/scala/kamon/trace/instrumentation/RunnableTracing.scala
+++ b/kamon-trace/src/main/scala/kamon/trace/instrumentation/FutureTracing.scala
@@ -5,7 +5,7 @@ import org.aspectj.lang.ProceedingJoinPoint
 import kamon.trace.{ContextAware, TraceContext, Trace}
 
 @Aspect
-class RunnableTracing {
+class FutureTracing {
 
   /**
    *  These are the Runnables that need to be instrumented and make the TraceContext available
@@ -15,16 +15,11 @@ class RunnableTracing {
   def onCompleteCallbacksRunnable: ContextAware = ContextAware.default
 
 
-  /**
-   *  Pointcuts
-   */
-
   @Pointcut("execution(kamon.trace.ContextAware+.new(..)) && this(runnable)")
   def instrumentedRunnableCreation(runnable: ContextAware): Unit = {}
 
   @Pointcut("execution(* kamon.trace.ContextAware+.run()) && this(runnable)")
-  def runnableExecution(runnable: ContextAware) = {}
-
+  def futureRunnableExecution(runnable: ContextAware) = {}
 
 
   @After("instrumentedRunnableCreation(runnable)")
@@ -33,12 +28,11 @@ class RunnableTracing {
     runnable.traceContext
   }
 
-
-  @Around("runnableExecution(runnable)")
+  @Around("futureRunnableExecution(runnable)")
   def around(pjp: ProceedingJoinPoint, runnable: ContextAware): Any = {
     import pjp._
 
-    Trace.traceContext.withValue(runnable.traceContext) {
+    Trace.withValue(runnable.traceContext) {
       proceed()
     }
   }
diff --git a/kamon-trace/src/test/scala/kamon/FutureTracingSpec.scala b/kamon-trace/src/test/scala/kamon/FutureTracingSpec.scala
new file mode 100644
index 00000000..7ff1b96d
--- /dev/null
+++ b/kamon-trace/src/test/scala/kamon/FutureTracingSpec.scala
@@ -0,0 +1,56 @@
+package kamon
+
+import scala.concurrent.{ExecutionContext, Await, Promise, Future}
+import org.scalatest.{Matchers, OptionValues, WordSpec}
+import org.scalatest.concurrent.{ScalaFutures, PatienceConfiguration}
+import java.util.UUID
+import scala.util.{Random, Success}
+import scala.concurrent.duration._
+import java.util.concurrent.TimeUnit
+import akka.actor.{Actor, ActorSystem}
+import kamon.trace.{Trace, TraceContext}
+
+
+class FutureTracingSpec extends WordSpec with Matchers with ScalaFutures with PatienceConfiguration with OptionValues {
+
+  implicit val execContext = ExecutionContext.Implicits.global
+
+  "a Future created with FutureTracing" should {
+    "capture the TraceContext available when created" which {
+      "must be available when executing the future's body" in new TraceContextFixture {
+        var future: Future[Option[TraceContext]] = _
+
+        Trace.withValue(testTraceContext) {
+          future = Future(Trace.context)
+        }
+
+        whenReady(future)( ctxInFuture =>
+          ctxInFuture should equal(testTraceContext)
+        )
+      }
+
+      "must be available when executing callbacks on the future" in new TraceContextFixture {
+        var future: Future[Option[TraceContext]] = _
+
+        Trace.withValue(testTraceContext) {
+          future = Future("Hello Kamon!")
+            // The TraceContext is expected to be available during all intermediate processing.
+            .map (_.length)
+            .flatMap(len => Future(len.toString))
+            .map (s => Trace.context())
+        }
+
+        whenReady(future)( ctxInFuture =>
+          ctxInFuture should equal(testTraceContext)
+        )
+      }
+    }
+  }
+
+  trait TraceContextFixture {
+    val random = new Random(System.nanoTime)
+    val testTraceContext = Some(TraceContext(Actor.noSender, random.nextInt))
+  }
+}
+
+
diff --git a/kamon-trace/src/test/scala/kamon/RunnableInstrumentationSpec.scala b/kamon-trace/src/test/scala/kamon/RunnableInstrumentationSpec.scala
deleted file mode 100644
index f968fa83..00000000
--- a/kamon-trace/src/test/scala/kamon/RunnableInstrumentationSpec.scala
+++ /dev/null
@@ -1,85 +0,0 @@
-package kamon
-
-import scala.concurrent.{Await, Promise, Future}
-import org.scalatest.{Matchers, OptionValues, WordSpec}
-import org.scalatest.concurrent.{ScalaFutures, PatienceConfiguration}
-import java.util.UUID
-import scala.util.Success
-import scala.concurrent.duration._
-import java.util.concurrent.TimeUnit
-import akka.actor.{Actor, ActorSystem}
-import kamon.trace.{Trace, TraceContext}
-
-
-class RunnableInstrumentationSpec extends WordSpec with Matchers with ScalaFutures with PatienceConfiguration with OptionValues {
-
-  "a instrumented runnable" when {
-    "created in a thread that does have a TraceContext" must {
-      "preserve the TraceContext" which {
-        "should be available during the run method execution" in new FutureWithContextFixture {
-
-/*            whenReady(futureWithContext) { result =>
-              result.value should equal(testContext)
-            }*/
-        }
-
-        "should be available during the execution of onComplete callbacks" in new FutureWithContextFixture {
-
-          val onCompleteContext = Promise[Option[TraceContext]]()
-
-/*          Tracer.traceContext.withValue(Some(testContext)) {
-            futureWithContext.onComplete({
-              case _ => println("Completing second promise from: "+Thread.currentThread().getName + " With Context: " + Tracer.traceContext.value); onCompleteContext.complete(Success(Tracer.traceContext.value))
-            })
-          }*/
-
-          whenReady(onCompleteContext.future) { result =>
-            result should equal(Some(testContext))
-          }
-        }
-      }
-    }
-
-    "created in a thread that doest have a TraceContext" must {
-      "not capture any TraceContext for the body execution" in new FutureWithoutContextFixture{
-          whenReady(futureWithoutContext) { result =>
-            result should equal(None)
-          }
-      }
-
-      "not make any TraceContext available during the onComplete callback" in new FutureWithoutContextFixture {
-        val onCompleteContext = Promise[Option[TraceContext]]()
-
-        futureWithoutContext.onComplete {
-          case _ => onCompleteContext.complete(Success(Trace.context()))
-        }
-
-        whenReady(onCompleteContext.future) { result =>
-          result should equal(None)
-        }
-      }
-    }
-  }
-
-
-  /**
-   *  We are using Futures for the test since they exercise Runnables in the back and also resemble the real use case we have.
-   */
-  implicit val testActorSystem = ActorSystem("test-actorsystem")
-  implicit val execContext = testActorSystem.dispatcher
-
-  class FutureWithContextFixture {
-    val testContext = TraceContext(Actor.noSender, 1)
-
-/*    var futureWithContext: Future[Option[TraceContext]] = _
-    Tracer.context.withValue(Some(testContext)) {
-      futureWithContext = Future { Tracer.traceContext.value }
-    }*/
-  }
-
-  trait FutureWithoutContextFixture {
-    val futureWithoutContext = Future { Trace.context.value }
-  }
-}
-
-