Skip to content

Commit

Permalink
fix: Move bucket metrics to ConcurrentHashMaps
Browse files Browse the repository at this point in the history
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
  • Loading branch information
chriswk committed Oct 24, 2023
1 parent 46f2844 commit 2fc6953
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 10 deletions.
7 changes: 4 additions & 3 deletions src/main/kotlin/io/getunleash/metrics/MetricsReporter.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -56,11 +57,11 @@ class NonReporter : MetricsReporter {
}
}

data class EvaluationCount(var yes: Int, var no: Int, val variants: MutableMap<String, Int> = mutableMapOf())
data class EvaluationCount(var yes: Int, var no: Int, val variants: ConcurrentHashMap<String, Int> = ConcurrentHashMap())
data class Bucket(
val start: Date,
var stop: Date? = null,
val toggles: MutableMap<String, EvaluationCount> = mutableMapOf()
val toggles: ConcurrentHashMap<String, EvaluationCount> = ConcurrentHashMap()
)

data class Report(val appName: String, val environment: String, val instanceId: String, val bucket: Bucket)
Expand All @@ -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())
Expand Down
23 changes: 16 additions & 7 deletions src/test/kotlin/io/getunleash/metrics/MetricsTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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 <K, V> concurrentHashMapOf(vararg pairs: Pair<K, V>): ConcurrentHashMap<K, V> {
val map: ConcurrentHashMap<K, V> = ConcurrentHashMap()
pairs.forEach { (key, value) ->
map[key] = value
}
return map
}
class MetricsTest {

lateinit var server: MockWebServer
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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\"")
}

}

0 comments on commit 2fc6953

Please sign in to comment.