diff --git a/sdk/common/shared/src/main/scala/org/typelevel/otel4s/sdk/internal/ComponentRegistry.scala b/sdk/common/shared/src/main/scala/org/typelevel/otel4s/sdk/internal/ComponentRegistry.scala index ca22f10ba..e57b29372 100644 --- a/sdk/common/shared/src/main/scala/org/typelevel/otel4s/sdk/internal/ComponentRegistry.scala +++ b/sdk/common/shared/src/main/scala/org/typelevel/otel4s/sdk/internal/ComponentRegistry.scala @@ -24,7 +24,7 @@ import cats.effect.std.AtomicCell import cats.syntax.functor._ import org.typelevel.otel4s.sdk.common.InstrumentationScope -/** A registry that caches components by `key`, `version`, and `schemaUrl`. +/** A registry that caches components by `key`, `version`, `schemaUrl`, and `attributes`. * * @tparam F * the higher-kinded type of a polymorphic effect @@ -34,12 +34,7 @@ import org.typelevel.otel4s.sdk.common.InstrumentationScope */ private[sdk] sealed trait ComponentRegistry[F[_], A] { - /** Returns the component associated with the `name`, `version`, and `schemaUrl`. - * - * '''Note''': `attributes` are not part of component identity. - * - * Behavior is undefined when different `attributes` are provided where `name`, `version`, and `schemaUrl` are - * identical. + /** Returns the component associated with the `name`, `version`, `schemaUrl`, and `attributes`. * * @param name * the name to associate with a component @@ -53,12 +48,7 @@ private[sdk] sealed trait ComponentRegistry[F[_], A] { * @param attributes * the attributes to associate with a component */ - def get( - name: String, - version: Option[String], - schemaUrl: Option[String], - attributes: Attributes - ): F[A] + def get(name: String, version: Option[String], schemaUrl: Option[String], attributes: Attributes): F[A] /** Returns the collection of the registered components. */ @@ -80,44 +70,28 @@ private[sdk] object ComponentRegistry { * @tparam A * the type of the component */ - def create[F[_]: Concurrent, A]( - buildComponent: InstrumentationScope => F[A] - ): F[ComponentRegistry[F, A]] = + def create[F[_]: Concurrent, A](buildComponent: InstrumentationScope => F[A]): F[ComponentRegistry[F, A]] = for { - cache <- AtomicCell[F].of(Map.empty[Key, A]) + cache <- AtomicCell[F].of(Map.empty[InstrumentationScope, A]) } yield new Impl(cache, buildComponent) - private final case class Key( - name: String, - version: Option[String], - schemaUrl: Option[String] - ) - private final class Impl[F[_]: Applicative, A]( - cache: AtomicCell[F, Map[Key, A]], + cache: AtomicCell[F, Map[InstrumentationScope, A]], buildComponent: InstrumentationScope => F[A] ) extends ComponentRegistry[F, A] { - def get( - name: String, - version: Option[String], - schemaUrl: Option[String], - attributes: Attributes - ): F[A] = + def get(name: String, version: Option[String], schemaUrl: Option[String], attributes: Attributes): F[A] = cache.evalModify { cache => - val key = Key(name, version, schemaUrl) + val scope = InstrumentationScope(name, version, schemaUrl, attributes) - cache.get(key) match { + cache.get(scope) match { case Some(component) => Applicative[F].pure((cache, component)) case None => - val scope = - InstrumentationScope(name, version, schemaUrl, attributes) - for { component <- buildComponent(scope) - } yield (cache.updated(key, component), component) + } yield (cache.updated(scope, component), component) } } diff --git a/sdk/common/shared/src/test/scala/org/typelevel/otel4s/sdk/internal/ComponentRegistrySuite.scala b/sdk/common/shared/src/test/scala/org/typelevel/otel4s/sdk/internal/ComponentRegistrySuite.scala index d85efd12c..2a950ac8f 100644 --- a/sdk/common/shared/src/test/scala/org/typelevel/otel4s/sdk/internal/ComponentRegistrySuite.scala +++ b/sdk/common/shared/src/test/scala/org/typelevel/otel4s/sdk/internal/ComponentRegistrySuite.scala @@ -30,12 +30,14 @@ class ComponentRegistrySuite extends CatsEffectSuite { registryTest("get cached values (by name only)") { registry => for { + v0 <- registry.get(name, None, None, Attributes.empty) v1 <- registry.get(name, None, None, Attributes.empty) v2 <- registry.get(name, None, None, attributes) v3 <- registry.get(name, Some(version), None, attributes) v4 <- registry.get(name, Some(version), Some(schemaUrl), attributes) } yield { - assertEquals(v1, v2) + assertEquals(v0, v1) + assertNotEquals(v1, v2) assertNotEquals(v1, v3) assertNotEquals(v2, v3) assertNotEquals(v1, v4) @@ -45,11 +47,13 @@ class ComponentRegistrySuite extends CatsEffectSuite { registryTest("get cached values (by name and version)") { registry => for { + v0 <- registry.get(name, Some(version), None, Attributes.empty) v1 <- registry.get(name, Some(version), None, Attributes.empty) v2 <- registry.get(name, Some(version), None, attributes) v3 <- registry.get(name, Some(version), Some(schemaUrl), attributes) } yield { - assertEquals(v1, v2) + assertEquals(v0, v1) + assertNotEquals(v1, v2) assertNotEquals(v1, v3) assertNotEquals(v2, v3) } @@ -57,14 +61,23 @@ class ComponentRegistrySuite extends CatsEffectSuite { registryTest("get cached values (by name, version, and schema)") { registry => for { + v0 <- registry.get(name, Some(version), Some(schemaUrl), Attributes.empty) v1 <- registry.get(name, Some(version), Some(schemaUrl), Attributes.empty) v2 <- registry.get(name, Some(version), Some(schemaUrl), attributes) + } yield { + assertEquals(v0, v1) + assertNotEquals(v1, v2) + } + } + + registryTest("get cached values (by name, version, schema, and attributes)") { registry => + for { + v1 <- registry.get(name, Some(version), Some(schemaUrl), attributes) + v2 <- registry.get(name, Some(version), Some(schemaUrl), attributes) } yield assertEquals(v1, v2) } - private def registryTest( - name: String - )(body: ComponentRegistry[IO, TestComponent] => IO[Unit]): Unit = + private def registryTest(name: String)(body: ComponentRegistry[IO, TestComponent] => IO[Unit]): Unit = test(name) { for { registry <- ComponentRegistry.create(_ => IO.pure(new TestComponent()))