-
Notifications
You must be signed in to change notification settings - Fork 79
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
Introduce a TraceResource type class #527
Conversation
37864be
to
ba58b6d
Compare
This is basically Bayou with some deduplication because it lives in the same repo. |
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. |
Perhaps this achieves subtleties I don't appreciate. I'm not precisely sure what this means:
Are you talking about the |
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 |
Okay, I think I now see what you thought you saw. I think the entry method could return 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. |
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:
|
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 def continueOrRoot[A](fa: F[A], kernel: Kernel): F[A] |
ahah I keep getting confused because in my mind I think what you're saying is that if we wanted to have entry points in
ok this sounds good, so |
I think we need a |
I've been using the opposite terminology.
Yes. We could alternatively add these to
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 Alternatively, we can do this in this PR, and spans created while using the response would fall under 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)
Agree. I almost called it |
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
TraceResource[F]
and aMonadCancel[F]
.Applicative[F]
.Libraries that use this constraint trade the "local" instances (
Kleisli[F, Span[F], *]
andLocal[F, Span[F]]
) for the ability to span an entire resource.