natchez icon indicating copy to clipboard operation
natchez copied to clipboard

Introduce a TraceResource type class

Open rossabaker opened this issue 3 years ago • 10 comments

Alternative design to #526. Tracing a resource only works well with "stateful" instances. This acknowledges that by adding a sub class of Trace. Instances are provided for:

  • IO
  • Transformers given a TraceResource[F] and a MonadCancel[F].
  • A no-op for Applicative[F].

Libraries that use this constraint trade the "local" instances (Kleisli[F, Span[F], *] and Local[F, Span[F]]) for the ability to span an entire resource.

rossabaker avatar Mar 01 '22 04:03 rossabaker

This is basically Bayou with some deduplication because it lives in the same repo.

rossabaker avatar Mar 01 '22 04:03 rossabaker

or the ability to span an entire resource.

Not just that, I also think it makes the very common pattern of creating untraced dependencies whose operations need to be traced a lot more usable. Matter of fact, except for things like naming, this design is essentially what I had come up with just for the problem above without even thinking about tracing Resource, so with Bayou it's 3 data points that there might be something to it

SystemFw avatar Mar 06 '22 00:03 SystemFw

Perhaps this achieves subtleties I don't appreciate. I'm not precisely sure what this means:

pattern of creating untraced dependencies whose operations need to be traced a lot more usable.

Are you talking about the entryPointTrace idea? I'm not sure that this solves that. What this proposal still lacks is a nice way to inject a traced dependency in a wider scope than the entry point that uses it. For example, the database algebra that serves an HTTP request handler.

rossabaker avatar Mar 06 '22 15:03 rossabaker

I'm not sure that this solves that. What this proposal still lacks is a nice way to inject a traced dependency in a wider scope than the entry point that uses it. For example, the database algebra that serves an HTTP request handler.

mm, I thought it did, but I misinterpreted the "creates a new span" in the scaladoc, it's creating a child span whereas I thought it'd create a brand new one

SystemFw avatar Mar 06 '22 22:03 SystemFw

Okay, I think I now see what you thought you saw. I think the entry method could return Resource[F, Unit], but also would need a kernel, and have continueOrRoot-like semantics.

I was deferring dealing with the entrypoint abstraction until we settled between this and #526, because I think the problems are orthogonal. But the entry is only needed on the stateful, and this is the PR that introduces a distinction between stateful and stateless.

rossabaker avatar Mar 07 '22 01:03 rossabaker

Yeah they are orthogonal (and sorry for hijacking your discussion to an extent), although I think the problem solved by the entry method is much more common, where I was coming from is this:

that introduces a distinction between stateful and stateless.

SystemFw avatar Mar 07 '22 09:03 SystemFw

The entry point is supportable on the stateless instances, but only works with a signature that passes the continuation, and would not work particularly well for Resource. It's necessary only because of stateful Trace, but pushed down there, would let us continue to abstract over stateful vs. stateless:

def continueOrRoot[A](fa: F[A], kernel: Kernel): F[A]

rossabaker avatar Mar 07 '22 15:03 rossabaker

ahah I keep getting confused because in my mind Trace + Kleisli is more "stateful" (since it represents the current trace).

I think what you're saying is that if we wanted to have entry points in Trace, rather than TraceResource (really needs a nicer name), it would have to include an A => F[B] which is confusing for Resource? and that TraceResource doesn't have this problem?

def continueOrRoot[A](fa: F[A], kernel: Kernel): F[A]

ok this sounds good, so continueOrRoot won't be expressed in terms of Resource, you only see Resource where you want to trace an actual Resource?

SystemFw avatar Mar 07 '22 17:03 SystemFw

I think we need a continueOrRoot(kernel: Kernel): Resource[F, Unit] as well. I have effectively this in Bayou I believe.

armanbilge avatar Mar 07 '22 18:03 armanbilge

in my mind Trace + Kleisli is more "stateful"

I've been using the opposite terminology. Trace[IO] captures a root span as state when it is acquired. The statefulness is further reflected in that instances must be obtained in IO. Kleisli is "stateless" in that it's just a function of a span to be provided later. But if that's stateless, a state monad is just a function of a state to be provided later, so maybe these terms are bad.

I think what you're saying is that if we wanted to have entry points in Trace, rather than TraceResource (really needs a nicer name), it would have to include an A => F[B] which is confusing for Resource? and that TraceResource doesn't have this problem?

Yes. continueOrRoot[A](name: String, kernel: Kernel): Resource[F, Unit] (ed: I added the name parameter) has the same problem as spanR(name: String): Resource[F, Unit] for Kleisli: we span the use of the returned resource, but that span isn't ambient, or even accessible, to any of the three phases (acquisition, release, or use) of the resource.

We could alternatively add these to Trace, which would be the first instance of Span on the type class:

// Create a new span and return it as a resource
def spanR(name: String): Resource[F, Span[F]]

// Make the span ambient through `fa`
def scope[A](span: Span[F])(fa: F[A]): F[A]

Typed in GitHub, has never seen a compiler:

def traceHttp4sClient[F: Trace](c: Client[F]): Client[F] =
  Kleisli { req =>
     for {
       s <- Trace[F].spanR("whole-connection") // I span the whole connection
       r <- Resource(
         Trace[F].scope(s) { // so `s` is ambient
           Trace[F].span("response-and-headers") { // I span status and headers but not body
             Trace[F].put("http.uri", req.uri.toString) >>
             client.run(req).allocated
           }.flatMap { case (resp, release) =>
             Trace[F].put("http.status", resp.status.toInt) >>
             F.pure(resp -> Trace.scope(s) { // so `s` is ambient
               Trace[F].span("release") { // I don't span body, but how quickly connection closes
                 release
               }
             }
           }
         }
       )
     } yield (r)

That's harder to use, couples the type class to Span, and s is inaccessible to whoever uses the response. The time spent reading the body could be inferred from the time spanned by whole-connection minus response-and-headers or release, but we can't add any tracing.

Alternatively, we can do this in this PR, and spans created while using the response would fall under whole-connection:

def traceHttp4sClient[F: TraceResource](c: Client[F]): Client[F] =
  Kleisli { req =>
     for {
       _ <- TraceResource[F].spanR("whole-connection") // I span the whole connection
       r <- Resource(
         Trace[F].span("response-and-headers") { // I span status and headers but not body
           Trace[F].put("http.uri", req.uri.toString) >>
           client.run(req).allocated
         }.flatMap { case (resp, release) =>
           Trace[F].put("http.status", resp.status.toInt) >>
           F.pure(resp -> Trace[F].span("release") { // I don't span body, but how quickly connection closes
             release
           }
         }
       )
     } yield (r)

TraceResource (really needs a nicer name)

Agree. I almost called it StatefulTrace, but I guess that's confusing too.

rossabaker avatar Mar 07 '22 18:03 rossabaker