From 315d97c1d047db359c7b436c87a6a6b358f2a8de Mon Sep 17 00:00:00 2001 From: Diego Date: Sun, 14 Sep 2014 23:07:28 -0300 Subject: + play: * use the url template from the routes file as default trace name(basic implementation), and closes #82 * minor refactor in RequestInstrumentation --- .../instrumentation/RequestInstrumentation.scala | 56 ++++++++++---- .../kamon/play/RequestInstrumentationSpec.scala | 90 ++++++++++++++++++++-- .../scala/kamon/play/WSInstrumentationSpec.scala | 2 +- 3 files changed, 129 insertions(+), 19 deletions(-) diff --git a/kamon-play/src/main/scala/kamon/play/instrumentation/RequestInstrumentation.scala b/kamon-play/src/main/scala/kamon/play/instrumentation/RequestInstrumentation.scala index 8d8de230..2308e326 100644 --- a/kamon-play/src/main/scala/kamon/play/instrumentation/RequestInstrumentation.scala +++ b/kamon-play/src/main/scala/kamon/play/instrumentation/RequestInstrumentation.scala @@ -15,18 +15,25 @@ package kamon.play.instrumentation +import java.util.Locale + import kamon.Kamon import kamon.play.{ Play, PlayExtension } import kamon.trace.{ TraceContextAware, TraceRecorder } import org.aspectj.lang.ProceedingJoinPoint import org.aspectj.lang.annotation._ +import play.api.Routes import play.api.mvc.Results._ import play.api.mvc._ import play.libs.Akka +import scala.collection.concurrent.TrieMap + @Aspect class RequestInstrumentation { + import kamon.play.instrumentation.RequestInstrumentation._ + @DeclareMixin("play.api.mvc.RequestHeader+") def mixinContextAwareNewRequest: TraceContextAware = TraceContextAware.default @@ -36,7 +43,7 @@ class RequestInstrumentation { } @Before("call(* play.api.GlobalSettings+.onRouteRequest(..)) && args(requestHeader)") - def onRouteRequest(requestHeader: RequestHeader): Unit = { + def beforeRouteRequest(requestHeader: RequestHeader): Unit = { val system = Akka.system() val playExtension = Kamon(Play)(system) val defaultTraceName: String = s"${requestHeader.method}: ${requestHeader.uri}" @@ -51,20 +58,23 @@ class RequestInstrumentation { @Around("execution(* play.api.GlobalSettings+.doFilter(*)) && args(next)") def aroundDoFilter(pjp: ProceedingJoinPoint, next: EssentialAction): Any = { val essentialAction = (requestHeader: RequestHeader) ⇒ { - - val incomingContext = TraceRecorder.currentContext val executor = Kamon(Play)(Akka.system()).defaultDispatcher - next(requestHeader).map { - result ⇒ - TraceRecorder.finish() - incomingContext.map { ctx ⇒ - val playExtension = Kamon(Play)(ctx.system) - recordHttpServerMetrics(result.header, ctx.name, playExtension) - if (playExtension.includeTraceToken) result.withHeaders(playExtension.traceTokenHeaderName -> ctx.token) - else result - }.getOrElse(result) - }(executor) + def onResult(result: Result): Result = { + TraceRecorder.finish() + TraceRecorder.currentContext.map { ctx ⇒ + val playExtension = Kamon(Play)(ctx.system) + recordHttpServerMetrics(result.header, ctx.name, playExtension) + if (playExtension.includeTraceToken) result.withHeaders(playExtension.traceTokenHeaderName -> ctx.token) + else result + }.getOrElse(result) + } + + //override the current trace name + normaliseTraceName(requestHeader).map(TraceRecorder.rename(_)) + + // Invoke the action + next(requestHeader).map(onResult)(executor) } pjp.proceed(Array(EssentialAction(essentialAction))) } @@ -77,3 +87,23 @@ class RequestInstrumentation { private def recordHttpServerMetrics(header: ResponseHeader, traceName: String, playExtension: PlayExtension): Unit = playExtension.httpServerMetrics.recordResponse(traceName, header.status.toString) } + +object RequestInstrumentation { + private val cache = TrieMap.empty[String, String] + + def normaliseTraceName(requestHeader: RequestHeader): Option[String] = requestHeader.tags.get(Routes.ROUTE_VERB).map({ verb ⇒ + val path = requestHeader.tags(Routes.ROUTE_PATTERN) + cache.getOrElseUpdate(s"$verb$path", { + val traceName = { + // Convert paths of form GET /foo/bar/$paramname/blah to foo.bar.paramname.blah.get + val p = path.replaceAll("""\$([^<]+)<[^>]+>""", "$1").replace('/', '.').dropWhile(_ == '.') + val normalisedPath = { + if (p.lastOption.filter(_ != '.').isDefined) s"$p." + else p + } + s"$normalisedPath${verb.toLowerCase(Locale.ENGLISH)}" + } + traceName + }) + }) +} diff --git a/kamon-play/src/test/scala/kamon/play/RequestInstrumentationSpec.scala b/kamon-play/src/test/scala/kamon/play/RequestInstrumentationSpec.scala index 00adb99b..2afd31fd 100644 --- a/kamon-play/src/test/scala/kamon/play/RequestInstrumentationSpec.scala +++ b/kamon-play/src/test/scala/kamon/play/RequestInstrumentationSpec.scala @@ -17,17 +17,20 @@ package kamon.play import kamon.Kamon import kamon.http.HttpServerMetrics -import kamon.metric.{ CollectionContext, Metrics } +import kamon.metric.{ CollectionContext, Metrics, TraceMetrics } 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.libs.ws.WS import play.api.mvc.Results.Ok import play.api.mvc._ import play.api.test.Helpers._ import play.api.test._ +import play.core.Router.{ HandlerDef, Route, Routes } +import play.core.{ DynamicPart, PathPattern, Router, StaticPart } import play.libs.Akka import scala.concurrent.duration._ @@ -76,7 +79,11 @@ class RequestInstrumentationSpec extends PlaySpec with OneServerPerSuite { Action { Ok("retrieve from TraceLocal") } - }) + }, additionalConfiguration = Map( + ("application.router", "kamon.play.Routes"), + ("logger.root", "OFF"), + ("logger.play", "OFF"), + ("logger.application", "OFF"))) private val traceTokenValue = "kamon-trace-token-test" private val traceTokenHeaderName = "X-Trace-Token" @@ -109,10 +116,9 @@ class RequestInstrumentationSpec extends PlaySpec with OneServerPerSuite { } "respond to the Async Action with X-Trace-Token and the renamed trace" in { - val Some(result) = route(FakeRequest(GET, "/async-renamed").withHeaders(traceTokenHeader)) - Thread.sleep(500) // wait to complete the future + val result = Await.result(route(FakeRequest(GET, "/async-renamed").withHeaders(traceTokenHeader)).get, 10 seconds) TraceRecorder.currentContext.map(_.name) must be(Some("renamed-trace")) - header(traceTokenHeaderName, result) must be(expectedToken) + Some(result.header.headers(traceTokenHeaderName)) must be(expectedToken) } "propagate the TraceContext and LocalStorage through of filters in the current request" in { @@ -120,6 +126,21 @@ class RequestInstrumentationSpec extends PlaySpec with OneServerPerSuite { TraceLocal.retrieve(TraceLocalKey).get must be(traceLocalStorageValue) } + "response to the getRouted Action and normalise the current TraceContext name" in { + Await.result(WS.url("http://localhost:19001/getRouted").get, 10 seconds) + Kamon(Metrics)(Akka.system()).storage.get(TraceMetrics("getRouted.get")) must not be (empty) + } + + "response to the postRouted Action and normalise the current TraceContext name" in { + Await.result(WS.url("http://localhost:19001/postRouted").post("content"), 10 seconds) + Kamon(Metrics)(Akka.system()).storage.get(TraceMetrics("postRouted.post")) must not be (empty) + } + + "response to the showRouted Action and normalise the current TraceContext name" in { + Await.result(WS.url("http://localhost:19001/showRouted/2").get, 10 seconds) + Kamon(Metrics)(Akka.system()).storage.get(TraceMetrics("show.some.id.get")) must not be (empty) + } + "record http server metrics for all processed requests" in { val collectionContext = CollectionContext(100) Kamon(Metrics)(Akka.system()).register(HttpServerMetrics, HttpServerMetrics.Factory).get.collect(collectionContext) @@ -174,3 +195,62 @@ class RequestInstrumentationSpec extends PlaySpec with OneServerPerSuite { } } +object Routes extends Router.Routes { + private var _prefix = "/" + + def setPrefix(prefix: String) { + _prefix = prefix + List[(String, Routes)]().foreach { + case (p, router) ⇒ router.setPrefix(prefix + (if (prefix.endsWith("/")) "" else "/") + p) + } + } + + def prefix = _prefix + + lazy val defaultPrefix = { + if (Routes.prefix.endsWith("/")) "" else "/" + } + // Gets + private[this] lazy val Application_getRouted = + Route("GET", PathPattern(List(StaticPart(Routes.prefix), StaticPart(Routes.defaultPrefix), StaticPart("getRouted")))) + + private[this] lazy val Application_show = + Route("GET", PathPattern(List(StaticPart(Routes.prefix), StaticPart(Routes.defaultPrefix), StaticPart("showRouted/"), DynamicPart("id", """[^/]+""", true)))) + + //Posts + private[this] lazy val Application_postRouted = + Route("POST", PathPattern(List(StaticPart(Routes.prefix), StaticPart(Routes.defaultPrefix), StaticPart("postRouted")))) + + def documentation = Nil // Documentation not needed for tests + + def routes: PartialFunction[RequestHeader, Handler] = { + case Application_getRouted(params) ⇒ call { + createInvoker(controllers.Application.getRouted, + HandlerDef(this.getClass.getClassLoader, "", "controllers.Application", "getRouted", Nil, "GET", """some comment""", Routes.prefix + """getRouted""")).call(controllers.Application.getRouted) + } + case Application_postRouted(params) ⇒ call { + createInvoker(controllers.Application.postRouted, + HandlerDef(this.getClass.getClassLoader, "", "controllers.Application", "postRouted", Nil, "POST", """some comment""", Routes.prefix + """postRouted""")).call(controllers.Application.postRouted) + } + case Application_show(params) ⇒ call(params.fromPath[Int]("id", None)) { (id) ⇒ + createInvoker(controllers.Application.showRouted(id), + HandlerDef(this.getClass.getClassLoader, "", "controllers.Application", "showRouted", Seq(classOf[Int]), "GET", """""", Routes.prefix + """show/some/$id<[^/]+>""")).call(controllers.Application.showRouted(id)) + } + } +} + +object controllers { + import play.api.mvc._ + + object Application extends Controller { + val postRouted = Action { + Ok("invoked postRouted") + } + val getRouted = Action { + Ok("invoked getRouted") + } + def showRouted(id: Int) = Action { + Ok("invoked show with: " + id) + } + } +} \ No newline at end of file diff --git a/kamon-play/src/test/scala/kamon/play/WSInstrumentationSpec.scala b/kamon-play/src/test/scala/kamon/play/WSInstrumentationSpec.scala index aba1f1aa..8b0e0135 100644 --- a/kamon-play/src/test/scala/kamon/play/WSInstrumentationSpec.scala +++ b/kamon-play/src/test/scala/kamon/play/WSInstrumentationSpec.scala @@ -59,7 +59,7 @@ class WSInstrumentationSpec extends WordSpecLike with Matchers with OneServerPer }(Akka.system()) val snapshot = takeSnapshotOf("trace-outside-action") - println(snapshot) + println(snapshot) // force load snapshot.elapsedTime.numberOfMeasurements should be(1) snapshot.segments.size should be(1) snapshot.segments(HttpClientRequest("http://localhost:19001/outside")).numberOfMeasurements should be(1) -- cgit v1.2.3