Fix sets counters for telemetry (#546)

splitio · Oct 18, 2023 · 2d1147d · 2d1147d
1 parent f3029a6
commit 2d1147d
Show file tree

Hide file tree

Showing 7 changed files with 39 additions and 21 deletions.
diff --git a/src/androidTest/java/tests/integration/streaming/ControlTest.java b/src/androidTest/java/tests/integration/streaming/ControlTest.java
@@ -92,7 +92,8 @@ public void controlNotification() throws IOException, InterruptedException {
 
 
         CountDownLatch readyLatch = new CountDownLatch(1);
-        TestingHelper.TestEventTask updateTask = TestingHelper.testTask(new CountDownLatch(1), "CONTROL notif update task");
+        CountDownLatch updateLatch = new CountDownLatch(3);
+        SplitEventTaskHelper updateTask = new SplitEventTaskHelper(updateLatch);
 
         HttpClientMock httpClientMock = new HttpClientMock(createBasicResponseDispatcher());
 
@@ -133,19 +134,17 @@ public void controlNotification() throws IOException, InterruptedException {
         pushControl("STREAMING_RESUMED");
         synchronizerSpy.stopPeriodicFetchLatch.await(10, TimeUnit.SECONDS);
 
-        updateTask.mLatch = new CountDownLatch(1);
         pushMySegmentsUpdatePayload("new_segment");
-        updateTask.mLatch.await(10, TimeUnit.SECONDS);
+        updateLatch.await(10, TimeUnit.SECONDS);
 
         String treatmentEnabled = mClient.getTreatment(splitName);
 
         //Enable streaming, push a new my segments payload update and check data again
-        updateTask.mLatch = new CountDownLatch(1);
+        updateLatch = new CountDownLatch(1);
         pushControl("STREAMING_DISABLED");
-        updateTask.mLatch.await(5, TimeUnit.SECONDS);
+        updateLatch.await(5, TimeUnit.SECONDS);
         pushMySegmentsUpdatePayload("new_segment");
-        sleep(1000);
-
+        updateLatch.await(5, TimeUnit.SECONDS);
         String treatmentDisabled = mClient.getTreatment(splitName);
 
         assertTrue(telemetryStorage.popStreamingEvents().stream().anyMatch(event -> {
@@ -154,7 +153,6 @@ public void controlNotification() throws IOException, InterruptedException {
             }
             return false;
         }));
-        assertEquals(1, telemetryStorage.popTokenRefreshes());
         Assert.assertEquals("on", treatmentReady);
         Assert.assertEquals("on", treatmentPaused);
         Assert.assertEquals("free", treatmentEnabled);

diff --git a/src/androidTest/java/tests/integration/telemetry/TelemetryIntegrationTest.java b/src/androidTest/java/tests/integration/telemetry/TelemetryIntegrationTest.java
@@ -45,7 +45,6 @@
 import io.split.android.client.telemetry.model.MethodLatencies;
 import io.split.android.client.telemetry.storage.TelemetryStorage;
 import io.split.android.client.utils.Json;
-import io.split.android.client.utils.logger.Logger;
 import okhttp3.mockwebserver.Dispatcher;
 import okhttp3.mockwebserver.MockResponse;
 import okhttp3.mockwebserver.MockWebServer;
@@ -295,8 +294,8 @@ public MockResponse dispatch(RecordedRequest request) {
         initializeClient(false, "a", "_b", "a", "a", "c", "d", "_d");
         metricsLatch.await(20, TimeUnit.SECONDS);
         String s = metricsPayload.get();
-        assertTrue(s.contains("\"fsT\":5"));
-        assertTrue(s.contains("\"fsI\":2"));
+        assertTrue(s.contains("\"fsI\":4"));
+        assertTrue(s.contains("\"fsT\":7"));
     }
 
     private void initializeClient(boolean streamingEnabled, String ... sets) {

diff --git a/src/main/java/io/split/android/client/SplitFilter.java b/src/main/java/io/split/android/client/SplitFilter.java
@@ -61,6 +61,7 @@ public int maxValuesCount() {
     private final SplitFilter.Type mType;
     private final List<String> mValues;
     private int mInvalidValueCount;
+    private int mTotalValueCount;
 
     public static SplitFilter byName(@NonNull List<String> values) {
         return new SplitFilter(Type.BY_NAME, values);
@@ -71,6 +72,9 @@ public static SplitFilter byPrefix(@NonNull List<String> values) {
     }
 
     public static SplitFilter bySet(@NonNull List<String> values) {
+        if (values == null) {
+            values = new ArrayList<>();
+        }
         return new SplitFilter(Type.BY_SET, values, new FlagSetsValidatorImpl());
     }
 
@@ -89,6 +93,7 @@ public static SplitFilter bySet(@NonNull List<String> values) {
         SplitFilterValidator.ValidationResult validationResult = validator.cleanup("SDK config", values);
         mValues = validationResult.getValues();
         mInvalidValueCount = validationResult.getInvalidValueCount();
+        mTotalValueCount = (values != null) ? values.size() - validationResult.getInvalidValueCount() : 0;
     }
 
     public Type getType() {
@@ -102,4 +107,8 @@ public List<String> getValues() {
     public int getInvalidValueCount() {
         return mInvalidValueCount;
     }
+
+    public int getTotalValueCount() {
+        return mTotalValueCount;
+    }
 }
diff --git a/src/main/java/io/split/android/client/SyncConfig.java b/src/main/java/io/split/android/client/SyncConfig.java
@@ -13,11 +13,13 @@ public class SyncConfig {
 
     private final List<SplitFilter> mFilters;
 
-    private int mInvalidValueCount = 0;
+    private final int mInvalidValueCount;
+    private final int mTotalValueCount;
 
-    private SyncConfig(List<SplitFilter> filters, int invalidValueCount) {
+    private SyncConfig(List<SplitFilter> filters, int invalidValueCount, int totalValueCount) {
         mFilters = filters;
         mInvalidValueCount = invalidValueCount;
+        mTotalValueCount = totalValueCount;
     }
 
     public List<SplitFilter> getFilters() {
@@ -28,13 +30,18 @@ public int getInvalidValueCount() {
         return mInvalidValueCount;
     }
 
+    public int getTotalValueCount() {
+        return mTotalValueCount;
+    }
+
     public static Builder builder() {
         return new Builder();
     }
 
     public static class Builder {
         private final List<SplitFilter> mBuilderFilters = new ArrayList<>();
         private int mInvalidValueCount = 0;
+        private int mTotalValueCount = 0;
         private final SplitValidator mSplitValidator = new SplitValidatorImpl();
 
         public SyncConfig build() {
@@ -53,7 +60,7 @@ public SyncConfig build() {
                     validatedFilters.add(new SplitFilter(filter.getType(), validatedValues));
                 }
             }
-            return new SyncConfig(validatedFilters, mInvalidValueCount);
+            return new SyncConfig(validatedFilters, mInvalidValueCount, mTotalValueCount);
         }
 
         public Builder addSplitFilter(@NonNull SplitFilter filter) {
@@ -62,6 +69,7 @@ public Builder addSplitFilter(@NonNull SplitFilter filter) {
             }
             mBuilderFilters.add(filter);
             mInvalidValueCount += filter.getInvalidValueCount();
+            mTotalValueCount += filter.getTotalValueCount();
             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
@@ -200,14 +200,15 @@ public SplitInPlaceUpdateTask createSplitsUpdateTask(Split featureFlag, long sin
     @NonNull
     private TelemetryTaskFactory initializeTelemetryTaskFactory(@NonNull SplitClientConfig splitClientConfig, @Nullable Map<SplitFilter.Type, SplitFilter> filters, TelemetryStorage telemetryStorage) {
         final TelemetryTaskFactory mTelemetryTaskFactory;
-        int flagSetCount = 0;
         int invalidFlagSetCount = 0;
+        int totalFlagSetCount = 0;
         if (filters != null && !filters.isEmpty()) {
             SplitFilter bySetFilter = filters.get(SplitFilter.Type.BY_SET);
             if (bySetFilter != null) {
-                flagSetCount = bySetFilter.getValues().size();
-                invalidFlagSetCount = (splitClientConfig.syncConfig() != null) ?
-                        splitClientConfig.syncConfig().getInvalidValueCount() : 0;
+                if (splitClientConfig.syncConfig() != null) {
+                    invalidFlagSetCount = splitClientConfig.syncConfig().getInvalidValueCount();
+                    totalFlagSetCount = splitClientConfig.syncConfig().getTotalValueCount();
+                }
             }
         }
 
@@ -217,7 +218,7 @@ private TelemetryTaskFactory initializeTelemetryTaskFactory(@NonNull SplitClient
                 splitClientConfig,
                 mSplitsStorageContainer.getSplitsStorage(),
                 mSplitsStorageContainer.getMySegmentsStorageContainer(),
-                flagSetCount,
+                totalFlagSetCount,
                 invalidFlagSetCount);
         return mTelemetryTaskFactory;
     }

diff --git a/src/main/java/io/split/android/client/validators/FlagSetsValidatorImpl.java b/src/main/java/io/split/android/client/validators/FlagSetsValidatorImpl.java
@@ -47,7 +47,10 @@ public ValidationResult cleanup(String method, List<String> values) {
             }
 
             if (set.matches(FLAG_SET_REGEX)) {
-                cleanedUpSets.add(set);
+                if (!cleanedUpSets.add(set)) {
+                    Logger.w(method + ": you passed duplicated Flag Set. " + set + " was deduplicated");
+                    invalidValueCount++;
+                }
             } else {
                 invalidValueCount++;
                 Logger.w(method + ": you passed "+ set +", Flag Set must adhere to the regular expressions "+ FLAG_SET_REGEX +". This means a Flag Set must be start with a letter, be in lowercase, alphanumeric and have a max length of 50 characters. "+ set +" was discarded.");

diff --git a/src/test/java/io/split/android/client/validators/FlagSetsValidatorImplTest.java b/src/test/java/io/split/android/client/validators/FlagSetsValidatorImplTest.java
@@ -32,7 +32,7 @@ public void duplicatedInputValuesAreRemoved() {
         SplitFilterValidator.ValidationResult result = mValidator.cleanup("method", Arrays.asList("set1", "set1"));
         assertEquals(1, result.getValues().size());
         assertTrue(result.getValues().contains("set1"));
-        assertEquals(0, result.getInvalidValueCount());
+        assertEquals(1, result.getInvalidValueCount());
     }
 
     @Test