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 resources #514

Closed
rossabaker opened this issue Feb 13, 2022 · 55 comments
Closed

Tracing resources #514

rossabaker opened this issue Feb 13, 2022 · 55 comments

Comments

@rossabaker
Copy link
Member

Given this Client:

  type Client[F[_]] = Request => Resource[F, Response]

We would like to able to span the entire resource, with child spans for acquisition and release:

ambient
+- client
  +- acquire
  +- use
  +- release

Inspired by natchez-http4s and #19, we might try:

    def tracedClient[F[_]: MonadCancelThrow: Trace](client: Client[F]): Client[F] = { req =>
      Resource(Trace[F].span("client") {
        Trace[F].span("acquire")(client(req).allocated).map { case (resp, release) =>
          (resp, Trace[F].span("release")(release))
        }
      })
    }

But the span tree looks like this:

ambient
+- client
| +- acquire
+- use
+- release

We could introduce a more powerful TraceResource type class:

trait TraceResource extends Trace[F] {
  def spanResource(name: String): Resource[F, Unit]
}

Tracing a client becomes:

    def tracedClient[F[_]: MonadCancelThrow: TraceResource](client: Client[F]): Client[F] = { req =>
      TraceResource[F].spanResource("client") >>
      Resource(
        Trace[F].span("acquire")(client(req).allocated).map { case (resp, release) =>
          (resp, Trace[F].span("release")(release))
        }
      )
    }

Executable proof of concept:

$ scala-cli test -S 2.13 https://gist.github.com/rossabaker/8872792b06bd84e5be8fae3c9caf8731

We could avoid the second type class by adding the method to Trace. This would be a breaking change. In addition to the new abstract method, we'd have to mapK the Resource on all the transformed instances, which would require a MonadCancel where there are currently weaker constraints. All the base instances except noop already require MonadCancel, so maybe that's okay.

/cc @zmccoy

@rossabaker
Copy link
Member Author

I tried adding spanResource to Trace, and the KleisliTrace has sprained my brain. 🤕

@rossabaker
Copy link
Member Author

You can put yourself in a dark place with this if you use allocated and don't release in reverse order. It's totally fine to create a child span from a finished span, but we lose access to the root span. I'm worried how fs2 rescoping might play here:

  test("weird and misnested") {
    val tracer = new MockTracer
    for {
      root <- IO(MockSpan[IO](tracer, tracer.buildSpan("root").start()))
      _ <- TraceResource.ioTrace(root).flatMap { trace =>
        for {
          a <- trace.resource("a").allocated
          b <- trace.resource("b").allocated
          _ <- a._2
          // the ambient span is root
          _ <- b._2
          // the ambient span is a, which is finished
          c <- trace.resource("c").use_
        } yield ()
      }
      a <- lookupSpan(tracer, "a")
      c <- lookupSpan(tracer, "c")
      _ <- IO(root.span.finish())
      _ = assertEquals(c.parentId, a.context.spanId)
    } yield ()
  }

@rossabaker
Copy link
Member Author

def resource(name: String): Resource[F, Unit] on ioTrace (except the above footgun) and noop work great. I also tested them on spanning fs2 Streams.

I am struggling with the Resource[Kleisli[F, Span[F], *], Unit] that the KleisliTrace instance would need. This monstrosity spans the client request, but acquire, use, and release are all children of the root span. It could maybe expose the child Span to feed into any kleislis in the continuation to set the "ambient trace", but that's troubling for noop.

    def resource(name: String): Resource[Kleisli[F, Span[F], *], Unit] = {
      val wtf = Kleisli((span: Span[F]) => span.span(name).void).mapF(_.pure[F]).map(_.mapK(Kleisli.liftK[F, Span[F]]))
      Resource.suspend(wtf)
    }

I'm coming to appreciate the existing model more and more, but spanning a resource lifecycle seems pretty fundamental as well.

@rossabaker
Copy link
Member Author

I got the Kleisli instance working. I had to pass the resource as a continuation, similar to how we pass the effect as a continuation to span:

def resource[A](name: String)(r: Resource[F, A]): Resource[F, A]

The same traced client now spans the entire lifecycle of the request, and works for both Trace[IO] and Trace[Kleisli[F, Span[F], *]]. I don't yet love the API for tracing streams, but it also seems to work. The gist is updated, and now requires a --compiler-plugin org.typelevel:::kind-projector:0.13.2 argument.

@rossabaker
Copy link
Member Author

