From 6c1a1bbe4fc0775169f7a9d94f5f6055bee04f21 Mon Sep 17 00:00:00 2001 From: Stas Shevchenko Date: Thu, 3 Sep 2026 10:00:34 +0200 Subject: [PATCH] Report cancelled loads in load metrics --- build.sbt | 2 + .../com/evolution/scache/CacheMetered.scala | 12 +-- .../com/evolution/scache/CacheMetrics.scala | 73 ++++++++++++------- .../evolution/scache/CacheMeteredSpec.scala | 2 +- .../com/evolution/scache/CacheSpec.scala | 72 +++++++++--------- 5 files changed, 93 insertions(+), 68 deletions(-) diff --git a/build.sbt b/build.sbt index 2cd3197a..273a7c32 100644 --- a/build.sbt +++ b/build.sbt @@ -114,6 +114,8 @@ lazy val scache = (project in file("scache")) ProblemFilters.exclude[MissingClassProblem]("com.evolution.scache.LoadingCache$EntryRefs$"), ProblemFilters.exclude[MissingClassProblem]("com.evolution.scache.ExpiringCache$MapOps"), ProblemFilters.exclude[MissingClassProblem]("com.evolution.scache.ExpiringCache$MapOps$"), + // CacheMetrics.load takes LoadResult, reviewed in https://github.com/evolution-gaming/scache/issues/386 + ProblemFilters.exclude[ReversedMissingMethodProblem]("com.evolution.scache.CacheMetrics.load"), ), libraryDependencies ++= Seq( Cats.core, diff --git a/scache/src/main/scala/com/evolution/scache/CacheMetered.scala b/scache/src/main/scala/com/evolution/scache/CacheMetered.scala index 7e61d864..6f5d944c 100644 --- a/scache/src/main/scala/com/evolution/scache/CacheMetered.scala +++ b/scache/src/main/scala/com/evolution/scache/CacheMetered.scala @@ -1,9 +1,11 @@ package com.evolution.scache +import cats.effect.implicits.* import cats.effect.{Resource, Temporal} import cats.kernel.CommutativeMonoid import cats.syntax.all.* import com.evolution.scache.Cache.Directive +import com.evolution.scache.CacheMetrics.LoadResult import com.evolutiongaming.catshelper.{MeasureDuration, Schedule} import scala.concurrent.duration.* @@ -67,13 +69,13 @@ object CacheMetered { for { _ <- metrics.get(false) start <- MeasureDuration[F].start - value <- value.attempt + value <- value.attempt.onCancel { start.flatMap { metrics.load(_, LoadResult.Cancelled) } } duration <- start - loadSucceed = value match { - case Right(_) | Left(CacheOpsCompat.NoneError) => true - case Left(_) => false + result = value match { + case Right(_) | Left(CacheOpsCompat.NoneError) => LoadResult.Success + case Left(_) => LoadResult.Failure } - _ <- metrics.load(duration, loadSucceed) + _ <- metrics.load(duration, result) value <- value.liftTo[F] } yield { val (a, v, release) = value diff --git a/scache/src/main/scala/com/evolution/scache/CacheMetrics.scala b/scache/src/main/scala/com/evolution/scache/CacheMetrics.scala index 8b8a1f01..bbff2166 100644 --- a/scache/src/main/scala/com/evolution/scache/CacheMetrics.scala +++ b/scache/src/main/scala/com/evolution/scache/CacheMetrics.scala @@ -3,7 +3,7 @@ package com.evolution.scache import cats.effect.{Concurrent, Ref, Resource} import cats.syntax.all.* import cats.{Applicative, Monad} -import com.evolution.scache.CacheMetrics.Directive +import com.evolution.scache.CacheMetrics.{Directive, LoadResult} import com.evolutiongaming.smetrics.MetricsHelper.* import com.evolutiongaming.smetrics.{CollectorRegistry, LabelNames, Quantile, Quantiles} @@ -14,7 +14,11 @@ trait CacheMetrics[F[_]] { def get(hit: Boolean): F[Unit] - def load(time: FiniteDuration, success: Boolean): F[Unit] + def load(time: FiniteDuration, result: LoadResult): F[Unit] + + @deprecated("use load(time, LoadResult)", "7.0.0") + def load(time: FiniteDuration, success: Boolean): F[Unit] = + load(time, if (success) LoadResult.Success else LoadResult.Failure) def life(time: FiniteDuration): F[Unit] @@ -48,7 +52,7 @@ object CacheMetrics { def get(hit: Boolean) = unit - def load(time: FiniteDuration, success: Boolean) = unit + def load(time: FiniteDuration, result: LoadResult) = unit def life(time: FiniteDuration) = unit @@ -82,6 +86,25 @@ object CacheMetrics { case object Remove extends Directive } + /** + * Outcome of a value computation started by `getOrUpdate`: it produced a value, failed, or was + * cancelled before doing either. + */ + sealed trait LoadResult { + override def toString: Prefix = this match { + case LoadResult.Success => "success" + case LoadResult.Failure => "failure" + case LoadResult.Cancelled => "cancelled" + } + } + object LoadResult { + case object Success extends LoadResult + case object Failure extends LoadResult + case object Cancelled extends LoadResult + + val values: List[LoadResult] = List(Success, Failure, Cancelled) + } + type Name = String type Prefix = String @@ -121,7 +144,7 @@ object CacheMetrics { val loadResultCounter = collectorRegistry.counter( name = s"${ prefix }_load_result", - help = "Load result: success or failure", + help = "Load result: success, failure or cancelled", labels = LabelNames("name", "result"), ) @@ -176,13 +199,13 @@ object CacheMetrics { val missCounter = getsCounter.labels(name, "miss") - val successCounter = loadResultCounter.labels(name, "success") - - val failureCounter = loadResultCounter.labels(name, "failure") + val loadCounters = LoadResult.values.map { result => + (result, loadResultCounter.labels(name, result.toString)) + }.toMap - val successSummary = loadTimeSummary.labels(name, "success") - - val failureSummary = loadTimeSummary.labels(name, "failure") + val loadSummaries = LoadResult.values.map { result => + (result, loadTimeSummary.labels(name, result.toString)) + }.toMap val putCounter1 = putCounter.labels(name) @@ -205,12 +228,10 @@ object CacheMetrics { counter.inc() } - def load(time: FiniteDuration, success: Boolean) = { - val resultCounter = if (success) successCounter else failureCounter - val timeSummary = if (success) successSummary else failureSummary + def load(time: FiniteDuration, result: LoadResult) = { for { - _ <- resultCounter.inc() - _ <- timeSummary.observe(time.toNanos.nanosToSeconds) + _ <- loadCounters(result).inc() + _ <- loadSummaries(result).observe(time.toNanos.nanosToSeconds) } yield {} } @@ -283,7 +304,7 @@ object CacheMetrics { val loadResultCounter = collectorRegistry.counter( name = s"${ prefix }_load_result", - help = "Load result: success or failure", + help = "Load result: success, failure or cancelled", labels = LabelNames("name", "result"), ) @@ -336,13 +357,13 @@ object CacheMetrics { val missCounter = getsCounter.labels(name, "miss") - val successCounter = loadResultCounter.labels(name, "success") - - val failureCounter = loadResultCounter.labels(name, "failure") - - val successSummary = loadTimeSummary.labels(name, "success") + val loadCounters = LoadResult.values.map { result => + (result, loadResultCounter.labels(name, result.toString)) + }.toMap - val failureSummary = loadTimeSummary.labels(name, "failure") + val loadSummaries = LoadResult.values.map { result => + (result, loadTimeSummary.labels(name, result.toString)) + }.toMap val putCounter1 = putCounter.labels(name) @@ -365,12 +386,10 @@ object CacheMetrics { counter.inc() } - def load(time: FiniteDuration, success: Boolean) = { - val resultCounter = if (success) successCounter else failureCounter - val timeSummary = if (success) successSummary else failureSummary + def load(time: FiniteDuration, result: LoadResult) = { for { - _ <- resultCounter.inc() - _ <- timeSummary.observe(time.toNanos.nanosToSeconds) + _ <- loadCounters(result).inc() + _ <- loadSummaries(result).observe(time.toNanos.nanosToSeconds) } yield {} } diff --git a/scache/src/test/scala/com/evolution/scache/CacheMeteredSpec.scala b/scache/src/test/scala/com/evolution/scache/CacheMeteredSpec.scala index a90861d9..7f50acc3 100644 --- a/scache/src/test/scala/com/evolution/scache/CacheMeteredSpec.scala +++ b/scache/src/test/scala/com/evolution/scache/CacheMeteredSpec.scala @@ -11,7 +11,7 @@ class CacheMeteredSpec extends AsyncFunSuite with Matchers { private def sizeRecorder(sizes: Ref[IO, List[Int]]): CacheMetrics[IO] = new CacheMetrics[IO] { def get(hit: Boolean): IO[Unit] = IO.unit - def load(time: FiniteDuration, success: Boolean): IO[Unit] = IO.unit + def load(time: FiniteDuration, result: CacheMetrics.LoadResult): IO[Unit] = IO.unit def life(time: FiniteDuration): IO[Unit] = IO.unit def put: IO[Unit] = IO.unit def modify(entryExisted: Boolean, directive: CacheMetrics.Directive): IO[Unit] = IO.unit diff --git a/scache/src/test/scala/com/evolution/scache/CacheSpec.scala b/scache/src/test/scala/com/evolution/scache/CacheSpec.scala index 66b6c1fd..1206780a 100644 --- a/scache/src/test/scala/com/evolution/scache/CacheSpec.scala +++ b/scache/src/test/scala/com/evolution/scache/CacheSpec.scala @@ -93,7 +93,7 @@ class CacheSpec extends AsyncFunSuite with Matchers { _ <- metrics.expect( metrics.expectedGet(hit = false) -> 2, metrics.expectedGet(hit = true) -> 3, - metrics.expectedLoad(success = true) -> 1, + metrics.expectedLoad(CacheMetrics.LoadResult.Success) -> 1, metrics.expectedPut -> 1, ) } yield {} @@ -390,7 +390,7 @@ class CacheSpec extends AsyncFunSuite with Matchers { metrics.expectedPut -> 2, metrics.expectedLife -> 4, metrics.expectedClear -> 1, - metrics.expectedLoad(success = true) -> 2, + metrics.expectedLoad(CacheMetrics.LoadResult.Success) -> 2, ) } yield {} } @@ -434,7 +434,7 @@ class CacheSpec extends AsyncFunSuite with Matchers { _ <- IO { value1 shouldEqual 0 } _ <- metrics.expect( metrics.expectedGet(hit = false) -> 1, - metrics.expectedLoad(success = true) -> 1, + metrics.expectedLoad(CacheMetrics.LoadResult.Success) -> 1, metrics.expectedGet(hit = true) -> 1, ) } yield {} @@ -448,7 +448,7 @@ class CacheSpec extends AsyncFunSuite with Matchers { _ <- metrics.expect( metrics.expectedSize(0) -> 1, metrics.expectedGet(hit = false) -> 1, - metrics.expectedLoad(success = true) -> 1, + metrics.expectedLoad(CacheMetrics.LoadResult.Success) -> 1, ) } yield {} result.run() @@ -465,7 +465,7 @@ class CacheSpec extends AsyncFunSuite with Matchers { _ <- metrics.expect( metrics.expectedGet(hit = false) -> 2, metrics.expectedGet(hit = true) -> 1, - metrics.expectedLoad(success = true) -> 2, + metrics.expectedLoad(CacheMetrics.LoadResult.Success) -> 2, ) } yield {} } @@ -500,7 +500,7 @@ class CacheSpec extends AsyncFunSuite with Matchers { metrics.expectedGet(hit = false) -> 2, metrics.expectedLife -> 2, metrics.expectedClear -> 1, - metrics.expectedLoad(success = true) -> 2, + metrics.expectedLoad(CacheMetrics.LoadResult.Success) -> 2, ) } yield {} } @@ -525,7 +525,7 @@ class CacheSpec extends AsyncFunSuite with Matchers { _ <- metrics.expect( metrics.expectedGet(hit = false) -> 1, metrics.expectedGet(hit = true) -> 1, - metrics.expectedLoad(success = true) -> 1, + metrics.expectedLoad(CacheMetrics.LoadResult.Success) -> 1, ) } yield {} } @@ -556,7 +556,7 @@ class CacheSpec extends AsyncFunSuite with Matchers { _ <- released.complete(()) _ <- metrics.expect( metrics.expectedGet(hit = false) -> 2, - metrics.expectedLoad(success = true) -> 2, + metrics.expectedLoad(CacheMetrics.LoadResult.Success) -> 2, metrics.expectedGet(hit = true) -> 1, ) } yield {} @@ -573,7 +573,7 @@ class CacheSpec extends AsyncFunSuite with Matchers { _ <- IO { value shouldEqual 1.asLeft.some } _ <- metrics.expect( metrics.expectedGet(hit = false) -> 2, - metrics.expectedLoad(success = true) -> 2, + metrics.expectedLoad(CacheMetrics.LoadResult.Success) -> 2, ) } yield {} } @@ -586,7 +586,7 @@ class CacheSpec extends AsyncFunSuite with Matchers { _ <- metrics.expect( metrics.expectedSize(0) -> 1, metrics.expectedGet(hit = false) -> 1, - metrics.expectedLoad(success = true) -> 1, + metrics.expectedLoad(CacheMetrics.LoadResult.Success) -> 1, ) } yield {} result.run() @@ -600,7 +600,7 @@ class CacheSpec extends AsyncFunSuite with Matchers { _ <- metrics.expect( metrics.expectedSize(0) -> 1, metrics.expectedGet(hit = false) -> 1, - metrics.expectedLoad(success = false) -> 1, + metrics.expectedLoad(CacheMetrics.LoadResult.Failure) -> 1, ) } yield {} result.run() @@ -614,7 +614,7 @@ class CacheSpec extends AsyncFunSuite with Matchers { _ <- metrics.expect( metrics.expectedSize(0) -> 1, metrics.expectedGet(hit = false) -> 1, - metrics.expectedLoad(success = true) -> 1, + metrics.expectedLoad(CacheMetrics.LoadResult.Success) -> 1, ) } yield {} result.run() @@ -628,7 +628,7 @@ class CacheSpec extends AsyncFunSuite with Matchers { _ <- metrics.expect( metrics.expectedSize(0) -> 1, metrics.expectedGet(hit = false) -> 1, - metrics.expectedLoad(success = false) -> 1, + metrics.expectedLoad(CacheMetrics.LoadResult.Failure) -> 1, ) } yield {} result.run() @@ -648,7 +648,8 @@ class CacheSpec extends AsyncFunSuite with Matchers { _ <- metrics.expect( metrics.expectedGet(hit = false) -> 2, metrics.expectedGet(hit = true) -> 1, - metrics.expectedLoad(success = true) -> 1, + metrics.expectedLoad(CacheMetrics.LoadResult.Cancelled) -> 1, + metrics.expectedLoad(CacheMetrics.LoadResult.Success) -> 1, ) } yield {} } @@ -667,7 +668,7 @@ class CacheSpec extends AsyncFunSuite with Matchers { _ <- IO { value shouldEqual 1.some } _ <- metrics.expect( metrics.expectedGet(hit = true) -> 2, - metrics.expectedLoad(success = true) -> 1, + metrics.expectedLoad(CacheMetrics.LoadResult.Success) -> 1, metrics.expectedGet(hit = false) -> 1, metrics.expectedPut -> 1, metrics.expectedLife -> 1, @@ -691,7 +692,7 @@ class CacheSpec extends AsyncFunSuite with Matchers { _ <- IO { value shouldEqual 1.some } _ <- metrics.expect( metrics.expectedGet(hit = true) -> 2, - metrics.expectedLoad(success = true) -> 1, + metrics.expectedLoad(CacheMetrics.LoadResult.Success) -> 1, metrics.expectedGet(hit = false) -> 1, metrics.expectedPut -> 1, metrics.expectedLife -> 1, @@ -750,7 +751,7 @@ class CacheSpec extends AsyncFunSuite with Matchers { _ <- metrics.expect( metrics.expectedGet(hit = false) -> 1, metrics.expectedPut -> 1, - metrics.expectedLoad(success = false) -> 1, + metrics.expectedLoad(CacheMetrics.LoadResult.Failure) -> 1, metrics.expectedGet(hit = true) -> 2, ) } yield {} @@ -771,7 +772,7 @@ class CacheSpec extends AsyncFunSuite with Matchers { _ <- metrics.expect( metrics.expectedGet(hit = false) -> 1, metrics.expectedPut -> 1, - metrics.expectedLoad(success = false) -> 1, + metrics.expectedLoad(CacheMetrics.LoadResult.Failure) -> 1, metrics.expectedGet(hit = true) -> 2, ) } yield {} @@ -789,7 +790,7 @@ class CacheSpec extends AsyncFunSuite with Matchers { _ <- IO { value shouldEqual 0.some } _ <- metrics.expect( metrics.expectedGet(hit = false) -> 1, - metrics.expectedLoad(success = true) -> 1, + metrics.expectedLoad(CacheMetrics.LoadResult.Success) -> 1, metrics.expectedGet(hit = true) -> 1, ) } yield {} @@ -809,7 +810,7 @@ class CacheSpec extends AsyncFunSuite with Matchers { _ <- released.complete(()) _ <- metrics.expect( metrics.expectedGet(hit = false) -> 1, - metrics.expectedLoad(success = true) -> 1, + metrics.expectedLoad(CacheMetrics.LoadResult.Success) -> 1, metrics.expectedGet(hit = true) -> 1, ) } yield {} @@ -827,7 +828,7 @@ class CacheSpec extends AsyncFunSuite with Matchers { _ <- IO { value shouldEqual none[Int] } _ <- metrics.expect( metrics.expectedGet(hit = false) -> 2, - metrics.expectedLoad(success = false) -> 1, + metrics.expectedLoad(CacheMetrics.LoadResult.Failure) -> 1, ) } yield {} } @@ -844,7 +845,7 @@ class CacheSpec extends AsyncFunSuite with Matchers { _ <- IO { value shouldEqual none[Int] } _ <- metrics.expect( metrics.expectedGet(hit = false) -> 2, - metrics.expectedLoad(success = false) -> 1, + metrics.expectedLoad(CacheMetrics.LoadResult.Failure) -> 1, ) } yield {} } @@ -865,7 +866,7 @@ class CacheSpec extends AsyncFunSuite with Matchers { _ <- metrics.expect( metrics.expectedGet(hit = false) -> 1, metrics.expectedGet(hit = true) -> 1, - metrics.expectedLoad(success = true) -> 1, + metrics.expectedLoad(CacheMetrics.LoadResult.Success) -> 1, ) } yield {} } @@ -892,7 +893,7 @@ class CacheSpec extends AsyncFunSuite with Matchers { _ <- metrics.expect( metrics.expectedGet(hit = false) -> 1, metrics.expectedGet(hit = true) -> 1, - metrics.expectedLoad(success = true) -> 1, + metrics.expectedLoad(CacheMetrics.LoadResult.Success) -> 1, ) } yield {} } @@ -911,7 +912,7 @@ class CacheSpec extends AsyncFunSuite with Matchers { _ <- IO { value shouldEqual none } _ <- metrics.expect( metrics.expectedGet(hit = false) -> 2, - metrics.expectedLoad(success = false) -> 1, + metrics.expectedLoad(CacheMetrics.LoadResult.Failure) -> 1, ) } yield {} } @@ -930,7 +931,7 @@ class CacheSpec extends AsyncFunSuite with Matchers { _ <- IO { value shouldEqual none } _ <- metrics.expect( metrics.expectedGet(hit = false) -> 2, - metrics.expectedLoad(success = false) -> 1, + metrics.expectedLoad(CacheMetrics.LoadResult.Failure) -> 1, ) } yield {} } @@ -954,7 +955,7 @@ class CacheSpec extends AsyncFunSuite with Matchers { metrics.expectedGet(hit = false) -> 1, metrics.expectedLife -> 1, metrics.expectedClear -> 1, - metrics.expectedLoad(success = true) -> 1, + metrics.expectedLoad(CacheMetrics.LoadResult.Success) -> 1, ) } yield {} } @@ -981,7 +982,7 @@ class CacheSpec extends AsyncFunSuite with Matchers { metrics.expectedGet(hit = false) -> 1, metrics.expectedLife -> 1, metrics.expectedClear -> 1, - metrics.expectedLoad(success = true) -> 1, + metrics.expectedLoad(CacheMetrics.LoadResult.Success) -> 1, ) } yield {} } @@ -1011,7 +1012,7 @@ class CacheSpec extends AsyncFunSuite with Matchers { metrics.expectedGet(hit = false) -> 1, metrics.expectedLife -> 1, metrics.expectedClear -> 1, - metrics.expectedLoad(success = true) -> 1, + metrics.expectedLoad(CacheMetrics.LoadResult.Success) -> 1, ) } yield {} } @@ -1088,7 +1089,7 @@ class CacheSpec extends AsyncFunSuite with Matchers { metrics.expectedGet(hit = false) -> 1, metrics.expectedPut -> 3, metrics.expectedClear -> 1, - metrics.expectedLoad(success = true) -> 1, + metrics.expectedLoad(CacheMetrics.LoadResult.Success) -> 1, metrics.expectedValues -> 5, metrics.expectedLife -> 4, ) @@ -1108,6 +1109,7 @@ class CacheSpec extends AsyncFunSuite with Matchers { _ <- fiber.joinWithNever _ <- metrics.expect( metrics.expectedGet(hit = false) -> 2, + metrics.expectedLoad(CacheMetrics.LoadResult.Cancelled) -> 1, ) } yield {} } @@ -1124,8 +1126,8 @@ class CacheSpec extends AsyncFunSuite with Matchers { _ <- IO { result shouldEqual 0.asRight } _ <- metrics.expect( metrics.expectedGet(hit = false) -> 2, - metrics.expectedLoad(success = false) -> 1, - metrics.expectedLoad(success = true) -> 1, + metrics.expectedLoad(CacheMetrics.LoadResult.Failure) -> 1, + metrics.expectedLoad(CacheMetrics.LoadResult.Success) -> 1, metrics.expectedGet(hit = true) -> 1, ) } yield {} @@ -1173,7 +1175,7 @@ class CacheSpec extends AsyncFunSuite with Matchers { _ <- IO { a shouldEqual 6.pure[Outcome[IO, Throwable, *]] } _ <- metrics.expect( metrics.expectedGet(hit = false) -> 3, - metrics.expectedLoad(success = true) -> 3, + metrics.expectedLoad(CacheMetrics.LoadResult.Success) -> 3, metrics.expectedFoldMap -> 1, ) } yield {} @@ -1535,7 +1537,7 @@ object CacheSpec { } def expectedGet(hit: Boolean): String = s"get(hit=$hit)" - def expectedLoad(success: Boolean): String = s"load(time=..., success=$success)" + def expectedLoad(result: CacheMetrics.LoadResult): String = s"load(time=..., result=$result)" val expectedLife: String = "life(time=...)" val expectedPut: String = "put" def expectedModify(entryExisted: Boolean, directive: CacheMetrics.Directive): String = @@ -1548,7 +1550,7 @@ object CacheSpec { val expectedFoldMap: String = "foldMap(latency=...)" def get(hit: Boolean): IO[Unit] = inc(expectedGet(hit)) - def load(time: FiniteDuration, success: Boolean): IO[Unit] = inc(expectedLoad(success)) + def load(time: FiniteDuration, result: CacheMetrics.LoadResult): IO[Unit] = inc(expectedLoad(result)) def life(time: FiniteDuration): IO[Unit] = inc(expectedLife) def put: IO[Unit] = inc(expectedPut) def modify(entryExisted: Boolean, directive: CacheMetrics.Directive): IO[Unit] =