Fix bad config in StatsCollectorVerticle instantiation.

Change format of the client version stat string. Made client version stats recorder ignore blank versions.
IABTechLab · Sep 19, 2024 · 6df8f28 · 6df8f28
1 parent a0dd91f
commit 6df8f28
Show file tree

Hide file tree

Showing 7 changed files with 61 additions and 19 deletions.
diff --git a/src/main/java/com/uid2/operator/Main.java b/src/main/java/com/uid2/operator/Main.java
@@ -364,7 +364,7 @@ private Future<String> createAndDeployCloudSyncStoreVerticle(String name, ICloud
 
     private Future<String> createAndDeployStatsCollector() {
         Promise<String> promise = Promise.promise();
-        StatsCollectorVerticle statsCollectorVerticle = new StatsCollectorVerticle(60000, config.getInteger(Const.Config.MaxInvalidPaths, 50), config.getInteger(Const.Config.MaxInvalidPaths, config.getInteger(Const.Config.MaxVersionBucketsPerSite, 50)));
+        StatsCollectorVerticle statsCollectorVerticle = new StatsCollectorVerticle(60000, config.getInteger(Const.Config.MaxInvalidPaths, 50), config.getInteger(Const.Config.MaxVersionBucketsPerSite, 50));
         vertx.deployVerticle(statsCollectorVerticle, promise);
         _statsCollectorQueue = statsCollectorVerticle;
         return promise.future();

diff --git a/src/main/java/com/uid2/operator/model/StatsCollectorMessageItem.java b/src/main/java/com/uid2/operator/model/StatsCollectorMessageItem.java
@@ -8,7 +8,8 @@ public class StatsCollectorMessageItem {
     private String clientVersion;
 
     //USED by json serial
-    public StatsCollectorMessageItem(){}
+    public StatsCollectorMessageItem() {
+    }
 
     public StatsCollectorMessageItem(String path, String referer, String apiContact, Integer siteId, String clientVersion) {
         this.path = path;
@@ -51,6 +52,11 @@ public void setSiteId(Integer siteId) {
         this.siteId = siteId;
     }
 
-    public String getClientVersion() { return clientVersion; }
-    public void setClientVersion(String clientVersion) { this.clientVersion = clientVersion; }
+    public String getClientVersion() {
+        return clientVersion;
+    }
+
+    public void setClientVersion(String clientVersion) {
+        this.clientVersion = clientVersion;
+    }
 }
diff --git a/src/main/java/com/uid2/operator/monitoring/ClientVersionStatRecorder.java b/src/main/java/com/uid2/operator/monitoring/ClientVersionStatRecorder.java
@@ -13,13 +13,15 @@ public ClientVersionStatRecorder(int maxVersionBucketsPerSite) {
         this.siteClientBucketLimit = maxVersionBucketsPerSite;
     }
 
-    public Stream<SiteClientVersionStat> getStatsView() {
+    public Stream<ILoggedStat> getStatsView() {
         return siteIdToVersionCounts.entrySet().stream().map(entry -> new SiteClientVersionStat(entry.getKey(), entry.getValue()));
     }
 
     private void removeLowVersionCounts(int siteId) {
         var versionCounts = siteIdToVersionCounts.get(siteId);
-        if (versionCounts == null) return;
+        if (versionCounts == null) {
+            return;
+        }
 
         // Remove 3 items to avoid a couple of new version values from continuously evicting each other
         var lowestEntries = versionCounts.entrySet().stream()
@@ -35,7 +37,7 @@ private void removeLowVersionCounts(int siteId) {
     }
 
     public void add(Integer siteId, String clientVersion) {
-        if (siteId == null || clientVersion == null) {
+        if (siteId == null || clientVersion == null || clientVersion.isBlank()) {
             return;
         }
 

diff --git a/src/main/java/com/uid2/operator/monitoring/ILoggedStat.java b/src/main/java/com/uid2/operator/monitoring/ILoggedStat.java
@@ -0,0 +1,6 @@
+package com.uid2.operator.monitoring;
+
+public interface ILoggedStat {
+    public String GetLogPrefix();
+    public Object GetValueToLog();
+}
diff --git a/src/main/java/com/uid2/operator/monitoring/SiteClientVersionStat.java b/src/main/java/com/uid2/operator/monitoring/SiteClientVersionStat.java
@@ -1,6 +1,24 @@
 package com.uid2.operator.monitoring;
 
 import java.util.Map;
+import java.util.Objects;
 
-public record SiteClientVersionStat(Integer siteId, Map<String, Integer> versionCounts) {
+public final class SiteClientVersionStat implements ILoggedStat {
+    private final Integer siteId;
+    private final Map<String, Integer> versionCounts;
+
+    public SiteClientVersionStat(Integer siteId, Map<String, Integer> versionCounts) {
+        this.siteId = siteId;
+        this.versionCounts = versionCounts;
+    }
+
+    @Override
+    public String GetLogPrefix() {
+        return "version log; siteId=%d versions=".formatted(siteId);
+    }
+
+    @Override
+    public Object GetValueToLog() {
+        return versionCounts;
+    }
 }
diff --git a/src/main/java/com/uid2/operator/monitoring/StatsCollectorVerticle.java b/src/main/java/com/uid2/operator/monitoring/StatsCollectorVerticle.java
@@ -25,7 +25,7 @@ public class StatsCollectorVerticle extends AbstractVerticle implements IStatsCo
     private static final Logger LOGGER = LoggerFactory.getLogger(StatsCollectorVerticle.class);
     private HashMap<String, EndpointStat> pathMap;
 
-    private ClientVersionStatRecorder clientVersionStat;
+    private final ClientVersionStatRecorder clientVersionStat;
 
     private static final int MAX_AVAILABLE = 1000;
     private final int maxInvalidPaths;
@@ -153,12 +153,12 @@ public void handleMessage(Message message) {
         }
     }
 
-    private Void serializeToLogs(List<Object> stats) {
+    private Void serializeToLogs(List<ILoggedStat> stats) {
         LOGGER.debug("Starting JSON Serialize");
         ObjectMapper statMapper = new ObjectMapper();
-        for (Object stat : stats) {
+        for (var stat : stats) {
             try {
-                String jsonString = statMapper.writeValueAsString(stat);
+                String jsonString = "%s%s".formatted(stat.GetLogPrefix(), statMapper.writeValueAsString(stat.GetValueToLog()));
                 LOGGER.info(jsonString);
             } catch (JsonProcessingException e) {
                 LOGGER.error(e.getMessage(), e);
@@ -172,9 +172,9 @@ private EndpointStat mergeEndpoint(EndpointStat a, EndpointStat b) {
         return a;
     }
 
-    private List<Object> buildStatsList() {
-        Stream<?> pathMapStream = pathMap.values().stream();
-        Stream<?> clientVersionStream = clientVersionStat.getStatsView();
+    private List<ILoggedStat> buildStatsList() {
+        Stream<EndpointStat> pathMapStream = pathMap.values().stream();
+        Stream<ILoggedStat> clientVersionStream = clientVersionStat.getStatsView();
         var stats = Stream.concat(pathMapStream, clientVersionStream);
         return stats.toList();
     }
@@ -222,7 +222,7 @@ public void merge(DomainStat d) {
         }
     }
 
-    class EndpointStat {
+    class EndpointStat implements ILoggedStat {
         private final String endpoint;
         private final Integer siteId;
         private final String apiVersion;
@@ -273,6 +273,16 @@ else if(domainList.size() < MaxDomains) {
                 domainMissedCounter.increment();
             }
         }
+
+        @Override
+        public String GetLogPrefix() {
+            return "";
+        }
+
+        @Override
+        public Object GetValueToLog() {
+            return this;
+        }
     }
 }
 

diff --git a/src/test/java/com/uid2/operator/StatsCollectorVerticleTest.java b/src/test/java/com/uid2/operator/StatsCollectorVerticleTest.java
@@ -193,9 +193,9 @@ void clientVersionStats(Vertx vertx, VertxTestContext testContext) throws Interr
         triggerSerializeAndWait(testContext);
 
         var expectedLogs = List.of(
-                "{\"siteId\":1,\"versionCounts\":{\"uid2-sdk-3.0.01\":1,\"uid2-sdk-3.0.02\":1,\"uid2-sdk-3.0.00\":1}}",
-                "{\"siteId\":1,\"versionCounts\":{\"uid2-sdk-3.0.01\":2,\"uid2-sdk-3.0.02\":1,\"uid2-sdk-3.0.00\":1}}",
-                "{\"siteId\":2,\"versionCounts\":{\"<Not recorded>\":21,\"uid2-sdk-3.0.06\":7,\"uid2-sdk-3.0.07\":8,\"uid2-sdk-3.0.011\":12,\"uid2-sdk-3.0.08\":9,\"uid2-sdk-3.0.010\":11,\"uid2-sdk-3.0.09\":10,\"uid2-sdk-3.0.0single\":1}}"
+                "version log; siteId=1 versions={\"uid2-sdk-3.0.01\":1,\"uid2-sdk-3.0.02\":1,\"uid2-sdk-3.0.00\":1}",
+                "version log; siteId=1 versions={\"uid2-sdk-3.0.01\":2,\"uid2-sdk-3.0.02\":1,\"uid2-sdk-3.0.00\":1}",
+                "version log; siteId=2 versions={\"<Not recorded>\":21,\"uid2-sdk-3.0.06\":7,\"uid2-sdk-3.0.07\":8,\"uid2-sdk-3.0.011\":12,\"uid2-sdk-3.0.08\":9,\"uid2-sdk-3.0.010\":11,\"uid2-sdk-3.0.09\":10,\"uid2-sdk-3.0.0single\":1}"
         );
         var messages = getMessages();
         assertThat(messages).containsAll(expectedLogs);