From d2d8800e284c52d6d2945bc70af6375cbbb29362 Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Wed, 13 Sep 2023 15:30:50 -0300 Subject: [PATCH 1/4] Track invalid values in SyncConfig --- .../java/io/split/android/client/SyncConfig.java | 12 ++++++++++-- .../service/executor/SplitTaskFactoryImpl.java | 2 +- .../java/io/split/android/client/SyncConfigTest.java | 12 ++++++++++++ 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/src/main/java/io/split/android/client/SyncConfig.java b/src/main/java/io/split/android/client/SyncConfig.java index c0bd4e5fe..9311d757a 100644 --- a/src/main/java/io/split/android/client/SyncConfig.java +++ b/src/main/java/io/split/android/client/SyncConfig.java @@ -13,21 +13,28 @@ public class SyncConfig { private final List mFilters; + private int mInvalidValueCount = 0; - private SyncConfig(List filters) { + private SyncConfig(List filters, int invalidValueCount) { mFilters = filters; + mInvalidValueCount = invalidValueCount; } public List getFilters() { return mFilters; } + public int getInvalidValueCount() { + return mInvalidValueCount; + } + public static Builder builder() { return new Builder(); } public static class Builder { private List mBuilderFilters = new ArrayList<>(); + private int mInvalidValueCount = 0; private final SplitValidator mSplitValidator = new SplitValidatorImpl(); public SyncConfig build() { @@ -46,7 +53,7 @@ public SyncConfig build() { validatedFilters.add(new SplitFilter(filter.getType(), validatedValues)); } } - return new SyncConfig(validatedFilters); + return new SyncConfig(validatedFilters, mInvalidValueCount); } public Builder addSplitFilter(@NonNull SplitFilter filter) { @@ -54,6 +61,7 @@ public Builder addSplitFilter(@NonNull SplitFilter filter) { throw new IllegalArgumentException("Filter can't be null"); } mBuilderFilters.add(filter); + mInvalidValueCount += filter.getInvalidValueCount(); return this; } } diff --git a/src/main/java/io/split/android/client/service/executor/SplitTaskFactoryImpl.java b/src/main/java/io/split/android/client/service/executor/SplitTaskFactoryImpl.java index e91cd8c3a..d1b5a5382 100644 --- a/src/main/java/io/split/android/client/service/executor/SplitTaskFactoryImpl.java +++ b/src/main/java/io/split/android/client/service/executor/SplitTaskFactoryImpl.java @@ -204,7 +204,7 @@ private TelemetryTaskFactory initializeTelemetryTaskFactory(@NonNull SplitClient SplitFilter bySetFilter = filters.get(SplitFilter.Type.BY_SET); if (bySetFilter != null) { flagSetCount = bySetFilter.getValues().size(); - invalidFlagSetCount = bySetFilter.getInvalidValueCount(); + invalidFlagSetCount = splitClientConfig.syncConfig().getInvalidValueCount(); } } diff --git a/src/test/java/io/split/android/client/SyncConfigTest.java b/src/test/java/io/split/android/client/SyncConfigTest.java index d46b094c4..2356aeb4a 100644 --- a/src/test/java/io/split/android/client/SyncConfigTest.java +++ b/src/test/java/io/split/android/client/SyncConfigTest.java @@ -162,4 +162,16 @@ public void addingNullFilterToConfig() { Assert.assertTrue(exceptionThrown); Assert.assertNull(config); } + + @Test + public void invalidValuesAreTracked() { + // Currently only invalid values for {@link SplitFilter#BY_SET} are tracked, for telemetry + + SyncConfig config = SyncConfig.builder() + .addSplitFilter(SplitFilter.bySet(Arrays.asList("_f1", "f2", "f3"))) + .addSplitFilter(SplitFilter.bySet(Arrays.asList("f4", "_f5", "_f6", "_f6"))) + .build(); + + Assert.assertEquals(4, config.getInvalidValueCount()); + } } From 67f14d65f14d01a2c53b1ac94b397eefbb93f727 Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Wed, 13 Sep 2023 15:33:26 -0300 Subject: [PATCH 2/4] Sets in telemetry config test --- .../telemetry/TelemetryIntegrationTest.java | 65 +++++++++++++++++-- 1 file changed, 59 insertions(+), 6 deletions(-) diff --git a/src/androidTest/java/tests/integration/telemetry/TelemetryIntegrationTest.java b/src/androidTest/java/tests/integration/telemetry/TelemetryIntegrationTest.java index 414c3df83..1f9a132e7 100644 --- a/src/androidTest/java/tests/integration/telemetry/TelemetryIntegrationTest.java +++ b/src/androidTest/java/tests/integration/telemetry/TelemetryIntegrationTest.java @@ -11,6 +11,7 @@ import org.junit.Before; import org.junit.Test; +import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.concurrent.CountDownLatch; @@ -28,6 +29,8 @@ import io.split.android.client.SplitClient; import io.split.android.client.SplitClientConfig; import io.split.android.client.SplitFactory; +import io.split.android.client.SplitFilter; +import io.split.android.client.SyncConfig; import io.split.android.client.api.Key; import io.split.android.client.dtos.Split; import io.split.android.client.dtos.SplitChange; @@ -195,11 +198,54 @@ public void recordSessionLength() throws InterruptedException { assertTrue(sessionLength > 0); } - private void initializeClient(boolean streamingEnabled) { + @Test + public void flagSetsAreIncludedInPayload() throws InterruptedException { + CountDownLatch sseLatch = new CountDownLatch(1); + CountDownLatch metricsLatch = new CountDownLatch(2); + AtomicReference metricsPayload = new AtomicReference<>(); + final Dispatcher dispatcher = new Dispatcher() { + + @Override + public MockResponse dispatch(RecordedRequest request) { + String path = request.getPath(); + if (path.contains("/mySegments")) { + return new MockResponse().setResponseCode(200).setBody("{\"mySegments\":[{ \"id\":\"id1\", \"name\":\"segment1\"}, { \"id\":\"id1\", \"name\":\"segment2\"}]}"); + } else if (path.contains("/splitChanges")) { + long changeNumber = -1; + return new MockResponse().setResponseCode(200) + .setBody("{\"splits\":[], \"since\":" + changeNumber + ", \"till\":" + (changeNumber + 1000) + "}"); + } else if (path.contains("/events/bulk")) { + return new MockResponse().setResponseCode(200); + } else if (path.contains("metrics/usage")) { + metricsLatch.countDown(); + return new MockResponse().setResponseCode(200); + } else if (path.contains("metrics")) { + metricsPayload.set(request.getBody().readUtf8()); + return new MockResponse().setResponseCode(200); + } else if (path.contains("auth")) { + sseLatch.countDown(); + return new MockResponse().setResponseCode(401); + } else { + return new MockResponse().setResponseCode(404); + } + } + }; + + mWebServer.setDispatcher(dispatcher); + + initializeClient(false, "a", "_b", "a", "a", "c", "d", "_d"); + metricsLatch.await(20, TimeUnit.SECONDS); + String s = metricsPayload.get(); + Logger.w("Metrics payload: " + s); + assertTrue(s.contains("\"fsT\":5")); + assertTrue(s.contains("\"fsI\":2")); + } + + private void initializeClient(boolean streamingEnabled, String ... sets) { insertSplitsFromFileIntoDB(); CountDownLatch countDownLatch = new CountDownLatch(1); - client = getTelemetrySplitFactory(mWebServer, streamingEnabled).client(); + client = getTelemetrySplitFactory(mWebServer, streamingEnabled, sets).client(); TestingHelper.TestEventTask readyFromCacheTask = new TestingHelper.TestEventTask(countDownLatch); client.on(SplitEvent.SDK_READY, readyFromCacheTask); @@ -211,7 +257,7 @@ private void initializeClient(boolean streamingEnabled) { } } - private SplitFactory getTelemetrySplitFactory(MockWebServer webServer, boolean streamingEnabled) { + private SplitFactory getTelemetrySplitFactory(MockWebServer webServer, boolean streamingEnabled, String... sets) { final String url = webServer.url("/").url().toString(); ServiceEndpoints endpoints = ServiceEndpoints.builder() .eventsEndpoint(url) @@ -219,7 +265,7 @@ private SplitFactory getTelemetrySplitFactory(MockWebServer webServer, boolean s .sseAuthServiceEndpoint(url) .apiEndpoint(url).eventsEndpoint(url).build(); - SplitClientConfig config = new TestableSplitConfigBuilder() + TestableSplitConfigBuilder builder = new TestableSplitConfigBuilder() .serviceEndpoints(endpoints) .enableDebug() .telemetryRefreshRate(10) @@ -228,8 +274,15 @@ private SplitFactory getTelemetrySplitFactory(MockWebServer webServer, boolean s .impressionsRefreshRate(9999) .readTimeout(3000) .streamingEnabled(streamingEnabled) - .shouldRecordTelemetry(true) - .build(); + .shouldRecordTelemetry(true); + + if (sets != null && sets.length > 0) { + builder.syncConfig(SyncConfig.builder() + .addSplitFilter(SplitFilter.bySet(Arrays.asList(sets))) + .build()); + } + + SplitClientConfig config = builder.build(); mTelemetryStorage = StorageFactory.getTelemetryStorage(true); return IntegrationHelper.buildFactory( From 3b3caa69e429ce75bebabfe201876d644fd94834 Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Wed, 13 Sep 2023 16:10:03 -0300 Subject: [PATCH 3/4] Evaluation with flags tests in telemetry; --- .../telemetry/TelemetryIntegrationTest.java | 62 ++++++++++++++++++- 1 file changed, 60 insertions(+), 2 deletions(-) diff --git a/src/androidTest/java/tests/integration/telemetry/TelemetryIntegrationTest.java b/src/androidTest/java/tests/integration/telemetry/TelemetryIntegrationTest.java index 1f9a132e7..62cb20e86 100644 --- a/src/androidTest/java/tests/integration/telemetry/TelemetryIntegrationTest.java +++ b/src/androidTest/java/tests/integration/telemetry/TelemetryIntegrationTest.java @@ -2,6 +2,7 @@ import static junit.framework.TestCase.assertEquals; import static junit.framework.TestCase.assertFalse; +import static junit.framework.TestCase.assertNotNull; import static junit.framework.TestCase.assertTrue; import android.content.Context; @@ -82,12 +83,16 @@ public void telemetryInitTest() { @Test public void telemetryEvaluationLatencyTest() { - initializeClient(false); + initializeClient(false, "a", "b"); client.getTreatment("test_split"); client.getTreatments(Arrays.asList("test_split", "test_split_2"), null); client.getTreatmentWithConfig("test_split", null); client.getTreatmentsWithConfig(Arrays.asList("test_split", "test_split_2"), null); client.track("test_traffic_type", "test_split"); + client.getTreatmentsByFlagSet("a", null); + client.getTreatmentsByFlagSets(Arrays.asList("a", "b"), null); + client.getTreatmentsWithConfigByFlagSet("a", null); + client.getTreatmentsWithConfigByFlagSets(Arrays.asList("a", "b"), null); MethodLatencies methodLatencies = mTelemetryStorage.popLatencies(); assertFalse(methodLatencies.getTreatment().stream().allMatch(aLong -> aLong == 0L)); @@ -95,6 +100,60 @@ public void telemetryEvaluationLatencyTest() { assertFalse(methodLatencies.getTreatmentWithConfig().stream().allMatch(aLong -> aLong == 0L)); assertFalse(methodLatencies.getTreatmentsWithConfig().stream().allMatch(aLong -> aLong == 0L)); assertFalse(methodLatencies.getTrack().stream().allMatch(aLong -> aLong == 0L)); + assertFalse(methodLatencies.getTreatmentsByFlagSet().stream().allMatch(aLong -> aLong == 0L)); + assertFalse(methodLatencies.getTreatmentsByFlagSets().stream().allMatch(aLong -> aLong == 0L)); + assertFalse(methodLatencies.getTreatmentsWithConfigByFlagSet().stream().allMatch(aLong -> aLong == 0L)); + assertFalse(methodLatencies.getTreatmentsWithConfigByFlagSets().stream().allMatch(aLong -> aLong == 0L)); + } + + @Test + public void evaluationByFlagsInfoIsInPayload() throws InterruptedException { + CountDownLatch metricsLatch = new CountDownLatch(1); + AtomicReference metricsPayload = new AtomicReference<>(); + final Dispatcher dispatcher = new Dispatcher() { + + @Override + public MockResponse dispatch(RecordedRequest request) { + String path = request.getPath(); + if (path.contains("/mySegments")) { + return new MockResponse().setResponseCode(200).setBody("{\"mySegments\":[{ \"id\":\"id1\", \"name\":\"segment1\"}, { \"id\":\"id1\", \"name\":\"segment2\"}]}"); + } else if (path.contains("/splitChanges")) { + long changeNumber = -1; + return new MockResponse().setResponseCode(200) + .setBody("{\"splits\":[], \"since\":" + changeNumber + ", \"till\":" + (changeNumber + 1000) + "}"); + } else if (path.contains("/events/bulk")) { + return new MockResponse().setResponseCode(200); + } else if (path.contains("metrics/usage")) { + metricsPayload.set(request.getBody().readUtf8()); + metricsLatch.countDown(); + return new MockResponse().setResponseCode(200); + } else if (path.contains("metrics")) { + return new MockResponse().setResponseCode(200); + } else if (path.contains("auth")) { + return new MockResponse().setResponseCode(401); + } else { + return new MockResponse().setResponseCode(404); + } + } + }; + + mWebServer.setDispatcher(dispatcher); + + initializeClient(false, "a", "b"); + client.getTreatmentsByFlagSet("a", null); + client.getTreatmentsByFlagSets(Arrays.asList("a", "b"), null); + client.getTreatmentsWithConfigByFlagSet("a", null); + client.getTreatmentsWithConfigByFlagSets(Arrays.asList("a", "b"), null); + + boolean await = metricsLatch.await(10, TimeUnit.SECONDS); + + assertTrue(await); + assertTrue(metricsPayload.get().contains("\"tf\":[1,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0]")); + assertTrue(metricsPayload.get().contains("\"tfs\":[1,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0]")); + assertTrue(metricsPayload.get().contains("\"tcf\":[1,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0]")); + assertTrue(metricsPayload.get().contains("\"tcfs\":[1,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0]")); + assertTrue(metricsPayload.get().contains("\"tcf\":0,\"tcfs\":0")); + assertTrue(metricsPayload.get().contains("\"tf\":0,\"tfs\":0")); } @Test @@ -236,7 +295,6 @@ public MockResponse dispatch(RecordedRequest request) { initializeClient(false, "a", "_b", "a", "a", "c", "d", "_d"); metricsLatch.await(20, TimeUnit.SECONDS); String s = metricsPayload.get(); - Logger.w("Metrics payload: " + s); assertTrue(s.contains("\"fsT\":5")); assertTrue(s.contains("\"fsI\":2")); } From bf8bca2e93cacaf71d62414e636c69e21ebf9bc7 Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Wed, 13 Sep 2023 16:17:40 -0300 Subject: [PATCH 4/4] Extra safety check --- .../android/client/service/executor/SplitTaskFactoryImpl.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/io/split/android/client/service/executor/SplitTaskFactoryImpl.java b/src/main/java/io/split/android/client/service/executor/SplitTaskFactoryImpl.java index d1b5a5382..1341f4d5f 100644 --- a/src/main/java/io/split/android/client/service/executor/SplitTaskFactoryImpl.java +++ b/src/main/java/io/split/android/client/service/executor/SplitTaskFactoryImpl.java @@ -204,7 +204,8 @@ private TelemetryTaskFactory initializeTelemetryTaskFactory(@NonNull SplitClient SplitFilter bySetFilter = filters.get(SplitFilter.Type.BY_SET); if (bySetFilter != null) { flagSetCount = bySetFilter.getValues().size(); - invalidFlagSetCount = splitClientConfig.syncConfig().getInvalidValueCount(); + invalidFlagSetCount = (splitClientConfig.syncConfig() != null) ? + splitClientConfig.syncConfig().getInvalidValueCount() : 0; } }