From 87b1186653fea06c31b77ad7704ebfb16ecfdea2 Mon Sep 17 00:00:00 2001 From: Mike Holdsworth Date: Tue, 5 Dec 2023 09:44:34 +0800 Subject: [PATCH] refactor: Internalise locking of the Buffer and remove the need to sycronize in the filter --- examples/README.md | 11 ++++++- .../firetail/logging/core/FiretailBuffer.kt | 31 ++++++++++++++----- .../logging/servlet/FiretailFilter.kt | 6 ++-- .../logging/RequestInterceptorTests.kt | 3 +- 4 files changed, 37 insertions(+), 14 deletions(-) diff --git a/examples/README.md b/examples/README.md index 775b754..2dac8f4 100644 --- a/examples/README.md +++ b/examples/README.md @@ -2,12 +2,19 @@ Requires Java 17 -You will require an `application-local.yaml` file. It will look something like this: +You will require an `application-local.yml` file. It will look something like this: ```yaml firetail: apikey: "PS-02....441b09761c3" url: "https://your-apiapi.logging.eu-north-west-99.sandbox.firetail.app" + ## Cache control before dispatching logs to API + buffer: + # Millis + interval: 100000 + # Max capacity + capacity: 5 + ``` Firstly, build the Firetail-Java-Library @@ -19,6 +26,8 @@ cd .. # Run the example cd examples ./gradlew bootRun +# By default, you'll want to hit this endpoint 5 times before the logs are dispatched +# Otherwise hit it < 5 and wait for 10 seconds curl http://localhost:8080/hello ``` diff --git a/src/main/kotlin/io/firetail/logging/core/FiretailBuffer.kt b/src/main/kotlin/io/firetail/logging/core/FiretailBuffer.kt index 434d4c4..c43f14c 100644 --- a/src/main/kotlin/io/firetail/logging/core/FiretailBuffer.kt +++ b/src/main/kotlin/io/firetail/logging/core/FiretailBuffer.kt @@ -1,5 +1,6 @@ package io.firetail.logging.core +import io.firetail.logging.core.FiretailLogger.Companion.LOGGER import io.firetail.logging.servlet.FiretailMapper import io.firetail.logging.spring.FiretailConfig import org.slf4j.LoggerFactory @@ -7,6 +8,7 @@ import java.util.* import java.util.concurrent.Executors import java.util.concurrent.ScheduledExecutorService import java.util.concurrent.TimeUnit +import java.util.concurrent.locks.ReentrantLock class FiretailBuffer( private val firetailConfig: FiretailConfig, @@ -16,6 +18,7 @@ class FiretailBuffer( private val buffer: MutableList = mutableListOf() private val scheduler: ScheduledExecutorService = Executors.newSingleThreadScheduledExecutor() private val flushCallback = mutableListOf() + private val bufferLock = ReentrantLock() init { // Schedule the periodic flush task @@ -31,19 +34,31 @@ class FiretailBuffer( ) } + // Caller sycronizes access before calling this function fun add(item: FiretailData) { - buffer.add(item) - if (buffer.size >= firetailConfig.capacity) { - flush() + bufferLock.lock() + try { + buffer.add(item) + if (buffer.size >= firetailConfig.capacity) { + flush() + } + } finally { + bufferLock.unlock() } } + // Threadsafe - write and reset the cached data fun flush(): String { - if (buffer.isNotEmpty()) { - LOGGER.debug("Buffer flushing ${buffer.size}") - val result = firetailTemplate.send(buffer.toList()) - buffer.clear() - return firetailMapper.getResult(result) + bufferLock.lock() + try { + if (buffer.isNotEmpty()) { + LOGGER.debug("Buffer flushing ${buffer.size}") + val result = firetailTemplate.send(buffer) + buffer.clear() + return firetailMapper.getResult(result) + } + } finally { + bufferLock.unlock() } return "" } diff --git a/src/main/kotlin/io/firetail/logging/servlet/FiretailFilter.kt b/src/main/kotlin/io/firetail/logging/servlet/FiretailFilter.kt index 8bdb7c6..5d9823f 100644 --- a/src/main/kotlin/io/firetail/logging/servlet/FiretailFilter.kt +++ b/src/main/kotlin/io/firetail/logging/servlet/FiretailFilter.kt @@ -4,9 +4,9 @@ import io.firetail.logging.core.Constants.Companion.CORRELATION_ID import io.firetail.logging.core.Constants.Companion.OP_NAME import io.firetail.logging.core.Constants.Companion.REQUEST_ID import io.firetail.logging.core.FiretailBuffer -import io.firetail.logging.spring.FiretailConfig import io.firetail.logging.core.FiretailLogger import io.firetail.logging.core.FiretailTemplate +import io.firetail.logging.spring.FiretailConfig import io.firetail.logging.util.FiretailMDC import jakarta.servlet.FilterChain import jakarta.servlet.http.HttpServletRequest @@ -75,9 +75,7 @@ class FiretailFilter( ) CompletableFuture.runAsync { try { - synchronized(firetailBuffer) { - firetailBuffer.add(firetailLog) - } + firetailBuffer.add(firetailLog) } catch (e: Exception) { LOGGER.error(e.message) throw e diff --git a/src/test/kotlin/io/firetail/logging/RequestInterceptorTests.kt b/src/test/kotlin/io/firetail/logging/RequestInterceptorTests.kt index b606261..469f2c5 100644 --- a/src/test/kotlin/io/firetail/logging/RequestInterceptorTests.kt +++ b/src/test/kotlin/io/firetail/logging/RequestInterceptorTests.kt @@ -2,12 +2,12 @@ package io.firetail.logging import io.firetail.logging.core.Constants import io.firetail.logging.core.FiretailBuffer -import io.firetail.logging.spring.EnableFiretail import io.firetail.logging.core.FiretailLogger import io.firetail.logging.core.FiretailTemplate import io.firetail.logging.servlet.FiretailFilter import io.firetail.logging.servlet.FiretailHeaderInterceptor import io.firetail.logging.servlet.FiretailMapper +import io.firetail.logging.spring.EnableFiretail import io.firetail.logging.util.FiretailMDC import io.firetail.logging.util.StringUtils import org.assertj.core.api.Assertions.assertThat @@ -37,6 +37,7 @@ import org.springframework.web.client.RestTemplate @TestPropertySource( properties = [ "firetail.url=http://localhost:\${wiremock.server.port}", + "firetail.buffer.capacity=5", ], ) @EnableFiretail