I somehow overlooked @kubukoz's prior art in #72 and #73. Jakub twote:

If you dive deeper into the issues you'll see I battled with it before, but didn't consider adding a new typeclass

I don't think we have to introduce a second type class. My gist does, because I wanted a self-contained example. But I think the method could be added to Trace and work for all extant instances. Certainly ioTrace and noop and, horridly, kleisliTrace. I haven't thought through the other transformed instances.

A Trace[Resource[F, *]] and Trace[Stream[F, *]] could both be derived given Trace[F].resource. That could help get rid of the Stream.resource(Trace[F].resource("child")(Resource.unit)) in my gist. The very need for resource is getting into the same issues discussed in "Monadic Regions and Cancellation Semantics" in the original Cats Effect 3.0 proposal, but we didn't get the separate Region type class. The insights there might still inspire a nicer API here.

@armanbilge
Copy link
Member

Just noticed this thread. Didn't have a chance to read through yet, but I've been wanting this too. I made an experimental lib a little while back exploring this idea: https://github.com/armanbilge/bayou/

I couldn't get the Kleisli to work so I need to see how Ross did that :)

@rossabaker
Copy link
Member Author

I think my Kleisli instance is more complicated than it needs to be, but it took several hours of type tetris to get the one in the gist. There's no Traverse here to save us from the resource being inside the Kleisli when we want it outside the Kleisli.

@armanbilge
Copy link
Member

Before putting up that lib, I remembering experimenting with a signature similar to what you have, in the hope that it could do Stream and stuff as well.

def spanBracket[G[_]: FlatMap, A](f: Resource[F, *] ~> G)(name: String)(k: G[A]): G[A]

I've got to run, but I remember getting tripped up with Kleisli and came to some conclusion that it was only possible to span the acquire and release steps without actually spanning the use. But I could be wrong.

@armanbilge
Copy link
Member

Btw, I think IO-based tracing is better anyway in many cases for the reasons listed in my project's README.

@rossabaker
Copy link
Member Author

My unit test shows that the use is in the ambient span for use that is the same as acquire and release, even for the Kleisli. But it wasn't easy, and if you release resources in the wrong order with allocated an IO, woe be unto you.

@armanbilge
Copy link
Member

Well I've stared at it for the last hour and am still wrapping my head around it. Took me this long just to figure out the type parameters on the mapF but once I did it helped. In any case, the unit tests are convincing. This seems good.

def resource[A](name: String)(r: Resource[Kleisli[F, Span[F], *], A]): Resource[Kleisli[F, Span[F], *], A] =
  Resource.suspend[Kleisli[F, Span[F], *], A](
    Kleisli[Resource[F, *], Span[F], A]((span: Span[F]) =>
      span.span(name).flatMap(child => r.mapK(Kleisli.applyK(child)))
    ).mapF[F, Resource[Kleisli[F, Span[F], *], A]](ra =>
      ra.mapK[Kleisli[F, Span[F], *]](Kleisli.liftK[F, Span[F]]).pure[F]
    )
  )

@armanbilge
Copy link
Member

Btw, I've been kind of wishing for a natchez 0.2. Resource-tracing + CIString kernels + cats-uri instead of URI.

@rossabaker
Copy link
Member Author

I hadn't seen Bayou until you mentioned it, but you and @kubukoz and I all had the same basic idea. We all tried to return a Resource[F, Unit] instead of passing an Resource[F, A] to be transformed. I think that's why I eventually succeeded on the Kleisli instance. Returning Resource[Kleisli[F, Span[F]], Unit] lets us span the resource, but I don't see how to make it the ambient span without being passed a Kleisli to receive the child.

I do agree that it's a party trick compared to Trace[IO].resource. The only real value I see is permitting resource to live on the Trace abstraction without introducing a new type class.

@armanbilge
Copy link
Member

armanbilge commented Feb 23, 2022

We all tried to return a Resource[F, Unit] instead of passing an Resource[F, A] to be transformed.

Actually, I started with something similar to the latter idea as well :) I posted it on Discord but then couldn't figure out how to make it work for Kleisli. The signature was what I posted in #514 (comment). Edit: actually, if it's possible to implement it for a signature like that maybe it will be less awkward to trace a Stream? Idk.

The only real value I see is permitting resource to live on the Trace abstraction without introducing a new type class.

Actually, I think that's quite a lot of value. Party-trick or not, it's only an implementation detail at the end of the day :)

@rossabaker
Copy link
Member Author

