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

Does tracing work well without IOLocal? Should it? #88

Closed
rossabaker opened this issue Jan 23, 2023 · 15 comments
Closed

Does tracing work well without IOLocal? Should it? #88

rossabaker opened this issue Jan 23, 2023 · 15 comments
Labels
tracing Improvements to tracing module

Comments

@rossabaker
Copy link
Member

Natchez has Trace implementations for IOLocal, cats.mtl.Local, and various transformer stacks. IOLocal is different in that it's "stateful". It's probably the most ergonomic, but it also has some limitations discussed on #37.

Should we be supporting the cats.mtl.Local instances as well? If we do, what, if anything, needs to change?

@rossabaker rossabaker added the tracing Improvements to tracing module label Jan 23, 2023
@iRevive
Copy link
Contributor

iRevive commented Jan 24, 2023

The tracing scope is managed by TraceScope.

Specifically, only one method needs to be implemented:
https://github.com/typelevel/otel4s/blob/main/java/trace/src/main/scala/org/typelevel/otel4s/java/trace/TraceScope.scala#L84-L87

AFAIK we cannot manipulate Kleisli's value within Resource, aren't we?

def createScope(scope: String): Resource[Kleisli[F,  String, *], Unit] =
  Resource.make(Kleisli.ask <* Kleisli.pure(scope))(p => Kleisli.pure(scope))
  
scope("32").use(current => Kleisli.liftF(IO.println(current))).run("42") // prints "42"

Since Natchez does not offer a Resource API, the span can be propagated via Kleisli.local(), for example https://github.com/tpolecat/natchez/blob/d605b70ae7629abb5d51c6610f8809faf102f45c/modules/core/shared/src/main/scala/Trace.scala#L173.

Since cats.mtl.Local uses Kleisli under the hood, it does not offer Resource API too https://github.com/typelevel/cats-mtl/blob/main/core/src/main/scala/cats/mtl/Local.scala#L55.

@iRevive
Copy link
Contributor

iRevive commented Jan 24, 2023

I like the concept of IOLocal unless I heavily rely on IO.race, Stream's interrupt scopes, etc.

For some users, this behavior may be a deal breaker. Therefore, supporting cats.mtl.Local (and Kleisli, as a result) should be considered.

@iRevive
Copy link
Contributor

iRevive commented Jan 24, 2023

It would be interesting to see a mapK alternative but applied to a function passed to the use:

trait Resource[F[_], A] {
  def intercept(f: F ~> F): Resource[F, A] = ???
}

val res = Resource
  .unit[Kleisli[IO, String, *]]
  .intercept(new (Kleisli[IO, String, *] ~> Kleisli[IO, String, *]) { 
    def apply[A](f: Kleisli[IO, String, A]): Kleisli[IO, String, A] = 
      Kleisli.liftF(f.run("intercept"))
   })
   
res.use(current => IO.println(current)).use("external") // prints "intercept"

It sounds fun, but it may be dangerous and break a few laws.

@armanbilge
Copy link
Member

For some users, this behavior may be a deal breaker

Sorry, can you clarify this point? What is a situation where Kleisli works, but IOLocal does not?

@iRevive
Copy link
Contributor

iRevive commented Jan 24, 2023

@armanbilge unless I'm missing something, I do not understand how to implement the following service in terms of Kleisli:

case class State(value: String)

trait LocalState[F[_]] {
  def current: F[State]
  def scope(next: State): Resource[F, Unit]
}
The snippet
//> using scala "2.13.10"
//> using lib "org.typelevel::cats-effect::3.4.5"

import cats.data.Kleisli
import cats.effect.{IO, IOApp, IOLocal, Resource, Sync}
import cats.effect.std.Console
import cats.syntax.functor._
import cats.syntax.flatMap._

object Main extends IOApp.Simple {

  case class State(value: String)

  trait LocalState[F[_]] {
    def current: F[State]
    def scope(next: State): Resource[F, Unit]
  }

  def ioLocal(initial: State): IO[LocalState[IO]] =
    IOLocal(initial).map { local =>
      new LocalState[IO] {
        def current: IO[State] =
          local.get

        def scope(next: State): Resource[IO, Unit] =
          Resource.make(local.getAndSet(next))(previous => local.set(previous)).void
      }
    }

  type K[A] = Kleisli[IO, State, A]

  def kleisli: LocalState[K] =
    new LocalState[K] {
      def current: Kleisli[IO, State, State] =
        Kleisli.ask

      def scope(next: State): Resource[K, Unit] =
        ??? // how to implement the scope?
    }

  def test[F[_]: Sync: Console](state: LocalState[F]): F[Unit] =
    for {
      current <- state.current
      _ <- Console[F].println(current)
      _ <- state.scope(State("scope-specific")).surround {
        state.current.flatMap(state => Console[F].println(state))
      }
    } yield ()

  def run: IO[Unit] = {
    val initial = State("initial")
    ioLocal(initial).flatMap(state => test(state))
    // output:
    // State(initial)
    // State(scope-specific)

    //test(kleisli).run(initial)
  }

}

Nvm, I misread your question.

In some scenarios, IOLocal behaves differently than Kleisli:
typelevel/fs2#2842
typelevel/cats-effect#3100

@armanbilge
Copy link
Member

armanbilge commented Jan 24, 2023

In some scenarios, IOLocal behaves differently than Kleisli:
typelevel/fs2#2842
typelevel/cats-effect#3100

I'm familiar with those issues, but they rely on APIs that are not available for Kleisli :) e.g. set. If you use IOLocal only with Local-style, then you won't see those.

@rossabaker
Copy link
Member Author

What on earth, I missed a whole conversation.

Anyway, #102 and #103 offer competing views of the world, via cats-mtl. As discussed in the Natchez Resource Debates of 2022, there is a fundamental difference between Stateful and Local (what I called "stateless" in that repo).

  • Natchez is local, from the days before IOLocal, with known compromises to Resource and Stream.
  • Maksym's initial design is stateful, with nasty surprises in the issues mentioned above.

I think it's clear that IOLocal has won, but it can operate in either model, and the Cats MTL constraint makes the distinction clearer. I think we need to pick one.

--

I wanted intercept during the Natchez Resource Debates, but I couldn't figure out how to add it to CE3 Resource in a compatible fashion.

@armanbilge
Copy link
Member

What is the signature of intercept ?

@rossabaker
Copy link
Member Author

From @iRevive above:

trait Resource[F[_], A] {
  def intercept(f: F ~> F): Resource[F, A] = ???
}

@rossabaker
Copy link
Member Author

WIP: I have a gist that shows:

  • a Trace algebra
  • a Trace for cats.mtl.Local
  • a Trace for cats.mtl.Stateful
  • two Traces for IOLocal, one through each MTL class

Challenge: write a program where Local's output is preferred to Stateful's.

@rossabaker
Copy link
Member Author

I haven't made anything bad like typelevel/cats-effect#3100 happen yet, but typelevel/fs2#2842 is a real problem for spanR:

  def fs2_2842(T: Trace[IO]) = {
    val s =
      Stream.eval(T.current.flatMap(IO.println)) >>
      Stream.resource(T.spanR(1)) >>
      Stream.eval(T.current.flatMap(IO.println)) >>
      Stream.resource(T.spanR(2)) >>
      Stream.eval(T.current.flatMap(IO.println))

    IO.println("interrupt") >> s.interruptScope.compile.drain >>
    IO.println("scope") >> s.scope.compile.drain >>
    IO.println("plain") >> s.compile.drain
  }
Local
interrupt
0
0
0
scope
0
0
0
plain
0
0
0
()
Stateful
interrupt
0
0
0
scope
0
1
2
plain
0
1
2
()

@rossabaker
Copy link
Member Author

Limiting IOLocal to what can be done with Local does seem to make the sharp edges go away. Specifically, the Local[Stream[F, *], E] I derive from it seems to be immune to the interruptScope surprises.

  def localIO[E](e: E): IO[Local[IO, E]] =
    IOLocal(e).map { ioLocal =>
      new Local[IO, E] {
        def ask[E2 >: E] = ioLocal.get
        val applicative = Applicative[IO]
        def local[A](fa: IO[A])(f: E => E) =
          ioLocal.modify(p => (f(p), p))
            .bracket(Function.const(fa))(ioLocal.set)
      }
    }

That impacts Tracer, which has to see the continuation. Just sketching:

  trait Trace[F[_]] {
    def span[A](id: Int): Trace.SpanOps[F, A]
    def current: F[Int]
  }

  object Trace {
    class SpanOps[F[_], A](id: Int)(implicit L: Local[F, Int]) {
      // use this one to add events, links, etc.
      def use(k: Span => F[A]): F[A] =
        L.scope(k(Span(id)))(id)
      // use this just to span an effect
      def surround(k: F[A]): F[A] =
        L.scope(k)(id)
      // more resource-like methods?
      // maybe these go on SpanBuilder?
    }

    // ways to lift to Stream, Resource
  }

Use looks like what we have. The big bummer is that span is no longer returning a proper Resource:

   Trace.ioLocal.flatMap { implicit T =>
      T.span(1).use { span =>
        IO.println("Current span is "+span) >>
        T.current.flatMap(id => IO.println("Span context ID is "+id)) >>
        T.span(2).surround {
          T.current.flatMap(id => IO.println("Span context ID is "+id))
        }
      }
    }

@iRevive
Copy link
Contributor

iRevive commented Jan 30, 2023

I was thinking about something similar to SpanOps at some point. This approach looks good to me.

What are the benefits of expressing span as a Resource?

  1. Out-of-the-box integration with streams (unless you are using interruptScope, hehe)
  2. Better control (integrity?) over resource-based allocations (e.g. chaining several spans while making a db connection)

Can we do the following with Local?

val res: Resource[IO, Connection] = 
  for {
    _ <- T.span("first-step").start
    c <- makeConnection
    _ <- T.span("second-step").start
  } yield c
  
val r: IO[Unit] = res.use(_ =>  T.current.flatTap(IO.println)) // should be 'second-step' span

I guess the closest point we can get is:

val r: IO[Unit] = 
  Resource(T.span("first-step").surround(makeConnection.allocated)).use { c =>
    T.span("second-step").surround {
      T.current.flatTap(IO.println)
    }
  }

By the way, do Tracer.spanScope, Tracer.rootScope work fine with Local?

@rossabaker
Copy link
Member Author

Yeah, in the resource example, second-step needs to be moved into the use. With local semantics, a Resource has no way to intercept its use. It can only trace what it sees, and a resource is just acquire and release. I fought this problem for a long time in Natchez and couldn't beat it, which is why I was excited about the Stateful approach, but I think the interruptScope issue stops that cold.

I expect spanScope and rootScope work fine, as long as the effect that they scope is passed in. Everything just has to look like Local#local: a function to modify the environment and the effect in which it's modified.

@rossabaker
Copy link
Member Author

We now support any Local[F, Vault].

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

No branches or pull requests

3 participants