guard against errors in the HTTP client request instrumentation, fixes …

…#813
kamon-io · Jul 29, 2020 · ad586db · ad586db
1 parent 05613bf
commit ad586db
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 12 deletions.
diff --git a/...trumentation-common/src/main/scala/kamon/instrumentation/http/OperationNameSettings.scala b/...trumentation-common/src/main/scala/kamon/instrumentation/http/OperationNameSettings.scala
@@ -1,18 +1,30 @@
 package kamon.instrumentation.http
 
 import kamon.util.Filter
+import org.slf4j.LoggerFactory
+
+import scala.util.Try
+
+final case class OperationNameSettings(
+    defaultOperationName: String,
+    operationMappings: Map[Filter.Glob, String],
+    operationNameGenerator: HttpOperationNameGenerator) {
+
+  private val logger = LoggerFactory.getLogger(classOf[OperationNameSettings])
 
-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)
+    Try {
+      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)
+    } getOrElse(defaultOperationName)
   }
 }
diff --git a/...tion-common/src/test/scala/kamon/instrumentation/http/HttpClientInstrumentationSpec.scala b/...tion-common/src/test/scala/kamon/instrumentation/http/HttpClientInstrumentationSpec.scala
@@ -95,6 +95,18 @@ class HttpClientInstrumentationSpec extends WordSpec with Matchers with Instrume
         span.tags().get(plainLong("http.status_code")) shouldBe 200
         span.tags().get(plain("operation")) shouldBe "EventRSVPs"
       }
+
+      "fallback to the default operation name when there is a problem while figuring out a custom operation name" in {
+        val handler = httpClient().createHandler(fakeRequest("http://localhost:8080/", "/fail-operation-name", "GET", Map.empty), Context.Empty)
+        handler.processResponse(fakeResponse(200))
+
+        val span = handler.span
+        span.operationName() shouldBe "default-name"
+        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 "default-name"
+      }
     }
 
     "all capabilities are disabled" should {
@@ -133,7 +145,7 @@ class HttpClientInstrumentationSpec extends WordSpec with Matchers with Instrume
       initialHeaders.foreach { case (k, v) => _headers += (k -> v)}
 
       override def url: String = requestUrl
-      override def path: String = requestPath
+      override def path: String = if(requestPath == "/fail-operation-name") sys.error("fail") else requestPath
       override def method: String = requestMethod
       override def read(header: String): Option[String] = _headers.get(header)
       override def readAll(): Map[String, String] = _headers.toMap