I overlooked your spanBracket signature somehow. I think you're grasping for the Region type class in the old CE3 design doc. We ended up never getting that type class, which is why you needed the ~>.

Unfortunately, my Kleisli instance doesn't work either. I had an error in my unit test, and the Kleisli returned by use is a child of the root span. It's the same basic problem as the Resource[F, Unit]-returning signature: we don't get to hook into the final Kleisli to apply the child to it.

  test("nested kleislis") {
    val tracer = new MockTracer
    for {
      root <- IO(MockSpan[IO](tracer, tracer.buildSpan("root").start()))
      trace = TraceResource[Kleisli[IO, Span[IO], *]]
      _ <- trace.resource("a") {
        trace.resource("b") {
          Resource.unit
        }
      }.use(_ => trace.span("c")(Kleisli.pure(()))).run(root)
      a <- lookupSpan(tracer, "a")
      b <- lookupSpan(tracer, "b")
      c <- lookupSpan(tracer, "c")
      _ <- IO(root.span.finish())
      _ = assertEquals(b.parentId, a.context.spanId)
      // FAILS HERE
      _ = assertEquals(c.parentId, b.context.spanId)
    } yield ()
  }

We might be back to a TraceResource type class without a Kleisli[F, Span[F], *] instance, which would be super disappointing for improving the tracing middlewares.

@armanbilge
Copy link
Member

Oh darn 😕 still, I think IO-based tracing is mostly good, but not super popular. I haven't thought much about the release-order footgun yet.

@armanbilge
Copy link
Member

I got into IO-based tracing while working on a natchez integration for feral. I think it works fine so long as you are setting this stuff up close to your main method anyway. Here's an example that integrates it with http4s actually:

https://github.com/typelevel/feral/blob/440a62c3bffde6e59db46ab0c95949b25301ced1/examples/src/main/scala/feral/examples/Http4sLambda.scala#L63

@rossabaker
Copy link
Member Author

Your TracedHandler goes from Span[IO] => IO[Trace[IO]] in the obvious way. I have code that receives an Span[F] for which I'd like to generate a Trace[F] instance do similar (also serverside http4s). I sketched a TraceRoot type class this evening for that so that could can remain tagless, but IO is really the only useful implementation.

I guess we could still have resourceful tracing on the transformers over IO, but that base Kleisli[F, Span[F], *] instance -- the one we sold everybody in CE2 -- is going to get left behind if things like http4s start demanding a type class with a Resource operation. We were so close. 😞

@armanbilge
Copy link
Member

armanbilge commented Feb 23, 2022

If Trace exposed the current Span itself, I think that would get Kleisli back in action? Edit: it's past my stupid hour, so maybe ignore me. I'm trying to reconcile an old discord thread which discussed passing around Span explicitly as a solution for this problem.

Although I've appreciated that Trace is independent of Span, which I assumed is what you were alluding to above as well.

I'm coming to appreciate the existing model more and more, but spanning a resource lifecycle seems pretty fundamental as well.

@rossabaker
Copy link
Member Author

rossabaker commented Feb 23, 2022

@zmccoy and I discussed wishing to be able to get the ambient Span today, with regards to adding support for linking. It would require having a no-op Span in core for Trace.Implciits.noop to provide. But I'm not sure it helps with the Kleisli instance: span doesn't know it's been called inside a Resource, so what could its Kleisli definition be other than ask?

@armanbilge
Copy link
Member

armanbilge commented Feb 23, 2022

Right, sorry. A true Kleisli instance is definitely out of the question.

If you have direct access to the Span then at least you can manually wrap some resources. There would be no way to pass that Span or its children to inner effects due to the same constraints.

It's a compromise: this would be better than the current situation, where we can't span a Resource or Stream at all, and it would work for any F[_]: Trace, not just IO. But you sacrifice a lot of power.

@rossabaker
Copy link
Member Author

Manually passing the span seems intrusive to me. We can't trace how we use a resource without changing the very shape of the resource.

  def helper[F[_]: MonadCancelThrow, A](span: Span[F], name: String)(r: Resource[F, A]): Resource[F, (Span[F], A)] = {
    span.span(name).flatMap { case child =>
      Resource(child.span("acquire").use(_ => r.allocated).map { case (a, release) =>
        (child, a) -> child.span("release").use(_ => release)
      })
    }
  }

  def example[F[_]: MonadCancelThrow](span: Span[F]) = {
    helper(span, "resource")(Resource.pure("precious")).use {
      // Use went from `A => F[B]` to `(Span[F], A) => F[B]`   
      case (rSpan, precious) =>
        rSpan.span("use").use_
    }
  }

