+ adds configurable operation name mappings for http client instrumen…

…tation (#771)
kamon-io · Jun 16, 2020 · 29d82a7 · 29d82a7
1 parent e390184
commit 29d82a7
Show file tree

Hide file tree

Showing 11 changed files with 105 additions and 34 deletions.
diff --git a/...on-akka-http/src/main/scala/kamon/instrumentation/akka/http/AkkaHttpInstrumentation.scala b/...on-akka-http/src/main/scala/kamon/instrumentation/akka/http/AkkaHttpInstrumentation.scala
@@ -1,12 +1,10 @@
 package kamon.instrumentation.akka.http
 
-import akka.http.scaladsl.model.{HttpHeader, HttpRequest, HttpResponse}
+import akka.http.scaladsl.model.{HttpRequest, HttpResponse}
 import akka.http.scaladsl.model.headers.RawHeader
 import kamon.Kamon
 import kamon.instrumentation.http.HttpMessage
 
-import scala.collection.immutable
-
 object AkkaHttpInstrumentation {
 
   Kamon.onReconfigure(_ => AkkaHttpClientInstrumentation.rebuildHttpClientInstrumentation(): Unit)

diff --git a/instrumentation/kamon-akka-http/src/test/resources/application.conf b/instrumentation/kamon-akka-http/src/test/resources/application.conf
@@ -148,4 +148,14 @@ kamon.instrumentation.akka.http {
       }
     }
   }
+
+  client {
+    tracing {
+      operations {
+        mappings {
+          "/name-will-be-changed" = "named-via-config"
+        }
+      }
+    }
+  }
 }
diff --git a/...umentation/kamon-akka-http/src/test/scala/kamon/akka/http/AkkaHttpClientTracingSpec.scala b/...umentation/kamon-akka-http/src/test/scala/kamon/akka/http/AkkaHttpClientTracingSpec.scala
@@ -18,17 +18,16 @@ package kamon.akka.http
 
 import akka.actor.ActorSystem
 import akka.http.scaladsl.Http
-import akka.http.scaladsl.model.headers.RawHeader
 import akka.http.scaladsl.model.HttpRequest
+import akka.http.scaladsl.model.headers.RawHeader
 import akka.stream.ActorMaterializer
 import kamon.Kamon
+import kamon.tag.Lookups.{plain, plainBoolean, plainLong}
 import kamon.testkit._
-import kamon.trace.Span
-import kamon.tag.Lookups.{plain, plainLong, plainBoolean}
-import org.scalatest.concurrent.Eventually
-import org.scalatest.{BeforeAndAfterAll, Matchers, OptionValues, WordSpecLike}
 import org.json4s._
 import org.json4s.native.JsonMethods._
+import org.scalatest.concurrent.Eventually
+import org.scalatest.{BeforeAndAfterAll, Matchers, OptionValues, WordSpecLike}
 
 import scala.concurrent.duration._
 
@@ -115,6 +114,16 @@ class AkkaHttpClientTracingSpec extends WordSpecLike with Matchers with BeforeAn
         span.hasError shouldBe true
       }
     }
+
+    "keep operation names provided by the HTTP Client instrumentation" in {
+      val target = s"http://$interface:$port/name-will-be-changed"
+      Http().singleRequest(HttpRequest(uri = target)).map(_.discardEntityBytes())
+
+      eventually(timeout(10 seconds)) {
+        val span = testSpanReporter().nextSpan().value
+        span.operationName shouldBe "named-via-config"
+      }
+    }
   }
 
   override protected def afterAll(): Unit = {

diff --git a/...umentation/kamon-akka-http/src/test/scala/kamon/akka/http/AkkaHttpServerTracingSpec.scala b/...umentation/kamon-akka-http/src/test/scala/kamon/akka/http/AkkaHttpServerTracingSpec.scala
@@ -19,18 +19,16 @@ package kamon.akka.http
 import java.util.UUID
 
 import akka.actor.ActorSystem
-import akka.http.scaladsl.UseHttp2
-import akka.http.scaladsl.unmarshalling.Unmarshal
 import akka.stream.ActorMaterializer
 import javax.net.ssl.{HostnameVerifier, SSLSession}
-import kamon.testkit._
 import kamon.tag.Lookups.{plain, plainBoolean, plainLong}
+import kamon.testkit._
 import kamon.trace.Span.Mark
+import okhttp3.{OkHttpClient, Request}
 import org.scalatest._
 import org.scalatest.concurrent.{Eventually, ScalaFutures}
 
 import scala.concurrent.duration._
-import okhttp3.{OkHttpClient, Request}
 
 class AkkaHttpServerTracingSpec extends WordSpecLike with Matchers with ScalaFutures with Inside with BeforeAndAfterAll
     with MetricInspection.Syntax with Reconfigure with TestWebServer with Eventually with OptionValues with TestSpanReporter {

diff --git a/instrumentation/kamon-instrumentation-common/src/main/resources/reference.conf b/instrumentation/kamon-instrumentation-common/src/main/resources/reference.conf
@@ -218,6 +218,26 @@ kamon {
             #   - method: Uses the request HTTP method as the operation name.
             #
             name-generator = "method"
+
+            # Provides custom mappings from HTTP paths into operation names. Meant to be used in cases where the bytecode
+            # instrumentation is not able to provide a sensible operation name that is free of high cardinality values.
+            # For example, with the following configuration:
+            #   mappings {
+            #     "/organization/*/user/*/profile" = "/organization/:orgID/user/:userID/profile"
+            #     "/events/*/rsvps" = "EventRSVPs"
+            #   }
+            #
+            # Client requests to "/organization/3651/user/39652/profile" and "/organization/22234/user/54543/profile" will have
+            # the same operation name "/organization/:orgID/user/:userID/profile".
+            #
+            # Similarly, requests to "/events/aaa-bb-ccc/rsvps" and "/events/1234/rsvps" will have the same operation
+            # name "EventRSVPs".
+            #
+            # The patterns are expressed as globs and the operation names are free form.
+            #
+            mappings {
+
+            }
           }
         }
       }

