From 3676e998ba9ff49b04d815e7bacea9770c966100 Mon Sep 17 00:00:00 2001 From: Gregor Heine Date: Sun, 22 Feb 2026 23:35:42 +0100 Subject: [PATCH 1/2] FDN-4523: Add time-based throttling to RollbarLogger via atMostEvery Co-Authored-By: Claude Opus 4.6 --- CLAUDE.md | 59 +++++++++++++++++++ src/main/scala/io/flow/log/Rollbar.scala | 4 ++ .../scala/io/flow/log/RollbarLogger.scala | 22 ++++++- .../scala/io/flow/log/RollbarLoggerSpec.scala | 51 ++++++++++++++++ 4 files changed, 134 insertions(+), 2 deletions(-) create mode 100644 CLAUDE.md create mode 100644 src/test/scala/io/flow/log/RollbarLoggerSpec.scala diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..f2f0ec0 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,59 @@ +# CLAUDE.md + +This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. + +## Overview + +lib-log (`lib-log-play29`) is a Scala logging library for Flow Commerce Play 2.9 services. It provides a fluent, injectable logger (`RollbarLogger`) that integrates with Rollbar for production error tracking and supports structured logging via Logstash/Sumo. + +## Build Commands + +```bash +sbt compile # Compile +sbt test # Run all tests +sbt scalafmtAll # Format all code (must run before pushing) +sbt "testOnly io.flow.log.LogUtilSpec" # Run a single test class +sbt clean coverage test coverageReport # Run tests with coverage report +sbt publishLocal # Build and publish locally +``` + +## Code Coverage + +Enforced minimums: 25% statement, 15% branch. Build fails if not met. + +## Code Style + +- Scalafmt 3.5.9 with `scalafmtOnCompile := true` (auto-formats on compile) +- Max line width: 120 characters +- Trailing commas: always +- Alignment: none +- Scala 2.13 dialect +- Compiler uses `-Xfatal-warnings` — all warnings are errors + +## Architecture + +All code lives in package `io.flow.log` under `src/main/scala/`. Four source files: + +- **RollbarLogger** — Immutable case class with fluent builder methods (`.withKeyValue()`, `.fingerprint()`, `.organization()`, etc.). Only `warn` and `error` are sent to Rollbar; `debug` and `info` go to logs only. Supports frequency-based sampling via `.withFrequency()`. Use `RollbarLogger.SimpleLogger` in tests. + +- **Rollbar** (`Rollbar.scala`) — Guice module (`RollbarModule`), provider (`RollbarProvider`), and factory (`RollbarFactory`). Uses AssistedInject factory pattern so `RollbarLogger` instances can be copied while sharing a single Rollbar notifier. Includes custom Jackson serializer for Play JSON types and custom fingerprint generator. + +- **LogUtil** — Injectable utility for timing sync (`duration()`) and async (`durationF()`) operations with structured attributes. Supports frequency-based sampling. + +- **SerializePlayJson** — Jackson module registration for Play JSON serialization. + +## Key Patterns + +- **Dependency injection**: Google Guice with AssistedInject for the logger factory +- **Immutable builders**: `RollbarLogger` is a case class; fluent methods return new copies via `.copy()` +- **Testing**: ScalaTest WordSpec with Matchers (`src/test/scala/io/flow/log/`) +- **No `return` statements** — strictly avoided per Scala conventions + +## CI/CD + +- **Jenkins**: `skeletonLibraryPipeline()` in `Jenkinsfile` +- **GitHub Actions**: PR title must start with a JIRA ticket (e.g., `FDN-1234`, pattern: `^[A-Z]{3,}-(?!0+\b)\d{3,6}\b`). Auto-merge workflow for PRs labeled `auto-merge`. + +## Publishing + +Artifacts publish to Flow Artifactory (`flow.jfrog.io`). Requires `ARTIFACTORY_USERNAME` and `ARTIFACTORY_PASSWORD` environment variables. diff --git a/src/main/scala/io/flow/log/Rollbar.scala b/src/main/scala/io/flow/log/Rollbar.scala index 1b46c36..356c75b 100644 --- a/src/main/scala/io/flow/log/Rollbar.scala +++ b/src/main/scala/io/flow/log/Rollbar.scala @@ -3,8 +3,11 @@ package io.flow.log import com.fasterxml.jackson.core.JsonGenerator import com.fasterxml.jackson.databind.module.SimpleModule import com.fasterxml.jackson.databind.{ObjectMapper, SerializerProvider} +import java.util.concurrent.atomic.AtomicLong import com.google.inject.assistedinject.{AssistedInject, FactoryModuleBuilder} import com.google.inject.{AbstractModule, Provider, TypeLiteral} + +import scala.concurrent.duration.FiniteDuration import com.rollbar.api.payload.Payload import com.rollbar.api.payload.data.Data import com.rollbar.notifier.Rollbar @@ -20,6 +23,7 @@ import javax.inject.{Inject, Singleton} class RollbarModule extends AbstractModule { override def configure(): Unit = { bind(new TypeLiteral[Option[Rollbar]]() {}).toProvider(classOf[RollbarProvider]) + bind(new TypeLiteral[Option[(FiniteDuration, AtomicLong)]]() {}).toInstance(None) install(new FactoryModuleBuilder().build(classOf[RollbarLogger.Factory])) bind(classOf[RollbarLogger]).toProvider(classOf[RollbarLoggerProvider]) () diff --git a/src/main/scala/io/flow/log/RollbarLogger.scala b/src/main/scala/io/flow/log/RollbarLogger.scala index e3160e6..35b75b9 100644 --- a/src/main/scala/io/flow/log/RollbarLogger.scala +++ b/src/main/scala/io/flow/log/RollbarLogger.scala @@ -8,6 +8,8 @@ import net.logstash.logback.marker.Markers.appendEntries import org.slf4j.LoggerFactory import play.api.libs.json.{JsValue, Json, Writes} +import java.util.concurrent.atomic.AtomicLong +import scala.concurrent.duration.FiniteDuration import scala.jdk.CollectionConverters._ import scala.util.control.NonFatal import scala.util.{Random, Try} @@ -74,6 +76,7 @@ case class RollbarLogger @AssistedInject() ( @Assisted legacyMessage: Option[String], @Assisted shouldSendToRollbar: Boolean = true, @Assisted frequency: Long = 1L, + interval: Option[(FiniteDuration, AtomicLong)] = None, ) { private[this] val MaxValuesToWrite = 10 @@ -86,6 +89,12 @@ case class RollbarLogger @AssistedInject() ( */ def withFrequency(frequency: Long): RollbarLogger = this.copy(frequency = frequency) + /** Throttle logging to at most once per the given duration. Thread-safe via compare-and-set. The throttle state is + * shared across .copy() calls, so chained builder methods preserve it. + */ + def atMostEvery(d: FiniteDuration): RollbarLogger = + this.copy(interval = Some((d, new AtomicLong(0L)))) + def withKeyValue[T: Writes](keyValue: (String, T)): RollbarLogger = withKeyValue(keyValue._1, keyValue._2) def withKeyValue[T: Writes](key: String, value: T): RollbarLogger = this.copy(attributes = attributes + (key -> Json.toJson(value))) @@ -167,7 +176,16 @@ case class RollbarLogger @AssistedInject() ( rollbar.foreach(_.error(error, convert(attributes), message)) } - private def shouldLog: Boolean = - frequency == 1L || (Random.nextInt() % frequency == 0) + private[log] def shouldLog: Boolean = { + val frequencyCheck = frequency == 1L || (Random.nextInt() % frequency == 0) + val timeThrottleCheck = interval match { + case Some((minInterval, lastLoggedAt)) => + val now = System.currentTimeMillis() + val last = lastLoggedAt.get() + now - last >= minInterval.toMillis && lastLoggedAt.compareAndSet(last, now) + case None => true + } + frequencyCheck && timeThrottleCheck + } } diff --git a/src/test/scala/io/flow/log/RollbarLoggerSpec.scala b/src/test/scala/io/flow/log/RollbarLoggerSpec.scala new file mode 100644 index 0000000..43ad730 --- /dev/null +++ b/src/test/scala/io/flow/log/RollbarLoggerSpec.scala @@ -0,0 +1,51 @@ +package io.flow.log + +import org.scalatest.matchers.must.Matchers +import org.scalatest.wordspec.AnyWordSpec + +import scala.concurrent.duration._ + +class RollbarLoggerSpec extends AnyWordSpec with Matchers { + + "atMostEvery" should { + + "default to no time throttle" in { + RollbarLogger.SimpleLogger.interval mustBe None + } + + "allow the first call immediately" in { + val logger = RollbarLogger.SimpleLogger.atMostEvery(1.second) + logger.shouldLog mustBe true + } + + "suppress a second call within the interval" in { + val logger = RollbarLogger.SimpleLogger.atMostEvery(10.seconds) + logger.shouldLog mustBe true + logger.shouldLog mustBe false + } + + "allow a call after the interval has elapsed" in { + val logger = RollbarLogger.SimpleLogger.atMostEvery(50.millis) + logger.shouldLog mustBe true + Thread.sleep(100) + logger.shouldLog mustBe true + } + + "share throttle state across builder calls" in { + val base = RollbarLogger.SimpleLogger.atMostEvery(10.seconds) + val derived = base.withKeyValue("key", "value") + (base.interval.get._2 eq derived.interval.get._2) mustBe true + + base.shouldLog mustBe true + derived.shouldLog mustBe false + } + + "combine correctly with withFrequency" in { + val logger = RollbarLogger.SimpleLogger + .withFrequency(100) + .atMostEvery(1.second) + logger.frequency mustBe 100L + logger.interval.map(_._1) mustBe Some(1.second) + } + } +} From 78f65fce0dec88388b8f03adb6bc97bdb4dedcca Mon Sep 17 00:00:00 2001 From: Gregor Heine Date: Mon, 23 Feb 2026 10:05:10 +0100 Subject: [PATCH 2/2] Fix: only evaluate time throttle when frequency check passes Prevents skipped frequency samples from advancing the lastLoggedAt timestamp, which would suppress later logs more than intended. Co-Authored-By: Claude Opus 4.6 --- src/main/scala/io/flow/log/RollbarLogger.scala | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/scala/io/flow/log/RollbarLogger.scala b/src/main/scala/io/flow/log/RollbarLogger.scala index 35b75b9..2ba832d 100644 --- a/src/main/scala/io/flow/log/RollbarLogger.scala +++ b/src/main/scala/io/flow/log/RollbarLogger.scala @@ -178,12 +178,12 @@ case class RollbarLogger @AssistedInject() ( private[log] def shouldLog: Boolean = { val frequencyCheck = frequency == 1L || (Random.nextInt() % frequency == 0) - val timeThrottleCheck = interval match { - case Some((minInterval, lastLoggedAt)) => + val timeThrottleCheck = (interval, frequencyCheck) match { + case (Some((minInterval, lastLoggedAt)), true) => val now = System.currentTimeMillis() val last = lastLoggedAt.get() now - last >= minInterval.toMillis && lastLoggedAt.compareAndSet(last, now) - case None => true + case _ => true } frequencyCheck && timeThrottleCheck }