ovotech / natchez-extras

Integrations between Natchez, Doobie, HTTP4s, Log4cats and Datadog. Formerly called effect-utils.
https://ovotech.github.io/natchez-extras/
Apache License 2.0
31 stars 18 forks source link

Add support for tracing HttpRoutes #59

Closed xeinherjar closed 3 years ago

xeinherjar commented 3 years ago

This PR adds support for HttpRoutes.
We needed support for not tracing some routes, for example health checks.

Please let me know if you would like any additional changes, thanks!

timbess commented 3 years ago

@ericaovo What do you think about this change? We're migrating to natchez-extras but we need this to keep from tracing kubernetes healthcheck requests.

tomverran commented 3 years ago

Hi, thanks for the PR!

I think it should probably be possible to achieve what you want without needing this function, am I right in assuming you're composing together the various routes you have as HttpRoutes and then only turning them into HttpApp right at the end?

Something like

import cats.data.Kleisli
import cats.effect.IO
import cats.syntax.semigroupk._
import com.ovoenergy.natchez.extras.http4s.Configuration
import com.ovoenergy.natchez.extras.http4s.server.TraceMiddleware
import natchez.{EntryPoint, Span}
import org.http4s._
import org.http4s.syntax.kleisli._

val ep: EntryPoint[IO] = ???
val conf: Configuration[IO] = Configuration.default()

type TraceIO[A] = Kleisli[IO, Span[IO], A]
val appRoutes: HttpRoutes[TraceIO] = ???
val healthRoutes: HttpRoutes[TraceIO] = ???

val traced: HttpApp[IO] =
  TraceMiddleware(ep, conf)((healthRoutes <+> appRoutes).orNotFound) // will trace health checks

The reason to avoid tracing HttpRoutes is that if none of the routes match the middleware won't be able to flag that the response was a 404 as all that comes out of the routes is None instead of an HTTP response.

What we did to avoid tracing our health route was create a small .getOrElse extension method so we can combine together non traced HttpRoutes with a traced HttpApp (both of which under the hood are Kleislis):

import cats.Monad
import cats.data.Kleisli
import cats.effect.IO
import com.ovoenergy.natchez.extras.http4s.Configuration
import com.ovoenergy.natchez.extras.http4s.server.TraceMiddleware
import natchez.{EntryPoint, Span}
import org.http4s._
import org.http4s.syntax.kleisli._

implicit class HttpAppSyntax[F[_]: Monad](a: HttpRoutes[F]) {
  def getOrElse(b: HttpApp[F]): HttpApp[F] = Kleisli(r => a.run(r).getOrElseF(b.run(r)))
}

val ep: EntryPoint[IO] = ???
val conf: Configuration[IO] = Configuration.default()

type TraceIO[A] = Kleisli[IO, Span[IO], A]
val appRoutes: HttpRoutes[TraceIO] = ???
val healthRoutes: HttpRoutes[IO] = ??? // using IO as there's no tracing

val traced: HttpApp[IO] =
  healthRoutes.getOrElse(TraceMiddleware(ep, conf)(appRoutes.orNotFound))

Hopefully that makes sense, let me know if you have any questions

timbess commented 3 years ago

Ahh that's a clever snippet, yeah that's basically what we're doing so that works. That's a good point about 404s, it's definitely a dangerous pitfall that could leave off data from potentially important traces.

Thanks!

xeinherjar commented 3 years ago

The above works, thanks for the feedback!