As soon as we trace a client, every call is going to have that extra value to deal with. Worse, that span won't bear any relation to the ambient span of Trace, if we're mixing and matching.

@gvolpe
Copy link
Member

gvolpe commented Feb 23, 2022

So far, I haven't had the need for tracing Resource / Stream, though, it would be nice to have it. So forgive me if the problem I'm about to describe belongs somewhere else...

The issue I think it surfaces more to me is tracing HttpRoutes where you have something like this:

final class Routes[F[_]: GenUUID: Monad: Trace](
    users: UsersDB[F]
) extends Http4sDsl[F]

trait UsersDB[F[_]]:
  def get(id: UUID): F[Option[User]]

object UsersDB:
  def make[F[_]: MonadThrow: Trace]: F[UsersDB[F]] = ???

To construct my HttpRoutes, I first need to construct a UsersDB, so I need to satisfy the Trace constraint before the HTTP routes is able to pass its span on every request. So far, my only option has been to instantiate F as Kleisli[F, Span[F], *] from the main class.

Using Trace.ioTrace with an initial root span satisfies the Trace constraint in both as well, but the HttpRoutes span won't be propagated to the UsersDB.

I'm probably repeating myself here since I saw you both already mention the limitations of not being able to get a Trace[F] instance in a tagless world but I might learn a few things from your experience by exposing my issue here.

The same goes for consumer / producer examples, where I would like to resume a trace from a given kernel, can't be done it in an abstract F.


Additionally, natchez-http4s adds an extension method named liftT (and a few others) to magically satisfy the Trace constraint and run it in Kleisli. In a similar way, I created a liftRoutes extension method (only Scala 3 for now, using context fucntions) that runs it in IO instead. E.g.

/* it uses `Trace.ioTrace` under the hood, so only for IO */
def ctxIOLocal(ep: EntryPoint[IO]) =
  for
    db <- Resource.eval(UsersDB.noTrace[IO])
    routes = ep.liftRoutes(Routes[IO](db).routes)
    server = Ember.routes[IO](port"9000", routes)
  yield db -> server

Linking it here in case it is helpful to anyone.

@armanbilge
Copy link
Member

armanbilge commented Feb 23, 2022

Using Trace.ioTrace with an initial root span satisfies the Trace constraint in both as well, but the HttpRoutes span won't be propagated to the UsersDB.

@gvolpe Thanks for chiming in! Yes, I am acutely aware of this problem as a proponent of IO-based tracing. I proposed a workaround in Bayou and Ross also had an idea for a TraceRoot typeclass that I think might be related, but I'm not sure.

@rossabaker
Copy link
Member Author

My TraceRoot isn't particularly satisfactory. It's very IO-centric.

trait TraceRoot[F[_]] {
  def trace(root: Span[F]): F[Trace[F]]
}

object TraceRoot {
  def apply[F[_]](implicit ev: TraceRoot[F]): ev.type = ev

  implicit val traceRootForIO: TraceRoot[IO] =
    (root: Span[IO]) => Trace.ioTrace(root)
}

A Kleisli implementation would, like, what, take a Span[Kleisli[F, Span, *]] as input?

@armanbilge
Copy link
Member

@rossabaker
Copy link
Member Author

Using Trace.ioTrace with an initial root span satisfies the Trace constraint in both as well, but the HttpRoutes span won't be propagated to the UsersDB.

Is this suggesting that UsersDB needs to be made per request for its traces to be caused by the routes' traces?

@armanbilge
Copy link
Member

I think so, yes, unless you have a mechanism for injecting a new span on Trace itself.

@gvolpe
Copy link
Member

gvolpe commented Feb 23, 2022

Is this suggesting that UsersDB needs to be made per request for its traces to be caused by the routes' traces?

@rossabaker not the instance itself (that would be criminal), but the Trace instance needs to be able to pass a unique span on every HTTP request, which is only possible with Kleisli as far as I understand. I ran some tests with this sometime ago, but please correct me if I'm wrong, I can try to test this once again.

@rossabaker
Copy link
Member Author

Yes, Trace[IO] is constructed with a specific Span, typically a root or continued span from an EntryPoint. You would typically want one instance of this per request. And Trace[Kleisli[F, Span[F], *]] generates functions that you can call repeatedly with different Spans. Arman's IOTrace in Bayou attempts to fix this, but if your code is in Trace[F], there's no abstract way to bring your own span.

