Skip to content

Commit

Permalink
Merge pull request #11 from sriraamas/avoid-global-bodyparser
Browse files Browse the repository at this point in the history
Avoid global bodyparser
  • Loading branch information
sriraamas authored Aug 2, 2021
2 parents a1dfbf0 + 9f86ab2 commit 74298b1
Show file tree
Hide file tree
Showing 13 changed files with 95 additions and 52 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ jobs:
- name: Set SBT_OPTS
run: bash gen_sbt_opts.sh
- name: Build
run: sbt "package;packageDoc;packageSrc"
run: sbt "scalafmt::test;package;packageDoc;packageSrc"
- uses: actions/upload-artifact@v2
with:
name: jars
Expand Down
16 changes: 16 additions & 0 deletions .scalafmt.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
align.openParenCallSite = false
align.openParenDefnSite = false
continuationIndent.defnSite = 2
danglingParentheses = true
docstrings = JavaDoc
importSelectors = singleLine
maxColumn = 120
newlines.afterImplicitKWInVerticalMultiline = true
rewrite.redundantBraces.stringInterpolation = true
rewrite.rules = [
RedundantBraces,
RedundantParens,
PreferCurlyFors,
SortImports
]
unindentTopLevelOperators = true
1 change: 1 addition & 0 deletions play/build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,4 @@ moduleName := "opentracing-play"
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
3 changes: 1 addition & 2 deletions play/src/main/scala/io/opentracing/play/ContentTagger.scala
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,11 @@ import play.api.mvc.{RequestHeader, Result}
class ContentTagger extends SpanTagger {
import ContentTagger._

def tag(span: Span, request: RequestHeader, result: Option[Result]) = {
def tag(span: Span, request: RequestHeader, result: Option[Result]) =
result.foreach { result =>
result.body.contentLength.foreach(span.setTag(contentLengthTag, _))
result.body.contentType.foreach(span.setTag(contentTypeTag, _))
}
}
}

object ContentTagger extends ContentTagger {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import io.opentracing.threadcontext.ContextSpan
import scala.concurrent.ExecutionContext

@deprecated("Filters don't work well for setting thread local storage, use DefaultTracingAction instead", "0.12")
class DefaultTracingFilter(taggers: Traversable[SpanTagger])(implicit executionContext: ExecutionContext) extends TracingFilter(ContextSpan.DEFAULT, taggers) {
class DefaultTracingFilter(taggers: Traversable[SpanTagger])(implicit executionContext: ExecutionContext)
extends TracingFilter(ContextSpan.DEFAULT, taggers) {
protected[this] def tracer = GlobalTracer.get
}
7 changes: 4 additions & 3 deletions play/src/main/scala/io/opentracing/play/HeadersTextMap.scala
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@ import play.api.mvc.Headers
import scala.collection.JavaConverters._

class HeadersTextMap(headers: Headers) extends TextMap {
def iterator = {
headers.toMap.iterator.map { case (name, values) => Maps.immutableEntry(name, values.mkString(",")) }.asJava
}
def iterator =
headers.toMap.iterator.map {
case (name, values) => Maps.immutableEntry(name, values.mkString(","))
}.asJava

def put(key: String, value: String) = throw new UnsupportedOperationException
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,8 @@ import play.api.mvc.{RequestHeader, Result}
class HttpVersionTagger extends SpanTagger {
import HttpVersionTagger._

def tag(span: Span, request: RequestHeader, result: Option[Result]) = {
def tag(span: Span, request: RequestHeader, result: Option[Result]) =
span.setTag(HttpVersionTag, request.version)
}
}

object HttpVersionTagger extends HttpVersionTagger {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,12 @@ import play.api.mvc.{RequestHeader, Result}
*/
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]) =
attributeKeys.foreach { key =>
key.displayName.foreach { name =>
request.attrs.get(key).foreach(value => span.setTag(s"play.$name", value))
request.attrs
.get(key)
.foreach(value => span.setTag(s"play.$name", value))
}
}
}
}
7 changes: 4 additions & 3 deletions play/src/main/scala/io/opentracing/play/Routes.scala
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@ import play.api.routing.Router

object Routes {

def endpointName(request: RequestHeader): Option[String] = request.attrs.get(Router.Attrs.HandlerDef).map { handler =>
s"${handler.controller}.${handler.method}"
}
def endpointName(request: RequestHeader): Option[String] =
request.attrs.get(Router.Attrs.HandlerDef).map { handler =>
s"${handler.controller}.${handler.method}"
}

}
10 changes: 7 additions & 3 deletions play/src/main/scala/io/opentracing/play/StandardSpanTagger.scala
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,15 @@ class StandardSpanTagger extends SpanTagger {
Tags.HTTP_STATUS.set(span, result.header.status)
}
Tags.HTTP_URL.set(span, request.uri)
request.headers.get("X-Forwarded-Port").flatMap(port => Try(port.toInt.toShort).toOption)
request.headers
.get("X-Forwarded-Port")
.flatMap(port => Try(port.toInt.toShort).toOption)
.foreach(Tags.PEER_PORT.set(span, _))
Try(InetAddresses.forString(request.remoteAddress)).foreach {
case ip: Inet4Address => Tags.PEER_HOST_IPV4.set(span, ByteBuffer.wrap(ip.getAddress).getInt)
case ip: Inet6Address => Tags.PEER_HOST_IPV6.set(span, ip.getHostAddress.takeWhile(_ != '%'))
case ip: Inet4Address =>
Tags.PEER_HOST_IPV4.set(span, ByteBuffer.wrap(ip.getAddress).getInt)
case ip: Inet6Address =>
Tags.PEER_HOST_IPV6.set(span, ip.getHostAddress.takeWhile(_ != '%'))
}

}
Expand Down
49 changes: 30 additions & 19 deletions play/src/main/scala/io/opentracing/play/TracingAction.scala
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@ import play.api.mvc._
import scala.concurrent.{ExecutionContext, Future}

