Simplify interop between instrumented java libraries and otel4s

[iRevive]

The PR is based on the Discord discussion. It also slightly eases the pain of #202.

Motivation

1. Allow running effect using the explicit context

Use case: you have a Spring server and a full set of Open Telemetry Java instrumentations.
You want to invoke IO and it must capture the current tracing context:

@RestController
class HelloController {
  private val otel4s: OtelJava[IO] = ???
  private val tracer: Tracer[IO] = ???
  
  @GetMapping("/find-user")
  def findUser: String = // run the IO using the currently active context
    otel4s.withJContext(JContext.current())(findUser).unsafeRunSync()
  
  private def findUser: IO[String] = 
    tracer.span("find-tracing").surround {
      IO.pure("the-result")
    }
}

2. Provide access to the current context and store it in the thread-local

Use case: you want to invoke an instrumented Java library inside the IO. The instrumentation works only with thread-local context sharing:

val rabbitMQ: RabbitMQ = ??? // instrumented library
val io = tracer.span("otel4s-span").surround {
  otel4s.useJContextUnsafe { _ =>
    val span = jTracer.spanBuilder("java-span-inside-io").startSpan()
    span.storeInContext(JContext.current()).makeCurrent()
    rabbitMQ.sendBlocking("my-message")
    span.end()
  }
}

[NthPortal]

I have a couple of thoughts:

1. these methods indirectly expose a decent amount of the API of Local; would it be reasonable for OtelJava to explicitly expose the LocalContext? and if so, would it be reasonable to extend that upwards to Otel4s (probably promoting its Ctx type member to a type parameter at the same time)?
2. the names—particularly useJContextUnsafe—aren't super clear that they use or manipulate the LocalContext. are there decent names that could be clearer about that fact? perhaps something like withLocalAsJContext(Unsafe?) rather than useJContextUnsafe?

overall, I love this PR. it addresses the same problem #214 tries to solve in a more limited but much simpler fashion, and is not held up for an unknown amount of time by reliance on an unreleased feature

[NthPortal]

makeCurrent() needs to be closed (2 instances in the doc example here)

[NthPortal]

makeCurrent() needs to be closed here too

[NthPortal]

do you think there should be a specialisation of this something like as follows?

final def withCurrentJContext[A](fa: F[A]): F[A] = withJContext(JContext.current())(fa)

[NthPortal]

given that this method calls makeCurrent() for you, should it just take a => A instead of a JContext => A?

(also, the parameter probably shouldn't be called fa)

[armanbilge]

It looks to me like this method is intended for suspending a side-effect, but there is no delay/blocking/interruptible used here.

Which opens a bigger issue, that this API doesn't currently capture the different kinds of side-effects.

[armanbilge]

it addresses the same problem #214 tries to solve in a more limited but much simpler fashion, and is not held up for an unknown amount of time by reliance on an unreleased feature

I think this summarizes it really well. Essentially we have to make contextual counterparts of delay/blocking/interruptible. It also means it won't help with libraries such as fs2-grpc or http4s-netty (or doobie, does JDBC have otel integration too?). So unfortunately it's not very composable or orthogonal.

I'm not sure I would call it "simpler", at least for UX. I would agree that it's simpler to implement!

In an ideal world I'd say we should wait for #214 to do it right. But if there are real time needs, then that's motivation to move forward with this, but we should think about if/how we will migrate/deprecate these APIs in the future.

[armanbilge]

It looks to me like this method is intended for suspending a side-effect, but there is no delay/blocking/interruptible used here.

Which opens a bigger issue, that this API doesn't currently capture the different kinds of side-effects.

[iRevive]

@armanbilge @NthPortal thanks for the feedback!

I tend to agree that the whole interop thing increases the complexity.

In an ideal world I'd say we should wait for #214 to do it right.

Definitely.

But if there are real time needs, then that's motivation to move forward with this, but we should think about if/how we will migrate/deprecate these APIs in the future.

Even now, a library user can implement the same interop logic in their codebase. All necessary API is available.

What if we go in a different direction? Rather than extending the API, I can make a documentation page that explains how to interop with Java-instrumented libraries.

Then, a user can choose whether the side effect is blocking, interruptible, etc.

It should be a sufficient solution.

[armanbilge]

Rather than extending the API, I can make a documentation page that explains how to interop with Java-instrumented libraries.

Yes, that sounds better 👍