Merge pull request #62 from drpacman/master

Issue #61 - enable exclusion of specific query and header values from traces

core/src/test/scala/com/github/levkhomich/akka/tracing/MockCollector.scala

@@ -170,6 +170,10 @@ trait MockCollector { this: Specification =>
     checkBinaryAnnotationInt(span, key, expValue) { bytes => ByteBuffer.wrap(bytes) }
   }
 
+  def checkAbsentBinaryAnnotation(span: thrift.Span, key: String): MatchResult[Any] = {
+    span.binary_annotations.find(_.get_key == key) must beNone
+  }
+
   def checkAnnotation(span: thrift.Span, expValue: String): MatchResult[Any] = {
     span.annotations.find(_.get_value == expValue).isDefined mustEqual true
   }

play/src/main/scala/com/github/levkhomich/akka/tracing/play/TracingSettings.scala

@@ -30,7 +30,11 @@ import com.github.levkhomich.akka.tracing.http.TracingHeaders
 trait TracingSettings extends GlobalSettings with PlayControllerTracing {
 
   lazy val serviceName = play.libs.Akka.system.name
-
+
+  lazy val excludedQueryParams = Set.empty[String]
+
+  lazy val excludedHeaders = Set.empty[String]
+
   protected def sample(request: RequestHeader): Unit = {
     trace.sample(request, serviceName)
   }
@@ -43,11 +47,17 @@ trait TracingSettings extends GlobalSettings with PlayControllerTracing {
     trace.recordKeyValue(request, "request.proto", request.version)
     trace.recordKeyValue(request, "client.address", request.remoteAddress)
     // TODO: separate cookie records
-    request.queryString.foreach {
+    request.queryString.filter {
+      case (key, values) =>
+        !excludedQueryParams(key)
+    }.foreach {
       case (key, values) =>
         values.foreach(trace.recordKeyValue(request, "request.query." + key, _))
     }
-    request.headers.toMap.foreach {
+    request.headers.toMap.filter {
+      case (key, values) =>
+        !excludedHeaders(key)
+    }.foreach {
       case (key, values) =>
         values.foreach(trace.recordKeyValue(request, "request.headers." + key, _))
     }

play/src/test/scala/com/github/levkhomich/akka/tracing/play/PlayTracingSpec.scala

@@ -18,17 +18,19 @@ package com.github.levkhomich.akka.tracing.play
 
 import scala.concurrent.{ Await, Future }
 import scala.util.Random
-
+import scala.collection.immutable.Set
 import play.api.{ GlobalSettings, Play }
 import play.api.http.Writeable
 import play.api.libs.iteratee.Enumerator
 import play.api.mvc._
 import play.api.test._
+import org.specs2.matcher._
 
 import com.github.levkhomich.akka.tracing._
 import com.github.levkhomich.akka.tracing.http.TracingHeaders
+import scala.collection.JavaConversions._
 
-class PlayTracingSpec extends PlaySpecification with TracingTestCommons with MockCollector with Results {
+class PlayTracingSpec extends PlaySpecification with TracingTestCommons with MockCollector with Results with ResultMatchers {
 
   sequential
 
@@ -37,22 +39,35 @@ class PlayTracingSpec extends PlaySpecification with TracingTestCommons with Moc
   val npe = new NullPointerException
   implicit def trace: TracingExtensionImpl = TracingExtension(_root_.play.libs.Akka.system)
 
+  val configuration = Map(
+    TracingExtension.AkkaTracingPort -> collectorPort
+  )
+  val routes: PartialFunction[(String, String), Handler] = {
+    case ("GET", TestPath) =>
+      Action {
+        Ok("response") as "text/plain"
+      }
+    case ("GET", TestErrorPath) =>
+      Action {
+        throw npe
+        Ok("response") as "text/plain"
+      }
+  }
+
   def fakeApplication: FakeApplication = FakeApplication(
-    withRoutes = {
-      case ("GET", TestPath) =>
-        Action {
-          Ok("response") as "text/plain"
-        }
-      case ("GET", TestErrorPath) =>
-        Action {
-          throw npe
-          Ok("response") as "text/plain"
-        }
-    },
+    withRoutes = routes,
     withGlobal = Some(new GlobalSettings with TracingSettings),
-    additionalConfiguration = Map(
-      TracingExtension.AkkaTracingPort -> collectorPort
-    )
+    additionalConfiguration = configuration
+  )
+
+  def overriddenApplication(overriddenServiceName: String = "test", queryParams: Set[String] = Set.empty, headerKeys: Set[String] = Set.empty) = FakeApplication(
+    withRoutes = routes,
+    withGlobal = Some(new GlobalSettings with TracingSettings {
+      override lazy val serviceName = overriddenServiceName
+      override lazy val excludedQueryParams = queryParams
+      override lazy val excludedHeaders = headerKeys
+    }),
+    additionalConfiguration = configuration
   )
 
   "Play tracing" should {
@@ -62,6 +77,18 @@ class PlayTracingSpec extends PlaySpecification with TracingTestCommons with Moc
       success
     }
 
+    "use play application name as the default end point name" in new WithApplication(fakeApplication) {
+      val result = route(FakeRequest("GET", TestPath)).map(Await.result(_, defaultAwaitTimeout.duration))
+      val span = receiveSpan()
+      span.annotations.map(_.get_host().get_service_name()) must beEqualTo(_root_.play.libs.Akka.system.name).forall
+    }
+
+    "enable overriding the service name for the end point name" in new WithApplication(overriddenApplication(overriddenServiceName = "test service")) {
+      val result = route(FakeRequest("GET", TestPath)).map(Await.result(_, defaultAwaitTimeout.duration))
+      val span = receiveSpan()
+      span.annotations.map(_.get_host().get_service_name()) must beEqualTo("test service").forall
+    }
+
     "not allow to use RequestHeaders as child of other request" in new WithApplication(fakeApplication) {
       val parent = new TracingSupport {}
       val request = FakeRequest("GET", TestPath)
@@ -88,6 +115,28 @@ class PlayTracingSpec extends PlaySpecification with TracingTestCommons with Moc
       checkBinaryAnnotation(span, "request.query.key", "value")
     }
 
+    "exclude specific query values from annotations when configured" in new WithApplication(overriddenApplication(queryParams = Set("excludedParam"))) {
+      val result = route(FakeRequest("GET", TestPath + "?key=value&excludedParam=value",
+        FakeHeaders(Seq("Content-Type" -> Seq("text/plain"))), AnyContentAsEmpty
+      )).map(Await.result(_, defaultAwaitTimeout.duration))
+      val span = receiveSpan()
+      checkBinaryAnnotation(span, "request.query.key", "value")
+      checkAbsentBinaryAnnotation(span, "request.query.excludedParam")
+    }
+
+    "exclude specific header fields from annotations when configured" in new WithApplication(overriddenApplication(headerKeys = Set("Excluded"))) {
+      val result = route(FakeRequest("GET", TestPath,
+        FakeHeaders(Seq(
+          "Content-Type" -> Seq("text/plain"),
+          "Excluded" -> Seq("test"),
+          "Included" -> Seq("value")
+        )), AnyContentAsEmpty
+      )).map(Await.result(_, defaultAwaitTimeout.duration))
+      val span = receiveSpan()
+      checkBinaryAnnotation(span, "request.headers.Included", "value")
+      checkAbsentBinaryAnnotation(span, "request.headers.Excluded")
+    }
+
     "propagate tracing headers" in new WithApplication(fakeApplication) {
       val spanId = Random.nextLong
       val parentId = Random.nextLong
@@ -134,4 +183,4 @@ class PlayTracingSpec extends PlaySpecification with TracingTestCommons with Moc
     result
   }
 
-}
+}