From b1045af164612580993a2eeec870727b67b0983b Mon Sep 17 00:00:00 2001 From: Maksym Ochenashko Date: Sat, 4 May 2024 15:46:17 +0300 Subject: [PATCH] sdk-metrics: improve console messages - add class name, show all duplicates --- .../sdk/metrics/internal/MetricStorageRegistry.scala | 9 +++++---- .../metrics/internal/storage/AsynchronousStorage.scala | 3 ++- .../sdk/metrics/internal/storage/MetricStorage.scala | 3 ++- .../metrics/internal/storage/SynchronousStorage.scala | 1 + .../metrics/internal/MetricStorageRegistrySuite.scala | 10 +++++----- 5 files changed, 15 insertions(+), 11 deletions(-) diff --git a/sdk/metrics/src/main/scala/org/typelevel/otel4s/sdk/metrics/internal/MetricStorageRegistry.scala b/sdk/metrics/src/main/scala/org/typelevel/otel4s/sdk/metrics/internal/MetricStorageRegistry.scala index 3dd080ee7..ab1da6da7 100644 --- a/sdk/metrics/src/main/scala/org/typelevel/otel4s/sdk/metrics/internal/MetricStorageRegistry.scala +++ b/sdk/metrics/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/sdk/metrics/src/main/scala/org/typelevel/otel4s/sdk/metrics/internal/storage/AsynchronousStorage.scala b/sdk/metrics/src/main/scala/org/typelevel/otel4s/sdk/metrics/internal/storage/AsynchronousStorage.scala index 12d3b473e..ea165c14f 100644 --- a/sdk/metrics/src/main/scala/org/typelevel/otel4s/sdk/metrics/internal/storage/AsynchronousStorage.scala +++ b/sdk/metrics/src/main/scala/org/typelevel/otel4s/sdk/metrics/internal/storage/AsynchronousStorage.scala @@ -71,7 +71,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) @@ -117,6 +117,7 @@ private final class AsynchronousStorage[ private def cardinalityWarning: F[Unit] = MetricStorage.cardinalityWarning( + "AsynchronousStorage", metricDescriptor.sourceInstrument, maxCardinality ) diff --git a/sdk/metrics/src/main/scala/org/typelevel/otel4s/sdk/metrics/internal/storage/MetricStorage.scala b/sdk/metrics/src/main/scala/org/typelevel/otel4s/sdk/metrics/internal/storage/MetricStorage.scala index dbe144da5..5d2aba3e8 100644 --- a/sdk/metrics/src/main/scala/org/typelevel/otel4s/sdk/metrics/internal/storage/MetricStorage.scala +++ b/sdk/metrics/src/main/scala/org/typelevel/otel4s/sdk/metrics/internal/storage/MetricStorage.scala @@ -79,11 +79,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/sdk/metrics/src/main/scala/org/typelevel/otel4s/sdk/metrics/internal/storage/SynchronousStorage.scala b/sdk/metrics/src/main/scala/org/typelevel/otel4s/sdk/metrics/internal/storage/SynchronousStorage.scala index 605b6570e..c9e7e51ca 100644 --- a/sdk/metrics/src/main/scala/org/typelevel/otel4s/sdk/metrics/internal/storage/SynchronousStorage.scala +++ b/sdk/metrics/src/main/scala/org/typelevel/otel4s/sdk/metrics/internal/storage/SynchronousStorage.scala @@ -162,6 +162,7 @@ private final class SynchronousStorage[ private def cardinalityWarning: F[Unit] = MetricStorage.cardinalityWarning( + "SynchronousStorage", metricDescriptor.sourceInstrument, maxCardinality ) diff --git a/sdk/metrics/src/test/scala/org/typelevel/otel4s/sdk/metrics/internal/MetricStorageRegistrySuite.scala b/sdk/metrics/src/test/scala/org/typelevel/otel4s/sdk/metrics/internal/MetricStorageRegistrySuite.scala index e897c4e9a..a50af39c7 100644 --- a/sdk/metrics/src/test/scala/org/typelevel/otel4s/sdk/metrics/internal/MetricStorageRegistrySuite.scala +++ b/sdk/metrics/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))