From 967e47863373a2a1d1e42067f8630b12513c0c2f Mon Sep 17 00:00:00 2001 From: Sriraam AS Date: Wed, 8 Sep 2021 23:48:00 -0400 Subject: [PATCH 1/5] handle failures when underlying actionbuilder throws exception --- .../io/opentracing/play/TracingAction.scala | 34 +++++++++++++------ 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/play/src/main/scala/io/opentracing/play/TracingAction.scala b/play/src/main/scala/io/opentracing/play/TracingAction.scala index 64aaaa4..387a921 100644 --- a/play/src/main/scala/io/opentracing/play/TracingAction.scala +++ b/play/src/main/scala/io/opentracing/play/TracingAction.scala @@ -11,6 +11,7 @@ import akka.stream.ActorMaterializer import play.api.mvc._ import scala.concurrent.{ExecutionContext, Future} +import scala.util.{Failure, Success, Try} private object RequestSpan { def apply(tracer: Tracer, request: RequestHeader): Span = @@ -53,18 +54,29 @@ class TracingActionBuilder( contextSpan .set(span) .call(new Callable[Future[Result]] { + + private def failureHandler(exception: Throwable): Future[Result] = { + Tags.ERROR.set(tracingRequest.span, true) + finishSpan(tracingRequest, None) + Future.failed(exception) + } + def call() = - block(tracingRequest) - .map { result => - finishSpan(tracingRequest, Some(result)) - result - } - .recoverWith { - case e => - Tags.ERROR.set(tracingRequest.span, true) - finishSpan(tracingRequest, None) - Future.failed(e) - } + Try(block(tracingRequest)) match { + case Failure(exception) => + failureHandler(exception) + case Success(resultFuture) => + resultFuture + .map { result => + finishSpan(tracingRequest, Some(result)) + result + } + .recoverWith { + case exception => + failureHandler(exception) + } + } + }) } From 6bfca12e1f20c1ea2193a561150025fd3307d898 Mon Sep 17 00:00:00 2001 From: Sriraam AS Date: Wed, 8 Sep 2021 23:48:53 -0400 Subject: [PATCH 2/5] use materializer instead of actor materializer when using default body parser --- play/src/main/scala/io/opentracing/play/TracingAction.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/play/src/main/scala/io/opentracing/play/TracingAction.scala b/play/src/main/scala/io/opentracing/play/TracingAction.scala index 387a921..9b984c4 100644 --- a/play/src/main/scala/io/opentracing/play/TracingAction.scala +++ b/play/src/main/scala/io/opentracing/play/TracingAction.scala @@ -7,7 +7,7 @@ import io.opentracing.tag.Tags import io.opentracing.threadcontext.ContextSpan import java.util.concurrent.Callable -import akka.stream.ActorMaterializer +import akka.stream.Materializer import play.api.mvc._ import scala.concurrent.{ExecutionContext, Future} @@ -88,5 +88,5 @@ class TracingActionBuilder( */ class DefaultTracingActionBuilder(taggers: Traversable[SpanTagger])( implicit ec: ExecutionContext, - mat: ActorMaterializer + mat: Materializer ) extends TracingActionBuilder(GlobalTracer.get, ContextSpan.DEFAULT, taggers)(new BodyParsers.Default) From 7fcca8c34bb3285d5dc66978f364fd39f29855dc Mon Sep 17 00:00:00 2001 From: Sriraam AS Date: Thu, 9 Sep 2021 07:41:05 -0400 Subject: [PATCH 3/5] enable fatal warnings --- play/build.sbt | 4 +++- play/src/main/scala/io/opentracing/play/HeadersTextMap.scala | 2 +- play/src/main/scala/io/opentracing/play/SpanTagger.scala | 2 +- play/src/main/scala/io/opentracing/play/TracingAction.scala | 4 ++-- 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/play/build.sbt b/play/build.sbt index 469f3be..904ac38 100644 --- a/play/build.sbt +++ b/play/build.sbt @@ -5,7 +5,8 @@ libraryDependencies ++= Seq( "com.google.guava" % "guava" % "21.0", "com.typesafe.play" %% "play" % SettingKey[String]("playVersion").value, "io.opentracing" % "opentracing-api" % "0.31.0", - "io.opentracing" % "opentracing-util" % "0.31.0" + "io.opentracing" % "opentracing-util" % "0.31.0", + "org.scala-lang.modules" %% "scala-collection-compat" % "2.5.0" ) moduleName := "opentracing-play" @@ -13,3 +14,4 @@ publishTo := sonatypePublishToBundle.value // This is needed because the play axis changes the version sonatypeBundleDirectory := (ThisBuild / baseDirectory).value / "target" / "sonatype-staging" / (Global / version).value scalafmtOnCompile := true +scalacOptions ++= Seq("-unchecked", "-deprecation", "-feature", "-Xfatal-warnings") diff --git a/play/src/main/scala/io/opentracing/play/HeadersTextMap.scala b/play/src/main/scala/io/opentracing/play/HeadersTextMap.scala index de70745..63792eb 100644 --- a/play/src/main/scala/io/opentracing/play/HeadersTextMap.scala +++ b/play/src/main/scala/io/opentracing/play/HeadersTextMap.scala @@ -3,7 +3,7 @@ package io.opentracing.play import com.google.common.collect.Maps import io.opentracing.propagation.TextMap import play.api.mvc.Headers -import scala.collection.JavaConverters._ +import scala.jdk.CollectionConverters._ class HeadersTextMap(headers: Headers) extends TextMap { def iterator = diff --git a/play/src/main/scala/io/opentracing/play/SpanTagger.scala b/play/src/main/scala/io/opentracing/play/SpanTagger.scala index 8c9afb4..937cf4f 100644 --- a/play/src/main/scala/io/opentracing/play/SpanTagger.scala +++ b/play/src/main/scala/io/opentracing/play/SpanTagger.scala @@ -4,5 +4,5 @@ import io.opentracing.Span import play.api.mvc.{RequestHeader, Result} abstract class SpanTagger { - def tag(span: Span, request: RequestHeader, result: Option[Result]) + def tag(span: Span, request: RequestHeader, result: Option[Result]): Unit } diff --git a/play/src/main/scala/io/opentracing/play/TracingAction.scala b/play/src/main/scala/io/opentracing/play/TracingAction.scala index 9b984c4..ea69525 100644 --- a/play/src/main/scala/io/opentracing/play/TracingAction.scala +++ b/play/src/main/scala/io/opentracing/play/TracingAction.scala @@ -36,7 +36,7 @@ class TracingRequest[+A](val span: Span, request: Request[A]) extends WrappedReq class TracingActionBuilder( protected[this] val tracer: Tracer, protected[this] val contextSpan: ContextSpan, - taggers: Traversable[SpanTagger] + taggers: Iterable[SpanTagger] )(val parser: BodyParser[AnyContent])(implicit ec: ExecutionContext) extends ActionBuilder[TracingRequest, AnyContent] { @@ -86,7 +86,7 @@ class TracingActionBuilder( /** * Like TracingRequest but uses ContextSpan.DEFAULT and GlobalTracer for the context and tracer. */ -class DefaultTracingActionBuilder(taggers: Traversable[SpanTagger])( +class DefaultTracingActionBuilder(taggers: Iterable[SpanTagger])( implicit ec: ExecutionContext, mat: Materializer ) extends TracingActionBuilder(GlobalTracer.get, ContextSpan.DEFAULT, taggers)(new BodyParsers.Default) From 3266a12844b38729ee8e0531c190616cf169ccd1 Mon Sep 17 00:00:00 2001 From: Sriraam AS Date: Thu, 9 Sep 2021 08:03:26 -0400 Subject: [PATCH 4/5] upgrade some package dependencies --- build.sbt | 12 ++++++------ play/build.sbt | 1 - 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/build.sbt b/build.sbt index 273a95c..1c490e0 100644 --- a/build.sbt +++ b/build.sbt @@ -7,17 +7,17 @@ def play = Project("play", file("play")).cross.cross(playAxis) def `play-active` = Project("play-active", file("play-active")) .cross.cross(playAxis).dependsOn(CrossableProject.toDependency(play)) -lazy val `play_2.13_2.8` = play("2.8.2")("2.13.2") +lazy val `play_2.13_2.8` = play("2.8.8")("2.13.6") -lazy val `play_2.13_2.7` = play("2.7.5")("2.13.2") +lazy val `play_2.13_2.7` = play("2.7.9")("2.13.6") -lazy val `play_2.12_2.8` = play("2.7.5")("2.12.12") +lazy val `play_2.12_2.8` = play("2.8.8")("2.12.14") -lazy val `play_2.12_2.7` = play("2.7.5")("2.12.12") +lazy val `play_2.12_2.7` = play("2.7.9")("2.12.12") -lazy val `play_2.12_2.6` = play("2.6.25")("2.12.12") +lazy val `play_2.12_2.6` = play("2.6.25")("2.12.14") -lazy val `play_2.11_2.7` = play("2.7.5")("2.11.12") +lazy val `play_2.11_2.7` = play("2.7.9")("2.11.12") lazy val `play_2.11_2.6` = play("2.6.25")("2.11.12") diff --git a/play/build.sbt b/play/build.sbt index 904ac38..0f3fbb3 100644 --- a/play/build.sbt +++ b/play/build.sbt @@ -2,7 +2,6 @@ description := "Opentracing for Play" libraryDependencies ++= Seq( "com.lucidchart" % "opentracing-thread-context" % "0.5", - "com.google.guava" % "guava" % "21.0", "com.typesafe.play" %% "play" % SettingKey[String]("playVersion").value, "io.opentracing" % "opentracing-api" % "0.31.0", "io.opentracing" % "opentracing-util" % "0.31.0", From c04446f70e17efe41a07864220b57b554d38f818 Mon Sep 17 00:00:00 2001 From: Sriraam AS Date: Thu, 9 Sep 2021 08:27:14 -0400 Subject: [PATCH 5/5] fix scaladoc in requestattributesspantagger --- .../io/opentracing/play/RequestAttributesSpanTagger.scala | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/play/src/main/scala/io/opentracing/play/RequestAttributesSpanTagger.scala b/play/src/main/scala/io/opentracing/play/RequestAttributesSpanTagger.scala index afefee7..48a32be 100644 --- a/play/src/main/scala/io/opentracing/play/RequestAttributesSpanTagger.scala +++ b/play/src/main/scala/io/opentracing/play/RequestAttributesSpanTagger.scala @@ -5,17 +5,17 @@ import play.api.libs.typedmap.TypedKey import play.api.mvc.{RequestHeader, Result} /** - * Applies attributes from [[RequestHeader#attrs]]. + * Applies attributes from [[https://www.playframework.com/documentation/2.8.x/api/scala/play/api/mvc/RequestHeader.html RequestHeader#attrs]]. * - * @param attributeKeys The set of attributes to read from the [[RequestHeader]]. Must be explicitly stated because - * [[RequestHeader#attrs]] does not support iterating over all attributes on a request. + * @param attributeKeys The set of attributes to read from the [[https://www.playframework.com/documentation/2.8.x/api/scala/play/api/mvc/RequestHeader.html RequestHeader]]. Must be explicitly stated because + * [[https://www.playframework.com/documentation/2.8.x/api/scala/play/api/mvc/RequestHeader.html RequestHeader#attrs]] does not support iterating over all attributes on a request. * * Attributes may or may not have a displayName. In order to ensure that the tags set on the Span are sensible, only * Attributes with a displayName set will be used to tag the Span. */ class RequestAttributesSpanTagger(attributeKeys: Set[TypedKey[String]]) extends SpanTagger { - def tag(span: Span, request: RequestHeader, result: Option[Result]) = + def tag(span: Span, request: RequestHeader, result: Option[Result]): Unit = attributeKeys.foreach { key => key.displayName.foreach { name => request.attrs