So a global Trace[Kleisli[F, Span[F], *]] or a global Bayou IOTrace can be shared between HttpRoutes and UsersDb, but a Trace[IO] from natchez needs to be request-scoped. That then suggests either your UsersDb needs to be request scoped (hopefully without request-scoping any connection pool!), or the Trace[F] constraint needs to be moved to the operations.

@gvolpe
Copy link
Member

gvolpe commented Feb 24, 2022

Exactly as I understand it as well, thanks for confirming 👍🏽

I'm trying to understand by explaining the different ways of doing tracing with a few simple (but not as simple as the documentation) examples: https://github.com/gvolpe/trading/blob/main/modules/x-demo/src/main/scala/demo/tracer/TraceApp.scala

Another alternative is to work with two different effect types, but not that pretty.

def alt[F[_]: MonadThrow: Ref.Make, G[_]: MonadThrow: Trace](using NT[F, G]): F[UsersDB[G]] = ???

@rossabaker
Copy link
Member Author

Avoiding that second type parameter may be why Trace doesn't expose Span directly. Otherwise, Trace[Kleisli[F, Span[F], *]] would have to return a Span[Kleisli[F, Span[F], Span[F]]]. I suppose we could... the other operations on Trace are essentially just an unbundled version of that.

Abstracting over Trace[F] is so tricky because Trace[IO] is stateful and Trace[Kleisli[F, Span[F], *]] is stateless. The latter has semantics like cats.mtl.Local, where the scope operation is only observable through the result. Trace[Kleisli[F, Span[F], *]] is incapable of supplying a different ambient span to the use of a Resource, and Trace.ioTrace can't be shared beyond the scope of its trace (which its construction in IO is trying to tell us!) Proper use of the abstraction then carries both limitations.

@armanbilge
Copy link
Member

Proper use of the abstraction then carries both limitations.

💯 that was a really nice summary.

Trace.ioTrace can't be shared beyond the scope of its trace

Do you mean, it can't be shared beyond the scope of its "seed" span?

The interesting thing is (unless I'm missing something) while we cannot fix Resource tracing for Kleisli, we can still inject other root spans into an IOTrace after-the-fact. It just needs a default "seed" span to create the instance.

@armanbilge
Copy link
Member

we cannot fix Resource tracing for Kleisli

Well, the problem as formulated here is effectively how to get from Trace[F] => Trace[Resource[F, *]] or Trace[F] => Trace[Stream[F, *]]. IO can do this, but not Kleisli.

However, can we go Trace[Resource[F, *]] => Trace[F]? Something like this:

def downgradeTrace[F[_]: MonadCancelThrow](resourceTrace: Trace[Resource[F, *]]): Trace[F] =
  new Trace[F] {
    def span[A](name: String)(fa: F[A]): F[A] =
      resourceTrace.span(name)(Resource.eval(fa)).use(_.pure)
  }

This would suggest APIs should ask for the "strongest" Traces they need according to a monad partial order. Wouldn't be terribly efficient, but would this work? Apologies if I'm off here :)

@rossabaker
Copy link
Member Author

I'm having trouble conceptualizing what that looks like. I haven not run this by scalac whatsoever:

def traceClient[F[_]: MonadCancelThrow](client: Client[F])(impliciit T: Trace[Resource[F, *]]): Client[F] = { req: Request =>
  T.span("client")(
    Resource(
      downgradeTrace(T).span("acquire")(client(req)).map { (resp, release) =>
        resp -> downgrateTrace(T).span("release")(release)
      }
    )    
  }
}

If my Trace[Resource[F, *]] is not stateful, how does it span use?

@armanbilge
Copy link
Member

Er, crap. That thought came out half-baked if we're being generous. So sorry.

What I think I was trying to get at was working in terms of Span[Resource[F, *]] rather than Span[F], but I stupidly wrote it in terms of Trace.

Let me actually spend some quality time with scalac and see if I can get it to agree with my fantasies.

@armanbilge
Copy link
Member

Ok, I think I got where I intended to be this morning 😅 at least this time, scalac is backing me up.

https://gist.github.com/armanbilge/3a2622d2d3834fe8c487ad74f2eb957a

So far what we know is:

  1. Trace works well with APIs written in terms of an abstract effect F[_], because we can always choose an effect s.t. Trace[F] exists
  2. Trace does not work well with APIs written in concrete effects that don't implement Trace
  3. even though F[_] is abstract, Resource[F, *] and Stream[F, *] are concrete

