Merge pull request #921 from IABTechLab/llp-UID2-937-log-sdk-version-…

…metrics Start collecting per-site stats of client versions.
IABTechLab · Sep 20, 2024 · ccf665d · ccf665d
2 parents effef5f + 6df8f28
commit ccf665d
Show file tree

Hide file tree

Showing 12 changed files with 248 additions and 212 deletions.
diff --git a/src/main/java/com/uid2/operator/Const.java b/src/main/java/com/uid2/operator/Const.java
@@ -28,5 +28,6 @@ public class Config extends com.uid2.shared.Const.Config {
         public static final String OptOutStatusApiEnabled = "optout_status_api_enabled";
         public static final String OptOutStatusMaxRequestSize = "optout_status_max_request_size";
         public static final String MaxInvalidPaths = "logging_limit_max_invalid_paths_per_interval";
+        public static final String MaxVersionBucketsPerSite = "logging_limit_max_version_buckets_per_site";
     }
 }
diff --git a/src/main/java/com/uid2/operator/Main.java b/src/main/java/com/uid2/operator/Main.java
@@ -365,7 +365,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));
+        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
@@ -5,15 +5,18 @@ public class StatsCollectorMessageItem {
     private String referer;
     private String apiContact;
     private Integer siteId;
+    private String clientVersion;
 
     //USED by json serial
-    public StatsCollectorMessageItem(){}
+    public StatsCollectorMessageItem() {
+    }
 
-    public StatsCollectorMessageItem(String path, String referer, String apiContact, Integer siteId){
+    public StatsCollectorMessageItem(String path, String referer, String apiContact, Integer siteId, String clientVersion) {
         this.path = path;
         this.referer = referer;
         this.apiContact = apiContact;
         this.siteId = siteId;
+        this.clientVersion = clientVersion;
     }
 
 
@@ -48,4 +51,12 @@ public Integer getSiteId() {
     public void setSiteId(Integer siteId) {
         this.siteId = siteId;
     }
+
+    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
@@ -0,0 +1,52 @@
+package com.uid2.operator.monitoring;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.stream.Stream;
+
+public class ClientVersionStatRecorder {
+    private static final String NOT_RECORDED = "<Not recorded>";
+    private final int siteClientBucketLimit;
+    private final Map<Integer, Map<String, Integer>> siteIdToVersionCounts = new HashMap<>();
+
+    public ClientVersionStatRecorder(int maxVersionBucketsPerSite) {
+        this.siteClientBucketLimit = maxVersionBucketsPerSite;
+    }
+
+    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;
+        }
+
+        // Remove 3 items to avoid a couple of new version values from continuously evicting each other
+        var lowestEntries = versionCounts.entrySet().stream()
+                .sorted(Map.Entry.comparingByValue())
+                .filter(entry -> !entry.getKey().equals(NOT_RECORDED))
+                .limit(3)
+                .toList();
+        for (var entry : lowestEntries) {
+            var notRecordedCount = versionCounts.getOrDefault(NOT_RECORDED, 0);
+            versionCounts.put(NOT_RECORDED, notRecordedCount + entry.getValue());
+            versionCounts.remove(entry.getKey());
+        }
+    }
+
+    public void add(Integer siteId, String clientVersion) {
+        if (siteId == null || clientVersion == null || clientVersion.isBlank()) {
+            return;
+        }
+
+        var clientVersionCounts = siteIdToVersionCounts.computeIfAbsent(siteId, k -> new HashMap<>());
+
+        var count = clientVersionCounts.getOrDefault(clientVersion, 0);
+        if (count == 0 && clientVersionCounts.size() >= siteClientBucketLimit) {
+            removeLowVersionCounts(siteId);
+        }
+        clientVersionCounts.put(clientVersion, count + 1);
+    }
+}
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
@@ -0,0 +1,24 @@
+package com.uid2.operator.monitoring;
+
+import java.util.Map;
+import java.util.Objects;
+
+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/StatsCollectorHandler.java b/src/main/java/com/uid2/operator/monitoring/StatsCollectorHandler.java
@@ -1,6 +1,7 @@
 package com.uid2.operator.monitoring;
 
 import com.uid2.operator.model.StatsCollectorMessageItem;
+import com.uid2.shared.Const;
 import com.uid2.shared.auth.ClientKey;
 import com.uid2.shared.middleware.AuthMiddleware;
 import io.vertx.core.Handler;
@@ -33,10 +34,19 @@ private void addStatsMessageToQueue(RoutingContext routingContext) {
         final ClientKey clientKey = (ClientKey) AuthMiddleware.getAuthClient(routingContext);
         final String apiContact = clientKey == null ? null : clientKey.getContact();
         final Integer siteId = clientKey == null ? null : clientKey.getSiteId();
+        final String clientVersion = getClientVersion(routingContext);
 
-        final StatsCollectorMessageItem messageItem = new StatsCollectorMessageItem(path, referer, apiContact, siteId);
+        final StatsCollectorMessageItem messageItem = new StatsCollectorMessageItem(path, referer, apiContact, siteId, clientVersion);
 
         _statCollectorQueue.enqueue(vertx, messageItem);
     }
 
+    private String getClientVersion(RoutingContext routingContext) {
+        String clientVersion = routingContext.request().headers().get(Const.Http.ClientVersionHeader);
+        if (clientVersion == null) {
+            clientVersion =  !routingContext.queryParam("client").isEmpty() ? routingContext.queryParam("client").get(0) : null;
+        }
+        return clientVersion;
+    }
+
 }
