Rename WrappedSpanContext to SpanContextConversion

typelevel · Nov 10, 2023 · dc2f0ed · dc2f0ed
1 parent bdb12be
commit dc2f0ed
Show file tree

Hide file tree

Showing 6 changed files with 54 additions and 52 deletions.
diff --git a/core/trace/src/main/scala/org/typelevel/otel4s/trace/SpanContext.scala b/core/trace/src/main/scala/org/typelevel/otel4s/trace/SpanContext.scala
@@ -161,14 +161,25 @@ object SpanContext {
       traceState: TraceState,
       remote: Boolean
   ): SpanContext =
-    createInternal(
-      traceId = traceId,
-      spanId = spanId,
-      traceFlags = traceFlags,
-      traceState = traceState,
-      remote = remote,
-      skipIdValidation = false
-    )
+    if (TraceId.isValid(traceId) && SpanId.isValid(spanId)) {
+      createInternal(
+        traceId,
+        spanId,
+        traceFlags,
+        traceState,
+        remote,
+        isValid = true
+      )
+    } else {
+      createInternal(
+        TraceId.Invalid,
+        SpanId.Invalid,
+        traceFlags,
+        traceState,
+        remote,
+        isValid = false
+      )
+    }
 
   implicit val spanContextHash: Hash[SpanContext] =
     Hash.by { ctx =>
@@ -221,44 +232,27 @@ object SpanContext {
     * @param remote
     *   whether the span is propagated from the remote parent or not
     *
-    * @param skipIdValidation
-    *   pass true to skip validation of trace ID and span ID as an optimization
-    *   in cases where they are known to have been already validated
+    * @param isValid
+    *   whether the span is valid or not
     */
   private[otel4s] def createInternal(
       traceId: ByteVector,
       spanId: ByteVector,
       traceFlags: TraceFlags,
       traceState: TraceState,
       remote: Boolean,
-      skipIdValidation: Boolean
-  ): SpanContext = {
-    if (
-      skipIdValidation || (TraceId.isValid(traceId) && SpanId.isValid(spanId))
-    ) {
-      SpanContextImpl(
-        traceId = traceId,
-        traceIdHex = traceId.toHex,
-        spanId = spanId,
-        spanIdHex = spanId.toHex,
-        traceFlags = traceFlags,
-        traceState = traceState,
-        isRemote = remote,
-        isValid = true
-      )
-    } else {
-      SpanContextImpl(
-        traceId = TraceId.Invalid,
-        traceIdHex = TraceId.Invalid.toHex,
-        spanId = SpanId.Invalid,
-        spanIdHex = SpanId.Invalid.toHex,
-        traceFlags = traceFlags,
-        traceState = traceState,
-        isRemote = remote,
-        isValid = false
-      )
-    }
-  }
+      isValid: Boolean
+  ): SpanContext =
+    SpanContextImpl(
+      traceId = traceId,
+      traceIdHex = traceId.toHex,
+      spanId = spanId,
+      spanIdHex = spanId.toHex,
+      traceFlags = traceFlags,
+      traceState = traceState,
+      isRemote = remote,
+      isValid = isValid
+    )
 
   private final case class SpanContextImpl(
       traceId: ByteVector,

diff --git a/java/trace/src/main/scala/org/typelevel/otel4s/java/trace/SpanBackendImpl.scala b/java/trace/src/main/scala/org/typelevel/otel4s/java/trace/SpanBackendImpl.scala
@@ -104,7 +104,10 @@ private[java] class SpanBackendImpl[F[_]: Sync](
 
 private[java] object SpanBackendImpl {
   def fromJSpan[F[_]: Sync](jSpan: JSpan): SpanBackendImpl[F] =
-    new SpanBackendImpl(jSpan, WrappedSpanContext.wrap(jSpan.getSpanContext))
+    new SpanBackendImpl(
+      jSpan,
+      SpanContextConversion.fromJSpanContext(jSpan.getSpanContext)
+    )
 
   private def toJStatus(status: Status): JStatusCode =
     status match {

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
@@ -105,7 +105,7 @@ private[java] final case class SpanBuilderImpl[F[_]: Sync](
     startTimestamp.foreach(d => b.setStartTimestamp(d.length, d.unit))
     links.foreach { case (ctx, attributes) =>
       b.addLink(
-        WrappedSpanContext.unwrap(ctx),
+        SpanContextConversion.toJSpanContext(ctx),
         Conversions.toJAttributes(attributes)
       )
     }
@@ -136,7 +136,7 @@ private[java] final case class SpanBuilderImpl[F[_]: Sync](
             case Parent.Propagate => underlying
             case Parent.Explicit(parent) =>
               JSpan
-                .wrap(WrappedSpanContext.unwrap(parent))
+                .wrap(SpanContextConversion.toJSpanContext(parent))
                 .storeInContext(underlying)
           }
         }

diff --git a/...tel4s/java/trace/WrappedSpanContext.scala → ...4s/java/trace/SpanContextConversion.scala b/...tel4s/java/trace/WrappedSpanContext.scala → ...4s/java/trace/SpanContextConversion.scala
@@ -24,23 +24,24 @@ import org.typelevel.otel4s.trace.TraceFlags
 import org.typelevel.otel4s.trace.TraceState
 import scodec.bits.ByteVector
 
-private[otel4s] object WrappedSpanContext {
+private[otel4s] object SpanContextConversion {
 
-  def wrap(context: JSpanContext): SpanContext = {
+  def fromJSpanContext(context: JSpanContext): SpanContext = {
     val entries = Vector.newBuilder[(String, String)]
     context.getTraceState.forEach((k, v) => entries.addOne(k -> v))
     val traceState = TraceState.fromVectorUnsafe(entries.result())
 
-    SpanContext(
+    SpanContext.createInternal(
       traceId = ByteVector(context.getTraceIdBytes),
       spanId = ByteVector(context.getSpanIdBytes),
       traceFlags = TraceFlags.fromByte(context.getTraceFlags.asByte),
       traceState = traceState,
-      remote = context.isRemote
+      remote = context.isRemote,
+      isValid = context.isValid
     )
   }
 
-  def unwrap(context: SpanContext): JSpanContext = {
+  def toJSpanContext(context: SpanContext): JSpanContext = {
     val traceId = context.traceIdHex
     val spanId = context.spanIdHex
     val flags = JTraceFlags.fromByte(context.traceFlags.toByte)

diff --git a/java/trace/src/main/scala/org/typelevel/otel4s/java/trace/TracerImpl.scala b/java/trace/src/main/scala/org/typelevel/otel4s/java/trace/TracerImpl.scala
@@ -45,7 +45,9 @@ private[java] class TracerImpl[F[_]: Sync](
       case Context.Noop => None
       case Context.Wrapped(underlying) =>
         Option(JSpan.fromContextOrNull(underlying))
-          .map(jSpan => WrappedSpanContext.wrap(jSpan.getSpanContext))
+          .map(jSpan =>
+            SpanContextConversion.fromJSpanContext(jSpan.getSpanContext)
+          )
     }
 
   def currentSpanOrNoop: F[Span[F]] =
@@ -62,7 +64,9 @@ private[java] class TracerImpl[F[_]: Sync](
 
   def childScope[A](parent: SpanContext)(fa: F[A]): F[A] =
     L.local(fa) {
-      _.map(JSpan.wrap(WrappedSpanContext.unwrap(parent)).storeInContext)
+      _.map(
+        JSpan.wrap(SpanContextConversion.toJSpanContext(parent)).storeInContext
+      )
     }
 
   def rootScope[A](fa: F[A]): F[A] =

diff --git a/java/trace/src/test/scala/org/typelevel/otel4s/java/trace/SpanContextSuite.scala b/java/trace/src/test/scala/org/typelevel/otel4s/java/trace/SpanContextSuite.scala
@@ -60,7 +60,7 @@ class SpanContextSuite extends ScalaCheckSuite {
 
   test("SpanContext to JSpanContext") {
     Prop.forAll(spanContextGen) { ctx =>
-      val jCtx = WrappedSpanContext.unwrap(ctx)
+      val jCtx = SpanContextConversion.toJSpanContext(ctx)
 
       assert(ctx.traceId.toArray.sameElements(jCtx.getTraceIdBytes))
       assert(ctx.spanId.toArray.sameElements(jCtx.getSpanIdBytes))
@@ -76,8 +76,8 @@ class SpanContextSuite extends ScalaCheckSuite {
 
   test("back and forth conversion") {
     Prop.forAll(spanContextGen) { ctx =>
-      val jCtx = WrappedSpanContext.unwrap(ctx)
-      assertEquals(WrappedSpanContext.wrap(jCtx), ctx)
+      val jCtx = SpanContextConversion.toJSpanContext(ctx)
+      assertEquals(SpanContextConversion.fromJSpanContext(jCtx), ctx)
     }
   }