ptrdom commented on code in PR #522:
URL:
https://github.com/apache/pekko-persistence-r2dbc/pull/522#discussion_r4375654791
##########
core/src/main/scala/org/apache/pekko/persistence/r2dbc/internal/R2dbcExecutor.scala:
##########
@@ -134,7 +134,9 @@ class R2dbcExecutor(
private def nanoTime(): Long =
if (logDbCallsExceedingEnabled) System.nanoTime() else 0L
- private def durationInMicros(startTime: Long): Long = (nanoTime() -
startTime) / 1000
+ // Long.MinValue when disabled, so that `durationMicros >=
logDbCallsExceedingMicros` is never true
+ private def durationInMicros(startTime: Long): Long =
+ if (logDbCallsExceedingEnabled) (nanoTime() - startTime) / 1000 else
Long.MinValue
Review Comment:
Nit (optional): in the enabled branch `nanoTime()` re-checks
`logDbCallsExceedingEnabled`; `System.nanoTime() - startTime` would be a bit
more direct.
##########
core/src/test/scala/org/apache/pekko/persistence/r2dbc/internal/R2dbcExecutorSpec.scala:
##########
@@ -82,6 +84,43 @@ class R2dbcExecutorSpec
"R2dbcExecutor" should {
+ "not log db calls when log-db-calls-exceeding is disabled" in {
+ val executor = new R2dbcExecutor(
+ connectionFactoryProvider,
+ LoggerFactory.getLogger(getClass),
+ logDbCallsExceeding = -1.millis,
+ closeCallsExceeding)(typedSystem.executionContext, typedSystem)
+
+ LoggingTestKit.info("getConnection took").withOccurrences(0).expect {
+ LoggingTestKit.info("Selected").withOccurrences(0).expect {
+ LoggingTestKit.info("DB call completed").withOccurrences(0).expect {
+ executor
+ .select("test")(
+ _.createStatement(s"select col from $table"),
+ row => Row(row.get("col", classOf[String])))
+ .futureValue
+ executor.updateOne("test")(_.createStatement(s"delete from $table
where col = 'x'")).futureValue
+ }
+ }
+ }
+ }
+
+ "log db calls when log-db-calls-exceeding is zero" in {
+ val executor = new R2dbcExecutor(
+ connectionFactoryProvider,
+ LoggerFactory.getLogger(getClass),
+ logDbCallsExceeding = Duration.Zero,
+ closeCallsExceeding)(typedSystem.executionContext, typedSystem)
+
+ LoggingTestKit.info("Selected").expect {
Review Comment:
Nit (optional): this only asserts the select log. Also expecting
`getConnection took` and `DB call completed` (via an `updateOne`) would mirror
the disabled test and cover all log sites on the enabled path.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
