From f3fb46ae9c3cf76a06028fc5c61a31216b76e49d Mon Sep 17 00:00:00 2001 From: Weihao Ding <158090588+weihao-statsig@users.noreply.github.com> Date: Mon, 21 Oct 2024 11:30:19 -0700 Subject: [PATCH] refac: modify configSyncDetails (#396) the idea here is it's a bit odd for people who use this api like this: `configSyncDetails.getDetails().isConfigSpecReady();` so rather that wrap previous `initDetails`, create its own `configSyncDetails` --- src/main/kotlin/com/statsig/sdk/Evaluator.kt | 4 ++-- .../com/statsig/sdk/InitializationDetails.kt | 10 +++++++-- src/main/kotlin/com/statsig/sdk/SpecStore.kt | 12 +++++++++-- src/main/kotlin/com/statsig/sdk/Statsig.kt | 2 +- .../kotlin/com/statsig/sdk/StatsigServer.kt | 21 ++++++------------- .../statsig/sdk/StatsigReinitializeTest.java | 12 +++++------ 6 files changed, 33 insertions(+), 28 deletions(-) diff --git a/src/main/kotlin/com/statsig/sdk/Evaluator.kt b/src/main/kotlin/com/statsig/sdk/Evaluator.kt index 7a2ef72..a5bea9b 100644 --- a/src/main/kotlin/com/statsig/sdk/Evaluator.kt +++ b/src/main/kotlin/com/statsig/sdk/Evaluator.kt @@ -83,9 +83,9 @@ internal class Evaluator( specStore.shutdown() } - suspend fun syncConfigSpecs(): FailureDetails? { + suspend fun syncConfigSpecs(): ConfigSyncDetails { if (!isInitialized) { - return FailureDetails(FailureReason.EMPTY_SPEC) + return ConfigSyncDetails(0L, false, FailureDetails(FailureReason.EMPTY_SPEC), 0L) } return specStore.syncConfigSpecs() } diff --git a/src/main/kotlin/com/statsig/sdk/InitializationDetails.kt b/src/main/kotlin/com/statsig/sdk/InitializationDetails.kt index 7afc612..ee8c8b9 100644 --- a/src/main/kotlin/com/statsig/sdk/InitializationDetails.kt +++ b/src/main/kotlin/com/statsig/sdk/InitializationDetails.kt @@ -14,8 +14,14 @@ data class InitializationDetails( ) data class ConfigSyncDetails( - @SerializedName("details") - var details: InitializationDetails + @SerializedName("duration") + var duration: Long, + @SerializedName("configSpecReady") + var configSpecReady: Boolean, + @SerializedName("failureDetails") + var failureDetails: FailureDetails? = null, + @SerializedName("lcut") + var lcut: Long? = null, ) data class FailureDetails( diff --git a/src/main/kotlin/com/statsig/sdk/SpecStore.kt b/src/main/kotlin/com/statsig/sdk/SpecStore.kt index 4224209..708e96b 100644 --- a/src/main/kotlin/com/statsig/sdk/SpecStore.kt +++ b/src/main/kotlin/com/statsig/sdk/SpecStore.kt @@ -67,8 +67,16 @@ internal class SpecStore( this.specUpdater.shutdown() } - suspend fun syncConfigSpecs(): FailureDetails? { - return initializeSpecs() + suspend fun syncConfigSpecs(): ConfigSyncDetails { + val startTime = System.currentTimeMillis() + val failureDetails = initializeSpecs() + val endTime = System.currentTimeMillis() + return ConfigSyncDetails( + duration = endTime - startTime, + configSpecReady = failureDetails == null, + failureDetails, + specUpdater.lastUpdateTime + ) } fun setDownloadedConfigs(downloadedConfig: APIDownloadedConfigs, isFromBootstrap: Boolean = false): Boolean { diff --git a/src/main/kotlin/com/statsig/sdk/Statsig.kt b/src/main/kotlin/com/statsig/sdk/Statsig.kt index 52eed36..16774e6 100644 --- a/src/main/kotlin/com/statsig/sdk/Statsig.kt +++ b/src/main/kotlin/com/statsig/sdk/Statsig.kt @@ -790,7 +790,7 @@ class Statsig { @JvmStatic fun syncConfigSpecs(): CompletableFuture { if (!checkInitialized()) { - return CompletableFuture.completedFuture(ConfigSyncDetails(InitializationDetails(0, false, false))) + return CompletableFuture.completedFuture(ConfigSyncDetails(0, false, null)) } return statsigServer.syncConfigSpecs() } diff --git a/src/main/kotlin/com/statsig/sdk/StatsigServer.kt b/src/main/kotlin/com/statsig/sdk/StatsigServer.kt index 0ce0708..66534bd 100644 --- a/src/main/kotlin/com/statsig/sdk/StatsigServer.kt +++ b/src/main/kotlin/com/statsig/sdk/StatsigServer.kt @@ -586,25 +586,16 @@ private class StatsigServerImpl() : } override fun syncConfigSpecs(): CompletableFuture { + val startTime = System.currentTimeMillis() return statsigScope.future { errorBoundary.capture("syncConfigSpecs", { - val failureDetails = evaluator.syncConfigSpecs() - ConfigSyncDetails( - InitializationDetails( - System.currentTimeMillis() - setupStartTime, - isSDKReady = isSDKInitialized(), - configSpecReady = failureDetails == null, - failureDetails, - ) - ) + evaluator.syncConfigSpecs() }, { ConfigSyncDetails( - InitializationDetails( - System.currentTimeMillis() - setupStartTime, - isSDKReady = isSDKInitialized(), - configSpecReady = false, - FailureDetails(FailureReason.INTERNAL_ERROR), - ) + duration = System.currentTimeMillis() - startTime, + configSpecReady = false, + FailureDetails(FailureReason.INTERNAL_ERROR), + lcut = 0L, ) }) } diff --git a/src/test/java/com/statsig/sdk/StatsigReinitializeTest.java b/src/test/java/com/statsig/sdk/StatsigReinitializeTest.java index b326de8..95cb241 100644 --- a/src/test/java/com/statsig/sdk/StatsigReinitializeTest.java +++ b/src/test/java/com/statsig/sdk/StatsigReinitializeTest.java @@ -47,7 +47,7 @@ public MockResponse dispatch(@NotNull RecordedRequest request) { } }); - server.start(1080); + server.start(9874); options = new StatsigOptions(); options.setApi(server.url("/v1").toString()); @@ -80,8 +80,8 @@ public void testReinitialize() throws Exception { CompletableFuture details2Future = Statsig.syncConfigSpecs(); ConfigSyncDetails details2 = details2Future.get(); Assert.assertNotNull(details2); - Assert.assertTrue(details2.getDetails().isSDKReady()); - Assert.assertFalse(details2.getDetails().getConfigSpecReady()); // Second try should also fail + Assert.assertFalse(details2.getConfigSpecReady()); // Second try should also fail + Assert.assertNotNull(details2.getLcut()); Assert.assertFalse(Statsig.checkGateSync(user, "always_on_gate")); Assert.assertFalse(Statsig.checkGateSync(user, "on_for_statsig_email")); Layer layer2 = Statsig.getLayerSync(user, "c_layer_with_holdout"); @@ -90,9 +90,9 @@ public void testReinitialize() throws Exception { CompletableFuture details3Future = Statsig.syncConfigSpecs(); ConfigSyncDetails details3 = details3Future.get(); Assert.assertNotNull(details3); - Assert.assertTrue(details3.getDetails().isSDKReady()); - Assert.assertTrue(details3.getDetails().getConfigSpecReady()); // Third try should succeed with 200 response - Assert.assertNull(details3.getDetails().getFailureDetails()); // No failure details expected for successful init + Assert.assertTrue(details3.getConfigSpecReady()); // Third try should succeed with 200 response + Assert.assertNull(details3.getFailureDetails()); // No failure details expected for successful init + Assert.assertNotNull(details3.getLcut()); Assert.assertTrue(Statsig.checkGateSync(user, "always_on_gate")); Assert.assertTrue(Statsig.checkGateSync(user, "on_for_statsig_email")); Layer layer3 = Statsig.getLayerSync(user, "c_layer_with_holdout");