From 29d82a7dc5f54c3d32bf53c764e5f9a42f74aa90 Mon Sep 17 00:00:00 2001 From: Lawrence Carvalho Date: Tue, 16 Jun 2020 09:50:27 +0100 Subject: [PATCH] + adds configurable operation name mappings for http client instrumentation (#771) --- .../akka/http/AkkaHttpInstrumentation.scala | 4 +--- .../src/test/resources/application.conf | 10 +++++++++ .../akka/http/AkkaHttpClientTracingSpec.scala | 19 ++++++++++++----- .../akka/http/AkkaHttpServerTracingSpec.scala | 6 ++---- .../src/main/resources/reference.conf | 20 ++++++++++++++++++ .../http/HttpClientInstrumentation.scala | 17 +++++++++------ .../http/HttpServerInstrumentation.scala | 21 ++++++------------- .../http/OperationNameSettings.scala | 18 ++++++++++++++++ .../src/test/resources/application.conf | 10 +++++++++ .../http/HttpClientInstrumentationSpec.scala | 12 +++++++++++ .../jdbc/StatementInstrumentationSpec.scala | 2 +- 11 files changed, 105 insertions(+), 34 deletions(-) create mode 100644 instrumentation/kamon-instrumentation-common/src/main/scala/kamon/instrumentation/http/OperationNameSettings.scala diff --git a/instrumentation/kamon-akka-http/src/main/scala/kamon/instrumentation/akka/http/AkkaHttpInstrumentation.scala b/instrumentation/kamon-akka-http/src/main/scala/kamon/instrumentation/akka/http/AkkaHttpInstrumentation.scala index bd9a5ff91..f49484783 100644 --- a/instrumentation/kamon-akka-http/src/main/scala/kamon/instrumentation/akka/http/AkkaHttpInstrumentation.scala +++ b/instrumentation/kamon-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 index 795b2a76a..bc5b127c4 100644 --- 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" + } + } + } + } } \ No newline at end of file diff --git a/instrumentation/kamon-akka-http/src/test/scala/kamon/akka/http/AkkaHttpClientTracingSpec.scala b/instrumentation/kamon-akka-http/src/test/scala/kamon/akka/http/AkkaHttpClientTracingSpec.scala index f9b9b0c7a..5c7e9ab3d 100644 --- a/instrumentation/kamon-akka-http/src/test/scala/kamon/akka/http/AkkaHttpClientTracingSpec.scala +++ b/instrumentation/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/instrumentation/kamon-akka-http/src/test/scala/kamon/akka/http/AkkaHttpServerTracingSpec.scala b/instrumentation/kamon-akka-http/src/test/scala/kamon/akka/http/AkkaHttpServerTracingSpec.scala index 3ad93fdc6..28fb8277e 100644 --- a/instrumentation/kamon-akka-http/src/test/scala/kamon/akka/http/AkkaHttpServerTracingSpec.scala +++ b/instrumentation/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 index 81b5cb253..354fa850d 100644 --- 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/instrumentation/kamon-instrumentation-common/src/main/scala/kamon/instrumentation/http/HttpClientInstrumentation.scala b/instrumentation/kamon-instrumentation-common/src/main/scala/kamon/instrumentation/http/HttpClientInstrumentation.scala index a3591ba60..896479673 100644 --- a/instrumentation/kamon-instrumentation-common/src/main/scala/kamon/instrumentation/http/HttpClientInstrumentation.scala +++ b/instrumentation/kamon-instrumentation-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/instrumentation/kamon-instrumentation-common/src/main/scala/kamon/instrumentation/http/HttpServerInstrumentation.scala b/instrumentation/kamon-instrumentation-common/src/main/scala/kamon/instrumentation/http/HttpServerInstrumentation.scala index cfd6e5a90..367317c1d 100644 --- a/instrumentation/kamon-instrumentation-common/src/main/scala/kamon/instrumentation/http/HttpServerInstrumentation.scala +++ b/instrumentation/kamon-instrumentation-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/instrumentation/kamon-instrumentation-common/src/main/scala/kamon/instrumentation/http/OperationNameSettings.scala b/instrumentation/kamon-instrumentation-common/src/main/scala/kamon/instrumentation/http/OperationNameSettings.scala new file mode 100644 index 000000000..52c831ebf --- /dev/null +++ b/instrumentation/kamon-instrumentation-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 index 47273e9db..ec5fba492 100644 --- 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/instrumentation/kamon-instrumentation-common/src/test/scala/kamon/instrumentation/http/HttpClientInstrumentationSpec.scala b/instrumentation/kamon-instrumentation-common/src/test/scala/kamon/instrumentation/http/HttpClientInstrumentationSpec.scala index 53ae43822..d8f613d7f 100644 --- a/instrumentation/kamon-instrumentation-common/src/test/scala/kamon/instrumentation/http/HttpClientInstrumentationSpec.scala +++ b/instrumentation/kamon-instrumentation-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/instrumentation/kamon-jdbc/src/test/scala/kamon/instrumentation/jdbc/StatementInstrumentationSpec.scala b/instrumentation/kamon-jdbc/src/test/scala/kamon/instrumentation/jdbc/StatementInstrumentationSpec.scala index 2bfd07455..d6ef7bd4d 100644 --- a/instrumentation/kamon-jdbc/src/test/scala/kamon/instrumentation/jdbc/StatementInstrumentationSpec.scala +++ b/instrumentation/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"