From ec21382ff632681ca04ae8c6d2171f3a76a27d28 Mon Sep 17 00:00:00 2001 From: Marissa/Princess Date: Thu, 6 Jul 2023 15:54:13 -0400 Subject: [PATCH] Replace `SpanBuilder#wrapResource` API Replace `SpanBuilder#wrapResource` with `SpanOps#resource:Resource[F,F~>F]`, which no longer automatically provides 'acquire', 'use' and 'release' spans. This API change allows further simplification of the API in future commits. --- .../typelevel/otel4s/trace/TracerMacro.scala | 19 +++--- .../typelevel/otel4s/trace/TracerMacro.scala | 18 +++--- .../org/typelevel/otel4s/trace/Span.scala | 29 --------- .../typelevel/otel4s/trace/SpanBuilder.scala | 52 ++------------- .../org/typelevel/otel4s/trace/SpanOps.scala | 5 ++ .../org/typelevel/otel4s/trace/Tracer.scala | 11 ++-- examples/src/main/scala/TraceExample.scala | 3 +- examples/src/main/scala/TracingExample.scala | 3 +- .../otel4s/java/trace/SpanBuilderImpl.scala | 7 +- .../otel4s/java/trace/SpanRunner.scala | 52 --------------- .../otel4s/java/trace/TracerSuite.scala | 64 ++++++++++++++----- 11 files changed, 90 insertions(+), 173 deletions(-) diff --git a/core/trace/src/main/scala-2/org/typelevel/otel4s/trace/TracerMacro.scala b/core/trace/src/main/scala-2/org/typelevel/otel4s/trace/TracerMacro.scala index 43f930c2b..043102487 100644 --- a/core/trace/src/main/scala-2/org/typelevel/otel4s/trace/TracerMacro.scala +++ b/core/trace/src/main/scala-2/org/typelevel/otel4s/trace/TracerMacro.scala @@ -17,7 +17,8 @@ package org.typelevel.otel4s package trace -import cats.effect.kernel.Resource +import cats.effect.Resource +import cats.~> private[otel4s] trait TracerMacro[F[_]] { self: Tracer[F] => @@ -101,10 +102,11 @@ private[otel4s] trait TracerMacro[F[_]] { * @param attributes * the set of attributes to associate with the span */ - def resourceSpan[A](name: String, attributes: Attribute[_]*)( - resource: Resource[F, A] - ): SpanOps.Aux[F, Span.Res[F, A]] = - macro TracerMacro.resourceSpan[F, A] + def spanResource( + name: String, + attributes: Attribute[_]* + ): Resource[F, F ~> F] = + macro TracerMacro.spanResource } object TracerMacro { @@ -128,14 +130,13 @@ object TracerMacro { q"(if ($meta.isEnabled) ${c.prefix}.spanBuilder($name).root.addAttributes(..$attributes) else $meta.noopSpanBuilder).build" } - def resourceSpan[F[_], A](c: blackbox.Context)( + def spanResource(c: blackbox.Context)( name: c.Expr[String], attributes: c.Expr[Attribute[_]]* - )(resource: c.Expr[Resource[F, A]]): c.universe.Tree = { + ): c.universe.Tree = { import c.universe._ val meta = q"${c.prefix}.meta" - - q"if ($meta.isEnabled) ${c.prefix}.spanBuilder($name).addAttributes(..$attributes).wrapResource($resource).build else $meta.noopResSpan($resource).build" + q"if ($meta.isEnabled) ${c.prefix}.spanBuilder($name).addAttributes(..$attributes).build.resource else $meta.noopResSpan" } } diff --git a/core/trace/src/main/scala-3/org/typelevel/otel4s/trace/TracerMacro.scala b/core/trace/src/main/scala-3/org/typelevel/otel4s/trace/TracerMacro.scala index 3e0ec38bb..a0ef61b1d 100644 --- a/core/trace/src/main/scala-3/org/typelevel/otel4s/trace/TracerMacro.scala +++ b/core/trace/src/main/scala-3/org/typelevel/otel4s/trace/TracerMacro.scala @@ -17,6 +17,7 @@ package org.typelevel.otel4s package trace +import cats.~> import cats.effect.kernel.Resource import scala.quoted.* @@ -106,11 +107,11 @@ private[otel4s] trait TracerMacro[F[_]] { * @param attributes * the set of attributes to associate with the span */ - inline def resourceSpan[A]( + inline def spanResource( inline name: String, inline attributes: Attribute[_]* - )(inline resource: Resource[F, A]): SpanOps.Aux[F, Span.Res[F, A]] = - ${ TracerMacro.resourceSpan('self, 'name, 'attributes, 'resource) } + ): Resource[F, F ~> F] = + ${ TracerMacro.spanResource('self, 'name, 'attributes) } } @@ -138,20 +139,19 @@ object TracerMacro { else $tracer.meta.noopSpanBuilder.build } - def resourceSpan[F[_], A]( + def spanResource[F[_]]( tracer: Expr[Tracer[F]], name: Expr[String], - attributes: Expr[Seq[Attribute[_]]], - resource: Expr[Resource[F, A]] - )(using Quotes, Type[F], Type[A]) = + attributes: Expr[Seq[Attribute[_]]] + )(using Quotes, Type[F]) = '{ if ($tracer.meta.isEnabled) $tracer .spanBuilder($name) .addAttributes($attributes*) - .wrapResource($resource) .build - else $tracer.meta.noopResSpan($resource).build + .resource + else $tracer.meta.noopResSpan } } diff --git a/core/trace/src/main/scala/org/typelevel/otel4s/trace/Span.scala b/core/trace/src/main/scala/org/typelevel/otel4s/trace/Span.scala index 991f53e6a..2a1258526 100644 --- a/core/trace/src/main/scala/org/typelevel/otel4s/trace/Span.scala +++ b/core/trace/src/main/scala/org/typelevel/otel4s/trace/Span.scala @@ -157,33 +157,4 @@ object Span { new Span[F] { def backend: Backend[F] = back } - - /** The allocation and release stages of a supplied resource are traced by - * separate spans. Carries a value of a wrapped resource. - * - * The structure of the inner spans: - * {{{ - * > span-name - * > acquire - * > use - * > release - * }}} - */ - trait Res[F[_], A] extends Span[F] { - def value: A - } - - object Res { - def unapply[F[_], A](span: Span.Res[F, A]): Option[A] = - Some(span.value) - - private[otel4s] def fromBackend[F[_], A]( - a: A, - back: Backend[F] - ): Res[F, A] = - new Res[F, A] { - def value: A = a - def backend: Backend[F] = back - } - } } diff --git a/core/trace/src/main/scala/org/typelevel/otel4s/trace/SpanBuilder.scala b/core/trace/src/main/scala/org/typelevel/otel4s/trace/SpanBuilder.scala index 3e47b32f4..4f982cecb 100644 --- a/core/trace/src/main/scala/org/typelevel/otel4s/trace/SpanBuilder.scala +++ b/core/trace/src/main/scala/org/typelevel/otel4s/trace/SpanBuilder.scala @@ -18,8 +18,10 @@ package org.typelevel.otel4s package trace import cats.Applicative -import cats.effect.kernel.MonadCancelThrow -import cats.effect.kernel.Resource +import cats.arrow.FunctionK +import cats.effect.MonadCancelThrow +import cats.effect.Resource +import cats.~> import scala.concurrent.duration.FiniteDuration @@ -110,41 +112,6 @@ trait SpanBuilder[F[_]] { */ def withParent(parent: SpanContext): Builder - /** Wraps the given resource to trace it upon the start. - * - * The span is started upon resource allocation and ended upon finalization. - * The allocation and release stages of the `resource` are traced by separate - * spans. Carries a value of the given `resource`. - * - * The structure of the inner spans: - * {{{ - * > span-name - * > acquire - * > use - * > release - * }}} - * - * The finalization strategy is determined by [[SpanFinalizer.Strategy]]. By - * default, the abnormal termination (error, cancelation) is recorded. - * - * @see - * default finalization strategy [[SpanFinalizer.Strategy.reportAbnormal]] - * @example - * {{{ - * val tracer: Tracer[F] = ??? - * val resource: Resource[F, String] = Resource.eval(Sync[F].delay("string")) - * val ok: F[Unit] = - * tracer.spanBuilder("wrapped-resource").wrapResource(resource).build.use { case span @ Span.Res(value) => - * span.setStatus(Status.Ok, s"all good. resource value: $${value}") - * } - * }}} - * @param resource - * the resource to trace - */ - def wrapResource[A]( - resource: Resource[F, A] - )(implicit ev: Result =:= Span[F]): SpanBuilder.Aux[F, Span.Res[F, A]] - def build: SpanOps.Aux[F, Result] } @@ -168,14 +135,6 @@ object SpanBuilder { private val span: Span[F] = Span.fromBackend(back) - def wrapResource[A]( - resource: Resource[F, A] - )(implicit ev: Result =:= Span[F]): SpanBuilder.Aux[F, Span.Res[F, A]] = - make( - back, - resource.map(r => Span.Res.fromBackend(r, back)) - ) - def addAttribute[A](attribute: Attribute[A]): Builder = this def addAttributes(attributes: Attribute[_]*): Builder = this @@ -207,6 +166,9 @@ object SpanBuilder { def surround[A](fa: F[A]): F[A] = fa + + def resource: Resource[F, F ~> F] = + Resource.pure(FunctionK.id) } } diff --git a/core/trace/src/main/scala/org/typelevel/otel4s/trace/SpanOps.scala b/core/trace/src/main/scala/org/typelevel/otel4s/trace/SpanOps.scala index a06d8c603..b1410f981 100644 --- a/core/trace/src/main/scala/org/typelevel/otel4s/trace/SpanOps.scala +++ b/core/trace/src/main/scala/org/typelevel/otel4s/trace/SpanOps.scala @@ -16,6 +16,9 @@ package org.typelevel.otel4s.trace +import cats.effect.Resource +import cats.~> + trait SpanOps[F[_]] { type Result <: Span[F] @@ -104,6 +107,8 @@ trait SpanOps[F[_]] { * See [[use]] for more details regarding lifecycle strategy */ def surround[A](fa: F[A]): F[A] + + def resource: Resource[F, F ~> F] } object SpanOps { diff --git a/core/trace/src/main/scala/org/typelevel/otel4s/trace/Tracer.scala b/core/trace/src/main/scala/org/typelevel/otel4s/trace/Tracer.scala index c800a627e..ff841ba2b 100644 --- a/core/trace/src/main/scala/org/typelevel/otel4s/trace/Tracer.scala +++ b/core/trace/src/main/scala/org/typelevel/otel4s/trace/Tracer.scala @@ -18,8 +18,9 @@ package org.typelevel.otel4s package trace import cats.Applicative -import cats.effect.kernel.MonadCancelThrow -import cats.effect.kernel.Resource +import cats.effect.MonadCancelThrow +import cats.effect.Resource +import cats.~> import org.typelevel.otel4s.meta.InstrumentMeta @annotation.implicitNotFound(""" @@ -180,10 +181,8 @@ object Tracer { trait Meta[F[_]] extends InstrumentMeta[F] { def noopSpanBuilder: SpanBuilder.Aux[F, Span[F]] - final def noopResSpan[A]( - resource: Resource[F, A] - ): SpanBuilder.Aux[F, Span.Res[F, A]] = - noopSpanBuilder.wrapResource(resource) + final def noopResSpan: Resource[F, F ~> F] = + noopSpanBuilder.build.resource } object Meta { diff --git a/examples/src/main/scala/TraceExample.scala b/examples/src/main/scala/TraceExample.scala index 41c366d68..cd253721c 100644 --- a/examples/src/main/scala/TraceExample.scala +++ b/examples/src/main/scala/TraceExample.scala @@ -136,7 +136,8 @@ object TraceExample extends IOApp.Simple { val resource: Resource[IO, Unit] = Resource.make(IO.sleep(50.millis))(_ => IO.sleep(100.millis)) tracer - .resourceSpan("Start up")(resource) + .spanResource("Start up") + .flatMap(resource.mapK(_)) .surround( userIdAlg .getAllUsersForInstitution( diff --git a/examples/src/main/scala/TracingExample.scala b/examples/src/main/scala/TracingExample.scala index 27ffce992..959e4b7de 100644 --- a/examples/src/main/scala/TracingExample.scala +++ b/examples/src/main/scala/TracingExample.scala @@ -65,7 +65,8 @@ object TracingExample extends IOApp.Simple { val resource: Resource[IO, Unit] = Resource.make(IO.sleep(50.millis))(_ => IO.sleep(100.millis)) tracer - .resourceSpan("resource")(resource) + .spanResource("resource") + .flatMap(resource.mapK(_)) .surround( Work[IO].request( Map( diff --git a/java/trace/src/main/scala/org/typelevel/otel4s/java/trace/SpanBuilderImpl.scala b/java/trace/src/main/scala/org/typelevel/otel4s/java/trace/SpanBuilderImpl.scala index 232485996..03acc137c 100644 --- a/java/trace/src/main/scala/org/typelevel/otel4s/java/trace/SpanBuilderImpl.scala +++ b/java/trace/src/main/scala/org/typelevel/otel4s/java/trace/SpanBuilderImpl.scala @@ -78,11 +78,6 @@ private[java] final case class SpanBuilderImpl[F[_]: Sync, Res <: Span[F]]( def withFinalizationStrategy(strategy: SpanFinalizer.Strategy): Builder = copy(finalizationStrategy = strategy) - def wrapResource[A]( - resource: Resource[F, A] - )(implicit ev: Result =:= Span[F]): SpanBuilder.Aux[F, Span.Res[F, A]] = - copy(runner = SpanRunner.resource(scope, resource, jTracer)) - def build: SpanOps.Aux[F, Result] = new SpanOps[F] { type Result = Res @@ -98,6 +93,8 @@ private[java] final case class SpanBuilderImpl[F[_]: Sync, Res <: Span[F]]( def surround[A](fa: F[A]): F[A] = use(_ => fa) + def resource: Resource[F, F ~> F] = start.map(_._2) + private def start: Resource[F, (Result, F ~> F)] = Resource.eval(runnerContext).flatMap(ctx => runner.start(ctx)) } diff --git a/java/trace/src/main/scala/org/typelevel/otel4s/java/trace/SpanRunner.scala b/java/trace/src/main/scala/org/typelevel/otel4s/java/trace/SpanRunner.scala index 20f8f0824..32d636b78 100644 --- a/java/trace/src/main/scala/org/typelevel/otel4s/java/trace/SpanRunner.scala +++ b/java/trace/src/main/scala/org/typelevel/otel4s/java/trace/SpanRunner.scala @@ -25,7 +25,6 @@ import cats.syntax.foldable._ import cats.syntax.functor._ import cats.~> import io.opentelemetry.api.trace.{SpanBuilder => JSpanBuilder} -import io.opentelemetry.api.trace.{Tracer => JTracer} import io.opentelemetry.context.{Context => JContext} import org.typelevel.otel4s.trace.Span import org.typelevel.otel4s.trace.SpanFinalizer @@ -61,57 +60,6 @@ private[java] object SpanRunner { } } - def resource[F[_]: Sync, A]( - scope: TraceScope[F], - resource: Resource[F, A], - jTracer: JTracer - ): SpanRunner[F, Span.Res[F, A]] = - new SpanRunner[F, Span.Res[F, A]] { - def start( - ctx: Option[RunnerContext] - ): Resource[F, (Span.Res[F, A], F ~> F)] = - ctx match { - case Some(RunnerContext(builder, parent, hasStartTimestamp, fin)) => - def child( - name: String, - parent: JContext - ): Resource[F, (SpanBackendImpl[F], F ~> F)] = - startManaged( - builder = jTracer.spanBuilder(name).setParent(parent), - hasStartTimestamp = false, - finalizationStrategy = fin, - scope = scope - ) - - for { - rootBackend <- startManaged( - builder = builder, - hasStartTimestamp = hasStartTimestamp, - finalizationStrategy = fin, - scope = scope - ) - - rootCtx <- Resource.pure(parent.`with`(rootBackend._1.jSpan)) - - pair <- Resource.make( - child("acquire", rootCtx).use(b => b._2(resource.allocated)) - ) { case (_, release) => - child("release", rootCtx).use(b => b._2(release)) - } - (value, _) = pair - - pair2 <- child("use", rootCtx) - (useSpanBackend, nt) = pair2 - } yield (Span.Res.fromBackend(value, useSpanBackend), nt) - - case None => - resource.map(a => - (Span.Res.fromBackend(a, Span.Backend.noop), FunctionK.id) - ) - } - - } - def startUnmanaged[F[_]: Sync](context: Option[RunnerContext]): F[Span[F]] = context match { case Some(RunnerContext(builder, _, ts, _)) => diff --git a/java/trace/src/test/scala/org/typelevel/otel4s/java/trace/TracerSuite.scala b/java/trace/src/test/scala/org/typelevel/otel4s/java/trace/TracerSuite.scala index c4642ea11..3358da379 100644 --- a/java/trace/src/test/scala/org/typelevel/otel4s/java/trace/TracerSuite.scala +++ b/java/trace/src/test/scala/org/typelevel/otel4s/java/trace/TracerSuite.scala @@ -19,7 +19,10 @@ package org.typelevel.otel4s.java.trace import cats.effect.IO import cats.effect.IOLocal import cats.effect.Resource +import cats.effect.kernel.MonadCancelThrow import cats.effect.testkit.TestControl +import cats.implicits.toFunctorOps +import cats.~> import io.opentelemetry.api.common.{AttributeKey => JAttributeKey} import io.opentelemetry.api.common.Attributes import io.opentelemetry.api.trace.StatusCode @@ -397,6 +400,28 @@ class TracerSuite extends CatsEffectSuite { } } + private def wrapResource[F[_]: MonadCancelThrow, A]( + tracer: Tracer[F], + resource: Resource[F, A], + attributes: Attribute[_]* + ): Resource[F, F ~> F] = { + tracer + .spanResource("resource-span", attributes: _*) + .flatMap { nt => + Resource[F, A] { + nt { + tracer + .span("acquire") + .surround { + resource.allocated.map { case (acquired, release) => + acquired -> nt(tracer.span("release").use(_ => release)) + } + } + } + }.map(_ => nt) + } + } + test("trace resource with use") { val attribute = Attribute("string-attribute", "value") @@ -464,14 +489,17 @@ class TracerSuite extends CatsEffectSuite { now <- IO.monotonic.delayBy(1.second) // otherwise returns 0 sdk <- makeSdk() tracer <- sdk.provider.get("tracer") - _ <- tracer - .resourceSpan("resource-span", attribute)(mkRes(tracer)) - .use { _ => - for { - _ <- tracer.span("body-1").surround(IO.sleep(100.millis)) - _ <- tracer.span("body-2").surround(IO.sleep(200.millis)) - _ <- tracer.span("body-3").surround(IO.sleep(50.millis)) - } yield () + _ <- wrapResource(tracer, mkRes(tracer), attribute) + .use { + _ { + tracer.span("use").surround { + for { + _ <- tracer.span("body-1").surround(IO.sleep(100.millis)) + _ <- tracer.span("body-2").surround(IO.sleep(200.millis)) + _ <- tracer.span("body-3").surround(IO.sleep(50.millis)) + } yield () + } + } } spans <- sdk.finishedSpans tree <- IO.pure(SpanNode.fromSpans(spans)) @@ -527,11 +555,14 @@ class TracerSuite extends CatsEffectSuite { now <- IO.monotonic.delayBy(1.second) // otherwise returns 0 sdk <- makeSdk() tracer <- sdk.provider.tracer("tracer").get - _ <- tracer - .resourceSpan("resource-span")(mkRes) - .surround( - tracer.span("body").surround(IO.sleep(100.millis)) - ) + _ <- wrapResource(tracer, mkRes) + .use { + _ { + tracer.span("use").surround { + tracer.span("body").surround(IO.sleep(100.millis)) + } + } + } spans <- sdk.finishedSpans tree <- IO.pure(SpanNode.fromSpans(spans)) } yield assertEquals(tree, List(expected(now))) @@ -581,9 +612,10 @@ class TracerSuite extends CatsEffectSuite { now <- IO.monotonic.delayBy(1.second) // otherwise returns 0 sdk <- makeSdk() tracer <- sdk.provider.tracer("tracer").get - _ <- tracer - .resourceSpan("resource-span")(mkRes) - .use_ + _ <- wrapResource(tracer, mkRes) + .use { + _(tracer.span("use").use_) + } spans <- sdk.finishedSpans tree <- IO.pure(SpanNode.fromSpans(spans)) } yield assertEquals(tree, List(expected(now)))