diff --git a/core/trace/src/main/scala-2/org/typelevel/otel4s/trace/SpanBuilderMacro.scala b/core/trace/src/main/scala-2/org/typelevel/otel4s/trace/SpanBuilderMacro.scala index 32c158920..87c8eb876 100644 --- a/core/trace/src/main/scala-2/org/typelevel/otel4s/trace/SpanBuilderMacro.scala +++ b/core/trace/src/main/scala-2/org/typelevel/otel4s/trace/SpanBuilderMacro.scala @@ -153,7 +153,11 @@ object SpanBuilderMacro { attributes: c.Expr[Attribute[_]]* ): c.universe.Tree = { import c.universe._ - whenEnabled(c)(q"_.addAttributes(_root_.scala.Seq(..$attributes))") + if (attributes.nonEmpty) { + whenEnabled(c)(q"_.addAttributes(_root_.scala.Seq(..$attributes))") + } else { + c.prefix.tree + } } def addAttributesColl(c: blackbox.Context)( diff --git a/core/trace/src/main/scala-2/org/typelevel/otel4s/trace/SpanMacro.scala b/core/trace/src/main/scala-2/org/typelevel/otel4s/trace/SpanMacro.scala index 02d76b588..4f6468c09 100644 --- a/core/trace/src/main/scala-2/org/typelevel/otel4s/trace/SpanMacro.scala +++ b/core/trace/src/main/scala-2/org/typelevel/otel4s/trace/SpanMacro.scala @@ -223,7 +223,12 @@ object SpanMacro { attributes: c.Expr[Attribute[_]]* ): c.universe.Tree = { import c.universe._ - addAttributesColl(c)(c.Expr(q"_root_.scala.Seq(..$attributes)")) + + if (attributes.nonEmpty) { + addAttributesColl(c)(c.Expr(q"_root_.scala.Seq(..$attributes)")) + } else { + q"${c.prefix}.backend.meta.unit" + } } def addAttributesColl(c: blackbox.Context)( 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 b154b24b9..dbe54f209 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 @@ -137,7 +137,11 @@ object TracerMacro { attributes: c.Expr[Attribute[_]]* ): c.universe.Tree = { import c.universe._ - spanColl(c)(name, c.Expr(q"_root_.scala.Seq(..$attributes)")) + if (attributes.nonEmpty) { + spanColl(c)(name, c.Expr(q"_root_.scala.Seq(..$attributes)")) + } else { + q"${c.prefix}.spanBuilder($name).build" + } } def spanColl(c: blackbox.Context)( @@ -153,7 +157,11 @@ object TracerMacro { attributes: c.Expr[Attribute[_]]* ): c.universe.Tree = { import c.universe._ - rootSpanColl(c)(name, c.Expr(q"_root_.scala.Seq(..$attributes)")) + if (attributes.nonEmpty) { + rootSpanColl(c)(name, c.Expr(q"_root_.scala.Seq(..$attributes)")) + } else { + q"${c.prefix}.spanBuilder($name).root.build" + } } def rootSpanColl(c: blackbox.Context)( diff --git a/core/trace/src/main/scala-3/org/typelevel/otel4s/trace/SpanBuilderMacro.scala b/core/trace/src/main/scala-3/org/typelevel/otel4s/trace/SpanBuilderMacro.scala index 48d76ab4d..5bd0a0b02 100644 --- a/core/trace/src/main/scala-3/org/typelevel/otel4s/trace/SpanBuilderMacro.scala +++ b/core/trace/src/main/scala-3/org/typelevel/otel4s/trace/SpanBuilderMacro.scala @@ -159,7 +159,7 @@ object SpanBuilderMacro { attributes: Expr[immutable.Iterable[Attribute[_]]] )(using Quotes, Type[F]) = '{ - if ($builder.meta.isEnabled) + if ($builder.meta.isEnabled && $attributes.nonEmpty) $builder.modifyState(_.addAttributes($attributes)) else $builder } diff --git a/core/trace/src/main/scala-3/org/typelevel/otel4s/trace/SpanMacro.scala b/core/trace/src/main/scala-3/org/typelevel/otel4s/trace/SpanMacro.scala index 0b274d0e5..296f84c87 100644 --- a/core/trace/src/main/scala-3/org/typelevel/otel4s/trace/SpanMacro.scala +++ b/core/trace/src/main/scala-3/org/typelevel/otel4s/trace/SpanMacro.scala @@ -230,7 +230,7 @@ object SpanMacro { attributes: Expr[immutable.Iterable[Attribute[_]]] )(using Quotes, Type[F]) = '{ - if ($span.backend.meta.isEnabled) + if ($span.backend.meta.isEnabled && $attributes.nonEmpty) $span.backend.addAttributes($attributes) else $span.backend.meta.unit } diff --git a/core/trace/src/test/scala/org/typelevel/otel4s/trace/SpanBuilderSuite.scala b/core/trace/src/test/scala/org/typelevel/otel4s/trace/SpanBuilderSuite.scala index 86d950b85..09293a870 100644 --- a/core/trace/src/test/scala/org/typelevel/otel4s/trace/SpanBuilderSuite.scala +++ b/core/trace/src/test/scala/org/typelevel/otel4s/trace/SpanBuilderSuite.scala @@ -277,14 +277,22 @@ class SpanBuilderSuite extends FunSuite { assertEquals(result.state, expected) } + test("addAttributes: eliminate empty varargs calls") { + val builder = InMemoryBuilder(InstrumentMeta.enabled, SpanBuilder.State.init) + val result = builder.addAttributes().asInstanceOf[InMemoryBuilder] + + assertEquals(result.modifications, 0) + } + case class InMemoryBuilder( meta: InstrumentMeta[IO], - state: SpanBuilder.State + state: SpanBuilder.State, + modifications: Int = 0 ) extends SpanBuilder[IO] { def modifyState( f: SpanBuilder.State => SpanBuilder.State ): SpanBuilder[IO] = - copy(state = f(state)) + copy(state = f(state), modifications = modifications + 1) def build: SpanOps[IO] = ??? diff --git a/core/trace/src/test/scala/org/typelevel/otel4s/trace/SpanSuite.scala b/core/trace/src/test/scala/org/typelevel/otel4s/trace/SpanSuite.scala new file mode 100644 index 000000000..4b283010c --- /dev/null +++ b/core/trace/src/test/scala/org/typelevel/otel4s/trace/SpanSuite.scala @@ -0,0 +1,104 @@ +/* + * Copyright 2022 Typelevel + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.typelevel.otel4s.trace + +import cats.effect.IO +import cats.effect.Ref +import munit._ +import org.typelevel.otel4s.Attribute +import org.typelevel.otel4s.meta.InstrumentMeta + +import scala.collection.immutable +import scala.concurrent.duration.FiniteDuration + +class SpanSuite extends CatsEffectSuite { + import SpanSuite._ + + test("addAttributes: eliminate empty varargs calls") { + for { + ops <- IO.ref(Vector.empty[BackendOp]) + span = Span.fromBackend(new OpsBackend(ops)) + _ <- span.addAttributes() + result <- ops.get + } yield assertEquals(result, Vector.empty) + } + +} + +object SpanSuite { + + private sealed trait BackendOp + private object BackendOp { + final case class UpdateName(name: String) extends BackendOp + final case class AddAttributes(attributes: immutable.Iterable[Attribute[_]]) extends BackendOp + final case class AddEvent( + name: String, + timestamp: Option[FiniteDuration], + attributes: immutable.Iterable[Attribute[_]] + ) extends BackendOp + final case class AddLink( + spanContext: SpanContext, + attributes: immutable.Iterable[Attribute[_]] + ) extends BackendOp + final case class RecordException( + exception: Throwable, + attributes: immutable.Iterable[Attribute[_]] + ) extends BackendOp + final case class SetStatus(status: StatusCode, description: Option[String]) extends BackendOp + final case class End(timestamp: Option[FiniteDuration]) extends BackendOp + } + + private class OpsBackend(state: Ref[IO, Vector[BackendOp]]) extends Span.Backend[IO] { + import BackendOp._ + + val meta: InstrumentMeta[IO] = InstrumentMeta.enabled + + def context: SpanContext = ??? + + def updateName(name: String): IO[Unit] = + state.update(_ :+ UpdateName(name)) + + def addAttributes(attributes: immutable.Iterable[Attribute[_]]): IO[Unit] = + state.update(_ :+ AddAttributes(attributes)) + + def addEvent(name: String, attributes: immutable.Iterable[Attribute[_]]): IO[Unit] = + state.update(_ :+ AddEvent(name, None, attributes)) + + def addEvent(name: String, timestamp: FiniteDuration, attributes: immutable.Iterable[Attribute[_]]): IO[Unit] = + state.update(_ :+ AddEvent(name, Some(timestamp), attributes)) + + def addLink(spanContext: SpanContext, attributes: immutable.Iterable[Attribute[_]]): IO[Unit] = + state.update(_ :+ AddLink(spanContext, attributes)) + + def recordException(exception: Throwable, attributes: immutable.Iterable[Attribute[_]]): IO[Unit] = + state.update(_ :+ RecordException(exception, attributes)) + + def setStatus(status: StatusCode): IO[Unit] = + state.update(_ :+ SetStatus(status, None)) + + def setStatus(status: StatusCode, description: String): IO[Unit] = + state.update(_ :+ SetStatus(status, Some(description))) + + def end: IO[Unit] = + state.update(_ :+ End(None)) + + def end(timestamp: FiniteDuration): IO[Unit] = + state.update(_ :+ End(Some(timestamp))) + + } + +} diff --git a/core/trace/src/test/scala/org/typelevel/otel4s/trace/TracerSuite.scala b/core/trace/src/test/scala/org/typelevel/otel4s/trace/TracerSuite.scala index a18198bd3..7e8a14632 100644 --- a/core/trace/src/test/scala/org/typelevel/otel4s/trace/TracerSuite.scala +++ b/core/trace/src/test/scala/org/typelevel/otel4s/trace/TracerSuite.scala @@ -17,8 +17,12 @@ package org.typelevel.otel4s package trace +import cats.Applicative import cats.effect.IO import munit.CatsEffectSuite +import org.typelevel.otel4s.context.propagation.TextMapGetter +import org.typelevel.otel4s.context.propagation.TextMapUpdater +import org.typelevel.otel4s.meta.InstrumentMeta import scala.concurrent.duration._ @@ -89,9 +93,99 @@ class TracerSuite extends CatsEffectSuite { } yield assert(!allocated) } + test("eliminate 'addAttributes' when varargs are empty") { + val tracer = new ProxyTracer(Tracer.noop[IO]) + val attribute = Attribute("key", "value") + + val expected = Vector( + Vector( + BuilderOp.Init("span"), + BuilderOp.Build + ), + Vector( + BuilderOp.Init("span-varargs"), + BuilderOp.ModifyState(SpanBuilder.State.init.addAttribute(Attribute("key", "value"))), + BuilderOp.Build + ), + Vector( + BuilderOp.Init("root-span"), + BuilderOp.Build + ), + Vector( + BuilderOp.Init("root-span-varargs"), + BuilderOp.ModifyState(SpanBuilder.State.init.addAttribute(Attribute("key", "value"))), + BuilderOp.Build + ) + ) + + for { + _ <- tracer.span("span").use_ + _ <- tracer.span("span-varargs", attribute).use_ + _ <- tracer.span("root-span").use_ + _ <- tracer.span("root-span-varargs", attribute).use_ + } yield assertEquals(tracer.builders.map(_.ops), expected) + } + test("`currentSpanOrNoop` is not valid when instrument is noop") { val tracer = Tracer.noop[IO] for (span <- tracer.currentSpanOrNoop) yield assert(!span.context.isValid) } + + // utility + + private sealed trait BuilderOp + private object BuilderOp { + case class Init(name: String) extends BuilderOp + case class ModifyState(state: SpanBuilder.State) extends BuilderOp + case object Build extends BuilderOp + } + + private final class ProxyBuilder[F[_]: Applicative]( + name: String, + var underlying: SpanBuilder[F] + ) extends SpanBuilder[F] { + private var state: SpanBuilder.State = SpanBuilder.State.init + private val builderOps = Vector.newBuilder[BuilderOp] + builderOps.addOne(BuilderOp.Init(name)) + + def ops: Vector[BuilderOp] = builderOps.result() + + def meta: InstrumentMeta[F] = InstrumentMeta.enabled[F] + + def modifyState(f: SpanBuilder.State => SpanBuilder.State): SpanBuilder[F] = { + state = f(state) + underlying = underlying.modifyState(f) + builderOps.addOne(BuilderOp.ModifyState(state)) + this + } + + def build: SpanOps[F] = { + builderOps.addOne(BuilderOp.Build) + underlying.build + } + } + + private class ProxyTracer[F[_]: Applicative](underlying: Tracer[F]) extends Tracer[F] { + private val proxyBuilders = Vector.newBuilder[ProxyBuilder[F]] + + def meta: InstrumentMeta[F] = InstrumentMeta.enabled[F] + def currentSpanContext: F[Option[SpanContext]] = underlying.currentSpanContext + def currentSpanOrNoop: F[Span[F]] = underlying.currentSpanOrNoop + def currentSpanOrThrow: F[Span[F]] = underlying.currentSpanOrThrow + def childScope[A](parent: SpanContext)(fa: F[A]): F[A] = underlying.childScope(parent)(fa) + def joinOrRoot[A, C: TextMapGetter](carrier: C)(fa: F[A]): F[A] = underlying.joinOrRoot(carrier)(fa) + def rootScope[A](fa: F[A]): F[A] = underlying.rootScope(fa) + def noopScope[A](fa: F[A]): F[A] = underlying.noopScope(fa) + def propagate[C: TextMapUpdater](carrier: C): F[C] = underlying.propagate(carrier) + + def spanBuilder(name: String): SpanBuilder[F] = { + val builder = new ProxyBuilder[F](name, underlying.spanBuilder(name)) + proxyBuilders.addOne(builder) + builder + } + + def builders: Vector[ProxyBuilder[F]] = + proxyBuilders.result() + } }