= play:

* remove from publishErrorMessage method
* refactor onError method in RequestInstrumentation
* refactor WSInstrumentation in order to propagate the TraceContext when a WS call is executed outside an Action
* improve tests
* closes #33

2 files changed, 65 insertions, 50 deletions

diff --git a/kamon-play/src/test/scala/kamon/play/RequestInstrumentationSpec.scala b/kamon-play/src/test/scala/kamon/play/RequestInstrumentationSpec.scala
index fc195580..00adb99b 100644
--- a/kamon-play/src/test/scala/kamon/play/RequestInstrumentationSpec.scala
+++ b/kamon-play/src/test/scala/kamon/play/RequestInstrumentationSpec.scala
@@ -15,13 +15,14 @@
 
 package kamon.play
 
-import scala.concurrent.duration._
 import kamon.Kamon
 import kamon.http.HttpServerMetrics
 import kamon.metric.{ CollectionContext, Metrics }
 import kamon.play.action.TraceName
 import kamon.trace.{ TraceLocal, TraceRecorder }
 import org.scalatestplus.play._
+import play.api.DefaultGlobal
+import play.api.http.Writeable
 import play.api.libs.concurrent.Execution.Implicits.defaultContext
 import play.api.mvc.Results.Ok
 import play.api.mvc._
@@ -29,6 +30,7 @@ import play.api.test.Helpers._
 import play.api.test._
 import play.libs.Akka
 
+import scala.concurrent.duration._
 import scala.concurrent.{ Await, Future }
 
 class RequestInstrumentationSpec extends PlaySpec with OneServerPerSuite {
@@ -49,6 +51,11 @@ class RequestInstrumentationSpec extends PlaySpec with OneServerPerSuite {
       Action {
         Results.NotFound
       }
+    case ("GET", "/error") ⇒
+      Action {
+        throw new Exception("This page generates an error!")
+        Ok("This page will generate an error!")
+      }
     case ("GET", "/redirect") ⇒
       Action {
         Results.Redirect("/redirected", MOVED_PERMANENTLY)
@@ -125,11 +132,17 @@ class RequestInstrumentationSpec extends PlaySpec with OneServerPerSuite {
         Await.result(route(FakeRequest(GET, "/notFound").withHeaders(traceTokenHeader)).get, 10 seconds)
       }
 
+      for (repetition ← 1 to 5) {
+        Await.result(routeWithOnError(FakeRequest(GET, "/error").withHeaders(traceTokenHeader)).get, 10 seconds)
+      }
+
       val snapshot = Kamon(Metrics)(Akka.system()).register(HttpServerMetrics, HttpServerMetrics.Factory).get.collect(collectionContext)
       snapshot.countsPerTraceAndStatusCode("GET: /default")("200").count must be(10)
       snapshot.countsPerTraceAndStatusCode("GET: /notFound")("404").count must be(5)
+      snapshot.countsPerTraceAndStatusCode("GET: /error")("500").count must be(5)
       snapshot.countsPerStatusCode("200").count must be(10)
       snapshot.countsPerStatusCode("404").count must be(5)
+      snapshot.countsPerStatusCode("500").count must be(5)
     }
   }
 
@@ -151,5 +164,13 @@ class RequestInstrumentationSpec extends PlaySpec with OneServerPerSuite {
       }
     }
   }
+
+  def routeWithOnError[T](req: Request[T])(implicit w: Writeable[T]): Option[Future[Result]] = {
+    route(req).map { result ⇒
+      result.recoverWith {
+        case t: Throwable ⇒ DefaultGlobal.onError(req, t)
+      }
+    }
+  }
 }
 
diff --git a/kamon-play/src/test/scala/kamon/play/WSInstrumentationSpec.scala b/kamon-play/src/test/scala/kamon/play/WSInstrumentationSpec.scala
index 775d3e26..cbd95db3 100644
--- a/kamon-play/src/test/scala/kamon/play/WSInstrumentationSpec.scala
+++ b/kamon-play/src/test/scala/kamon/play/WSInstrumentationSpec.scala
@@ -16,70 +16,64 @@
 
 package kamon.play
 
+import kamon.Kamon
+import kamon.metric.TraceMetrics.{ HttpClientRequest, TraceMetricsSnapshot }
+import kamon.metric.{ Metrics, TraceMetrics }
+import org.scalatest.{ Matchers, WordSpecLike }
+import org.scalatestplus.play.OneServerPerSuite
+import play.api.libs.ws.WS
 import play.api.mvc.Action
 import play.api.mvc.Results.Ok