However, what I think/hope my gist shows is we can have effect that is both Resource-like and Trace-able. If our APIs abstracted over the resource effect in addition to the base effect F[_], then we could instantiate them with e.g. Kleisli[Resource[F, *], Span[F], *] and Kleisli[F, Span[F], *] and I think things would work the way we want them to.

This might be obvious observation and not sure if it's any help in practice.

@rossabaker
Copy link
Member Author

Am I calling this right in your gist? I can't get it to compile, but I also haven't learned to read Scala 3 errors. I think I need a ResourceLike[Resource[F, *], F]?

def traceClient[F[_]: MonadCancelThrow](client: Client[F])(implicit TR: Trace[Resource[F, *]]): Client[F] = { (req: Request) =>
  implicit val T: Trace[F] = downgradeTrace[Resource[F, *], F]
  TR.span("client")(
    Resource(
      T.span("acquire")(client(req)).map { (resp, release) =>
        resp -> T.span("release")(release)
      }
    )
  )
}

@rossabaker
Copy link
Member Author

Meanwhile, if we have a stateful Trace:

trait TraceS[F[_]] extends Trace[F] {
  def resource(name: String): Resource[F, Unit]
  def withSpan(span: Span[F]): Resource[F, Unit]
}

then this seems to work pretty well:

  test("app-wiring".only) {
    trait Connection[F[_]]

    trait Pool[F[_]] {
      def borrow: Resource[F, Connection[F]]
    }

    object Pool {
      def heavyAssPool[F[_]: MonadCancelThrow: TraceS]: Resource[F, Pool[F]] =
        Resource.pure {
          new Pool[F] {
            def borrow = TraceS[F].resource("pool").as(new Connection[F] {})
          }
        }
    }

    trait UsersDB[F[_]] {
      def findUser(id: Long): F[Option[String]]
    }

    object UsersDB {
      def apply[F[_]: MonadCancelThrow: TraceS](pool: Pool[F]): UsersDB[F] =
       new UsersDB[F] {
         def findUser(id: Long): F[Option[String]] =
           pool.borrow.use(_ => TraceS[F].span("db")(Applicative[F].pure(Some("Joe"))))
       }
    }

    type Server[F[_]] = Request => Resource[F, Response]

    object Server {
      def apply[F[_]](f: Request => Resource[F, Response]): Server[F] = f

      def trace[F[_]: MonadCancelThrow: TraceS](entryPoint: EntryPoint[F])(server: Server[F]): Server[F] = { req =>
        entryPoint.root("server").flatMap(TraceS[F].withSpan) >>
        Resource(
          TraceS[F].span("acquire")(server(req).allocated).map { case (resp, release) =>
            (resp, TraceS[F].span("release")(release))
          }
        )
      }
    }

    def userServer[F[_]: Functor](usersDb: UsersDB[F]): Server[F] = {
      case req =>
        Resource.eval(usersDb.findUser(0L).map(_.fold(Response("oops"))(Response.apply)))
    }

    val entryPoint = new natchez.mock.MockEntrypoint[IO]()
    (for {
      root <- entryPoint.root("app")
      srv <- Resource.eval(TraceS.ioTrace(root)).flatMap { implicit trace =>
        for {
          pool <- Pool.heavyAssPool[IO]
          usersDb = UsersDB[IO](pool)
          server = userServer[IO](usersDb)
        } yield Server.trace(entryPoint)(server)
      }
      _ <- srv(Request("test"))
    } yield root).use_ >> {
      IO.println(entryPoint.mockTracer.finishedSpans())
    }
  }

The withSpan is like Bayou's IOTrace and allows us to trace UsersDB in request scope, but create a users DB in application scope. There's an irritating "app" span that lasts for the entire app, because we have to create the trace instance with something.

@armanbilge
Copy link
Member

armanbilge commented Feb 25, 2022

Yes, you need to implement ResourceLike[Resource[F, *], F].

But what I was trying to get at is that we should avoid pre-binding our resource-effect to Resource and instead make it abstract R[_]: ResourceLike: Trace. This lets us bind it as Kleisli[Resource[F, *], Span[F], *].

Meanwhile, if we have a stateful Trace.

Of course, if we buy into the stateful Trace then none of this is necessary :)

@SystemFw
Copy link

The issue I think it surfaces more to me is tracing HttpRoutes where you have something like this:

This is the issue most of the teams I work with encounter, and I have a solution to it which (after talking with @armanbilge), is more or less equivalent to spanWith, i.e. I was going to propose an entryPointTrace(name: String)(fa: F[A]): F[A] addition to Trace, and instantiate it with noOpSpan in main, which is pretty much what you can do with spanWith.

I think fundamentally what we are proposing is changing the semantics of Trace from "the current trace" to "the ability to trace", i.e. current Trace is a stateful abstraction over a single tree, whereas this addition represent the abstract ability to start new trees and trace within them, and in turn be able to

trace UsersDB in request scope, but create a users DB in application scope

@rossabaker
Copy link
Member Author

I think there are three possible semantics to this. New @SystemFw dropped while I was writing this...

    trait TraceR[F[_]] extends Trace[F] {
      def resource[A](name: String)(r: Resource[F, A]): Resource[F, A]
      def entryPointTrace(name: String)(fa: F[A]): 
    }
    object TraceR {
      def apply[F[_]](implicit ev: TraceR[F]): ev.type = ev
    }

    def blah[F[_]: MonadCancelThrow: TraceR, A](r: Resource[F, A]): F[Unit] = {
      TraceR[F].resource("resource")(
        Resource(
          TraceR[F].span("acquire")(r.allocated.map { case (a, release) =>
            a -> TraceR[F].span("release")(release)
          })
        )
      ).use(_ => TraceR[F].span("use")(Applicative[F].unit))
    }

I have not tested this table but for all the experiments above, so call me on my bullshit:

Stateless I Stateless II Stateful
acquire/release under resource 🚫
use under resource 🚫 🚫
Supports IO
Supports Kleisli 🚫
Supports Local 🚫
resource[A](name: String)(r: Resource[F, A]): Resource[F, A]
resource(name: String): Resource[F, Unit] 🚫
entryPointTrace[A](name: String)(fa: F[A]): F[A]

Stateless I adds the ability to span a resource, but doesn't parent anything. It says acquisition, use, and release are caused by whoever uses it, but doesn't bundle them at all. Ideally we'd at least link to the resource, but we don't support linking yet.

Stateless II says acquisition and release are caused by the resource, but use is caused by whoever uses it. This neatly matches the "lexical scope" of the trace calls, but still leaves no link between a resource and its use.

Stateful seems like an extension of or complement to Trace, but if it's the only thing that can do resources, it's probably the one I'd end up embedding in libraries and choosing for everyone else.

@armanbilge
Copy link
Member

armanbilge commented Feb 25, 2022

I'm still confused about this

def entryPointTrace(name: String)(fa: F[A]): F[A]

What does this do? How does it help me when I have a Kernel from an incoming request that I'd like to seed a new tree with?

@SystemFw
Copy link

What does this do?

creates a new Span

How does it help me when I have a Kernel from an incoming request that I'd like to seed a new tree with?

It doesn't in the current form because it's a sketch, but you can imagine either adding rootTrace, continueTrace etc instead of just entryPointTrace, or changing the type to entryPointTrace to take an Option[Kernel] or whatever other form of data modelling you want to use to model root vs continue

@rossabaker
Copy link
Member Author

Natchez is already integrated deeply into Skunk. There would be high upside to integrating it into the various backends of http4s. More effect libraries should instrument themselves. It will be unfortunate if we splinter libraries and end up with inconsistent constraints as we wire together apps.

I'd love to see us rally around either the Stateless II or Stateful semantics above. I don't see a lot of upside to Stateless I, other than a simpler Resource signature than Stateless II.

@armanbilge
Copy link
Member

Question: is there a reason we can't expose all three new APIs? For example, according to your table Stateless I and Stateful actually expose the same APIs. So the change in behavior for acquire/release/use is completely due to how they are implemented. But is that a bad thing?

This seems like a reasonable compromise: libraries can use the common Trace API to its maximum potential and meanwhile, users have the freedom to decide if they want to use Kleisli (and accept the caveats) or IOLocal-based tracing.

@rossabaker
Copy link
Member Author

Given:

def foo[F[_]: Trace]: F[Unit] = ???

