Skip to content

Commit

Permalink
Merge pull request #795 from iRevive/core-trace/macro-empty-varargs
Browse files Browse the repository at this point in the history
  • Loading branch information
iRevive authored Oct 28, 2024
2 parents 33ec7c0 + c4d3ec0 commit 076fb43
Show file tree
Hide file tree
Showing 8 changed files with 231 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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)(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)(
Expand All @@ -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)(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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] =
???
Expand Down
104 changes: 104 additions & 0 deletions core/trace/src/test/scala/org/typelevel/otel4s/trace/SpanSuite.scala
Original file line number Diff line number Diff line change
@@ -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)))

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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._

Expand Down Expand Up @@ -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()
}
}

0 comments on commit 076fb43

Please sign in to comment.