-import play.api.libs.ws.WS
-import org.scalatestplus.play.OneServerPerSuite
-import play.api.test._
 import play.api.test.Helpers._
-import akka.actor.ActorSystem
-import akka.testkit.{ TestKitBase, TestProbe }
+import play.api.test._
+import play.libs.Akka
 
-import com.typesafe.config.ConfigFactory
-import org.scalatest.{ Matchers, WordSpecLike }
-import kamon.Kamon
-import kamon.metric.{ TraceMetrics, Metrics }
-import kamon.metric.Subscriptions.TickMetricSnapshot
-import kamon.metric.TraceMetrics.ElapsedTime
+import scala.concurrent.Await
+import scala.concurrent.duration._
 
-class WSInstrumentationSpec extends TestKitBase with WordSpecLike with Matchers with OneServerPerSuite {
+class WSInstrumentationSpec extends WordSpecLike with Matchers with OneServerPerSuite {
 
   System.setProperty("config.file", "./kamon-play/src/test/resources/conf/application.conf")
 
-  import scala.collection.immutable.StringLike._
-  implicit lazy val system: ActorSystem = ActorSystem("play-ws-instrumentation-spec", ConfigFactory.parseString(
-    """
-      |akka {
-      |  loglevel = ERROR
-      |}
-      |
-      |kamon {
-      |  metrics {
-      |    tick-interval = 2 seconds
-      |
-      |    filters = [
-      |      {
-      |        trace {
-      |          includes = [ "*" ]
-      |          excludes = []
-      |        }
-      |      }
-      |    ]
-      |  }
-      |}
-    """.stripMargin))
-
   implicit override lazy val app = FakeApplication(withRoutes = {
-    case ("GET", "/async") ⇒ Action { Ok("ok") }
+    case ("GET", "/async")   ⇒ Action { Ok("ok") }
+    case ("GET", "/outside") ⇒ Action { Ok("ok") }
+    case ("GET", "/inside")  ⇒ callWSinsideController("http://localhost:19001/async")
   })
 
   "the WS instrumentation" should {
-    "respond to the Async Action and complete the WS request" in {
+    "propagate the TraceContext outside an Action and complete the WS request" in {
+      Await.result(WS.url("http://localhost:19001/outside").get(), 10 seconds)
+
+      val snapshot = takeSnapshotOf("http://localhost:19001/outside")
+      snapshot.elapsedTime.numberOfMeasurements should be(1)
+      snapshot.segments.size should be(1)
+      snapshot.segments(HttpClientRequest("http://localhost:19001/outside")).numberOfMeasurements should be(1)
+    }
 
-      val metricListener = TestProbe()
-      Kamon(Metrics)(system).subscribe(TraceMetrics, "*", metricListener.ref, permanently = true)
-      metricListener.expectMsgType[TickMetricSnapshot]
+    "propagate the TraceContext inside an Action and complete the WS request" in {
+      Await.result(route(FakeRequest(GET, "/inside")).get, 10 seconds)
+
+      val snapshot = takeSnapshotOf("GET: /inside")
+      snapshot.elapsedTime.numberOfMeasurements should be(2)
+      snapshot.segments.size should be(1)
+      snapshot.segments(HttpClientRequest("http://localhost:19001/async")).numberOfMeasurements should be(1)
+    }
+
+  }
+
+  def takeSnapshotOf(traceName: String): TraceMetricsSnapshot = {
+    val recorder = Kamon(Metrics)(Akka.system()).register(TraceMetrics(traceName), TraceMetrics.Factory)
+    val collectionContext = Kamon(Metrics)(Akka.system()).buildDefaultCollectionContext
+    recorder.get.collect(collectionContext)
+  }
 
-      val response = await(WS.url("http://localhost:19001/async").get())
-      response.status should be(OK)
+  def callWSinsideController(url: String) = Action.async {
+    import play.api.libs.concurrent.Execution.Implicits.defaultContext
+    import play.api.Play.current
 
-      //      val tickSnapshot = metricListener.expectMsgType[TickMetricSnapshot]
-      //      val traceMetrics = tickSnapshot.metrics.find { case (k, v) ⇒ k.name.contains("async") } map (_._2.metrics)
-      //      traceMetrics should not be empty
-      //
-      //      traceMetrics map { metrics ⇒
-      //        metrics(ElapsedTime).numberOfMeasurements should be(1L)
-      //      }
+    WS.url(url).get().map { response ⇒
+      Ok("Ok")
     }
   }
 }
\ No newline at end of file