From 2fc6953d606a900b59ec2c9d287fb80142c09ad6 Mon Sep 17 00:00:00 2001 From: Christopher Kolstad Date: Tue, 24 Oct 2023 12:04:55 +0200 Subject: [PATCH] fix: Move bucket metrics to ConcurrentHashMaps Since we're continuously logging metrics, we could happen to log while serializing the metrics bucket. Moving to a concurrenthashmap will allow us to insert into the map while reading from the iterator. fixes: #59 --- .../io/getunleash/metrics/MetricsReporter.kt | 7 +++--- .../io/getunleash/metrics/MetricsTest.kt | 23 +++++++++++++------ 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/src/main/kotlin/io/getunleash/metrics/MetricsReporter.kt b/src/main/kotlin/io/getunleash/metrics/MetricsReporter.kt index 2d6e4a7..949b650 100644 --- a/src/main/kotlin/io/getunleash/metrics/MetricsReporter.kt +++ b/src/main/kotlin/io/getunleash/metrics/MetricsReporter.kt @@ -17,6 +17,7 @@ import org.slf4j.LoggerFactory import java.io.Closeable import java.io.IOException import java.util.Date +import java.util.concurrent.ConcurrentHashMap import java.util.concurrent.TimeUnit interface MetricsReporter { @@ -56,11 +57,11 @@ class NonReporter : MetricsReporter { } } -data class EvaluationCount(var yes: Int, var no: Int, val variants: MutableMap = mutableMapOf()) +data class EvaluationCount(var yes: Int, var no: Int, val variants: ConcurrentHashMap = ConcurrentHashMap()) data class Bucket( val start: Date, var stop: Date? = null, - val toggles: MutableMap = mutableMapOf() + val toggles: ConcurrentHashMap = ConcurrentHashMap() ) data class Report(val appName: String, val environment: String, val instanceId: String, val bucket: Bucket) @@ -80,7 +81,7 @@ class HttpMetricsReporter(val config: UnleashConfig, val started: Date = Date()) appName = config.appName ?: "unknown", instanceId = config.instanceId ?: "not-set", environment = config.environment ?: "not-set", - bucket = bucket.copy(stop = Date()) + bucket = bucket ) val request = Request.Builder().header("Authorization", config.clientKey).url(metricsUrl).post( Parser.jackson.writeValueAsString(report).toRequestBody("application/json".toMediaType()) diff --git a/src/test/kotlin/io/getunleash/metrics/MetricsTest.kt b/src/test/kotlin/io/getunleash/metrics/MetricsTest.kt index e9329a8..9afef09 100644 --- a/src/test/kotlin/io/getunleash/metrics/MetricsTest.kt +++ b/src/test/kotlin/io/getunleash/metrics/MetricsTest.kt @@ -17,7 +17,15 @@ import java.io.File import java.time.ZoneOffset import java.time.ZonedDateTime import java.util.Date +import java.util.concurrent.ConcurrentHashMap +fun concurrentHashMapOf(vararg pairs: Pair): ConcurrentHashMap { + val map: ConcurrentHashMap = ConcurrentHashMap() + pairs.forEach { (key, value) -> + map[key] = value + } + return map +} class MetricsTest { lateinit var server: MockWebServer @@ -118,11 +126,13 @@ class MetricsTest { } val toggles = reporter.getToggles() - assertThat(toggles).containsAllEntriesOf(mutableMapOf( - "some-non-existing-toggle" to EvaluationCount(0, 100, mutableMapOf("disabled" to 100)), - "asdasd" to EvaluationCount(100, 0, mutableMapOf("123" to 100)), - "cache.buster" to EvaluationCount(100, 0, mutableMapOf("disabled" to 100)), - )) + assertThat(toggles).containsAllEntriesOf( + concurrentHashMapOf( + "some-non-existing-toggle" to EvaluationCount(0, 100, concurrentHashMapOf("disabled" to 100)), + "asdasd" to EvaluationCount(100, 0, concurrentHashMapOf("123" to 100)), + "cache.buster" to EvaluationCount(100, 0, concurrentHashMapOf("disabled" to 100)), + ) + ) } @Test @@ -169,7 +179,7 @@ class MetricsTest { "unleash-android-proxy-sdk" to EvaluationCount(0, 100), "non-existing-toggle" to EvaluationCount(0, 100), "Test_release" to EvaluationCount(100, 0), - "demoApp.step4" to EvaluationCount(100, 0, mutableMapOf("red" to 100)) + "demoApp.step4" to EvaluationCount(100, 0, concurrentHashMapOf("red" to 100)) )) server.enqueue(MockResponse()) // No activity since last report, bucket should be empty @@ -187,5 +197,4 @@ class MetricsTest { val output = Parser.jackson.writeValueAsString(Date.from(ZonedDateTime.of(2021, 6, 1, 15, 0, 0, 456000000, ZoneOffset.UTC).toInstant())) assertThat(output).isEqualTo("\"2021-06-01T15:00:00.456+00:00\"") } - }