Merge pull request #668 from iRevive/sdk-metrics/improve-messages

typelevel · May 4, 2024 · 9a61034 · 9a61034
2 parents 70542e4 + b1045af
commit 9a61034
Show file tree

Hide file tree

Showing 5 changed files with 15 additions and 11 deletions.
diff --git a/...rics/src/main/scala/org/typelevel/otel4s/sdk/metrics/internal/MetricStorageRegistry.scala b/...rics/src/main/scala/org/typelevel/otel4s/sdk/metrics/internal/MetricStorageRegistry.scala
@@ -59,16 +59,17 @@ private[metrics] final class MetricStorageRegistry[
           val updated = map.updated(descriptor, storage)
 
           // run a duplicate check
-          map.find(_._1.name == descriptor.name) match {
-            case Some((duplicate, _)) =>
+          map.keySet.filter(_.name == descriptor.name) match {
+            case duplicates if duplicates.nonEmpty =>
               def warn: F[Unit] =
                 Console[F].errorln(
-                  s"MetricStorageRegistry: found a duplicate $duplicate, source $descriptor"
+                  "MetricStorageRegistry: found a duplicate. " +
+                    s"The $descriptor has similar descriptors in the storage: ${duplicates.mkString(", ")}."
                 )
 
               (updated, warn.as(storage))
 
-            case None =>
+            case _ =>
               (updated, MonadCancelThrow[F].pure(storage))
           }
       }

diff --git a/...rc/main/scala/org/typelevel/otel4s/sdk/metrics/internal/storage/AsynchronousStorage.scala b/...rc/main/scala/org/typelevel/otel4s/sdk/metrics/internal/storage/AsynchronousStorage.scala
@@ -70,7 +70,7 @@ private final class AsynchronousStorage[
 
         if (points.contains(processed)) {
           Console[F].errorln(
-            s"Instrument [${metricDescriptor.sourceInstrument.name}] has recorded multiple values for the same attributes $processed"
+            s"AsynchronousStorage: instrument [${metricDescriptor.sourceInstrument.name}] has recorded multiple values for the same attributes $processed"
           )
         } else {
           val timeWindow = TimeWindow(start, measurement.timeWindow.end)
@@ -116,6 +116,7 @@ private final class AsynchronousStorage[
 
   private def cardinalityWarning: F[Unit] =
     MetricStorage.cardinalityWarning(
+      "AsynchronousStorage",
       metricDescriptor.sourceInstrument,
       maxCardinality
     )

diff --git a/...rics/src/main/scala/org/typelevel/otel4s/sdk/metrics/internal/storage/MetricStorage.scala b/...rics/src/main/scala/org/typelevel/otel4s/sdk/metrics/internal/storage/MetricStorage.scala
@@ -77,11 +77,12 @@ private[metrics] object MetricStorage {
     Attribute("otel.metric.overflow", true)
 
   private[storage] def cardinalityWarning[F[_]: Console](
+      storageName: String,
       instrument: InstrumentDescriptor,
       maxCardinality: Int
   ): F[Unit] =
     Console[F].errorln(
-      s"Instrument [${instrument.name}] has exceeded the maximum allowed cardinality [$maxCardinality]"
+      s"$storageName: instrument [${instrument.name}] has exceeded the maximum allowed cardinality [$maxCardinality]"
     )
 
   /** A storage for the metrics from the synchronous instruments.

diff --git a/...src/main/scala/org/typelevel/otel4s/sdk/metrics/internal/storage/SynchronousStorage.scala b/...src/main/scala/org/typelevel/otel4s/sdk/metrics/internal/storage/SynchronousStorage.scala
@@ -160,6 +160,7 @@ private final class SynchronousStorage[
 
   private def cardinalityWarning: F[Unit] =
     MetricStorage.cardinalityWarning(
+      "SynchronousStorage",
       metricDescriptor.sourceInstrument,
       maxCardinality
     )

diff --git a/...src/test/scala/org/typelevel/otel4s/sdk/metrics/internal/MetricStorageRegistrySuite.scala b/...src/test/scala/org/typelevel/otel4s/sdk/metrics/internal/MetricStorageRegistrySuite.scala
@@ -49,8 +49,8 @@ class MetricStorageRegistrySuite
   test("warn about duplicates") {
     PropF.forAllF(Gens.instrumentDescriptor) { descriptor =>
       InMemoryConsole.create[IO].flatMap { implicit C: InMemoryConsole[IO] =>
-        val sourceDescriptor = MetricDescriptor(None, descriptor)
-        val duplicateDescriptor = MetricDescriptor(
+        val first = MetricDescriptor(None, descriptor)
+        val second = MetricDescriptor(
           Some(View.builder.withDescription("desc").build),
           descriptor
         )
@@ -61,15 +61,15 @@ class MetricStorageRegistrySuite
           List(
             Entry(
               Op.Errorln,
-              s"MetricStorageRegistry: found a duplicate $sourceDescriptor, source $duplicateDescriptor"
+              s"MetricStorageRegistry: found a duplicate. The $second has similar descriptors in the storage: $first."
             )
           )
         }
 
         for {
           registry <- MetricStorageRegistry.create[IO]
-          source <- registry.register(metricStorage(sourceDescriptor))
-          duplicate <- registry.register(metricStorage(duplicateDescriptor))
+          source <- registry.register(metricStorage(first))
+          duplicate <- registry.register(metricStorage(second))
           storages <- registry.storages
           _ <- C.entries.assertEquals(consoleEntries)
         } yield assertEquals(storages, Vector(source, duplicate))