Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tracing doodles #50

Closed
wants to merge 10 commits into from
Closed

Conversation

armanbilge
Copy link
Member

Fork of #45 for me to doodle with how to integrate natchez with feral.


object TracedLambda {
def apply[F[_]: MonadCancelThrow, G[_], Event: HasKernel, Result](entryPoint: EntryPoint[F])(
lambda: Trace[G] => Lambda[G, Event, Result])(
Copy link
Member Author

@armanbilge armanbilge Nov 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure yet how I feel about injecting Trace[G] like this. It's a bit awkward, but I think it's a cleaner way to support IOLocal-based tracing, or generally tracing in a generic G.

Comment on lines +26 to +31
object HasKernel extends HasKernelLowPriority {
@inline def apply[E](implicit hk: HasKernel[E]): hk.type = hk

implicit val apiGateProxyEventV2Kernel: HasKernel[ApiGatewayProxyEventV2] =
e => Kernel(e.headers)
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One of the nice things about having all the events together in the same module is it makes it easy to provide these implicits. Although I'm doubtful if any will have a special implementation except this one.

Comment on lines +35 to +36
def viaSpan[F[_]: MonadCancelThrow, Event: HasKernel, Result](entryPoint: EntryPoint[F])(
lambda: Span[F] => Lambda[F, Event, Result]): Lambda[F, Event, Result] = {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bpholt Here's an alternative TracedLambda middleware, which only requires a single effect F and gives you a Span[F] instead of a Trace[G].

Does this mean less boilerplate? It depends. If all you wanted all along was a Span[F], then this is perfect.

But, if you actually want a Trace instance (e.g., to use natchez http4s middleware), then you have a couple options.

  1. Hardcode the traced effect (either Kleisli or IO) and run it with the given span. This is straightforward enough, but you lose some abstraction power.
  2. Re-invent LiftTrace[F, G] 😉

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works pretty well! Let's run with it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool! So you prefer this one to the other in terms of Trace[G]? I think I'd like to keep both forms, but maybe this should be the primary :)

Copy link
Member

@bpholt bpholt Nov 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm using it wrapped by XRayTracedLambda:

package feral.lambda.natchez.xray

import _root_.natchez._
import _root_.natchez.noop.NoopSpan
import _root_.natchez.xray.{XRay, XRayEnvironment}
import cats.data._
import cats.effect.std.Random
import cats.effect.{Trace => _, _}
import feral.lambda._
import feral.lambda.natchez._

object XRayTracedLambda {
  def apply[F[_]: Async, Event: HasKernel, Result](installer: Resource[Kleisli[F, Span[F], *], Lambda[Kleisli[F, Span[F], *], Event, Result]])
                                                  (implicit LT: LiftTrace[F, Kleisli[F, Span[F], *]]): Resource[F, Lambda[F, Event, Result]] =
    installer.mapK(Kleisli.applyK(new NoopSpan[F]())).flatMap(XRayTracedLambda(_))

  def apply[F[_]: Async, Event: HasKernel, Result](lambda: Span[F] => Lambda[F, Event, Result]): Resource[F, Lambda[F, Event, Result]] =
    Resource.eval(Random.scalaUtilRandom[F]).flatMap { implicit random =>
      Resource
        .eval(XRayEnvironment[F].daemonAddress)
        .flatMap {
          case Some(addr) => XRay.entryPoint[F](addr)
          case None => XRay.entryPoint[F]()
        }
        .map(TracedLambda.viaSpan(_)(lambda))
    }

  def apply[F[_]: Async, G[_], Event: HasKernel, Result](lambda: Lambda[G, Event, Result])
                                                        (implicit LT: LiftTrace[F, G]): Resource[F, Lambda[F, Event, Result]] =
    XRayTracedLambda { span => (event, context) =>
      LT.run(span) { _ => lambda(event, context.mapK(LT.liftK)) }
    }
}

Here's an example of a Lambda using that middleware: https://github.com/Dwolla/rabbitmq-topology-backup/blob/xray/src/main/scala/com/dwolla/rabbitmq/topology/LambdaHandler.scala#L24-L42

Basically:

def handler[F[_] : Async : Trace]: Resource[F, lambda.Lambda[F, Event, INothing]] =override def run: Resource[IO, lambda.Lambda[IO, Event, INothing]] =
  XRayTracedLambda(handler[Kleisli[IO, Span[IO], *]])

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bpholt what would you think about adding this stuff directly to the XRay integration in Natchez? Seems like common boilerplate?

    Resource.eval(Random.scalaUtilRandom[F]).flatMap { implicit random =>
      Resource
        .eval(XRayEnvironment[F].daemonAddress)
        .flatMap {
          case Some(addr) => XRay.entryPoint[F](addr)
          case None => XRay.entryPoint[F]()
        }

Also, what's the thread-safety story with the Random :)

@bpholt
Copy link
Member

bpholt commented Nov 19, 2021

I think this is in a pretty good state, pending what happens with typelevel/natchez#448. Is there anything we want to try or do before merging (and maybe publishing snapshots)?

@bpholt
Copy link
Member

bpholt commented Nov 19, 2021

Also, I have the XRay middleware updated to use this branch, but it doesn't build since the XRay support in Natchez hasn't been published yet. I was thinking if we plan to merge this branch soon, I can push it up as a followup PR (in its own module? I'm not sure how we want to organize things yet)

@armanbilge
Copy link
Member Author

Awesome! Actually, what I'd like to do is split the PRs again which got mixed up while we were experimenting :)

  1. A PR introducing the effect-agnostic Lambda, which hopefully @djspiewak can take a look at, since it's fairly different from his original concept gist and also since he was dubious about my last attempt at effect agnosticism :)
  2. A PR introducing the TracedLambda, at least with the Span[F]-based formulation since we can do this with Natchez as published today. If LiftTrace becomes a published thing, we can consider adding the Trace[G]-based formulation as well.
  3. A couple more tweak PRs with the INothing thing and maybe some other minor optimizations/fixes.

All of this should be publishable right now and I'm happy to do so while Daniel looks at (1).

Also, I have the XRay middleware updated to use this branch, but it doesn't build since the XRay support in Natchez hasn't been published yet.

Ooh, I'll take a look at this shortly! I thought we could get to a place where we don't need a specific integration, but maybe this was too ambitious 😆

in its own module? I'm not sure how we want to organize things yet

I'm like wayyy into over-modularization, so please push back on this. But yes that's my first instinct!

@armanbilge
Copy link
Member Author

@bpholt I just published everything in this branch, propagating now.

"com.armanbilge" %%% "feral-lambda-natchez" % "0.1-f497734" // and friends :)

@armanbilge
Copy link
Member Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants