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

Simplify interop between instrumented java libraries and otel4s #340

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 73 additions & 1 deletion java/all/src/main/scala/org/typelevel/otel4s/java/OtelJava.scala
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,11 @@ import cats.effect.IOLocal
import cats.effect.LiftIO
import cats.effect.Resource
import cats.effect.Sync
import cats.mtl.Local
import cats.syntax.all._
import io.opentelemetry.api.{OpenTelemetry => JOpenTelemetry}
import io.opentelemetry.api.GlobalOpenTelemetry
import io.opentelemetry.context.{Context => JContext}
import io.opentelemetry.sdk.{OpenTelemetrySdk => JOpenTelemetrySdk}
import org.typelevel.otel4s.ContextPropagators
import org.typelevel.otel4s.Otel4s
Expand All @@ -36,12 +38,71 @@ import org.typelevel.otel4s.java.trace.Traces
import org.typelevel.otel4s.metrics.MeterProvider
import org.typelevel.otel4s.trace.TracerProvider

sealed class OtelJava[F[_]] private (
import scala.util.Using

sealed abstract class OtelJava[F[_]] private (
val propagators: ContextPropagators[F, Context],
val meterProvider: MeterProvider[F],
val tracerProvider: TracerProvider[F],
) extends Otel4s[F] {
type Ctx = Context

/** Runs the given `fa` with the given `JContext`.
*
* Can be used to run the effect with the external Open Telemetry Java
* context.
*
* @see
* [[useJContextUnsafe]]
*
* @param ctx
* the Open Telemetry Java Context
*
* @param fa
* the effect to run
*/
def withJContext[A](ctx: JContext)(fa: F[A]): F[A]

/** Extracts the currently active context and sets it into the thread local
* variable.
*
* Can be used to interop with the Java libraries, that rely on the Open
* Telemetry Java context.
*
* @example
* {{{
* import io.opentelemetry.api.trace.{Span => JSpan}
* import io.opentelemetry.api.trace.{Tracer => JTracer}
* import io.opentelemetry.context.{Context => JContext}
*
* val jTracer: JTracer = ???
* val dispatcher: Dispatcher[IO] = ???
* val otel4s: OtelJava[IO] = ???
* val otel4sTracer: Tracer[IO] = ???
*
* val io = otel4sTracer.span("otel4s-span").surround {
* otel4s.useJContextUnsafe { _ =>
* val span = jTracer.spanBuilder("java-span-inside-io").startSpan()
* span.storeInContext(JContext.current()).makeCurrent()
* // invoke java code, e.g. instrumented RabbitMQ Java client
* span.end()
* }
* }
*
* val span = jTracer.span("java-span").startSpan()
* span.storeInContext(JContext.current()).makeCurrent() // store span in the context
Comment on lines +86 to +93
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

* otel4s.withJContext(JContext.current())(io).unsafeRunSync() // run IO using the external context
* span.end()
* }}}
*
* The hierarchy of spans will be:
* {{{
* > java-span
* > otel4s-span
* > java-span-inside-io
* }}}
*/
def useJContextUnsafe[A](fa: JContext => A): F[A]
}

object OtelJava {
Expand Down Expand Up @@ -76,6 +137,17 @@ object OtelJava {
traces.tracerProvider,
) {
override def toString: String = jOtel.toString

def withJContext[A](ctx: JContext)(fa: F[A]): F[A] =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Local[F, Context].scope(fa)(Context.wrap(ctx))

def useJContextUnsafe[A](fa: JContext => A): F[A] =
Local[F, Context].ask.flatMap { ctx =>
Async[F].fromTry {
val jContext = ctx.underlying
Using(jContext.makeCurrent())(_ => fa(jContext))
}
}
Comment on lines +144 to +150
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,12 @@
package org.typelevel.otel4s.java

import cats.effect.IO
import io.opentelemetry.api.trace.{Span => JSpan}
import io.opentelemetry.context.{Context => JContext}
import io.opentelemetry.sdk.{OpenTelemetrySdk => JOpenTelemetrySdk}
import io.opentelemetry.sdk.testing.exporter.InMemorySpanExporter
import io.opentelemetry.sdk.trace.SdkTracerProvider
import io.opentelemetry.sdk.trace.`export`.SimpleSpanProcessor
import munit.CatsEffectSuite

class OtelJavaSuite extends CatsEffectSuite {
Expand All @@ -26,9 +31,46 @@ class OtelJavaSuite extends CatsEffectSuite {
val testSdk: JOpenTelemetrySdk = JOpenTelemetrySdk.builder().build()
OtelJava
.forAsync[IO](testSdk)
.map(testOtel4s => {
.map { testOtel4s =>
val res = testOtel4s.toString()
assert(clue(res).startsWith("OpenTelemetrySdk"))
})
}
}

test("interop with java") {
val sdk = createSdk

OtelJava
.forAsync[IO](sdk)
.map { otel4s =>
val getCurrentSpan = otel4s.useJContextUnsafe(_ => JSpan.current())

val jTracer = sdk.getTracer("tracer")
val span = jTracer.spanBuilder("test").startSpan()

span.storeInContext(JContext.current()).makeCurrent()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makeCurrent() needs to be closed here too


val ioSpan = otel4s
.withJContext(JContext.current())(getCurrentSpan)
.unsafeRunSync()

span.end()

assertEquals(span, ioSpan)
}
}

private def createSdk: JOpenTelemetrySdk = {
val exporter = InMemorySpanExporter.create()

val builder = SdkTracerProvider
.builder()
.addSpanProcessor(SimpleSpanProcessor.create(exporter))

JOpenTelemetrySdk
.builder()
.setTracerProvider(builder.build())
.build()
}

}