From 360a5e2848ac2c43d9fb209d07c7390e94b438a9 Mon Sep 17 00:00:00 2001 From: Lionell Pack Date: Mon, 2 Sep 2024 17:57:17 +1000 Subject: [PATCH 01/10] Start collecting per-site stats of client versions. --- src/main/java/com/uid2/operator/Const.java | 1 + src/main/java/com/uid2/operator/Main.java | 2 +- .../model/StatsCollectorMessageItem.java | 7 +- .../monitoring/ClientVersionStatRecorder.java | 42 +++++ .../monitoring/SiteClientVersionStat.java | 6 + .../monitoring/StatsCollectorHandler.java | 11 +- .../monitoring/StatsCollectorVerticle.java | 32 ++-- .../vertx/ClientVersionCapturingHandler.java | 52 ------ .../operator/vertx/UIDOperatorVerticle.java | 1 - .../operator/StatsCollectorVerticleTest.java | 166 +++++++++++------- 10 files changed, 183 insertions(+), 137 deletions(-) create mode 100644 src/main/java/com/uid2/operator/monitoring/ClientVersionStatRecorder.java create mode 100644 src/main/java/com/uid2/operator/monitoring/SiteClientVersionStat.java delete mode 100644 src/main/java/com/uid2/operator/vertx/ClientVersionCapturingHandler.java diff --git a/src/main/java/com/uid2/operator/Const.java b/src/main/java/com/uid2/operator/Const.java index 9b9e23a24..bb51aed13 100644 --- a/src/main/java/com/uid2/operator/Const.java +++ b/src/main/java/com/uid2/operator/Const.java @@ -26,5 +26,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 index 37e06fbc0..34d948a6d 100644 --- a/src/main/java/com/uid2/operator/Main.java +++ b/src/main/java/com/uid2/operator/Main.java @@ -364,7 +364,7 @@ private Future createAndDeployCloudSyncStoreVerticle(String name, ICloud private Future createAndDeployStatsCollector() { Promise 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.MaxInvalidPaths, 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 index 409709911..0191386d3 100644 --- a/src/main/java/com/uid2/operator/model/StatsCollectorMessageItem.java +++ b/src/main/java/com/uid2/operator/model/StatsCollectorMessageItem.java @@ -5,15 +5,17 @@ public class StatsCollectorMessageItem { private String referer; private String apiContact; private Integer siteId; + private String clientVersion; //USED by json serial 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 +50,7 @@ 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 new file mode 100644 index 000000000..4d5c1371e --- /dev/null +++ b/src/main/java/com/uid2/operator/monitoring/ClientVersionStatRecorder.java @@ -0,0 +1,42 @@ +package com.uid2.operator.monitoring; + +import java.util.HashMap; +import java.util.Map; +import java.util.stream.Stream; + +public class ClientVersionStatRecorder { + private final int siteClientBucketLimit; + private final Map> clientVersionToSiteCount = new HashMap<>(); + + public ClientVersionStatRecorder(int maxVersionBucketsPerSite) { + this.siteClientBucketLimit = maxVersionBucketsPerSite; + } + + public Stream getStatsView() { + return clientVersionToSiteCount.entrySet().stream().map(entry -> new SiteClientVersionStat(entry.getKey(), entry.getValue())); + } + + public void clear() { + clientVersionToSiteCount.clear(); + } + + public void add(Integer siteId, String clientVersion) { + if (siteId == null || clientVersion == null) { + return; + } + + var clientVersionCounts = clientVersionToSiteCount.computeIfAbsent(siteId, k -> new HashMap<>()); + + var count = clientVersionCounts.get(clientVersion); + if (count == null && clientVersionCounts.size() >= siteClientBucketLimit) { + var notRecordedCount = clientVersionCounts.getOrDefault("NotRecorded", 0); + clientVersionCounts.put("NotRecorded", notRecordedCount + 1); + } + else if (count == null) { + clientVersionCounts.put(clientVersion, 1); + } + else { + clientVersionCounts.put(clientVersion, count + 1); + } + } +} diff --git a/src/main/java/com/uid2/operator/monitoring/SiteClientVersionStat.java b/src/main/java/com/uid2/operator/monitoring/SiteClientVersionStat.java new file mode 100644 index 000000000..747544941 --- /dev/null +++ b/src/main/java/com/uid2/operator/monitoring/SiteClientVersionStat.java @@ -0,0 +1,6 @@ +package com.uid2.operator.monitoring; + +import java.util.Map; + +public record SiteClientVersionStat(Integer siteId, Map versionCounts) { +} diff --git a/src/main/java/com/uid2/operator/monitoring/StatsCollectorHandler.java b/src/main/java/com/uid2/operator/monitoring/StatsCollectorHandler.java index c0b45dce3..f774def06 100644 --- 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; @@ -34,9 +35,17 @@ private void addStatsMessageToQueue(RoutingContext routingContext) { final String apiContact = clientKey == null ? null : clientKey.getContact(); final Integer siteId = clientKey == null ? null : clientKey.getSiteId(); - final StatsCollectorMessageItem messageItem = new StatsCollectorMessageItem(path, referer, apiContact, siteId); + final StatsCollectorMessageItem messageItem = new StatsCollectorMessageItem(path, referer, apiContact, siteId, getClientVersion(routingContext)); _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 index c764010b0..5c236461c 100644 --- 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 pathMap; + private 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.executeBlocking( promise -> promise.complete(this.serializeToLogs(stats)), res -> { @@ -144,13 +149,14 @@ public void handleMessage(Message message) { } ); pathMap.clear(); + clientVersionStat.clear(); } } } - private Void serializeToLogs(Object[] stats) { + private Void serializeToLogs(List stats) { LOGGER.debug("Starting JSON Serialize"); - ObjectMapper mapper = new ObjectMapper(); + ObjectMapper statMapper = new ObjectMapper(); for (Object stat : stats) { try { String jsonString = mapper.writeValueAsString(stat); @@ -167,19 +173,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 buildStatsList() { + Stream pathMapStream = pathMap.values().stream(); + Stream clientVersionStream = clientVersionStat.getStatsView(); + var stats = Stream.concat(pathMapStream, clientVersionStream); + return stats.toList(); } @Override diff --git a/src/main/java/com/uid2/operator/vertx/ClientVersionCapturingHandler.java b/src/main/java/com/uid2/operator/vertx/ClientVersionCapturingHandler.java deleted file mode 100644 index 2af4fb6b2..000000000 --- a/src/main/java/com/uid2/operator/vertx/ClientVersionCapturingHandler.java +++ /dev/null @@ -1,52 +0,0 @@ -package com.uid2.operator.vertx; - -import com.uid2.shared.Const; -import io.micrometer.core.instrument.Counter; -import io.micrometer.core.instrument.Metrics; -import io.vertx.core.Handler; -import io.vertx.ext.web.RoutingContext; - -import java.io.IOException; -import java.nio.file.DirectoryStream; -import java.nio.file.Files; -import java.nio.file.Path; -import java.nio.file.Paths; -import java.util.HashMap; -import java.util.Map; - -public class ClientVersionCapturingHandler implements Handler { - private final Map _clientVersionCounters = new HashMap<>(); - - public ClientVersionCapturingHandler(String dir, String whitelistGlob) throws IOException { - try (DirectoryStream dirStream = Files.newDirectoryStream(Paths.get(dir), whitelistGlob)) { - dirStream.forEach(path -> { - final String version = getFileNameWithoutExtension(path); - final Counter counter = Counter - .builder("uid2.client_sdk_versions") - .description("counter for how many http requests are processed per each client sdk version") - .tags("client_version", version) - .register(Metrics.globalRegistry); - _clientVersionCounters.put(version, counter); - }); - } - } - @Override - public void handle(RoutingContext context) { - String clientVersion = context.request().headers().get(Const.Http.ClientVersionHeader); - if (clientVersion == null) { - clientVersion = !context.queryParam("client").isEmpty() ? context.queryParam("client").get(0) : null; - } - if (clientVersion != null) { - final Counter counter = _clientVersionCounters.get(clientVersion); - if (counter != null) { - counter.increment(); - } - } - context.next(); - } - - private static String getFileNameWithoutExtension(Path path) { - final String fileName = path.getFileName().toString(); - return fileName.indexOf(".") > 0 ? fileName.substring(0, fileName.lastIndexOf(".")) : fileName; - } -} diff --git a/src/main/java/com/uid2/operator/vertx/UIDOperatorVerticle.java b/src/main/java/com/uid2/operator/vertx/UIDOperatorVerticle.java index 1c4996d36..23cbdf7ca 100644 --- 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) diff --git a/src/test/java/com/uid2/operator/StatsCollectorVerticleTest.java b/src/test/java/com/uid2/operator/StatsCollectorVerticleTest.java index ac594c44c..88cb51ca3 100644 --- a/src/test/java/com/uid2/operator/StatsCollectorVerticleTest.java +++ b/src/test/java/com/uid2/operator/StatsCollectorVerticleTest.java @@ -11,100 +11,120 @@ import io.vertx.core.Vertx; import io.vertx.junit5.VertxExtension; import io.vertx.junit5.VertxTestContext; +import static org.assertj.core.api.Assertions.*; + +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.slf4j.LoggerFactory; +import java.util.List; import java.util.Set; import java.util.concurrent.TimeUnit; +import java.util.stream.Collectors; @ExtendWith(VertxExtension.class) public class StatsCollectorVerticleTest { private static final int MAX_INVALID_PATHS = 5; - private StatsCollectorVerticle verticle; + private static final int MAX_CLIENT_VERSION_BUCKETS = 5; + private static final int JSON_INTERVAL = 200; + private static final int LOG_WAIT_INTERVAL = 50; + private static final String CLIENT_VERSION = "uid2-sdk-3.0.0"; + private final ObjectMapper mapper = new ObjectMapper(); + private ListAppender logWatcher; + private Vertx vertx; @BeforeEach void deployVerticle(Vertx vertx, VertxTestContext testContext) { - verticle = new StatsCollectorVerticle(1000, MAX_INVALID_PATHS); + this.vertx = vertx; + var verticle = new StatsCollectorVerticle(JSON_INTERVAL, MAX_INVALID_PATHS, MAX_CLIENT_VERSION_BUCKETS); vertx.deployVerticle(verticle, testContext.succeeding(id -> testContext.completeNow())); + + logWatcher = new ListAppender<>(); + logWatcher.start(); + ((Logger) LoggerFactory.getLogger(StatsCollectorVerticle.class)).addAppender(logWatcher); } + @AfterEach + void cleanupLogger() { + logWatcher.stop(); + ((Logger) LoggerFactory.getLogger(StatsCollectorVerticle.class)).detachAppender(logWatcher); + } @Test void verticleDeployed(Vertx vertx, VertxTestContext testContext) { testContext.completeNow(); } - @Test - void testJSONSerializeWithV0AndV1Paths(Vertx vertx, VertxTestContext testContext) throws InterruptedException, JsonProcessingException { - ObjectMapper mapper = new ObjectMapper(); - StatsCollectorMessageItem messageItem = new StatsCollectorMessageItem("/test", "https://test.com", "test", 1); - - vertx.eventBus().send(Const.Config.StatsCollectorEventBus, mapper.writeValueAsString(messageItem)); - vertx.eventBus().send(Const.Config.StatsCollectorEventBus, mapper.writeValueAsString(messageItem)); - vertx.eventBus().send(Const.Config.StatsCollectorEventBus, mapper.writeValueAsString(messageItem)); - - messageItem = new StatsCollectorMessageItem("/v1/test", "https://test.com", "test", 1); + private Set getMessages() { + return logWatcher.list.stream().map(ILoggingEvent::getFormattedMessage).collect(Collectors.toSet()); + } + private void sendStatMessage(StatsCollectorMessageItem messageItem) throws JsonProcessingException { vertx.eventBus().send(Const.Config.StatsCollectorEventBus, mapper.writeValueAsString(messageItem)); - vertx.eventBus().send(Const.Config.StatsCollectorEventBus, mapper.writeValueAsString(messageItem)); + } - testContext.awaitCompletion(2000, TimeUnit.MILLISECONDS); + @Test + void testJSONSerializeWithV0AndV1Paths(Vertx vertx, VertxTestContext testContext) throws InterruptedException, JsonProcessingException { + StatsCollectorMessageItem messageItem = new StatsCollectorMessageItem("/test", "https://test.com", "test", 1, CLIENT_VERSION); + sendStatMessage(messageItem); + sendStatMessage(messageItem); + sendStatMessage(messageItem); - String expected = - "{\"endpoint\":\"test\",\"siteId\":1,\"apiVersion\":\"v1\",\"domainList\":[{\"domain\":\"test.com\",\"count\":2,\"apiContact\":\"test\"}]}\n" - + "{\"endpoint\":\"test\",\"siteId\":1,\"apiVersion\":\"v0\",\"domainList\":[{\"domain\":\"test.com\",\"count\":3,\"apiContact\":\"test\"}]}\n"; + messageItem = new StatsCollectorMessageItem("/v1/test", "https://test.com", "test", 1, CLIENT_VERSION); + sendStatMessage(messageItem); + sendStatMessage(messageItem); + testContext.awaitCompletion(JSON_INTERVAL*2, TimeUnit.MILLISECONDS); - String results = verticle.getEndpointStats(); + StatsCollectorMessageItem triggerItem = new StatsCollectorMessageItem("/triggerSerialize", "https://test.com", "test", 1, CLIENT_VERSION); + sendStatMessage(triggerItem); + testContext.awaitCompletion(LOG_WAIT_INTERVAL, TimeUnit.MILLISECONDS); - Assertions.assertEquals(results, expected); + var expectedList = List.of("{\"endpoint\":\"test\",\"siteId\":1,\"apiVersion\":\"v1\",\"domainList\":[{\"domain\":\"test.com\",\"count\":2,\"apiContact\":\"test\"}]}", + "{\"endpoint\":\"test\",\"siteId\":1,\"apiVersion\":\"v0\",\"domainList\":[{\"domain\":\"test.com\",\"count\":3,\"apiContact\":\"test\"}]}"); + var messages = getMessages(); + assertThat(messages).containsAll(expectedList); testContext.completeNow(); } @Test void testJSONSerializeWithV2AndUnknownPaths(Vertx vertx, VertxTestContext testContext) throws InterruptedException, JsonProcessingException { - ObjectMapper mapper = new ObjectMapper(); - StatsCollectorMessageItem messageItem = new StatsCollectorMessageItem("/v2/test", "https://test.com", "test", 1); - - vertx.eventBus().send(Const.Config.StatsCollectorEventBus, mapper.writeValueAsString(messageItem)); - vertx.eventBus().send(Const.Config.StatsCollectorEventBus, mapper.writeValueAsString(messageItem)); - vertx.eventBus().send(Const.Config.StatsCollectorEventBus, mapper.writeValueAsString(messageItem)); - - messageItem = new StatsCollectorMessageItem("/v2", "https://test.com", "test", 1); - - vertx.eventBus().send(Const.Config.StatsCollectorEventBus, mapper.writeValueAsString(messageItem)); - vertx.eventBus().send(Const.Config.StatsCollectorEventBus, mapper.writeValueAsString(messageItem)); - - testContext.awaitCompletion(2000, TimeUnit.MILLISECONDS); - - String expected = - "{\"endpoint\":\"test\",\"siteId\":1,\"apiVersion\":\"v2\",\"domainList\":[{\"domain\":\"test.com\",\"count\":3,\"apiContact\":\"test\"}]}\n" - + "{\"endpoint\":\"v2\",\"siteId\":1,\"apiVersion\":\"unknown\",\"domainList\":[{\"domain\":\"test.com\",\"count\":2,\"apiContact\":\"test\"}]}\n"; - - String results = verticle.getEndpointStats(); - - Assertions.assertEquals(results, expected); + StatsCollectorMessageItem messageItem = new StatsCollectorMessageItem("/v2/test", "https://test.com", "test", 1, CLIENT_VERSION); + sendStatMessage(messageItem); + sendStatMessage(messageItem); + sendStatMessage(messageItem); + + messageItem = new StatsCollectorMessageItem("/v2", "https://test.com", "test", 1, CLIENT_VERSION); + sendStatMessage(messageItem); + testContext.awaitCompletion(JSON_INTERVAL*2, TimeUnit.MILLISECONDS); + sendStatMessage(messageItem); + testContext.awaitCompletion(LOG_WAIT_INTERVAL, TimeUnit.MILLISECONDS); + + var expectedList = List.of("{\"endpoint\":\"test\",\"siteId\":1,\"apiVersion\":\"v2\",\"domainList\":[{\"domain\":\"test.com\",\"count\":3,\"apiContact\":\"test\"}]}", + "{\"endpoint\":\"v2\",\"siteId\":1,\"apiVersion\":\"unknown\",\"domainList\":[{\"domain\":\"test.com\",\"count\":2,\"apiContact\":\"test\"}]}"); + var messages = getMessages(); + assertThat(messages).containsAll(expectedList); testContext.completeNow(); } @Test void allValidPathsAllowed(Vertx vertx, VertxTestContext testContext) throws InterruptedException, JsonProcessingException { - ObjectMapper mapper = new ObjectMapper(); Set validEndpoints = Endpoints.pathSet(); - for(String endpoint : validEndpoints) { - StatsCollectorMessageItem messageItem = new StatsCollectorMessageItem(endpoint, "https://test.com", "test", 1); - vertx.eventBus().send(Const.Config.StatsCollectorEventBus, mapper.writeValueAsString(messageItem)); + StatsCollectorMessageItem messageItem = new StatsCollectorMessageItem(endpoint, "https://test.com", "test", 1, CLIENT_VERSION); + sendStatMessage(messageItem); } - testContext.awaitCompletion(2000, TimeUnit.MILLISECONDS); - - String results = verticle.getEndpointStats(); + testContext.awaitCompletion(JSON_INTERVAL*2, TimeUnit.MILLISECONDS); + StatsCollectorMessageItem messageItem = new StatsCollectorMessageItem("/triggerSerialize", "https://test.com", "test", 1, CLIENT_VERSION); + sendStatMessage(messageItem); + testContext.awaitCompletion(LOG_WAIT_INTERVAL, TimeUnit.MILLISECONDS); + var messages = getMessages(); for(String endpoint: validEndpoints) { String withoutVersion = endpoint; if (endpoint.startsWith("/v1/") || endpoint.startsWith("/v2/")) { @@ -114,7 +134,7 @@ void allValidPathsAllowed(Vertx vertx, VertxTestContext testContext) throws Inte } String expected = "{\"endpoint\":\"" + withoutVersion + "\",\"siteId\":1,"; - Assertions.assertTrue(results.contains(expected)); + assertThat(messages).anyMatch(m -> m.contains(expected)); } testContext.completeNow(); @@ -122,37 +142,55 @@ void allValidPathsAllowed(Vertx vertx, VertxTestContext testContext) throws Inte @Test void invalidPathsLimit(Vertx vertx, VertxTestContext testContext) throws InterruptedException, JsonProcessingException { - ObjectMapper mapper = new ObjectMapper(); - for(int i = 0; i < MAX_INVALID_PATHS + Endpoints.pathSet().size() + 5; i++) { - StatsCollectorMessageItem messageItem = new StatsCollectorMessageItem("/bad" + i, "https://test.com", "test", 1); - vertx.eventBus().send(Const.Config.StatsCollectorEventBus, mapper.writeValueAsString(messageItem)); + StatsCollectorMessageItem messageItem = new StatsCollectorMessageItem("/bad" + i, "https://test.com", "test", 1, CLIENT_VERSION); + sendStatMessage(messageItem); } - testContext.awaitCompletion(2000, TimeUnit.MILLISECONDS); - String results = verticle.getEndpointStats(); + testContext.awaitCompletion(JSON_INTERVAL*2, TimeUnit.MILLISECONDS); + StatsCollectorMessageItem messageItem = new StatsCollectorMessageItem("/triggerSerialize", "https://test.com", "test", 1, CLIENT_VERSION); + sendStatMessage(messageItem); + testContext.awaitCompletion(LOG_WAIT_INTERVAL, TimeUnit.MILLISECONDS); + var messages = getMessages(); // MAX_INVALID_PATHS is not the hard limit. The maximum paths that can be recorded, including valid ones, is MAX_INVALID_PATHS + validPaths.size * 2 for(int i = 0; i < MAX_INVALID_PATHS + Endpoints.pathSet().size(); i++) { String expected = "{\"endpoint\":\"bad" + i + "\",\"siteId\":1,\"apiVersion\":\"v0\",\"domainList\":[{\"domain\":\"test.com\",\"count\":1,\"apiContact\":\"test\"}]}"; - Assertions.assertTrue(results.contains(expected)); + assertThat(messages).contains(expected); } for(int i = MAX_INVALID_PATHS + Endpoints.pathSet().size(); i < MAX_INVALID_PATHS + 5; i++) { String expected = "{\"endpoint\":\"bad" + i + "\",\"siteId\":1,\"apiVersion\":\"v0\",\"domainList\":[{\"domain\":\"test.com\",\"count\":1,\"apiContact\":\"test\"}]}"; - Assertions.assertFalse(results.contains(expected)); + assertThat(messages).contains(expected); } + assertThat(getMessages()).contains("max invalid paths reached; a large number of invalid paths have been requested from authenticated participants"); - ListAppender logWatcher = new ListAppender<>(); - logWatcher.start(); - ((Logger) LoggerFactory.getLogger(StatsCollectorVerticle.class)).addAppender(logWatcher); + testContext.completeNow(); + } - StatsCollectorMessageItem messageItem = new StatsCollectorMessageItem("/triggerSerialize", "https://test.com", "test", 1); - vertx.eventBus().send(Const.Config.StatsCollectorEventBus, mapper.writeValueAsString(messageItem)); + @Test + void clientVersionStats(Vertx vertx, VertxTestContext testContext) throws InterruptedException, JsonProcessingException { + for(int i = 0; i < 3; i++) { + // These should all be recorded. + StatsCollectorMessageItem messageItem = new StatsCollectorMessageItem("/test" + i, "https://test.com", "test", 1, CLIENT_VERSION + i); + sendStatMessage(messageItem); + } + for(int i = 0; i < 10; i++) { + // Only 5 of these should be recorded, but they should both have count of 2. The other 5 should result in 10 not-recorded entries. + StatsCollectorMessageItem messageItem = new StatsCollectorMessageItem("/test" + i, "https://test.com", "test", 2, CLIENT_VERSION + i); + sendStatMessage(messageItem); + sendStatMessage(messageItem); + } - testContext.awaitCompletion(1000, TimeUnit.MILLISECONDS); - Assertions.assertTrue(logWatcher.list.get(0).getFormattedMessage().contains("max invalid paths reached; a large number of invalid paths have been requested from authenticated participants")); + testContext.awaitCompletion(JSON_INTERVAL*2, TimeUnit.MILLISECONDS); + StatsCollectorMessageItem messageItem = new StatsCollectorMessageItem("/triggerSerialize", "https://test.com", "test", null, null); + sendStatMessage(messageItem); + testContext.awaitCompletion(LOG_WAIT_INTERVAL*10, TimeUnit.MILLISECONDS); + + 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\":2,\"versionCounts\":{\"uid2-sdk-3.0.01\":2,\"uid2-sdk-3.0.02\":2,\"uid2-sdk-3.0.03\":2,\"uid2-sdk-3.0.04\":2,\"NotRecorded\":10,\"uid2-sdk-3.0.00\":2}}"); + var messages = getMessages(); + assertThat(messages).containsAll(expectedLogs); testContext.completeNow(); } - } From a0f6f30aa9090bd69b0207c80401b513e41b6fb6 Mon Sep 17 00:00:00 2001 From: Lionell Pack Date: Mon, 2 Sep 2024 18:02:45 +1000 Subject: [PATCH 02/10] Remove outdated tests. --- .../operator/UIDOperatorVerticleTest.java | 70 ------------------- 1 file changed, 70 deletions(-) diff --git a/src/test/java/com/uid2/operator/UIDOperatorVerticleTest.java b/src/test/java/com/uid2/operator/UIDOperatorVerticleTest.java index 6b74a36bb..a9d8ed086 100644 --- a/src/test/java/com/uid2/operator/UIDOperatorVerticleTest.java +++ b/src/test/java/com/uid2/operator/UIDOperatorVerticleTest.java @@ -5090,74 +5090,4 @@ void secureLinkValidationFailsReturnsIdentityError(Vertx vertx, VertxTestContext testContext.completeNow(); }); } - - @ParameterizedTest // note that this test will be removed when we switch to logging versions - @ValueSource(strings = {"euid-sdk-1.0.0", "openid-sdk-1.0", "uid2-esp-0.0.1a", "uid2-sdk-0.0.1a", - "uid2-sdk-0.0.1b", "uid2-sdk-1.0.0", "uid2-sdk-2.0.0"}) - void clientVersionHeader(String clientVersion, Vertx vertx, VertxTestContext testContext) { - WebClient client = WebClient.create(vertx); - HttpRequest req = client.getAbs(getUrlForEndpoint("/any/endpoint")); - req.putHeader("X-UID2-Client-Version", clientVersion); - req.send(ar -> { - assertEquals(404, ar.result().statusCode()); - final double actual = Metrics.globalRegistry - .get("uid2.client_sdk_versions") - .tag("client_version", clientVersion) - .counter().count(); - assertEquals(1, actual); - testContext.completeNow(); - }); - } - - @ParameterizedTest // note that this test will be removed when we switch to logging versions - @ValueSource(strings = {"euid-sdk-1.0.0", "openid-sdk-1.0", "uid2-esp-0.0.1a", "uid2-sdk-0.0.1a", - "uid2-sdk-0.0.1b", "uid2-sdk-1.0.0", "uid2-sdk-2.0.0"}) - void clientVersionQueryParameter(String clientVersion, Vertx vertx, VertxTestContext testContext) { - WebClient client = WebClient.create(vertx); - HttpRequest req = client.getAbs(getUrlForEndpoint("/any/endpoint?client=" + clientVersion)); - req.send(ar -> { - assertEquals(404, ar.result().statusCode()); - final double actual = Metrics.globalRegistry - .get("uid2.client_sdk_versions") - .tag("client_version", clientVersion) - .counter().count(); - assertEquals(1, actual); - testContext.completeNow(); - }); - } - - @Test // note that this test will be removed when we switch to logging versions - void clientVersionHeaderNotFound(Vertx vertx, VertxTestContext testContext) { - WebClient client = WebClient.create(vertx); - HttpRequest req = client.getAbs(getUrlForEndpoint("/any/endpoint")); - String clientVersion = "invalid-sdk"; - req.putHeader("X-UID2-Client-Version", clientVersion); - req.send(ar -> { - assertEquals(404, ar.result().statusCode()); - assertThrows(MeterNotFoundException.class, () -> { - Metrics.globalRegistry - .get("uid2.client_sdk_versions") - .tag("client_version", clientVersion) - .counter(); - }); - testContext.completeNow(); - }); - } - - @Test // note that this test will be removed when we switch to logging versions - void clientVersionQueryParameterNotFound(Vertx vertx, VertxTestContext testContext) { - WebClient client = WebClient.create(vertx); - String clientVersion = "invalid-sdk"; - HttpRequest req = client.getAbs(getUrlForEndpoint("/any/endpoint?client=" + clientVersion)); - req.send(ar -> { - assertEquals(404, ar.result().statusCode()); - assertThrows(MeterNotFoundException.class, () -> { - Metrics.globalRegistry - .get("uid2.client_sdk_versions") - .tag("client_version", clientVersion) - .counter(); - }); - testContext.completeNow(); - }); - } } From 3d8eaf9c90881b35cfbd133eb4154980a39fe7b7 Mon Sep 17 00:00:00 2001 From: Lionell Pack Date: Tue, 3 Sep 2024 17:16:41 +1000 Subject: [PATCH 03/10] Switch to not clean the results between version stat runs. Refactor the tests to be easier to read. --- .../monitoring/ClientVersionStatRecorder.java | 37 ++++++----- .../monitoring/StatsCollectorVerticle.java | 1 - .../operator/StatsCollectorVerticleTest.java | 66 +++++++++++-------- 3 files changed, 59 insertions(+), 45 deletions(-) diff --git a/src/main/java/com/uid2/operator/monitoring/ClientVersionStatRecorder.java b/src/main/java/com/uid2/operator/monitoring/ClientVersionStatRecorder.java index 4d5c1371e..54b8cf437 100644 --- a/src/main/java/com/uid2/operator/monitoring/ClientVersionStatRecorder.java +++ b/src/main/java/com/uid2/operator/monitoring/ClientVersionStatRecorder.java @@ -5,19 +5,32 @@ import java.util.stream.Stream; public class ClientVersionStatRecorder { + private static final String NOT_RECORDED = ""; private final int siteClientBucketLimit; - private final Map> clientVersionToSiteCount = new HashMap<>(); + private final Map> siteIdToVersionCounts = new HashMap<>(); public ClientVersionStatRecorder(int maxVersionBucketsPerSite) { this.siteClientBucketLimit = maxVersionBucketsPerSite; } public Stream getStatsView() { - return clientVersionToSiteCount.entrySet().stream().map(entry -> new SiteClientVersionStat(entry.getKey(), entry.getValue())); + return siteIdToVersionCounts.entrySet().stream().map(entry -> new SiteClientVersionStat(entry.getKey(), entry.getValue())); } - public void clear() { - clientVersionToSiteCount.clear(); + 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 + versionCounts.entrySet().stream() + .sorted(Map.Entry.comparingByValue()) + .filter(entry -> !entry.getKey().equals(NOT_RECORDED)) + .limit(3) + .forEach(entry -> { + 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) { @@ -25,18 +38,12 @@ public void add(Integer siteId, String clientVersion) { return; } - var clientVersionCounts = clientVersionToSiteCount.computeIfAbsent(siteId, k -> new HashMap<>()); + var clientVersionCounts = siteIdToVersionCounts.computeIfAbsent(siteId, k -> new HashMap<>()); - var count = clientVersionCounts.get(clientVersion); - if (count == null && clientVersionCounts.size() >= siteClientBucketLimit) { - var notRecordedCount = clientVersionCounts.getOrDefault("NotRecorded", 0); - clientVersionCounts.put("NotRecorded", notRecordedCount + 1); - } - else if (count == null) { - clientVersionCounts.put(clientVersion, 1); - } - else { - clientVersionCounts.put(clientVersion, count + 1); + 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/StatsCollectorVerticle.java b/src/main/java/com/uid2/operator/monitoring/StatsCollectorVerticle.java index 5c236461c..318ac9b06 100644 --- a/src/main/java/com/uid2/operator/monitoring/StatsCollectorVerticle.java +++ b/src/main/java/com/uid2/operator/monitoring/StatsCollectorVerticle.java @@ -149,7 +149,6 @@ public void handleMessage(Message message) { } ); pathMap.clear(); - clientVersionStat.clear(); } } } diff --git a/src/test/java/com/uid2/operator/StatsCollectorVerticleTest.java b/src/test/java/com/uid2/operator/StatsCollectorVerticleTest.java index 88cb51ca3..bde73eeeb 100644 --- a/src/test/java/com/uid2/operator/StatsCollectorVerticleTest.java +++ b/src/test/java/com/uid2/operator/StatsCollectorVerticleTest.java @@ -14,7 +14,6 @@ import static org.assertj.core.api.Assertions.*; import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -28,7 +27,7 @@ @ExtendWith(VertxExtension.class) public class StatsCollectorVerticleTest { private static final int MAX_INVALID_PATHS = 5; - private static final int MAX_CLIENT_VERSION_BUCKETS = 5; + private static final int MAX_CLIENT_VERSION_BUCKETS = 8; private static final int JSON_INTERVAL = 200; private static final int LOG_WAIT_INTERVAL = 50; private static final String CLIENT_VERSION = "uid2-sdk-3.0.0"; @@ -58,14 +57,20 @@ void verticleDeployed(Vertx vertx, VertxTestContext testContext) { testContext.completeNow(); } - private Set getMessages() { - return logWatcher.list.stream().map(ILoggingEvent::getFormattedMessage).collect(Collectors.toSet()); + private List getMessages() { + return logWatcher.list.stream().map(ILoggingEvent::getFormattedMessage).collect(Collectors.toList()); } private void sendStatMessage(StatsCollectorMessageItem messageItem) throws JsonProcessingException { vertx.eventBus().send(Const.Config.StatsCollectorEventBus, mapper.writeValueAsString(messageItem)); } + private void triggerSerializeAndWait(VertxTestContext testContext) throws JsonProcessingException, InterruptedException { + StatsCollectorMessageItem triggerSerialize = new StatsCollectorMessageItem("/triggerSerialize", "https://test.com", "test", null, null); + sendStatMessage(triggerSerialize); + testContext.awaitCompletion(LOG_WAIT_INTERVAL, TimeUnit.MILLISECONDS); + } + @Test void testJSONSerializeWithV0AndV1Paths(Vertx vertx, VertxTestContext testContext) throws InterruptedException, JsonProcessingException { StatsCollectorMessageItem messageItem = new StatsCollectorMessageItem("/test", "https://test.com", "test", 1, CLIENT_VERSION); @@ -76,11 +81,9 @@ void testJSONSerializeWithV0AndV1Paths(Vertx vertx, VertxTestContext testContext messageItem = new StatsCollectorMessageItem("/v1/test", "https://test.com", "test", 1, CLIENT_VERSION); sendStatMessage(messageItem); sendStatMessage(messageItem); - testContext.awaitCompletion(JSON_INTERVAL*2, TimeUnit.MILLISECONDS); + waitForLogInterval(testContext); - StatsCollectorMessageItem triggerItem = new StatsCollectorMessageItem("/triggerSerialize", "https://test.com", "test", 1, CLIENT_VERSION); - sendStatMessage(triggerItem); - testContext.awaitCompletion(LOG_WAIT_INTERVAL, TimeUnit.MILLISECONDS); + triggerSerializeAndWait(testContext); var expectedList = List.of("{\"endpoint\":\"test\",\"siteId\":1,\"apiVersion\":\"v1\",\"domainList\":[{\"domain\":\"test.com\",\"count\":2,\"apiContact\":\"test\"}]}", "{\"endpoint\":\"test\",\"siteId\":1,\"apiVersion\":\"v0\",\"domainList\":[{\"domain\":\"test.com\",\"count\":3,\"apiContact\":\"test\"}]}"); @@ -90,6 +93,10 @@ void testJSONSerializeWithV0AndV1Paths(Vertx vertx, VertxTestContext testContext testContext.completeNow(); } + private static void waitForLogInterval(VertxTestContext testContext) throws InterruptedException { + testContext.awaitCompletion(JSON_INTERVAL*2, TimeUnit.MILLISECONDS); + } + @Test void testJSONSerializeWithV2AndUnknownPaths(Vertx vertx, VertxTestContext testContext) throws InterruptedException, JsonProcessingException { StatsCollectorMessageItem messageItem = new StatsCollectorMessageItem("/v2/test", "https://test.com", "test", 1, CLIENT_VERSION); @@ -99,9 +106,9 @@ void testJSONSerializeWithV2AndUnknownPaths(Vertx vertx, VertxTestContext testCo messageItem = new StatsCollectorMessageItem("/v2", "https://test.com", "test", 1, CLIENT_VERSION); sendStatMessage(messageItem); - testContext.awaitCompletion(JSON_INTERVAL*2, TimeUnit.MILLISECONDS); sendStatMessage(messageItem); - testContext.awaitCompletion(LOG_WAIT_INTERVAL, TimeUnit.MILLISECONDS); + waitForLogInterval(testContext); + triggerSerializeAndWait(testContext); var expectedList = List.of("{\"endpoint\":\"test\",\"siteId\":1,\"apiVersion\":\"v2\",\"domainList\":[{\"domain\":\"test.com\",\"count\":3,\"apiContact\":\"test\"}]}", "{\"endpoint\":\"v2\",\"siteId\":1,\"apiVersion\":\"unknown\",\"domainList\":[{\"domain\":\"test.com\",\"count\":2,\"apiContact\":\"test\"}]}"); @@ -119,10 +126,8 @@ void allValidPathsAllowed(Vertx vertx, VertxTestContext testContext) throws Inte sendStatMessage(messageItem); } - testContext.awaitCompletion(JSON_INTERVAL*2, TimeUnit.MILLISECONDS); - StatsCollectorMessageItem messageItem = new StatsCollectorMessageItem("/triggerSerialize", "https://test.com", "test", 1, CLIENT_VERSION); - sendStatMessage(messageItem); - testContext.awaitCompletion(LOG_WAIT_INTERVAL, TimeUnit.MILLISECONDS); + waitForLogInterval(testContext); + triggerSerializeAndWait(testContext); var messages = getMessages(); for(String endpoint: validEndpoints) { @@ -147,10 +152,8 @@ void invalidPathsLimit(Vertx vertx, VertxTestContext testContext) throws Interru sendStatMessage(messageItem); } - testContext.awaitCompletion(JSON_INTERVAL*2, TimeUnit.MILLISECONDS); - StatsCollectorMessageItem messageItem = new StatsCollectorMessageItem("/triggerSerialize", "https://test.com", "test", 1, CLIENT_VERSION); - sendStatMessage(messageItem); - testContext.awaitCompletion(LOG_WAIT_INTERVAL, TimeUnit.MILLISECONDS); + waitForLogInterval(testContext); + triggerSerializeAndWait(testContext); var messages = getMessages(); // MAX_INVALID_PATHS is not the hard limit. The maximum paths that can be recorded, including valid ones, is MAX_INVALID_PATHS + validPaths.size * 2 @@ -170,24 +173,29 @@ void invalidPathsLimit(Vertx vertx, VertxTestContext testContext) throws Interru @Test void clientVersionStats(Vertx vertx, VertxTestContext testContext) throws InterruptedException, JsonProcessingException { for(int i = 0; i < 3; i++) { - // These should all be recorded. StatsCollectorMessageItem messageItem = new StatsCollectorMessageItem("/test" + i, "https://test.com", "test", 1, CLIENT_VERSION + i); sendStatMessage(messageItem); } - for(int i = 0; i < 10; i++) { - // Only 5 of these should be recorded, but they should both have count of 2. The other 5 should result in 10 not-recorded entries. + for(int i = 0; i < 12; i++) { StatsCollectorMessageItem messageItem = new StatsCollectorMessageItem("/test" + i, "https://test.com", "test", 2, CLIENT_VERSION + i); - sendStatMessage(messageItem); - sendStatMessage(messageItem); + for (int count = 0; count <= i; count++) { + sendStatMessage(messageItem); + } } + sendStatMessage(new StatsCollectorMessageItem("/test", "https://test.com", "test", 2, CLIENT_VERSION + "single")); - testContext.awaitCompletion(JSON_INTERVAL*2, TimeUnit.MILLISECONDS); - StatsCollectorMessageItem messageItem = new StatsCollectorMessageItem("/triggerSerialize", "https://test.com", "test", null, null); - sendStatMessage(messageItem); - testContext.awaitCompletion(LOG_WAIT_INTERVAL*10, TimeUnit.MILLISECONDS); + waitForLogInterval(testContext); + triggerSerializeAndWait(testContext); + + waitForLogInterval(testContext); + sendStatMessage(new StatsCollectorMessageItem("/test", "https://test.com", "test", 1, CLIENT_VERSION + 1)); + 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\":2,\"versionCounts\":{\"uid2-sdk-3.0.01\":2,\"uid2-sdk-3.0.02\":2,\"uid2-sdk-3.0.03\":2,\"uid2-sdk-3.0.04\":2,\"NotRecorded\":10,\"uid2-sdk-3.0.00\":2}}"); + 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\":{\"\":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); From 34cebbd5b458a636ced9dd65b0420897bb9f4966 Mon Sep 17 00:00:00 2001 From: Lionell Pack Date: Tue, 3 Sep 2024 17:23:24 +1000 Subject: [PATCH 04/10] Use the local mapper. --- .../com/uid2/operator/monitoring/StatsCollectorVerticle.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/uid2/operator/monitoring/StatsCollectorVerticle.java b/src/main/java/com/uid2/operator/monitoring/StatsCollectorVerticle.java index 318ac9b06..d715714ec 100644 --- a/src/main/java/com/uid2/operator/monitoring/StatsCollectorVerticle.java +++ b/src/main/java/com/uid2/operator/monitoring/StatsCollectorVerticle.java @@ -158,7 +158,7 @@ private Void serializeToLogs(List stats) { ObjectMapper statMapper = new ObjectMapper(); for (Object stat : stats) { try { - String jsonString = mapper.writeValueAsString(stat); + String jsonString = statMapper.writeValueAsString(stat); LOGGER.info(jsonString); } catch (JsonProcessingException e) { LOGGER.error(e.getMessage(), e); From 9b0fe3ab103f291a1e31eab27d27af561909d29a Mon Sep 17 00:00:00 2001 From: Lionell Pack Date: Wed, 18 Sep 2024 12:45:36 +1000 Subject: [PATCH 05/10] Make sure iterator is reified before updating collection. --- .../monitoring/ClientVersionStatRecorder.java | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/uid2/operator/monitoring/ClientVersionStatRecorder.java b/src/main/java/com/uid2/operator/monitoring/ClientVersionStatRecorder.java index 54b8cf437..fc5a958fe 100644 --- a/src/main/java/com/uid2/operator/monitoring/ClientVersionStatRecorder.java +++ b/src/main/java/com/uid2/operator/monitoring/ClientVersionStatRecorder.java @@ -22,15 +22,16 @@ private void removeLowVersionCounts(int siteId) { if (versionCounts == null) return; // Remove 3 items to avoid a couple of new version values from continuously evicting each other - versionCounts.entrySet().stream() + var lowestEntries = versionCounts.entrySet().stream() .sorted(Map.Entry.comparingByValue()) .filter(entry -> !entry.getKey().equals(NOT_RECORDED)) .limit(3) - .forEach(entry -> { - var notRecordedCount = versionCounts.getOrDefault(NOT_RECORDED, 0); - versionCounts.put(NOT_RECORDED, notRecordedCount + entry.getValue()); - versionCounts.remove(entry.getKey()); - }); + .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) { From 821418b176fc3de26e5da12a76e066d145e4ca1e Mon Sep 17 00:00:00 2001 From: Lionell Pack Date: Wed, 18 Sep 2024 13:21:05 +1000 Subject: [PATCH 06/10] Made some code more style consistent. Updated a test to hopefully provide more detail about why it failed. --- .../com/uid2/operator/monitoring/StatsCollectorHandler.java | 3 ++- src/test/java/com/uid2/operator/UIDOperatorVerticleTest.java | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/uid2/operator/monitoring/StatsCollectorHandler.java b/src/main/java/com/uid2/operator/monitoring/StatsCollectorHandler.java index f774def06..04a36d9c1 100644 --- a/src/main/java/com/uid2/operator/monitoring/StatsCollectorHandler.java +++ b/src/main/java/com/uid2/operator/monitoring/StatsCollectorHandler.java @@ -34,8 +34,9 @@ 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, getClientVersion(routingContext)); + final StatsCollectorMessageItem messageItem = new StatsCollectorMessageItem(path, referer, apiContact, siteId, clientVersion); _statCollectorQueue.enqueue(vertx, messageItem); } diff --git a/src/test/java/com/uid2/operator/UIDOperatorVerticleTest.java b/src/test/java/com/uid2/operator/UIDOperatorVerticleTest.java index a9d8ed086..594283d95 100644 --- a/src/test/java/com/uid2/operator/UIDOperatorVerticleTest.java +++ b/src/test/java/com/uid2/operator/UIDOperatorVerticleTest.java @@ -55,6 +55,7 @@ import org.mockito.Mock; import org.mockito.MockitoAnnotations; import org.slf4j.LoggerFactory; +import static org.assertj.core.api.Assertions.*; import javax.crypto.SecretKey; import java.math.BigInteger; @@ -3304,7 +3305,7 @@ void cstgDomainNameCheckFailsAndLogInvalidHttpOrigin(String httpOrigin, Vertx ve assertFalse(respJson.containsKey("body")); assertEquals("unexpected http origin", respJson.getString("message")); assertEquals("invalid_http_origin", respJson.getString("status")); - Assertions.assertTrue(logWatcher.list.get(0).getFormattedMessage().contains("InvalidHttpOriginAndAppName: site test (123): http://gototest.com")); + assertThat(logWatcher.list.stream().map(ILoggingEvent::getFormattedMessage).collect(Collectors.toList())).contains("InvalidHttpOrfdiginAndAppName: site test (123): http://gototest.com"); assertTokenStatusMetrics( clientSideTokenGenerateSiteId, TokenResponseStatsCollector.Endpoint.ClientSideTokenGenerateV2, From ebe95a216d05cda21c691bf2258c165ddee2eef7 Mon Sep 17 00:00:00 2001 From: Lionell Pack Date: Wed, 18 Sep 2024 13:24:11 +1000 Subject: [PATCH 07/10] Fix typo. --- src/test/java/com/uid2/operator/UIDOperatorVerticleTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/com/uid2/operator/UIDOperatorVerticleTest.java b/src/test/java/com/uid2/operator/UIDOperatorVerticleTest.java index 594283d95..eafa14f9a 100644 --- a/src/test/java/com/uid2/operator/UIDOperatorVerticleTest.java +++ b/src/test/java/com/uid2/operator/UIDOperatorVerticleTest.java @@ -3305,7 +3305,7 @@ void cstgDomainNameCheckFailsAndLogInvalidHttpOrigin(String httpOrigin, Vertx ve assertFalse(respJson.containsKey("body")); assertEquals("unexpected http origin", respJson.getString("message")); assertEquals("invalid_http_origin", respJson.getString("status")); - assertThat(logWatcher.list.stream().map(ILoggingEvent::getFormattedMessage).collect(Collectors.toList())).contains("InvalidHttpOrfdiginAndAppName: site test (123): http://gototest.com"); + assertThat(logWatcher.list.stream().map(ILoggingEvent::getFormattedMessage).collect(Collectors.toList())).contains("InvalidHttpOriginAndAppName: site test (123): http://gototest.com"); assertTokenStatusMetrics( clientSideTokenGenerateSiteId, TokenResponseStatsCollector.Endpoint.ClientSideTokenGenerateV2, From a0dd91f3a27fa9e22796ac8f44a34d657f5c878b Mon Sep 17 00:00:00 2001 From: Lionell Pack Date: Wed, 18 Sep 2024 14:07:04 +1000 Subject: [PATCH 08/10] Add a null version entry to the test. --- src/test/java/com/uid2/operator/StatsCollectorVerticleTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/java/com/uid2/operator/StatsCollectorVerticleTest.java b/src/test/java/com/uid2/operator/StatsCollectorVerticleTest.java index bde73eeeb..0dc3b4c69 100644 --- a/src/test/java/com/uid2/operator/StatsCollectorVerticleTest.java +++ b/src/test/java/com/uid2/operator/StatsCollectorVerticleTest.java @@ -183,6 +183,7 @@ void clientVersionStats(Vertx vertx, VertxTestContext testContext) throws Interr } } sendStatMessage(new StatsCollectorMessageItem("/test", "https://test.com", "test", 2, CLIENT_VERSION + "single")); + sendStatMessage(new StatsCollectorMessageItem("/test", "https://test.com", "test", 2, null)); waitForLogInterval(testContext); triggerSerializeAndWait(testContext); From 6df8f28fe1c1f5fcc00a13731ca36d4d9030476e Mon Sep 17 00:00:00 2001 From: Lionell Pack Date: Thu, 19 Sep 2024 16:52:12 +1000 Subject: [PATCH 09/10] Fix bad config in StatsCollectorVerticle instantiation. Change format of the client version stat string. Made client version stats recorder ignore blank versions. --- src/main/java/com/uid2/operator/Main.java | 2 +- .../model/StatsCollectorMessageItem.java | 12 ++++++--- .../monitoring/ClientVersionStatRecorder.java | 8 +++--- .../uid2/operator/monitoring/ILoggedStat.java | 6 +++++ .../monitoring/SiteClientVersionStat.java | 20 +++++++++++++- .../monitoring/StatsCollectorVerticle.java | 26 +++++++++++++------ .../operator/StatsCollectorVerticleTest.java | 6 ++--- 7 files changed, 61 insertions(+), 19 deletions(-) create mode 100644 src/main/java/com/uid2/operator/monitoring/ILoggedStat.java diff --git a/src/main/java/com/uid2/operator/Main.java b/src/main/java/com/uid2/operator/Main.java index 34d948a6d..26f3f5dc3 100644 --- a/src/main/java/com/uid2/operator/Main.java +++ b/src/main/java/com/uid2/operator/Main.java @@ -364,7 +364,7 @@ private Future createAndDeployCloudSyncStoreVerticle(String name, ICloud private Future createAndDeployStatsCollector() { Promise 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 index 0191386d3..bcf06a523 100644 --- 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 index fc5a958fe..5abeb41d2 100644 --- 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 getStatsView() { + public Stream 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 new file mode 100644 index 000000000..2ea9f777f --- /dev/null +++ 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 index 747544941..b9e953e49 100644 --- 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 versionCounts) { +public final class SiteClientVersionStat implements ILoggedStat { + private final Integer siteId; + private final Map versionCounts; + + public SiteClientVersionStat(Integer siteId, Map 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 index d715714ec..8e49ec1f8 100644 --- 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 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 stats) { + private Void serializeToLogs(List 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 buildStatsList() { - Stream pathMapStream = pathMap.values().stream(); - Stream clientVersionStream = clientVersionStat.getStatsView(); + private List buildStatsList() { + Stream pathMapStream = pathMap.values().stream(); + Stream 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 index 0dc3b4c69..d0f089da4 100644 --- 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\":{\"\":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={\"\":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); From 8f114ca795b496f10f72240ac0563a596ccb2642 Mon Sep 17 00:00:00 2001 From: Release Workflow Date: Thu, 19 Sep 2024 19:02:33 +0000 Subject: [PATCH 10/10] [CI Pipeline] Released Patch version: 5.39.9 --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 327a8fa6f..8bdf6c842 100644 --- a/pom.xml +++ b/pom.xml @@ -6,7 +6,7 @@ com.uid2 uid2-operator - 5.39.3-alpha-119-SNAPSHOT + 5.39.9 UTF-8