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

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

Open
iRevive opened this issue Aug 9, 2023 · 2 comments
Open

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

iRevive opened this issue Aug 9, 2023 · 2 comments
Labels
tracing Improvements to tracing module

Comments

@iRevive
Copy link
Contributor

iRevive commented Aug 9, 2023

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
Copy link
Contributor Author

iRevive commented Aug 9, 2023

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
Copy link
Contributor Author

iRevive commented Aug 9, 2023

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 iRevive added the tracing Improvements to tracing module label Oct 27, 2023
@iRevive iRevive changed the title SpanOps#startUnmanaged does not store a Span in Vault SpanOps#startUnmanaged does not store a Span in Context Oct 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tracing Improvements to tracing module
Projects
None yet
Development

No branches or pull requests

1 participant