From 5e44d700552ffbf5689c1c60481e469edf333440 Mon Sep 17 00:00:00 2001 From: dmytr Date: Sat, 6 May 2023 14:22:28 +0200 Subject: [PATCH] Remove PropagatingSupervisor --- docs/opentelemetry.md | 46 +++++++++---------- .../context/ContextStorage.scala | 6 +-- .../internal/PropagatingSupervisor.scala | 45 ------------------ .../opentelemetry/tracing/Tracing.scala | 12 ----- 4 files changed, 26 insertions(+), 83 deletions(-) delete mode 100644 opentelemetry/src/main/scala/zio/telemetry/opentelemetry/internal/PropagatingSupervisor.scala diff --git a/docs/opentelemetry.md b/docs/opentelemetry.md index f3d3f8ae..8c954a33 100644 --- a/docs/opentelemetry.md +++ b/docs/opentelemetry.md @@ -104,35 +104,35 @@ ZIO.serviceWithZIO[Tracing] { tracing => } ``` -### [Experimental] Usage with OpenTelemetry automatic instrumentation +### Usage with OpenTelemetry automatic instrumentation OpenTelemetry provides a [JVM agent for automatic instrumentation](https://opentelemetry.io/docs/instrumentation/java/automatic/) which supports -many [popular Java libraries](https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/docs/supported-libraries.md) -. +many [popular Java libraries](https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/docs/supported-libraries.md). -This automatic instrumentation relies on the default OpenTelemetry context storage which is based on `ThreadLocal`. So -it doesn't work with ZIO out of the box. +Since [version 1.25.0](https://github.com/open-telemetry/opentelemetry-java-instrumentation/releases/tag/v1.25.0) +OpenTelemetry JVM agent supports ZIO. -`zio-opentelemetry` provides an experimental version of `Tracing` which bidirectionally propagates tracing context -between ZIO and non-ZIO code, enabling interoperability with _most_ libraries that use the default OpenTelemetry context -storage. +To enable interoperability between automatic instrumentation and `zio-opentelemetry`, `Tracing` has to be created +using `ContextStorage` backed by OpenTelemetry's `Context`. -To enable this experimental propagation, you will need to create `Tracing` using `Tracing.propagating` constructor ( -instead of `Tracing.live`). - -Please note that whether context propagation will work correctly depends on which specific ZIO wrappers around non-ZIO -libraries you are using. So please, test your specific setup. - -It was reported that it works with: - -* `zhttp` -* `sttp` with Java 11+ HTTP client backend -* `zio-kafka` -* `doobie` -* `redis4cats` +```scala +import zio.telemetry.opentelemetry.tracing.Tracing +import zio.telemetry.opentelemetry.context.ContextStorage +import zio.telemetry.opentelemetry.example.JaegerTracer +import io.opentelemetry.api.trace.{SpanKind, StatusCode} +import zio._ -It was reported that it does not work with: +val errorMapper = ErrorMapper[Throwable] { case _ => StatusCode.UNSET } -* `sttp` with `armeria` backend +val app = + ZIO.serviceWithZIO[Tracing] { tracing => + import tracing.aspects._ + ZIO.logInfo("Hello") @@ root("root span", SpanKind.INTERNAL, errorMapper) + }.provide( + Tracing.live, + ContextStorage.openTelemetryContext, // <<< + JaegerTracer.live + ) +``` diff --git a/opentelemetry/src/main/scala/zio/telemetry/opentelemetry/context/ContextStorage.scala b/opentelemetry/src/main/scala/zio/telemetry/opentelemetry/context/ContextStorage.scala index d7d03763..1575abfd 100644 --- a/opentelemetry/src/main/scala/zio/telemetry/opentelemetry/context/ContextStorage.scala +++ b/opentelemetry/src/main/scala/zio/telemetry/opentelemetry/context/ContextStorage.scala @@ -51,10 +51,10 @@ object ContextStorage { ) /** - * Uses OpenTelemetry's default context storage which is backed by a [[java.lang.ThreadLocal]]. This makes sense only - * if `zio.telemetry.opentelemetry.internal.PropagatingSupervisor` is used. + * Uses OpenTelemetry's context storage which is backed by a [[java.lang.ThreadLocal]]. This makes sense only if + * [[https://github.com/open-telemetry/opentelemetry-java-instrumentation OTEL instrumentation agent]] is used. */ - val threadLocal: ULayer[ContextStorage] = + val openTelemetryContext: ULayer[ContextStorage] = ZLayer.succeed { new ContextStorage { override def get(implicit trace: Trace): UIO[Context] = ZIO.succeed(Context.current()) diff --git a/opentelemetry/src/main/scala/zio/telemetry/opentelemetry/internal/PropagatingSupervisor.scala b/opentelemetry/src/main/scala/zio/telemetry/opentelemetry/internal/PropagatingSupervisor.scala deleted file mode 100644 index 35527427..00000000 --- a/opentelemetry/src/main/scala/zio/telemetry/opentelemetry/internal/PropagatingSupervisor.scala +++ /dev/null @@ -1,45 +0,0 @@ -package zio.telemetry.opentelemetry.internal - -import io.opentelemetry.api.trace.Span -import io.opentelemetry.context.Context -import zio._ - -import java.util.concurrent.ConcurrentHashMap -import scala.annotation.nowarn - -@nowarn("msg=discarded non-Unit value") -private[opentelemetry] final class PropagatingSupervisor extends Supervisor[Unit] { - - private val storage = new ConcurrentHashMap[FiberId, Span]() - - override def value(implicit trace: Trace): UIO[Unit] = ZIO.unit - - override def onStart[R, E, A]( - environment: ZEnvironment[R], - effect: ZIO[R, E, A], - parent: Option[Fiber.Runtime[Any, Any]], - fiber: Fiber.Runtime[E, A] - )(implicit unsafe: Unsafe): Unit = { - val span = Span.current() - if (span != null) storage.put(fiber.id, span) - else storage.put(fiber.id, Span.fromContext(Context.root())) - } - - override def onEnd[R, E, A](value: Exit[E, A], fiber: Fiber.Runtime[E, A])(implicit unsafe: Unsafe): Unit = { - storage.remove(fiber.id) - Context.root().makeCurrent() - } - - override def onSuspend[E, A](fiber: Fiber.Runtime[E, A])(implicit unsafe: Unsafe): Unit = { - val span = Span.current() - if (span != null) storage.put(fiber.id, span) - else storage.put(fiber.id, Span.fromContext(Context.root())) - Context.root().makeCurrent() - } - - override def onResume[E, A](fiber: Fiber.Runtime[E, A])(implicit unsafe: Unsafe): Unit = { - val span = storage.get(fiber.id) - if (span != null) span.makeCurrent() - } - -} diff --git a/opentelemetry/src/main/scala/zio/telemetry/opentelemetry/tracing/Tracing.scala b/opentelemetry/src/main/scala/zio/telemetry/opentelemetry/tracing/Tracing.scala index 51d049aa..d127158d 100644 --- a/opentelemetry/src/main/scala/zio/telemetry/opentelemetry/tracing/Tracing.scala +++ b/opentelemetry/src/main/scala/zio/telemetry/opentelemetry/tracing/Tracing.scala @@ -5,7 +5,6 @@ import io.opentelemetry.api.trace._ import io.opentelemetry.context.Context import zio._ import zio.telemetry.opentelemetry.context.{ ContextStorage, IncomingContextCarrier, OutgoingContextCarrier } -import zio.telemetry.opentelemetry.internal.PropagatingSupervisor import zio.telemetry.opentelemetry.tracing.propagation.TraceContextPropagator import java.util.concurrent.TimeUnit @@ -479,17 +478,6 @@ object Tracing { } yield tracing } - /** - * Tracing context will be bidirectionally propagated between ZIO and non-ZIO code. - * - * Since context propagation adds a performance overhead, it is recommended to use [[Tracing.live]] in most cases. - * - * [[Tracing.propagating]] should be used in combination with automatic instrumentation via OpenTelemetry JVM agent - * only. - */ - def propagating: URLayer[Tracer with ContextStorage, Tracing] = - Runtime.addSupervisor(new PropagatingSupervisor) ++ live - def scoped(tracer: Tracer, ctxStorage: ContextStorage): URIO[Scope, Tracing] = { val acquire = ZIO.succeed {