diff --git a/Confidence/src/main/java/com/spotify/confidence/RemoteFlagResolver.kt b/Confidence/src/main/java/com/spotify/confidence/RemoteFlagResolver.kt index b4e2d9dd..f4214c8a 100644 --- a/Confidence/src/main/java/com/spotify/confidence/RemoteFlagResolver.kt +++ b/Confidence/src/main/java/com/spotify/confidence/RemoteFlagResolver.kt @@ -3,6 +3,7 @@ package com.spotify.confidence import com.spotify.confidence.client.ResolveResponse import com.spotify.confidence.client.Sdk import com.spotify.confidence.client.await +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext @@ -16,7 +17,9 @@ import okhttp3.OkHttpClient import okhttp3.Request import okhttp3.RequestBody.Companion.toRequestBody import okhttp3.Response +import java.net.NoRouteToHostException import java.net.SocketTimeoutException +import java.net.UnknownHostException internal interface FlagResolver { suspend fun resolve(flags: List, context: Map): Result @@ -55,18 +58,32 @@ internal class RemoteFlagResolver( val httpRequest = requestBuilder.build() val startTime = System.nanoTime() - var status = Telemetry.RequestStatus.SUCCESS + var status: Telemetry.RequestStatus? = Telemetry.RequestStatus.SUCCESS try { httpClient.newCall(httpRequest).await().use { it.toResolveFlags() } + } catch (e: CancellationException) { + status = null + throw e } catch (e: SocketTimeoutException) { status = Telemetry.RequestStatus.TIMEOUT throw e + } catch (e: UnknownHostException) { + status = Telemetry.RequestStatus.OFFLINE + throw e + } catch (e: NoRouteToHostException) { + status = Telemetry.RequestStatus.OFFLINE + throw e } catch (e: Exception) { status = Telemetry.RequestStatus.ERROR throw e + } catch (e: Error) { + status = Telemetry.RequestStatus.ERROR + throw e } finally { - val elapsedMs = (System.nanoTime() - startTime) / 1_000_000 - telemetry.trackResolveLatency(elapsedMs, status) + if (status != null) { + val elapsedMs = (System.nanoTime() - startTime) / 1_000_000 + telemetry.trackResolveLatency(elapsedMs, status) + } } } diff --git a/Confidence/src/main/java/com/spotify/confidence/Telemetry.kt b/Confidence/src/main/java/com/spotify/confidence/Telemetry.kt index 440e60be..5e28d413 100644 --- a/Confidence/src/main/java/com/spotify/confidence/Telemetry.kt +++ b/Confidence/src/main/java/com/spotify/confidence/Telemetry.kt @@ -239,7 +239,8 @@ internal class Telemetry( UNSPECIFIED(0), SUCCESS(1), ERROR(2), - TIMEOUT(3) + TIMEOUT(3), + OFFLINE(5) } enum class EvaluationReason(val value: Int) { diff --git a/Confidence/src/test/java/com/spotify/confidence/ConfidenceRemoteClientTests.kt b/Confidence/src/test/java/com/spotify/confidence/ConfidenceRemoteClientTests.kt index 18ffcab2..a22e36a2 100644 --- a/Confidence/src/test/java/com/spotify/confidence/ConfidenceRemoteClientTests.kt +++ b/Confidence/src/test/java/com/spotify/confidence/ConfidenceRemoteClientTests.kt @@ -10,11 +10,16 @@ import com.spotify.confidence.client.FlagApplierClientImpl import com.spotify.confidence.client.Flags import com.spotify.confidence.client.ResolveFlags import com.spotify.confidence.client.ResolvedFlag +import com.spotify.telemetry.v1.Types.Monitoring import io.mockk.every import io.mockk.mockkStatic import io.mockk.unmockkStatic import junit.framework.TestCase.assertEquals +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.async +import kotlinx.coroutines.delay import kotlinx.coroutines.test.UnconfinedTestDispatcher import kotlinx.coroutines.test.runTest import okhttp3.OkHttpClient @@ -23,6 +28,8 @@ import okhttp3.mockwebserver.MockResponse import okhttp3.mockwebserver.MockWebServer import okhttp3.mockwebserver.RecordedRequest import org.junit.After +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertNull import org.junit.Assert.assertThrows import org.junit.Assert.assertTrue import org.junit.Before @@ -31,6 +38,8 @@ import org.mockito.kotlin.mock import org.mockito.kotlin.whenever import java.time.Instant import java.util.Date +import java.util.concurrent.TimeUnit +import com.spotify.telemetry.v1.Types.LibraryTraces.Trace.RequestTrace.Status as ProtoStatus internal class ConfidenceRemoteClientTests { private val mockWebServer = MockWebServer() @@ -1096,6 +1105,142 @@ internal class ConfidenceRemoteClientTests { assertEquals(Result.Success(Unit), result) } + @Test + fun testCancelledResolveDoesNotTrackTelemetry() = runTest { + val telemetry = Telemetry("", Telemetry.Library.CONFIDENCE, "") + mockWebServer.enqueue( + MockResponse() + .setResponseCode(200) + .setBodyDelay(2, TimeUnit.SECONDS) + .setBody("""{"resolvedFlags": [], "resolveToken": "t"}""") + ) + + val resolver = RemoteFlagResolver( + clientSecret = "", + region = ConfidenceRegion.EUROPE, + baseUrl = mockWebServer.url("/v1/flags:resolve"), + dispatcher = Dispatchers.IO, + httpClient = OkHttpClient(), + telemetry = telemetry + ) + + val job = async(Dispatchers.IO) { + resolver.resolve(listOf(), mapOf()) + } + + delay(200) + job.cancel() + + try { + job.await() + } catch (_: CancellationException) { + // expected + } + + delay(100) + assertNull( + "Cancelled resolve should not produce a telemetry trace", + telemetry.encodedHeaderValue() + ) + } + + @Test + fun testSuccessfulResolveTracksTelemetry() = runTest { + val tel = Telemetry("", Telemetry.Library.CONFIDENCE, "") + mockWebServer.enqueue( + MockResponse() + .setResponseCode(200) + .setBody("""{"resolvedFlags": [], "resolveToken": "t"}""") + ) + + val resolver = RemoteFlagResolver( + clientSecret = "", + region = ConfidenceRegion.EUROPE, + baseUrl = mockWebServer.url("/v1/flags:resolve"), + dispatcher = Dispatchers.IO, + httpClient = OkHttpClient(), + telemetry = tel + ) + + resolver.resolve(listOf(), mapOf()) + + val headerValue = tel.encodedHeaderValue() + assertNotNull("Successful resolve should produce a telemetry trace", headerValue) + val monitoring = Monitoring.parseFrom(java.util.Base64.getDecoder().decode(headerValue!!)) + val trace = monitoring.getLibraryTraces(0).getTraces(0) + assertEquals(ProtoStatus.STATUS_SUCCESS, trace.requestTrace.status) + } + + @Test + fun testFailedResolveTracksTelemetryAsError() = runTest { + val tel = Telemetry("", Telemetry.Library.CONFIDENCE, "") + mockWebServer.enqueue( + MockResponse().setResponseCode(500) + ) + + val resolver = RemoteFlagResolver( + clientSecret = "", + region = ConfidenceRegion.EUROPE, + baseUrl = mockWebServer.url("/v1/flags:resolve"), + dispatcher = Dispatchers.IO, + httpClient = OkHttpClient(), + telemetry = tel + ) + + try { + resolver.resolve(listOf(), mapOf()) + } catch (_: ConfidenceError.HttpError) { + // expected + } + + val headerValue = tel.encodedHeaderValue() + assertNotNull("Failed resolve should produce a telemetry trace", headerValue) + val monitoring = Monitoring.parseFrom(java.util.Base64.getDecoder().decode(headerValue!!)) + val trace = monitoring.getLibraryTraces(0).getTraces(0) + assertEquals(ProtoStatus.STATUS_ERROR, trace.requestTrace.status) + } + + @Test + fun testOfflineResolveTracksTelemetryAsOffline() = runTest { + val tel = Telemetry("", Telemetry.Library.CONFIDENCE, "") + + val resolver = RemoteFlagResolver( + clientSecret = "", + region = ConfidenceRegion.EUROPE, + baseUrl = okhttp3.HttpUrl.Builder() + .scheme("http") + .host("host.invalid") + .port(1) + .build(), + dispatcher = Dispatchers.IO, + httpClient = OkHttpClient.Builder() + .dns(object : okhttp3.Dns { + override fun lookup(hostname: String): List { + throw java.net.UnknownHostException(hostname) + } + }) + .build(), + telemetry = tel + ) + + try { + resolver.resolve(listOf(), mapOf()) + } catch (_: java.net.UnknownHostException) { + // expected + } + + val headerValue = tel.encodedHeaderValue() + assertNotNull( + "Offline resolve should produce a telemetry trace", + headerValue + ) + + val bytes = java.util.Base64.getDecoder().decode(headerValue!!) + val monitoring = Monitoring.parseFrom(bytes) + val trace = monitoring.getLibraryTraces(0).tracesList.first() + assertEquals(ProtoStatus.STATUS_OFFLINE, trace.requestTrace.status) + } + @Test fun testApplyReturnsFailureAfter500() = runTest { val testDispatcher = UnconfinedTestDispatcher(testScheduler) diff --git a/Confidence/src/test/java/com/spotify/confidence/TelemetryTest.kt b/Confidence/src/test/java/com/spotify/confidence/TelemetryTest.kt index b45bed55..d70ac831 100644 --- a/Confidence/src/test/java/com/spotify/confidence/TelemetryTest.kt +++ b/Confidence/src/test/java/com/spotify/confidence/TelemetryTest.kt @@ -29,7 +29,8 @@ import com.spotify.telemetry.v1.Types.LibraryTraces.Trace.EvaluationTrace.Evalua import com.spotify.telemetry.v1.Types.LibraryTraces.Trace.RequestTrace.Status as ProtoStatus import com.spotify.telemetry.v1.Types.Platform as ProtoPlatform -private fun decodeMonitoring(headerValue: String): Monitoring { +private fun decodeMonitoring(headerValue: String?): Monitoring { + requireNotNull(headerValue) { "Expected non-null telemetry header value" } val bytes = java.util.Base64.getDecoder().decode(headerValue) return Monitoring.parseFrom(bytes) } diff --git a/Confidence/src/test/proto/confidence/telemetry/v1/types.proto b/Confidence/src/test/proto/confidence/telemetry/v1/types.proto index 851db7b8..d72916e8 100644 --- a/Confidence/src/test/proto/confidence/telemetry/v1/types.proto +++ b/Confidence/src/test/proto/confidence/telemetry/v1/types.proto @@ -50,6 +50,7 @@ message LibraryTraces { STATUS_ERROR = 2; STATUS_TIMEOUT = 3; STATUS_CACHED = 4; + STATUS_OFFLINE = 5; } }