otel4s icon indicating copy to clipboard operation
otel4s copied to clipboard

`SpanOps#startUnmanaged` does not store a Span in Context

Open iRevive opened this issue 2 years ago • 2 comments

The follow-up to https://discord.com/channels/632277896739946517/1093154207328108634/1138014456278954014.

An example:

import cats.effect.kernel.Async
import cats.effect.std.{Console, Random}
import cats.effect.{IO, IOApp, Resource}
import cats.syntax.all._
import org.typelevel.otel4s.Attribute
import org.typelevel.otel4s.java.OtelJava
import org.typelevel.otel4s.trace.Tracer

import scala.concurrent.duration._

object Example extends IOApp.Simple {

  trait Work[F[_]] {
    def doWork: F[Unit]
  }

  def work[F[_]: Tracer: Async: Console] =
    new Work[F] {
      def doWork: F[Unit] =
        Resource.make(Tracer[F].span("Work.DoWork").startUnmanaged)(_.end).use {
          span =>
            span.addEvent("Starting the work.") *>
              doWorkInternal(steps = 10) *>
              span.addEvent("Finished working.")
        }

      def doWorkInternal(steps: Int): F[Unit] = {
        val step = Tracer[F].currentSpanContext.flatMap { case Some(parent) =>
          Tracer[F]
            .spanBuilder("internal")
            .addAttribute(Attribute("steps", steps.toLong))
            .withParent(parent)
            .build
            .use { _ =>
              for {
                random <- Random.scalaUtilRandom
                delay <- random.nextIntBounded(1000)
                _ <- Async[F].sleep(delay.millis)
                _ <- Console[F].println("Doin' work")
              } yield ()
            }
        }

        if (steps > 0) step *> doWorkInternal(steps - 1) else step
      }
    }

  def run: IO[Unit] = {
    OtelJava.global[IO].flatMap { otel4s =>
      otel4s.tracerProvider
        .get("com.service.runtime")
        .flatMap { implicit tracer: Tracer[IO] =>
          work[IO].doWork
        }
    }
  }
}

The span started by startUnmanaged isn't stored in a Vault. And from what I see, it's by design.

The context management logic revolves around Local semantics. For example, another SpanOps API (e.g. def use[A](f: Span[F] => F[A]): F[A]) has a fixed scope where the context can be manipulated.

With startUnmanaged situation is different. If we try to set this span directly into the Vault it may outlive the 'scope' eventually, because span.end does not reset the context.

Let's assume we always store the active span in the Vault, the following example is problematic:

Tracer[F].span("Work.DoWork").startUnmanaged.flatMap { span =>
  for {
    _ <- Tracer[F].span("child-1").use(span => span.addEvent("Starting the work.")) // child of a "Work.DoWork"
    _ <- span.end // "Work.DoWork" is closed
    _ <- Tracer[F].span("child-2").use(span => span.addEvent("Ending the work.")) // child of a "Work.DoWork" again
  } yield ()
}

I would say, it adds more burden than benefits.

iRevive avatar Aug 09 '23 14:08 iRevive

Another tricky case in the current implementation:

Tracer[F].span("auto").use { span =>
  Tracer[F].span("unmanaged").startUnmanaged.flatMap { unmanagedSpan =>
    Tracer[F].span("child-1").use(span => ...) // child of the "auto" span
  }
}

iRevive avatar Aug 09 '23 14:08 iRevive

I don't think we should change the existing behavior. But we definitely need to improve the documentation:

  • [ ] Update Scaladoc
  • [ ] Add examples to the site (pass parent explicitly, Tracer[F].childScope(span.spanContext)(), etc)

iRevive avatar Aug 09 '23 14:08 iRevive