private object RequestSpan {
def apply(tracer: Tracer, request: RequestHeader): Span = {
tracer.buildSpan(Routes.endpointName(request).getOrElse(s"HTTP ${request.method}"))
def apply(tracer: Tracer, request: RequestHeader): Span =
tracer
.buildSpan(Routes.endpointName(request).getOrElse(s"HTTP ${request.method}"))
.asChildOf(tracer.extract(Format.Builtin.HTTP_HEADERS, new HeadersTextMap(request.headers)))
.withTag(Tags.SPAN_KIND.getKey, Tags.SPAN_KIND_SERVER)
.start()
}
}
class TracingRequest[+A](val span: Span, request: Request[A]) extends WrappedRequest(request)

Expand All @@ -32,8 +32,13 @@ class TracingRequest[+A](val span: Span, request: Request[A]) extends WrappedReq
* object TracingAction extends TracingActionBuilder(ContextSpan.DEFAULT, Nil)
* }}}
*/
class TracingActionBuilder(protected[this] val tracer: Tracer, protected[this] val contextSpan: ContextSpan, taggers: Traversable[SpanTagger])
(implicit ec: ExecutionContext, mat: ActorMaterializer) extends ActionBuilder[TracingRequest, AnyContent] {
class TracingActionBuilder(
protected[this] val tracer: Tracer,
protected[this] val contextSpan: ContextSpan,
taggers: Traversable[SpanTagger]
)(val parser: BodyParser[AnyContent])(implicit ec: ExecutionContext)
extends ActionBuilder[TracingRequest, AnyContent] {

/**
* Finish tagging the span and finish it. A subclass may wish to
*/
Expand All @@ -45,25 +50,31 @@ class TracingActionBuilder(protected[this] val tracer: Tracer, protected[this] v
final def invokeBlock[A](request: Request[A], block: TracingRequest[A] => Future[Result]): Future[Result] = {
val span = RequestSpan(tracer, request)
val tracingRequest = new TracingRequest(span, request)
contextSpan.set(span).call(new Callable[Future[Result]] {
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)
}
})
contextSpan
.set(span)
.call(new Callable[Future[Result]] {
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)
}
})
}

override protected def executionContext: ExecutionContext = ec

override def parser: BodyParser[AnyContent] = new BodyParsers.Default
}

/**
* Like TracingRequest but uses ContextSpan.DEFAULT and GlobalTracer for the context and tracer.
*/
class DefaultTracingActionBuilder(taggers: Traversable[SpanTagger])
(implicit ec: ExecutionContext, mat: ActorMaterializer) extends TracingActionBuilder(GlobalTracer.get, ContextSpan.DEFAULT, taggers)
class DefaultTracingActionBuilder(taggers: Traversable[SpanTagger])(
implicit ec: ExecutionContext,
mat: ActorMaterializer
) extends TracingActionBuilder(GlobalTracer.get, ContextSpan.DEFAULT, taggers)(new BodyParsers.Default)
37 changes: 22 additions & 15 deletions play/src/main/scala/io/opentracing/play/TracingFilter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -9,30 +9,37 @@ import play.api.mvc._
import scala.concurrent.{ExecutionContext, Future}

@deprecated("Filters don't work well for setting thread local storage, use TracingAction instead", "0.12")
abstract class TracingFilter(protected[this] val contextSpan: ContextSpan, taggers: Traversable[SpanTagger])(implicit ec: ExecutionContext) extends EssentialFilter {
abstract class TracingFilter(protected[this] val contextSpan: ContextSpan, taggers: Traversable[SpanTagger])(
implicit ec: ExecutionContext
) extends EssentialFilter {

private[this] def toSupplier[A](a: => A) = new Supplier[A] { def get() = a }

protected[this] def tracer: Tracer


def apply(next: EssentialAction) = EssentialAction { request =>
val span = tracer.buildSpan(Routes.endpointName(request).getOrElse(s"HTTP ${request.method}"))
val span = tracer
.buildSpan(Routes.endpointName(request).getOrElse(s"HTTP ${request.method}"))
.asChildOf(tracer.extract(Format.Builtin.HTTP_HEADERS, new HeadersTextMap(request.headers)))
.withTag(Tags.SPAN_KIND.getKey, Tags.SPAN_KIND_SERVER)
.start()
contextSpan.set(span).supply(toSupplier {
next(request).map { result =>
taggers.foreach(_.tag(span, request, Some(result)))
span.finish()
result
}.recoverWith { case e =>
Tags.ERROR.set(span, true)
taggers.foreach(_.tag(span, request, None))
span.finish()
Future.failed(e)
}
})
contextSpan
.set(span)
.supply(toSupplier {
next(request)
.map { result =>
taggers.foreach(_.tag(span, request, Some(result)))
span.finish()
result
}
.recoverWith {
case e =>
Tags.ERROR.set(span, true)
taggers.foreach(_.tag(span, request, None))
span.finish()
Future.failed(e)
}
})
}

}
2 changes: 2 additions & 0 deletions project/build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,5 @@ addSbtPlugin("com.jsuereth" % "sbt-pgp" % "2.0.0")
addSbtPlugin("com.lucidchart" % "sbt-cross" % "4.0")

addSbtPlugin("org.xerial.sbt" % "sbt-sonatype" % "3.8")

addSbtPlugin("com.lucidchart" % "sbt-scalafmt" % "1.16")

0 comments on commit 74298b1

Please sign in to comment.