diff --git a/...entation-common/src/main/scala/kamon/instrumentation/http/HttpClientInstrumentation.scala b/...entation-common/src/main/scala/kamon/instrumentation/http/HttpClientInstrumentation.scala
@@ -10,6 +10,7 @@ import kamon.instrumentation.trace.SpanTagger
 import kamon.instrumentation.trace.SpanTagger.TagMode
 import kamon.tag.Lookups.option
 import kamon.trace.Span
+import kamon.util.Filter
 import org.slf4j.LoggerFactory
 
 import scala.util.Try
@@ -131,7 +132,7 @@ object HttpClientInstrumentation {
 
     private def createClientSpan(requestMessage: HttpMessage.Request, context: Context): Span = {
       val span = Kamon
-        .clientSpanBuilder(operationName(requestMessage), component)
+        .clientSpanBuilder(settings.operationNameSettings.operationName(requestMessage), component)
         .context(context)
 
       if(!settings.enableSpanMetrics)
@@ -149,12 +150,9 @@ object HttpClientInstrumentation {
 
       span.start()
     }
-
-    private def operationName(requestMessage: HttpMessage.Request): String =
-      settings.operationNameGenerator.name(requestMessage).getOrElse(settings.defaultOperationName)
   }
 
-  case class Settings (
+  final case class Settings (
     enableContextPropagation: Boolean,
     propagationChannel: String,
     enableTracing: Boolean,
@@ -164,8 +162,11 @@ object HttpClientInstrumentation {
     statusCodeTagMode: TagMode,
     contextTags: Map[String, TagMode],
     defaultOperationName: String,
+    operationMappings: Map[Filter.Glob, String],
     operationNameGenerator: HttpOperationNameGenerator
-  )
+  ) {
+    val operationNameSettings = OperationNameSettings(defaultOperationName, operationMappings, operationNameGenerator)
+  }
 
   object Settings {
 
@@ -197,6 +198,9 @@ object HttpClientInstrumentation {
           _log.warn("Failed to create an HTTP Operation Name Generator, falling back to the default operation name", t)
           new HttpOperationNameGenerator.Static(defaultOperationName)
       }
+      val operationMappings = config.getConfig("tracing.operations.mappings").pairs.map {
+        case (pattern, operationName) => (Filter.Glob(pattern), operationName)
+      }
 
       Settings(
         enablePropagation,
@@ -208,6 +212,7 @@ object HttpClientInstrumentation {
         statusCodeTagMode,
         contextTags,
         defaultOperationName,
+        operationMappings,
         operationNameGenerator.get
       )
     }

diff --git a/...entation-common/src/main/scala/kamon/instrumentation/http/HttpServerInstrumentation.scala b/...entation-common/src/main/scala/kamon/instrumentation/http/HttpServerInstrumentation.scala
@@ -295,7 +295,7 @@ object HttpServerInstrumentation {
 
 
     private def buildServerSpan(context: Context, request: HttpMessage.Request, deferSamplingDecision: Boolean): Span = {
-      val span = Kamon.serverSpanBuilder(operationName(request), component)
+      val span = Kamon.serverSpanBuilder(settings.operationNameSettings.operationName(request), component)
       span.context(context)
 
       if(!settings.enableSpanMetrics)
@@ -322,21 +322,10 @@ object HttpServerInstrumentation {
 
       span.start()
     }
-
-    private def operationName(request: HttpMessage.Request): String = {
-      val requestPath = request.path
-      //first apply any mappings rules
-      val customMapping = settings.operationMappings.collectFirst {
-        case (pattern, operationName) if pattern.accept(requestPath) => operationName
-      }.orElse( //fallback to use any configured name generator
-        settings.operationNameGenerator.name(request)
-     )
-      customMapping.getOrElse(settings.defaultOperationName)
-    }
   }
 
 
-  case class Settings(
+  final case class Settings(
     enableContextPropagation: Boolean,
     propagationChannel: String,
     enableServerMetrics: Boolean,
@@ -353,7 +342,9 @@ object HttpServerInstrumentation {
     unhandledOperationName: String,
     operationMappings: Map[Filter.Glob, String],
     operationNameGenerator: HttpOperationNameGenerator
-  )
+  ) {
+    val operationNameSettings = OperationNameSettings(defaultOperationName, operationMappings, operationNameGenerator)
+  }
 
   object Settings {
 
@@ -395,7 +386,7 @@ object HttpServerInstrumentation {
       }
       val unhandledOperationName = config.getString("tracing.operations.unhandled")
       val operationMappings = config.getConfig("tracing.operations.mappings").pairs.map {
-        case (pattern, operationName) => (new Filter.Glob(pattern), operationName)
+        case (pattern, operationName) => (Filter.Glob(pattern), operationName)
       }
 
       Settings(

diff --git a/...trumentation-common/src/main/scala/kamon/instrumentation/http/OperationNameSettings.scala b/...trumentation-common/src/main/scala/kamon/instrumentation/http/OperationNameSettings.scala
@@ -0,0 +1,18 @@
+package kamon.instrumentation.http
+
+import kamon.util.Filter
+
+final case class OperationNameSettings(defaultOperationName: String,
+                                       operationMappings: Map[Filter.Glob, String],
+                                       operationNameGenerator: HttpOperationNameGenerator) {
+  private[http] def operationName(request: HttpMessage.Request): String = {
+    val requestPath = request.path
+    //first apply any mappings rules
+    val customMapping = operationMappings.collectFirst {
+      case (pattern, operationName) if pattern.accept(requestPath) => operationName
+    }.orElse( //fallback to use any configured name generator
+      operationNameGenerator.name(request)
+    )
+    customMapping.getOrElse(defaultOperationName)
+  }
+}
diff --git a/instrumentation/kamon-instrumentation-common/src/test/resources/application.conf b/instrumentation/kamon-instrumentation-common/src/test/resources/application.conf
@@ -50,6 +50,16 @@ kamon {
     }
 
     http-client {
+      default {
+        tracing {
+          operations {
+            default = "default-name"
+            mappings {
+              "/events/*/rsvps" = "EventRSVPs"
+            }
+          }
+        }
+      }
 
       no-span-metrics {
         tracing.span-metrics = off

diff --git a/...tion-common/src/test/scala/kamon/instrumentation/http/HttpClientInstrumentationSpec.scala b/...tion-common/src/test/scala/kamon/instrumentation/http/HttpClientInstrumentationSpec.scala
@@ -83,6 +83,18 @@ class HttpClientInstrumentationSpec extends WordSpec with Matchers with Instrume
         handler.request.read("X-B3-ParentSpanId").value shouldBe parent.id.string
         handler.request.read("X-B3-Sampled") shouldBe defined
       }
+
+      "honour user provided operation name mappings" in {
+        val handler = httpClient().createHandler(fakeRequest("http://localhost:8080/", "/events/123/rsvps", "GET", Map.empty), Context.Empty)
+        handler.processResponse(fakeResponse(200))
+
+        val span = handler.span
+        span.operationName() shouldBe "EventRSVPs"
+        span.tags().get(plain("http.method")) shouldBe "GET"
+        span.tags().get(plain("http.url")) shouldBe "http://localhost:8080/"
+        span.tags().get(plainLong("http.status_code")) shouldBe 200
+        span.tags().get(plain("operation")) shouldBe "EventRSVPs"
+      }
     }
 
     "all capabilities are disabled" should {

diff --git a/...n/kamon-jdbc/src/test/scala/kamon/instrumentation/jdbc/StatementInstrumentationSpec.scala b/...n/kamon-jdbc/src/test/scala/kamon/instrumentation/jdbc/StatementInstrumentationSpec.scala
@@ -89,7 +89,7 @@ class StatementInstrumentationSpec extends WordSpec with Matchers with Eventuall
           statement.execute()
           validateNextRow(statement.getResultSet, valueNr = 1, valueName = "foo")
 
-          eventually(timeout(scaled(5 seconds))) {
+          eventually(timeout(scaled(5 seconds)), interval(200 millis)) {
             val span = testSpanReporter().nextSpan().value
             span.operationName shouldBe StatementTypes.GenericExecute
             span.metricTags.get(plain("component")) shouldBe "jdbc"