Conversation
| @@ -3,14 +3,14 @@ package kyo.stats | |||
| import kyo.stats.Attributes.AsAttribute | |||
| import scala.annotation.implicitNotFound | |||
|
|
|||
| case class Attributes(get: List[Attributes.Attribute]) extends AnyVal { | |||
There was a problem hiding this comment.
Used only for traces now, so I moved it to from kyo-scheduler to kyo-core, which uses Scala 3 syntax.
|
|
||
| def stop(): Unit = { | ||
| gauge.close() | ||
| def stop(): Unit = |
There was a problem hiding this comment.
The gauge will now be automatically collected when there are no strong references to it anymore.
| protected def measure(v: Long): Unit = { | ||
| probesCompleted.increment() | ||
| stats.measurement.observe(v.toDouble) | ||
| stats.measurement.observe(Math.max(0, v.toDouble)) |
There was a problem hiding this comment.
Workaround because HdrHistogram doesn't support negative values and there's a test that reproduces negative measurements.
| @@ -6,15 +6,15 @@ import kyo.stats.Attributes | |||
| import kyo.stats.Attributes.Attribute | |||
There was a problem hiding this comment.
kyo-stats-otel wan't getting cross-compiled to Scala 2
| import java.util.concurrent.atomic.LongAdder | ||
|
|
||
| class UnsafeCounter { | ||
| private var last = 0L |
There was a problem hiding this comment.
Do we need to worry about rollover for long running applications? I realize it's mostly unavoidable.
There was a problem hiding this comment.
Good point! I'll create an issue for it.
Problem
Kyo's
Statswas designed to be a lightweight wrapper for OpenTelemetry but the approach has some important downsides:closeon metrics to release them when not used anymore. It's not difficult to introduce a memory leak with the current approach.Solution
This PR introduces a new approach where
StatsRegistryholds weak references to all metrics and garbage collects empty references periodically. The integration with OTel is done via a periodic synchronization with a service-loadedStatsExporter.This change introduces a performance penalty: metrics will always be collected even if no exporters are present. I think it's a reasonable tradeoff and the overhead of metrics is typically not very significant.