Provide custom OkHttpClient for MetricsReporter (#65)

vitoksmile · web-flow · commit a8acf75d0b3b · 2024-01-23T12:13:03.000+02:00
* Provide custom OkHttpClient for MetricsReporter

* Updated tests
diff --git a/src/main/kotlin/io/getunleash/UnleashClient.kt b/src/main/kotlin/io/getunleash/UnleashClient.kt
@@ -40,7 +40,7 @@ class UnleashClient(
             )
         ).build(),
     val cache: ToggleCache = InMemoryToggleCache(),
-    val metricsReporter: MetricsReporter = unleashConfig.reportMetrics?.let { HttpMetricsReporter(unleashConfig) } ?: NonReporter()
+    val metricsReporter: MetricsReporter = unleashConfig.reportMetrics?.let { HttpMetricsReporter(unleashConfig, it.httpClient) } ?: NonReporter()
 ) : UnleashClientSpec {
     private val fetcher: UnleashFetcher = UnleashFetcher(unleashConfig = unleashConfig, httpClient = httpClient)
     private val refreshPolicy: RefreshPolicy = when (unleashConfig.pollingMode) {
diff --git a/src/main/kotlin/io/getunleash/UnleashConfig.kt b/src/main/kotlin/io/getunleash/UnleashConfig.kt
@@ -2,13 +2,18 @@ package io.getunleash
 
 import io.getunleash.polling.AutoPollingMode
 import io.getunleash.polling.PollingMode
+import okhttp3.OkHttpClient
 import java.util.UUID
+import java.util.concurrent.TimeUnit
 
+data class ReportMetrics(
+    val metricsInterval: Long,
+    val httpClient: OkHttpClient,
+)
 
-data class ReportMetrics(val metricsInterval: Long = 60000)
 /**
  * Represents configuration for Unleash.
- * @property url HTTP(s) URL to the Unleash Proxy (Required).
+ * @property proxyUrl HTTP(s) URL to the Unleash Proxy (Required).
  * @property clientKey the key added as the Authorization header sent to the unleash-proxy (Required)
  * @property appName: name of the underlying application. Will be used as default in the [io.getunleash.UnleashContext] call (Required).
  * @property environment which environment is the application running in. Will be used as default argument for the [io.getunleash.UnleashContext]. (Optional - Defaults to 'default')
@@ -69,9 +74,9 @@ data class UnleashConfig(
         var httpClientReadTimeout: Long? = null,
         var httpClientCacheSize: Long? = null,
         var enableMetrics: Boolean = false,
+        var metricsHttpClient: OkHttpClient? = null,
         var metricsInterval: Long? = null,
         var instanceId: String? = null,
-
     ) {
         fun proxyUrl(proxyUrl: String) = apply { this.proxyUrl = proxyUrl }
 
@@ -88,6 +93,7 @@ data class UnleashConfig(
         fun httpClientCacheSize(cacheSize: Long) = apply { this.httpClientCacheSize = cacheSize }
         fun enableMetrics() = apply { this.enableMetrics = true }
         fun disableMetrics() = apply { this.enableMetrics = false }
+        fun metricsHttpClient(client: OkHttpClient) = apply { this.metricsHttpClient = client }
         fun metricsInterval(intervalInMs: Long) = apply { this.metricsInterval = intervalInMs }
         fun metricsIntervalInSeconds(seconds: Long) = apply { this.metricsInterval = seconds * 1000 }
         fun instanceId(id: String) = apply { this.instanceId = id }
@@ -102,7 +108,13 @@ data class UnleashConfig(
             httpClientReadTimeout = httpClientReadTimeout ?: 5000,
             httpClientCacheSize = httpClientCacheSize ?: (1024 * 1024 * 10),
             reportMetrics = if (enableMetrics) {
-                ReportMetrics(metricsInterval ?: 60000)
+                ReportMetrics(
+                    metricsInterval = metricsInterval ?: 60000,
+                    httpClient = metricsHttpClient ?: OkHttpClient.Builder()
+                        .connectTimeout(httpClientConnectionTimeout ?: 2000, TimeUnit.MILLISECONDS)
+                        .readTimeout(httpClientReadTimeout ?: 5000, TimeUnit.MILLISECONDS)
+                        .build(),
+                )
             } else {
                 null
             },
diff --git a/src/main/kotlin/io/getunleash/metrics/MetricsReporter.kt b/src/main/kotlin/io/getunleash/metrics/MetricsReporter.kt
@@ -66,13 +66,20 @@ data class Bucket(
 
 data class Report(val appName: String, val environment: String, val instanceId: String, val bucket: Bucket)
 
-class HttpMetricsReporter(val config: UnleashConfig, val started: Date = Date()) : MetricsReporter,
+/**
+ * @param config - Configuration for unleash - see docs for [io.getunleash.UnleashConfig]
+ * @param httpClient - the http client to use for reporting metrics to Unleash proxy.
+ */
+class HttpMetricsReporter(
+    val config: UnleashConfig,
+    val httpClient: OkHttpClient,
+    started: Date = Date(),
+) : MetricsReporter,
     Closeable {
     companion object {
         val logger: Logger = LoggerFactory.getLogger(MetricsReporter::class.java)
     }
 
-    val client = OkHttpClient.Builder().callTimeout(2, TimeUnit.SECONDS).readTimeout(5, TimeUnit.SECONDS).build()
     val metricsUrl = config.proxyUrl.toHttpUrl().newBuilder().addPathSegment("client").addPathSegment("metrics").build()
     private var bucket: Bucket = Bucket(start = started)
 
@@ -86,7 +93,7 @@ class HttpMetricsReporter(val config: UnleashConfig, val started: Date = Date())
         val request = Request.Builder().header("Authorization", config.clientKey).url(metricsUrl).post(
             Parser.jackson.writeValueAsString(report).toRequestBody("application/json".toMediaType())
         ).build()
-        client.newCall(request).enqueue(object : Callback {
+        httpClient.newCall(request).enqueue(object : Callback {
             override fun onFailure(call: Call, e: IOException) {
                 logger.info("Failed to report metrics for interval")
             }
@@ -123,8 +130,8 @@ class HttpMetricsReporter(val config: UnleashConfig, val started: Date = Date())
     }
 
     override fun close() {
-        client.connectionPool.evictAll()
-        client.cache?.closeQuietly()
-        client.dispatcher.executorService.shutdown()
+        httpClient.dispatcher.executorService.shutdownNow()
+        httpClient.connectionPool.evictAll()
+        httpClient.cache?.closeQuietly()
     }
 }
diff --git a/src/main/kotlin/io/getunleash/polling/UnleashFetcher.kt b/src/main/kotlin/io/getunleash/polling/UnleashFetcher.kt
@@ -121,8 +121,8 @@ open class UnleashFetcher(
     }
 
     override fun close() {
-        this.httpClient.dispatcher.executorService.shutdownNow()
-        this.httpClient.connectionPool.evictAll()
-        this.httpClient.cache?.closeQuietly()
+        httpClient.dispatcher.executorService.shutdownNow()
+        httpClient.connectionPool.evictAll()
+        httpClient.cache?.closeQuietly()
     }
 }
diff --git a/src/test/kotlin/io/getunleash/UnleashConfigTest.kt b/src/test/kotlin/io/getunleash/UnleashConfigTest.kt
@@ -1,8 +1,11 @@
 package io.getunleash
 
+import okhttp3.OkHttpClient
 import org.assertj.core.api.Assertions.assertThat
 import org.assertj.core.api.Assertions.assertThatThrownBy
 import org.junit.jupiter.api.Test
+import java.util.concurrent.TimeUnit
+import kotlin.test.assertEquals
 
 class UnleashConfigTest {
 
@@ -107,4 +110,34 @@ class UnleashConfigTest {
         val configWithMetricsSetInSeconds = config.newBuilder().enableMetrics().metricsIntervalInSeconds(5).build()
         assertThat(configInMs.reportMetrics!!.metricsInterval).isEqualTo(configWithMetricsSetInSeconds.reportMetrics!!.metricsInterval)
     }
+
+    @Test
+    fun `Default http client for metrics uses config`() {
+        val config = UnleashConfig(
+            proxyUrl = "https://localhost:4242/proxy",
+            clientKey = "some-key",
+            appName = "my-app",
+            environment = "default"
+        )
+        assertThat(config.reportMetrics).isNull()
+        val withMetrics = config.newBuilder().enableMetrics().build()
+        assertThat(withMetrics.reportMetrics).isNotNull
+        assertThat(withMetrics.reportMetrics!!.httpClient.connectTimeoutMillis).isEqualTo(config.httpClientConnectionTimeout)
+        assertThat(withMetrics.reportMetrics!!.httpClient.readTimeoutMillis).isEqualTo(config.httpClientReadTimeout)
+    }
+
+    @Test
+    fun `Can override http client for metrics`() {
+        val config = UnleashConfig(
+            proxyUrl = "https://localhost:4242/proxy",
+            clientKey = "some-key",
+            appName = "my-app",
+            environment = "default"
+        )
+        assertThat(config.reportMetrics).isNull()
+        val okHttpClient = OkHttpClient.Builder().build()
+        val withMetrics = config.newBuilder().enableMetrics().metricsHttpClient(okHttpClient).build()
+        assertThat(withMetrics.reportMetrics).isNotNull
+        assertEquals(okHttpClient, withMetrics.reportMetrics!!.httpClient)
+    }
 }
diff --git a/src/test/kotlin/io/getunleash/metrics/HttpMetricsReporterTest.kt b/src/test/kotlin/io/getunleash/metrics/HttpMetricsReporterTest.kt
@@ -2,16 +2,19 @@ package io.getunleash.metrics
 
 import io.getunleash.UnleashConfig
 import io.getunleash.data.Variant
+import okhttp3.OkHttpClient
 import okhttp3.mockwebserver.MockResponse
 import okhttp3.mockwebserver.MockWebServer
 import org.assertj.core.api.Assertions.assertThat
 import org.junit.jupiter.api.Test
+import java.util.concurrent.TimeUnit
 
 class HttpMetricsReporterTest {
 
     @Test
     fun metricsUrlIsCorrect() {
-        HttpMetricsReporter(UnleashConfig.newBuilder().proxyUrl("http://localhost:4242/proxy").clientKey("some-key").build()).use { reporter ->
+        val okHttpClient = OkHttpClient.Builder().build()
+        HttpMetricsReporter(UnleashConfig.newBuilder().proxyUrl("http://localhost:4242/proxy").clientKey("some-key").build(), okHttpClient).use { reporter ->
             assertThat(reporter.metricsUrl.toString()).isEqualTo("http://localhost:4242/proxy/client/metrics")
         }
     }
diff --git a/src/test/kotlin/io/getunleash/metrics/MetricsTest.kt b/src/test/kotlin/io/getunleash/metrics/MetricsTest.kt
@@ -7,6 +7,7 @@ import io.getunleash.UnleashContext
 import io.getunleash.data.Parser
 import io.getunleash.data.Variant
 import io.getunleash.polling.PollingModes
+import okhttp3.OkHttpClient
 import okhttp3.mockwebserver.MockResponse
 import okhttp3.mockwebserver.MockWebServer
 import org.assertj.core.api.Assertions.assertThat
@@ -18,6 +19,7 @@ import java.time.ZoneOffset
 import java.time.ZonedDateTime
 import java.util.Date
 import java.util.concurrent.ConcurrentHashMap
+import java.util.concurrent.TimeUnit
 
 fun <K, V> concurrentHashMapOf(vararg pairs: Pair<K, V>): ConcurrentHashMap<K, V> {
     val map: ConcurrentHashMap<K, V> = ConcurrentHashMap()
@@ -158,7 +160,8 @@ class MetricsTest {
 
     @Test
     fun `http reporter does actually report toggles to metrics endpoint`() {
-        val reporter = HttpMetricsReporter(config)
+        val okHttpClient = OkHttpClient.Builder().build()
+        val reporter = HttpMetricsReporter(config, okHttpClient)
         val client = UnleashClient(config, context, metricsReporter = reporter)
         repeat(100) {
             client.isEnabled("unleash-android-proxy-sdk")

Original file line number	Diff line number	Diff line change
`@@ -40,7 +40,7 @@ class UnleashClient(`
`40`	`40`	`)`
`41`	`41`	`).build(),`
`42`	`42`	`val cache: ToggleCache = InMemoryToggleCache(),`
`43`		`- val metricsReporter: MetricsReporter = unleashConfig.reportMetrics?.let { HttpMetricsReporter(unleashConfig) } ?: NonReporter()`
	`43`	`+ val metricsReporter: MetricsReporter = unleashConfig.reportMetrics?.let { HttpMetricsReporter(unleashConfig, it.httpClient) } ?: NonReporter()`
`44`	`44`	`) : UnleashClientSpec {`
`45`	`45`	`private val fetcher: UnleashFetcher = UnleashFetcher(unleashConfig = unleashConfig, httpClient = httpClient)`
`46`	`46`	`private val refreshPolicy: RefreshPolicy = when (unleashConfig.pollingMode) {`
Original file line number	Diff line number	Diff line change
`@@ -121,8 +121,8 @@ open class UnleashFetcher(`
`121`	`121`	`}`
`122`	`122`
`123`	`123`	`override fun close() {`
`124`		`- this.httpClient.dispatcher.executorService.shutdownNow()`
`125`		`- this.httpClient.connectionPool.evictAll()`
`126`		`- this.httpClient.cache?.closeQuietly()`
	`124`	`+ httpClient.dispatcher.executorService.shutdownNow()`
	`125`	`+ httpClient.connectionPool.evictAll()`
	`126`	`+ httpClient.cache?.closeQuietly()`
`127`	`127`	`}`
`128`	`128`	`}`