val rootSpan: Span[IO] = ???
def x = Trace.ioTrace(rootSpan).flatMap { implicit trace => foo[IO] }
def y = foo[Kleisli[IO, Span[IO], *].run(rootSpan)

x and y should result in the same structure (parent-child and tags) beneath rootSpan. We have this "law" today between the stateless and stateful, but resource introduces a way to observe this state externally, so now we have to decide. We could support two (or more) type classes, but it sucks if Skunk picks one and http4s picks the other.

I did not compile these examples, but I think they're close...

*** Stateless I compatible

trait Trace[IO] {
  def resource(name: String): Resource[IO, Unit] =
     for {
       parent <- Resource.eval(local.get)
       _ <- parent.span(name)
     } yield ()
}

*** Stateless II compatible

trait Trace[IO] {
  def resource[A](name: String)(r: Resource[IO, A]): Resource[IO, A] =
     for {
       parent <- Resource.eval(local.get)
       child <- parent.span(name)
       a <- Resource(local.set(child).bracket(_ =>
         resource.allocated.map { case (a, release) =>
           a -> local.set(child).bracket(_ => release)(_ => local.set(parent))
         })(_ => local.set(parent)))
     } yield a
}

*** Stateful

These are the only way the span of a resource is the ambient trace when using the resource, but this semantic can't be implemented with Local or Kleisli.

trait Trace[IO] {
  def resource(name: String): Resource[IO, Unit] =
    for {
      parent <- Resource.eval(local.get)
      child <- parent.span(name)
      _ <- Resource.make(local.set(child))(_ => local.set(parent))
  } yield ()

  // or 

  def resource[A](name: String)(r: Resource[IO, A]): Resource[IO, A] =
     for {
       parent <- Resource.eval(local.get)
       child <- parent.span(name)
       _ <- Resource.make(local.set(child))(_ => local.set(parent))
      a <- r
    } yield a
}

@armanbilge
Copy link
Member

armanbilge commented Feb 27, 2022

We could support two (or more) type classes, but it sucks if Skunk picks one and http4s picks the other.

Exactly. So my question is, why can't we have one type class, and no law? That seems a far better compromise and in practice a Span can do whatever it wants anyway (such as no-op). Tracing is fundamentally ultimately a side-effect and I struggle to see the point in getting all rigorous over "laws" which can't be expressed anyway at the risk of ecosystem fragmentation.

Edit: well, that's probably too extreme a stance, sorry. Looking harder at your snippet I guess that is something we could test, with a special law-testing span.

def foo[F[_]: Trace]: F[Unit] = ???

val rootSpan: Span[IO] = ???
def x = Trace.ioTrace(rootSpan).flatMap { implicit trace => foo[IO] }
def y = foo[Kleisli[IO, Span[IO], *].run(rootSpan)

@armanbilge
Copy link
Member

For sake of argument, your law makes the no-op Trace unlawful or simply "out-of-bounds" since it has no relationship to Span.
https://github.com/tpolecat/natchez/blob/3efa0e7783ec569b39cd600b32c96d296a1651a8/modules/core/shared/src/main/scala/Trace.scala#L75-L88

Is Trace fundamentally related to Span? If I have a Trace implementation that isn't based on Span at all, would it be wrong to use it instead?

@rossabaker
Copy link
Member Author

That's a smart point. I think it would be highly surprising that the IO and the Kleisli that produces the IO from the same span would behave differently, but the law can only be expressed with the operation that injects the root span, and only applies to instances that deal in spans. It's more good manners than law.

@armanbilge
Copy link
Member

As library authors working with Trace, the only law we care about is that it won't mess up the semantics of our program.

Since the Trace[IO] instance always has to be constructed explicitly, we could offer multiple implementations, document their trade-offs, and indicate which one is compatible with the Kleisli instance.

@rossabaker
Copy link
Member Author

I started trying a PR, but the StateT instance is eluding me:

implicit def liftStateT[F[_]: Monad, S](implicit trace: Trace[F]): Trace[StateT[F, S, *]] = {
  def resource[A](name: String)(r: Resource[StateT[F, S, *], A]): Resource[StateT[F, S, *], A] = ???
}

I think we want to call r.mapK to use with trace.resource, but there is no natural transformation StateT[F, S, *] ~> F.

Bayou has no problem because it just returns a Resource[F, Unit] instead of being passed a resource to trace, but this prevents the Kleisli instance from being able to wrap acquire and release.

@SystemFw
Copy link

I started trying a PR

I think it would be very helpful to have a draft PR if you have time, even if broken or incomplete, seeing the actual code would make further discussion easier for me

@rossabaker
Copy link
Member Author

Okay, my WIP is at #526, with some inline discussion.

@rossabaker
Copy link
Member Author

See also #527.

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

No branches or pull requests

5 participants