diff --git a/src/main/java/com/uid2/operator/monitoring/StatsCollectorVerticle.java b/src/main/java/com/uid2/operator/monitoring/StatsCollectorVerticle.java
@@ -19,11 +19,14 @@
 import java.time.Instant;
 import java.util.*;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.stream.Stream;
 
 public class StatsCollectorVerticle extends AbstractVerticle implements IStatsCollectorQueue {
     private static final Logger LOGGER = LoggerFactory.getLogger(StatsCollectorVerticle.class);
     private HashMap<String, EndpointStat> pathMap;
 
+    private final ClientVersionStatRecorder clientVersionStat;
+
     private static final int MAX_AVAILABLE = 1000;
     private final int maxInvalidPaths;
 
@@ -41,7 +44,7 @@ public class StatsCollectorVerticle extends AbstractVerticle implements IStatsCo
     private final ObjectMapper mapper;
     private final Counter queueFullCounter;
 
-    public StatsCollectorVerticle(long jsonIntervalMS, int maxInvalidPaths) {
+    public StatsCollectorVerticle(long jsonIntervalMS, int maxInvalidPaths, int maxVersionBucketsPerSite) {
         pathMap = new HashMap<>();
 
         _statsCollectorCount = new AtomicInteger();
@@ -65,6 +68,7 @@ public StatsCollectorVerticle(long jsonIntervalMS, int maxInvalidPaths) {
                 .register(Metrics.globalRegistry);
 
         mapper = new ObjectMapper();
+        clientVersionStat = new ClientVersionStatRecorder(maxVersionBucketsPerSite);
     }
 
     @Override
@@ -121,6 +125,7 @@ public void handleMessage(Message message) {
             pathMap.merge(path, endpointStat, this::mergeEndpoint);
         }
 
+        clientVersionStat.add(siteId, messageItem.getClientVersion());
 
         _statsCollectorCount.decrementAndGet();
 
@@ -133,7 +138,7 @@ public void handleMessage(Message message) {
                 if(pathMap.size() == this.maxInvalidPaths + validPaths.size()) {
                     LOGGER.error("max invalid paths reached; a large number of invalid paths have been requested from authenticated participants");
                 }
-                Object[] stats = pathMap.values().toArray();
+                var stats = buildStatsList();
                 this.jsonSerializerExecutor.<Void>executeBlocking(
                         promise -> promise.complete(this.serializeToLogs(stats)),
                         res -> {
@@ -148,12 +153,12 @@ public void handleMessage(Message message) {
         }
     }
 
-    private Void serializeToLogs(Object[] stats) {
+    private Void serializeToLogs(List<ILoggedStat> stats) {
         LOGGER.debug("Starting JSON Serialize");
-        ObjectMapper mapper = new ObjectMapper();
-        for (Object stat : stats) {
+        ObjectMapper statMapper = new ObjectMapper();
+        for (var stat : stats) {
             try {
-                String jsonString = mapper.writeValueAsString(stat);
+                String jsonString = "%s%s".formatted(stat.GetLogPrefix(), statMapper.writeValueAsString(stat.GetValueToLog()));
                 LOGGER.info(jsonString);
             } catch (JsonProcessingException e) {
                 LOGGER.error(e.getMessage(), e);
@@ -167,19 +172,11 @@ private EndpointStat mergeEndpoint(EndpointStat a, EndpointStat b) {
         return a;
     }
 
-
-    public String getEndpointStats() {
-        Object[] stats = pathMap.values().toArray();
-        StringBuilder completeStats = new StringBuilder();
-        for (Object stat : stats) {
-            try {
-                String jsonString = mapper.writeValueAsString(stat);
-                completeStats.append(jsonString).append("\n");
-            } catch (JsonProcessingException e) {
-                LOGGER.error(e.getMessage(), e);
-            }
-        }
-        return completeStats.toString();
+    private List<ILoggedStat> buildStatsList() {
+        Stream<EndpointStat> pathMapStream = pathMap.values().stream();
+        Stream<ILoggedStat> clientVersionStream = clientVersionStat.getStatsView();
+        var stats = Stream.concat(pathMapStream, clientVersionStream);
+        return stats.toList();
     }
 
     @Override
@@ -225,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;
@@ -276,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/main/java/com/uid2/operator/vertx/ClientVersionCapturingHandler.java b/src/main/java/com/uid2/operator/vertx/ClientVersionCapturingHandler.java
diff --git a/src/main/java/com/uid2/operator/vertx/UIDOperatorVerticle.java b/src/main/java/com/uid2/operator/vertx/UIDOperatorVerticle.java
@@ -218,7 +218,6 @@ private Router createRoutesSetup() throws IOException {
 
         router.allowForward(AllowForwardHeaders.X_FORWARD);
         router.route().handler(new RequestCapturingHandler());
-        router.route().handler(new ClientVersionCapturingHandler("static/js", "*.js"));
         router.route().handler(CorsHandler.create()
                 .addRelativeOrigin(".*.")
                 .allowedMethod(io.vertx.core.http.HttpMethod.GET)