From ce035b5e3ffeb2f7b127ca6694cb87afb20bd939 Mon Sep 17 00:00:00 2001 From: Yaroslav Derman Date: Thu, 21 Jan 2021 00:07:23 +0200 Subject: [PATCH 1/4] play - allow to define custom router name generator --- instrumentation/kamon-play/build.sbt | 16 +++--- .../src/main/resources/reference.conf | 4 ++ .../play/GrpcRouterNameGenerator.scala | 15 +++++ .../play/PlayServerInstrumentation.scala | 55 +++++++++++++++---- .../play/RouterOperationNameGenerator.scala | 23 ++++++++ 5 files changed, 93 insertions(+), 20 deletions(-) create mode 100644 instrumentation/kamon-play/src/main/scala/kamon/instrumentation/play/GrpcRouterNameGenerator.scala create mode 100644 instrumentation/kamon-play/src/main/scala/kamon/instrumentation/play/RouterOperationNameGenerator.scala diff --git a/instrumentation/kamon-play/build.sbt b/instrumentation/kamon-play/build.sbt index aae46459b..16ada95f4 100644 --- a/instrumentation/kamon-play/build.sbt +++ b/instrumentation/kamon-play/build.sbt @@ -1,9 +1,9 @@ import sbt.Tests._ import play.grpc.gen.scaladsl.{PlayScalaServerCodeGenerator, PlayScalaClientCodeGenerator} -val `Play-2.6-version` = "2.6.23" -val `Play-2.7-version` = "2.7.3" -val `Play-2.8-version` = "2.8.2" +val `Play-2.6-version` = "2.6.25" +val `Play-2.7-version` = "2.7.9" +val `Play-2.8-version` = "2.8.7" /** * Test Configurations @@ -45,8 +45,8 @@ libraryDependencies ++= { if(scalaBinaryVersion.value == "2.13") Seq.empty else )} libraryDependencies ++= { if(scalaBinaryVersion.value == "2.11") Seq.empty else Seq( - "com.lightbend.play" %% "play-grpc-runtime" % "0.9.0" % "test-play-2.8", - "com.lightbend.play" %% "play-grpc-scalatest" % "0.9.0" % "test-play-2.8", + "com.lightbend.play" %% "play-grpc-runtime" % "0.9.1" % "test-play-2.8", + "com.lightbend.play" %% "play-grpc-scalatest" % "0.9.1" % "test-play-2.8", "com.typesafe.play" %% "play-akka-http2-support" % `Play-2.8-version` % "test-play-2.8", "com.typesafe.play" %% "play" % `Play-2.8-version` % "test-play-2.8", "com.typesafe.play" %% "play-netty-server" % `Play-2.8-version` % "test-play-2.8", @@ -73,12 +73,12 @@ lazy val baseTestSettings = Seq( ) inConfig(TestCommon)(Defaults.testSettings ++ instrumentationSettings ++ baseTestSettings ++ Seq( - crossScalaVersions := Seq("2.11.12", "2.12.11") + crossScalaVersions := Seq("2.11.12", "2.12.13") )) inConfig(`Test-Play-2.6`)(Defaults.testSettings ++ instrumentationSettings ++ baseTestSettings ++ Seq( sources := joinSources(TestCommon, `Test-Play-2.6`).value, - crossScalaVersions := Seq("2.11.12", "2.12.11"), + crossScalaVersions := Seq("2.11.12", "2.12.13"), testGrouping := singleTestPerJvm(definedTests.value, javaOptions.value), unmanagedResourceDirectories ++= (unmanagedResourceDirectories in Compile).value, unmanagedResourceDirectories ++= (unmanagedResourceDirectories in TestCommon).value, @@ -93,7 +93,7 @@ inConfig(`Test-Play-2.7`)(Defaults.testSettings ++ instrumentationSettings ++ ba inConfig(`Test-Play-2.8`)(Defaults.testSettings ++ instrumentationSettings ++ baseTestSettings ++ Seq( sources := joinSources(TestCommon, `Test-Play-2.8`).value, - crossScalaVersions := Seq("2.12.11", "2.13.1"), + crossScalaVersions := Seq("2.12.13", "2.13.3"), akkaGrpcGeneratedSources := Seq(AkkaGrpc.Server, AkkaGrpc.Client), akkaGrpcGeneratedLanguages := Seq(AkkaGrpc.Scala), akkaGrpcExtraGenerators += PlayScalaServerCodeGenerator, diff --git a/instrumentation/kamon-play/src/main/resources/reference.conf b/instrumentation/kamon-play/src/main/resources/reference.conf index 6608ddc31..ec8a98364 100644 --- a/instrumentation/kamon-play/src/main/resources/reference.conf +++ b/instrumentation/kamon-play/src/main/resources/reference.conf @@ -139,6 +139,10 @@ kamon.instrumentation.play.http { } } } + + name-generator = "kamon.instrumentation.play.DefaultRouterOperationNameGenerator" + + grpc-name-generator = "kamon.instrumentation.play.DefaultGrpcRouterNameGenerator" } client { diff --git a/instrumentation/kamon-play/src/main/scala/kamon/instrumentation/play/GrpcRouterNameGenerator.scala b/instrumentation/kamon-play/src/main/scala/kamon/instrumentation/play/GrpcRouterNameGenerator.scala new file mode 100644 index 000000000..d2c455835 --- /dev/null +++ b/instrumentation/kamon-play/src/main/scala/kamon/instrumentation/play/GrpcRouterNameGenerator.scala @@ -0,0 +1,15 @@ +package kamon.instrumentation.play + +import akka.http.scaladsl.model.HttpRequest + +trait GrpcRouterNameGenerator { + + def generateOperationName(request: akka.http.scaladsl.model.HttpRequest): String + +} + +class DefaultGrpcRouterNameGenerator extends GrpcRouterNameGenerator { + + override def generateOperationName(request: HttpRequest): String = request.uri.toRelative.toString + +} \ No newline at end of file diff --git a/instrumentation/kamon-play/src/main/scala/kamon/instrumentation/play/PlayServerInstrumentation.scala b/instrumentation/kamon-play/src/main/scala/kamon/instrumentation/play/PlayServerInstrumentation.scala index 251de41b3..29c392a28 100644 --- a/instrumentation/kamon-play/src/main/scala/kamon/instrumentation/play/PlayServerInstrumentation.scala +++ b/instrumentation/kamon-play/src/main/scala/kamon/instrumentation/play/PlayServerInstrumentation.scala @@ -19,10 +19,11 @@ package kamon.instrumentation.play import java.time.Duration import java.util.concurrent.atomic.AtomicLong +import com.typesafe.config.Config import io.netty.channel.Channel import io.netty.handler.codec.http.{HttpRequest, HttpResponse} import io.netty.util.concurrent.GenericFutureListener -import kamon.Kamon +import kamon.{ClassLoading, Kamon} import kamon.context.Storage import kamon.instrumentation.akka.http.ServerFlowWrapper import kamon.instrumentation.context.{CaptureCurrentTimestampOnExit, HasTimestamp} @@ -33,12 +34,13 @@ import kanela.agent.api.instrumentation.InstrumentationBuilder import kanela.agent.api.instrumentation.classloader.ClassRefiner import kanela.agent.api.instrumentation.mixin.Initializer import kanela.agent.libs.net.bytebuddy.asm.Advice +import org.slf4j.LoggerFactory import play.api.mvc.RequestHeader import play.api.routing.{HandlerDef, Router} import play.core.server.NettyServer import scala.collection.JavaConverters.asScalaBufferConverter -import scala.collection.concurrent.TrieMap +import scala.util.Try import scala.util.{Failure, Success} class PlayServerInstrumentation extends InstrumentationBuilder { @@ -243,32 +245,61 @@ object HasServerInstrumentation { object GenerateOperationNameOnFilterHandler { - private val _operationNameCache = TrieMap.empty[String, String] - private val _normalizePattern = """\$([^<]+)<[^>]+>""".r + private val defaultRouterNameGenerator = new DefaultRouterOperationNameGenerator() + private val _logger = LoggerFactory.getLogger(GenerateOperationNameOnFilterHandler.getClass) + + @volatile private var _routerNameGenerator: RouterOperationNameGenerator = rebuildRouterNameGenerator(Kamon.config()) + + Kamon.onReconfigure(newConfig => _routerNameGenerator = rebuildRouterNameGenerator(newConfig)) + + private def rebuildRouterNameGenerator(config: Config): RouterOperationNameGenerator = { + val nameGeneratorClazz = config.getString("kamon.instrumentation.play.http.server.name-generator") + Try(ClassLoading.createInstance[RouterOperationNameGenerator](nameGeneratorClazz)) match { + case Failure(exception) => + _logger.error(s"Exception occurred on $nameGeneratorClazz instance creation, used default", exception) + defaultRouterNameGenerator + case Success(value) => + value + } + } @Advice.OnMethodEnter def enter(@Advice.Argument(0) request: RequestHeader): Unit = { request.attrs.get(Router.Attrs.HandlerDef).map(handler => { val span = Kamon.currentSpan() - span.name(generateOperationName(handler)) + span.name(_routerNameGenerator.generateOperationName(handler)) span.takeSamplingDecision() }) } - private def generateOperationName(handlerDef: HandlerDef): String = - _operationNameCache.getOrElseUpdate(handlerDef.path, { - // Convert paths of form /foo/bar/$paramname/blah to /foo/bar/paramname/blah - _normalizePattern.replaceAllIn(handlerDef.path, "$1") - }) - } object GenerateGRPCOperationName { + + private val defaultGrpcRouterNameGenerator = new DefaultGrpcRouterNameGenerator() + private val _logger = LoggerFactory.getLogger(GenerateOperationNameOnFilterHandler.getClass) + + @volatile private var _grpcRouterNameGenerator: GrpcRouterNameGenerator = rebuildRouterNameGenerator(Kamon.config()) + + Kamon.onReconfigure(newConfig => _grpcRouterNameGenerator = rebuildRouterNameGenerator(newConfig)) + + private def rebuildRouterNameGenerator(config: Config): GrpcRouterNameGenerator = { + val nameGeneratorClazz = config.getString("kamon.instrumentation.play.http.server.grpc-name-generator") + Try(ClassLoading.createInstance[GrpcRouterNameGenerator](nameGeneratorClazz)) match { + case Failure(exception) => + _logger.error(s"Exception occurred on $nameGeneratorClazz instance creation, used default", exception) + defaultGrpcRouterNameGenerator + case Success(value) => + value + } + } + @Advice.OnMethodEnter def enter(@Advice.Argument(0) request: akka.http.scaladsl.model.HttpRequest): Unit = { val span = Kamon.currentSpan() - span.name(request.uri.toRelative.toString) + span.name(_grpcRouterNameGenerator.generateOperationName(request)) span.tag("http.protocol", request.protocol.value) span.takeSamplingDecision() } + } diff --git a/instrumentation/kamon-play/src/main/scala/kamon/instrumentation/play/RouterOperationNameGenerator.scala b/instrumentation/kamon-play/src/main/scala/kamon/instrumentation/play/RouterOperationNameGenerator.scala new file mode 100644 index 000000000..906b93bd4 --- /dev/null +++ b/instrumentation/kamon-play/src/main/scala/kamon/instrumentation/play/RouterOperationNameGenerator.scala @@ -0,0 +1,23 @@ +package kamon.instrumentation.play + +import play.api.routing.HandlerDef + +import scala.collection.concurrent.TrieMap + +trait RouterOperationNameGenerator { + def generateOperationName(handlerDef: HandlerDef): String +} + +class DefaultRouterOperationNameGenerator extends RouterOperationNameGenerator { + + private val _operationNameCache = TrieMap.empty[String, String] + private val _normalizePattern = """\$([^<]+)<[^>]+>""".r + + def generateOperationName(handlerDef: HandlerDef): String = { + _operationNameCache.getOrElseUpdate(handlerDef.path, { + // Convert paths of form /foo/bar/$paramname/blah to /foo/bar/paramname/blah + _normalizePattern.replaceAllIn(handlerDef.path, "$1") + }) + } + +} From b774d8ccb89b0426ae1af58983cb929c07e8e729 Mon Sep 17 00:00:00 2001 From: Yaroslav Derman Date: Thu, 21 Jan 2021 00:08:56 +0200 Subject: [PATCH 2/4] added newline --- .../kamon/instrumentation/play/GrpcRouterNameGenerator.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/kamon-play/src/main/scala/kamon/instrumentation/play/GrpcRouterNameGenerator.scala b/instrumentation/kamon-play/src/main/scala/kamon/instrumentation/play/GrpcRouterNameGenerator.scala index d2c455835..5fcc256e3 100644 --- a/instrumentation/kamon-play/src/main/scala/kamon/instrumentation/play/GrpcRouterNameGenerator.scala +++ b/instrumentation/kamon-play/src/main/scala/kamon/instrumentation/play/GrpcRouterNameGenerator.scala @@ -12,4 +12,4 @@ class DefaultGrpcRouterNameGenerator extends GrpcRouterNameGenerator { override def generateOperationName(request: HttpRequest): String = request.uri.toRelative.toString -} \ No newline at end of file +} From c3c74211c8fb2a91ab6165706b4b8a030a58204c Mon Sep 17 00:00:00 2001 From: Yaroslav Derman Date: Wed, 17 Mar 2021 18:50:31 +0200 Subject: [PATCH 3/4] apply PR comments --- .../src/main/resources/reference.conf | 30 +++++++++++++++++-- .../play/PlayServerInstrumentation.scala | 4 +-- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/instrumentation/kamon-play/src/main/resources/reference.conf b/instrumentation/kamon-play/src/main/resources/reference.conf index ec8a98364..cc224b6e0 100644 --- a/instrumentation/kamon-play/src/main/resources/reference.conf +++ b/instrumentation/kamon-play/src/main/resources/reference.conf @@ -107,6 +107,12 @@ kamon.instrumentation.play.http { # Custom mappings between routes and operation names. operations { + # Default configuration for HttpOperationNameGenerator implementation, but it has never been used + # - default: Uses the set default operation name + # - method: Uses the request HTTP method as the operation name. + # + name-generator = "default" + # The default operation name to be used when creating Spans to handle the HTTP server requests. In most # cases it is not possible to define an operation name right at the moment of starting the HTTP server Span # and in those cases, this operation name will be initially assigned to the Span. Instrumentation authors @@ -140,9 +146,29 @@ kamon.instrumentation.play.http { } } - name-generator = "kamon.instrumentation.play.DefaultRouterOperationNameGenerator" + # + # Configure specefic tracing/monitoring elements which depends on `play-framework` internal APIs that might change in the future + # + extra { + # Framework-specific way to generate operations names instead of using `kamon.instrumentation.play.http.server.tracing.operations.name-generator` + # Used by default with implementations that converts paths of form `/foo/bar/$paramname/blah` to `/foo/bar/paramname/blah` + # + # FQCN for a kamon.instrumentation.play.RouterOperationNameGenerator implementation, or ony of the following shorthand forms: + # - default: Uses the set default operation name + # - method: Uses the request HTTP method as the operation name. + # + name-generator = "kamon.instrumentation.play.DefaultRouterOperationNameGenerator" + + # Framework-specific way to generate operations names instead of using `kamon.instrumentation.play.http.server.tracing.operations.name-generator` + # Used by default with setting operation name to `request.uri.toRelative` + # + # FQCN for a kamon.instrumentation.play.GrpcRouterNameGenerator implementation, or ony of the following shorthand forms: + # - default: Uses the set default operation name + # - method: Uses the request HTTP method as the operation name. + # + grpc-name-generator = "kamon.instrumentation.play.DefaultGrpcRouterNameGenerator" + } - grpc-name-generator = "kamon.instrumentation.play.DefaultGrpcRouterNameGenerator" } client { diff --git a/instrumentation/kamon-play/src/main/scala/kamon/instrumentation/play/PlayServerInstrumentation.scala b/instrumentation/kamon-play/src/main/scala/kamon/instrumentation/play/PlayServerInstrumentation.scala index 29c392a28..72feaeb50 100644 --- a/instrumentation/kamon-play/src/main/scala/kamon/instrumentation/play/PlayServerInstrumentation.scala +++ b/instrumentation/kamon-play/src/main/scala/kamon/instrumentation/play/PlayServerInstrumentation.scala @@ -253,7 +253,7 @@ object GenerateOperationNameOnFilterHandler { Kamon.onReconfigure(newConfig => _routerNameGenerator = rebuildRouterNameGenerator(newConfig)) private def rebuildRouterNameGenerator(config: Config): RouterOperationNameGenerator = { - val nameGeneratorClazz = config.getString("kamon.instrumentation.play.http.server.name-generator") + val nameGeneratorClazz = config.getString("kamon.instrumentation.play.http.server.extra.name-generator") Try(ClassLoading.createInstance[RouterOperationNameGenerator](nameGeneratorClazz)) match { case Failure(exception) => _logger.error(s"Exception occurred on $nameGeneratorClazz instance creation, used default", exception) @@ -284,7 +284,7 @@ object GenerateGRPCOperationName { Kamon.onReconfigure(newConfig => _grpcRouterNameGenerator = rebuildRouterNameGenerator(newConfig)) private def rebuildRouterNameGenerator(config: Config): GrpcRouterNameGenerator = { - val nameGeneratorClazz = config.getString("kamon.instrumentation.play.http.server.grpc-name-generator") + val nameGeneratorClazz = config.getString("kamon.instrumentation.play.http.server.extra.grpc-name-generator") Try(ClassLoading.createInstance[GrpcRouterNameGenerator](nameGeneratorClazz)) match { case Failure(exception) => _logger.error(s"Exception occurred on $nameGeneratorClazz instance creation, used default", exception) From 1fada3d87b0eb7525f8ac8a365810042550cd9db Mon Sep 17 00:00:00 2001 From: Yaroslav Derman Date: Wed, 17 Mar 2021 18:51:08 +0200 Subject: [PATCH 4/4] apply PR comments --- instrumentation/kamon-play/src/main/resources/reference.conf | 1 - 1 file changed, 1 deletion(-) diff --git a/instrumentation/kamon-play/src/main/resources/reference.conf b/instrumentation/kamon-play/src/main/resources/reference.conf index cc224b6e0..9ab09325a 100644 --- a/instrumentation/kamon-play/src/main/resources/reference.conf +++ b/instrumentation/kamon-play/src/main/resources/reference.conf @@ -168,7 +168,6 @@ kamon.instrumentation.play.http { # grpc-name-generator = "kamon.instrumentation.play.DefaultGrpcRouterNameGenerator" } - } client {