From 22dd76271dc27e864179e65f8c702a359bf66a06 Mon Sep 17 00:00:00 2001 From: Michal Wisniewski Date: Sun, 3 Nov 2024 11:20:58 +0100 Subject: [PATCH 01/39] SKYEDEN-3234 | Add marking and unmarking commands --- .../domain/detection/UnusedTopic.java | 8 ++++ .../detection/UnusedTopicsRepository.java | 7 ++++ .../MarkTopicAsUnusedRepositoryCommand.java | 41 ++++++++++++++++++ .../UnmarkTopicAsUnusedRepositoryCommand.java | 42 +++++++++++++++++++ 4 files changed, 98 insertions(+) create mode 100644 hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopic.java create mode 100644 hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsRepository.java create mode 100644 hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/command/MarkTopicAsUnusedRepositoryCommand.java create mode 100644 hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/command/UnmarkTopicAsUnusedRepositoryCommand.java diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopic.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopic.java new file mode 100644 index 0000000000..c794bc4565 --- /dev/null +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopic.java @@ -0,0 +1,8 @@ +package pl.allegro.tech.hermes.management.domain.detection; + +import pl.allegro.tech.hermes.api.TopicName; + +import java.time.Instant; + +public record UnusedTopic( + TopicName topicName, Instant lastPublishedMessage, Instant lastNotified, boolean whitelisted) {} diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsRepository.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsRepository.java new file mode 100644 index 0000000000..369e733af2 --- /dev/null +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsRepository.java @@ -0,0 +1,7 @@ +package pl.allegro.tech.hermes.management.domain.detection; + +public interface UnusedTopicsRepository { + void markAsUnused(UnusedTopic unusedTopic); + + void unmarkAsUnused(UnusedTopic unusedTopic); +} diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/command/MarkTopicAsUnusedRepositoryCommand.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/command/MarkTopicAsUnusedRepositoryCommand.java new file mode 100644 index 0000000000..3f0e7843ff --- /dev/null +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/command/MarkTopicAsUnusedRepositoryCommand.java @@ -0,0 +1,41 @@ +package pl.allegro.tech.hermes.management.domain.detection.command; + +import pl.allegro.tech.hermes.management.domain.dc.DatacenterBoundRepositoryHolder; +import pl.allegro.tech.hermes.management.domain.dc.RepositoryCommand; +import pl.allegro.tech.hermes.management.domain.detection.UnusedTopic; +import pl.allegro.tech.hermes.management.domain.detection.UnusedTopicsRepository; + +public class MarkTopicAsUnusedRepositoryCommand extends RepositoryCommand { + + private final UnusedTopic unusedTopic; + + public MarkTopicAsUnusedRepositoryCommand(UnusedTopic unusedTopic) { + this.unusedTopic = unusedTopic; + } + + @Override + public void backup(DatacenterBoundRepositoryHolder holder) { + // TODO + } + + @Override + public void execute(DatacenterBoundRepositoryHolder holder) { + holder.getRepository().markAsUnused(unusedTopic); + } + + @Override + public void rollback( + DatacenterBoundRepositoryHolder holder, Exception exception) { + // TODO + } + + @Override + public Class getRepositoryType() { + return UnusedTopicsRepository.class; + } + + @Override + public String toString() { + return String.format("MarkTopicAsUnused(%s)", unusedTopic.topicName().qualifiedName()); + } +} diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/command/UnmarkTopicAsUnusedRepositoryCommand.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/command/UnmarkTopicAsUnusedRepositoryCommand.java new file mode 100644 index 0000000000..e9b7d05adf --- /dev/null +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/command/UnmarkTopicAsUnusedRepositoryCommand.java @@ -0,0 +1,42 @@ +package pl.allegro.tech.hermes.management.domain.detection.command; + +import pl.allegro.tech.hermes.management.domain.dc.DatacenterBoundRepositoryHolder; +import pl.allegro.tech.hermes.management.domain.dc.RepositoryCommand; +import pl.allegro.tech.hermes.management.domain.detection.UnusedTopic; +import pl.allegro.tech.hermes.management.domain.detection.UnusedTopicsRepository; + +public class UnmarkTopicAsUnusedRepositoryCommand + extends RepositoryCommand { + + private final UnusedTopic unusedTopic; + + public UnmarkTopicAsUnusedRepositoryCommand(UnusedTopic unusedTopic) { + this.unusedTopic = unusedTopic; + } + + @Override + public void backup(DatacenterBoundRepositoryHolder holder) { + // TODO + } + + @Override + public void execute(DatacenterBoundRepositoryHolder holder) { + holder.getRepository().unmarkAsUnused(unusedTopic); + } + + @Override + public void rollback( + DatacenterBoundRepositoryHolder holder, Exception exception) { + // TODO + } + + @Override + public Class getRepositoryType() { + return UnusedTopicsRepository.class; + } + + @Override + public String toString() { + return String.format("UnmarkTopicAsUnused(%s)", unusedTopic.topicName().qualifiedName()); + } +} From 1e28093f89db080a4838d8027b4ad21632f94206 Mon Sep 17 00:00:00 2001 From: Michal Wisniewski Date: Sun, 3 Nov 2024 12:30:15 +0100 Subject: [PATCH 02/39] SKYEDEN-3234 | Change to batch upserting --- .../detection/UnusedTopicsRepository.java | 6 ++- ... MarkTopicsAsUnusedRepositoryCommand.java} | 12 +++--- .../UnmarkTopicAsUnusedRepositoryCommand.java | 42 ------------------- .../ZookeeperUnusedTopicsRepository.java | 4 ++ 4 files changed, 15 insertions(+), 49 deletions(-) rename hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/command/{MarkTopicAsUnusedRepositoryCommand.java => MarkTopicsAsUnusedRepositoryCommand.java} (74%) delete mode 100644 hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/command/UnmarkTopicAsUnusedRepositoryCommand.java create mode 100644 hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/zookeeper/detection/ZookeeperUnusedTopicsRepository.java diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsRepository.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsRepository.java index 369e733af2..b1c9b5ab88 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsRepository.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsRepository.java @@ -1,7 +1,9 @@ package pl.allegro.tech.hermes.management.domain.detection; +import java.util.List; + public interface UnusedTopicsRepository { - void markAsUnused(UnusedTopic unusedTopic); + void upsert(List unusedTopics); - void unmarkAsUnused(UnusedTopic unusedTopic); + List read(); } diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/command/MarkTopicAsUnusedRepositoryCommand.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/command/MarkTopicsAsUnusedRepositoryCommand.java similarity index 74% rename from hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/command/MarkTopicAsUnusedRepositoryCommand.java rename to hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/command/MarkTopicsAsUnusedRepositoryCommand.java index 3f0e7843ff..7454b25c3d 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/command/MarkTopicAsUnusedRepositoryCommand.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/command/MarkTopicsAsUnusedRepositoryCommand.java @@ -5,12 +5,14 @@ import pl.allegro.tech.hermes.management.domain.detection.UnusedTopic; import pl.allegro.tech.hermes.management.domain.detection.UnusedTopicsRepository; -public class MarkTopicAsUnusedRepositoryCommand extends RepositoryCommand { +import java.util.List; - private final UnusedTopic unusedTopic; +public class MarkTopicsAsUnusedRepositoryCommand extends RepositoryCommand { - public MarkTopicAsUnusedRepositoryCommand(UnusedTopic unusedTopic) { - this.unusedTopic = unusedTopic; + private final List unusedTopics; + + public MarkTopicsAsUnusedRepositoryCommand(List unusedTopics) { + this.unusedTopics = unusedTopics; } @Override @@ -20,7 +22,7 @@ public void backup(DatacenterBoundRepositoryHolder holde @Override public void execute(DatacenterBoundRepositoryHolder holder) { - holder.getRepository().markAsUnused(unusedTopic); + holder.getRepository().upsert(unusedTopics); } @Override diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/command/UnmarkTopicAsUnusedRepositoryCommand.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/command/UnmarkTopicAsUnusedRepositoryCommand.java deleted file mode 100644 index e9b7d05adf..0000000000 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/command/UnmarkTopicAsUnusedRepositoryCommand.java +++ /dev/null @@ -1,42 +0,0 @@ -package pl.allegro.tech.hermes.management.domain.detection.command; - -import pl.allegro.tech.hermes.management.domain.dc.DatacenterBoundRepositoryHolder; -import pl.allegro.tech.hermes.management.domain.dc.RepositoryCommand; -import pl.allegro.tech.hermes.management.domain.detection.UnusedTopic; -import pl.allegro.tech.hermes.management.domain.detection.UnusedTopicsRepository; - -public class UnmarkTopicAsUnusedRepositoryCommand - extends RepositoryCommand { - - private final UnusedTopic unusedTopic; - - public UnmarkTopicAsUnusedRepositoryCommand(UnusedTopic unusedTopic) { - this.unusedTopic = unusedTopic; - } - - @Override - public void backup(DatacenterBoundRepositoryHolder holder) { - // TODO - } - - @Override - public void execute(DatacenterBoundRepositoryHolder holder) { - holder.getRepository().unmarkAsUnused(unusedTopic); - } - - @Override - public void rollback( - DatacenterBoundRepositoryHolder holder, Exception exception) { - // TODO - } - - @Override - public Class getRepositoryType() { - return UnusedTopicsRepository.class; - } - - @Override - public String toString() { - return String.format("UnmarkTopicAsUnused(%s)", unusedTopic.topicName().qualifiedName()); - } -} diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/zookeeper/detection/ZookeeperUnusedTopicsRepository.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/zookeeper/detection/ZookeeperUnusedTopicsRepository.java new file mode 100644 index 0000000000..f46128a2be --- /dev/null +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/zookeeper/detection/ZookeeperUnusedTopicsRepository.java @@ -0,0 +1,4 @@ +package pl.allegro.tech.hermes.management.infrastructure.zookeeper.detection; + +public class ZookeeperUnusedTopicsRepository { +} From 0f05e49139e7c80ed79e54ef11bb6f1e1feb98a2 Mon Sep 17 00:00:00 2001 From: Michal Wisniewski Date: Sun, 3 Nov 2024 14:18:52 +0100 Subject: [PATCH 03/39] SKYEDEN-3234 | Implement zk unusted topics repo --- .../zookeeper/ZookeeperPaths.java | 5 ++ .../domain/detection/UnusedTopic.java | 9 ++-- .../ZookeeperUnusedTopicsRepository.java | 54 +++++++++++++++++++ .../ZookeeperUnusedTopicsRepository.java | 4 -- 4 files changed, 64 insertions(+), 8 deletions(-) create mode 100644 hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/ZookeeperUnusedTopicsRepository.java delete mode 100644 hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/zookeeper/detection/ZookeeperUnusedTopicsRepository.java diff --git a/hermes-common/src/main/java/pl/allegro/tech/hermes/infrastructure/zookeeper/ZookeeperPaths.java b/hermes-common/src/main/java/pl/allegro/tech/hermes/infrastructure/zookeeper/ZookeeperPaths.java index e6d44c5830..5592f9a3e5 100644 --- a/hermes-common/src/main/java/pl/allegro/tech/hermes/infrastructure/zookeeper/ZookeeperPaths.java +++ b/hermes-common/src/main/java/pl/allegro/tech/hermes/infrastructure/zookeeper/ZookeeperPaths.java @@ -30,6 +30,7 @@ public class ZookeeperPaths { public static final String DATACENTER_READINESS_PATH = "datacenter-readiness"; public static final String OFFLINE_RETRANSMISSION_PATH = "offline-retransmission"; public static final String OFFLINE_RETRANSMISSION_TASKS_PATH = "tasks"; + public static final String UNUSED_TOPICS_PATH = "unused-topics"; private final String basePath; @@ -182,6 +183,10 @@ public String offlineRetransmissionPath(String taskId) { .join(basePath, OFFLINE_RETRANSMISSION_PATH, OFFLINE_RETRANSMISSION_TASKS_PATH, taskId); } + public String unusedTopicsPath() { + return Joiner.on(URL_SEPARATOR).join(basePath, UNUSED_TOPICS_PATH); + } + public String join(String... parts) { return Joiner.on(URL_SEPARATOR).join(parts); } diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopic.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopic.java index c794bc4565..195ed3f19e 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopic.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopic.java @@ -1,8 +1,9 @@ package pl.allegro.tech.hermes.management.domain.detection; -import pl.allegro.tech.hermes.api.TopicName; - -import java.time.Instant; +import com.fasterxml.jackson.annotation.JsonProperty; public record UnusedTopic( - TopicName topicName, Instant lastPublishedMessage, Instant lastNotified, boolean whitelisted) {} + @JsonProperty String qualifiedTopicName, + @JsonProperty long lastPublishedMessageTimestampMs, + @JsonProperty long lastNotifiedTimestampMs, + @JsonProperty boolean whitelisted) {} diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/ZookeeperUnusedTopicsRepository.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/ZookeeperUnusedTopicsRepository.java new file mode 100644 index 0000000000..2413d6864a --- /dev/null +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/ZookeeperUnusedTopicsRepository.java @@ -0,0 +1,54 @@ +package pl.allegro.tech.hermes.management.infrastructure.detection; + +import com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.databind.ObjectMapper; +import java.util.Collections; +import java.util.List; +import org.apache.curator.framework.CuratorFramework; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import pl.allegro.tech.hermes.common.exception.InternalProcessingException; +import pl.allegro.tech.hermes.infrastructure.zookeeper.ZookeeperBasedRepository; +import pl.allegro.tech.hermes.infrastructure.zookeeper.ZookeeperPaths; +import pl.allegro.tech.hermes.management.domain.detection.UnusedTopic; +import pl.allegro.tech.hermes.management.domain.detection.UnusedTopicsRepository; + +public class ZookeeperUnusedTopicsRepository extends ZookeeperBasedRepository + implements UnusedTopicsRepository { + + private static final Logger logger = + LoggerFactory.getLogger(ZookeeperUnusedTopicsRepository.class); + + public ZookeeperUnusedTopicsRepository( + CuratorFramework curatorFramework, ObjectMapper objectMapper, ZookeeperPaths paths) { + super(curatorFramework, objectMapper, paths); + } + + @Override + public void upsert(List unusedTopics) { + logger.info( + "Saving unused topics metadata into zookeeper, number of unused topics: {}", + unusedTopics.size()); + String path = paths.unusedTopicsPath(); + try { + if (pathExists(path)) { + overwrite(path, unusedTopics); + } else { + createRecursively(path, unusedTopics); + } + } catch (Exception e) { + throw new InternalProcessingException(e); + } + } + + @Override + public List read() { + String path = paths.unusedTopicsPath(); + if (!pathExists(path)) { + logger.warn("Unused topics ZK node does not exist: {}", path); + return Collections.emptyList(); + } + return readFrom(paths.unusedTopicsPath(), new TypeReference>() {}, true) + .orElse(Collections.emptyList()); + } +} diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/zookeeper/detection/ZookeeperUnusedTopicsRepository.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/zookeeper/detection/ZookeeperUnusedTopicsRepository.java deleted file mode 100644 index f46128a2be..0000000000 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/zookeeper/detection/ZookeeperUnusedTopicsRepository.java +++ /dev/null @@ -1,4 +0,0 @@ -package pl.allegro.tech.hermes.management.infrastructure.zookeeper.detection; - -public class ZookeeperUnusedTopicsRepository { -} From c255ef5289dab6a3808c1de9558efa3e0fd47683 Mon Sep 17 00:00:00 2001 From: Michal Wisniewski Date: Sun, 3 Nov 2024 15:49:14 +0100 Subject: [PATCH 04/39] SKYEDEN-3234 | Implement unused topics service --- .../config/storage/StorageConfiguration.java | 9 ++ .../domain/detection/UnusedTopic.java | 4 +- .../domain/detection/UnusedTopicsService.java | 28 ++++ .../MarkTopicsAsUnusedRepositoryCommand.java | 7 +- .../zookeeper/ZookeeperRepositoryManager.java | 8 + .../detection/UnusedTopicsServiceTest.groovy | 151 ++++++++++++++++++ 6 files changed, 203 insertions(+), 4 deletions(-) create mode 100644 hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsService.java create mode 100644 hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsServiceTest.groovy diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/storage/StorageConfiguration.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/storage/StorageConfiguration.java index ef050ca2d5..32c65b414c 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/storage/StorageConfiguration.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/storage/StorageConfiguration.java @@ -31,10 +31,12 @@ import pl.allegro.tech.hermes.infrastructure.zookeeper.ZookeeperWorkloadConstraintsRepository; import pl.allegro.tech.hermes.management.domain.blacklist.TopicBlacklistRepository; import pl.allegro.tech.hermes.management.domain.dc.MultiDatacenterRepositoryCommandExecutor; +import pl.allegro.tech.hermes.management.domain.detection.UnusedTopicsRepository; import pl.allegro.tech.hermes.management.domain.mode.ModeService; import pl.allegro.tech.hermes.management.domain.readiness.DatacenterReadinessRepository; import pl.allegro.tech.hermes.management.domain.retransmit.OfflineRetransmissionRepository; import pl.allegro.tech.hermes.management.infrastructure.blacklist.ZookeeperTopicBlacklistRepository; +import pl.allegro.tech.hermes.management.infrastructure.detection.ZookeeperUnusedTopicsRepository; import pl.allegro.tech.hermes.management.infrastructure.metrics.SummedSharedCounter; import pl.allegro.tech.hermes.management.infrastructure.readiness.ZookeeperDatacenterReadinessRepository; import pl.allegro.tech.hermes.management.infrastructure.retransmit.ZookeeperOfflineRetransmissionRepository; @@ -177,4 +179,11 @@ DatacenterReadinessRepository readinessRepository() { return new ZookeeperDatacenterReadinessRepository( localClient.getCuratorFramework(), objectMapper, zookeeperPaths()); } + + @Bean + UnusedTopicsRepository unusedTopicsRepository() { + ZookeeperClient localClient = clientManager().getLocalClient(); + return new ZookeeperUnusedTopicsRepository( + localClient.getCuratorFramework(), objectMapper, zookeeperPaths()); + } } diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopic.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopic.java index 195ed3f19e..453b155bf4 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopic.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopic.java @@ -2,8 +2,10 @@ import com.fasterxml.jackson.annotation.JsonProperty; +import java.util.Optional; + public record UnusedTopic( @JsonProperty String qualifiedTopicName, @JsonProperty long lastPublishedMessageTimestampMs, - @JsonProperty long lastNotifiedTimestampMs, + @JsonProperty Optional lastNotifiedTimestampMs, @JsonProperty boolean whitelisted) {} diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsService.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsService.java new file mode 100644 index 0000000000..b27754cc31 --- /dev/null +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsService.java @@ -0,0 +1,28 @@ +package pl.allegro.tech.hermes.management.domain.detection; + +import org.springframework.stereotype.Service; +import pl.allegro.tech.hermes.management.domain.dc.MultiDatacenterRepositoryCommandExecutor; +import pl.allegro.tech.hermes.management.domain.detection.command.MarkTopicsAsUnusedRepositoryCommand; + +import java.util.List; + +@Service +public class UnusedTopicsService { + private final UnusedTopicsRepository unusedTopicsRepository; + private final MultiDatacenterRepositoryCommandExecutor multiDcExecutor; + + public UnusedTopicsService( + UnusedTopicsRepository unusedTopicsRepository, + MultiDatacenterRepositoryCommandExecutor multiDcExecutor) { + this.unusedTopicsRepository = unusedTopicsRepository; + this.multiDcExecutor = multiDcExecutor; + } + + public void markAsUnused(List unusedTopics) { + multiDcExecutor.execute(new MarkTopicsAsUnusedRepositoryCommand(unusedTopics)); + } + + public List readUnusedTopics() { + return unusedTopicsRepository.read(); + } +} diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/command/MarkTopicsAsUnusedRepositoryCommand.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/command/MarkTopicsAsUnusedRepositoryCommand.java index 7454b25c3d..726d5d7243 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/command/MarkTopicsAsUnusedRepositoryCommand.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/command/MarkTopicsAsUnusedRepositoryCommand.java @@ -10,6 +10,7 @@ public class MarkTopicsAsUnusedRepositoryCommand extends RepositoryCommand { private final List unusedTopics; + private List backup; public MarkTopicsAsUnusedRepositoryCommand(List unusedTopics) { this.unusedTopics = unusedTopics; @@ -17,7 +18,7 @@ public MarkTopicsAsUnusedRepositoryCommand(List unusedTopics) { @Override public void backup(DatacenterBoundRepositoryHolder holder) { - // TODO + backup = holder.getRepository().read(); } @Override @@ -28,7 +29,7 @@ public void execute(DatacenterBoundRepositoryHolder hold @Override public void rollback( DatacenterBoundRepositoryHolder holder, Exception exception) { - // TODO + holder.getRepository().upsert(backup); } @Override @@ -38,6 +39,6 @@ public Class getRepositoryType() { @Override public String toString() { - return String.format("MarkTopicAsUnused(%s)", unusedTopic.topicName().qualifiedName()); + return String.format("MarkTopicsAsUnused(number of topics=%d)", unusedTopics.size()); } } diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/zookeeper/ZookeeperRepositoryManager.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/zookeeper/ZookeeperRepositoryManager.java index 3082d76bb9..8f5fe88f08 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/zookeeper/ZookeeperRepositoryManager.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/zookeeper/ZookeeperRepositoryManager.java @@ -33,9 +33,11 @@ import pl.allegro.tech.hermes.management.domain.blacklist.TopicBlacklistRepository; import pl.allegro.tech.hermes.management.domain.dc.DatacenterBoundRepositoryHolder; import pl.allegro.tech.hermes.management.domain.dc.RepositoryManager; +import pl.allegro.tech.hermes.management.domain.detection.UnusedTopicsRepository; import pl.allegro.tech.hermes.management.domain.readiness.DatacenterReadinessRepository; import pl.allegro.tech.hermes.management.domain.retransmit.OfflineRetransmissionRepository; import pl.allegro.tech.hermes.management.infrastructure.blacklist.ZookeeperTopicBlacklistRepository; +import pl.allegro.tech.hermes.management.infrastructure.detection.ZookeeperUnusedTopicsRepository; import pl.allegro.tech.hermes.management.infrastructure.readiness.ZookeeperDatacenterReadinessRepository; import pl.allegro.tech.hermes.management.infrastructure.retransmit.ZookeeperOfflineRetransmissionRepository; @@ -67,6 +69,7 @@ public class ZookeeperRepositoryManager implements RepositoryManager { new HashMap<>(); private final Map offlineRetransmissionRepositoriesByDc = new HashMap<>(); + private final Map unusedTopicsRepositoriesByDc = new HashMap<>(); private final ZookeeperGroupRepositoryFactory zookeeperGroupRepositoryFactory; public ZookeeperRepositoryManager( @@ -138,6 +141,10 @@ public void start() { ZookeeperOfflineRetransmissionRepository offlineRetransmissionRepository = new ZookeeperOfflineRetransmissionRepository(zookeeper, mapper, paths); offlineRetransmissionRepositoriesByDc.put(dcName, offlineRetransmissionRepository); + + ZookeeperUnusedTopicsRepository unusedTopicsRepository = + new ZookeeperUnusedTopicsRepository(zookeeper, mapper, paths); + unusedTopicsRepositoriesByDc.put(dcName, unusedTopicsRepository); } } @@ -189,5 +196,6 @@ private void initRepositoryTypeMap() { repositoryByType.put(DatacenterReadinessRepository.class, readinessRepositoriesByDc); repositoryByType.put( OfflineRetransmissionRepository.class, offlineRetransmissionRepositoriesByDc); + repositoryByType.put(UnusedTopicsRepository.class, unusedTopicsRepositoriesByDc); } } diff --git a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsServiceTest.groovy b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsServiceTest.groovy new file mode 100644 index 0000000000..1b9c67bb95 --- /dev/null +++ b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsServiceTest.groovy @@ -0,0 +1,151 @@ +package pl.allegro.tech.hermes.management.domain.detection + +import com.fasterxml.jackson.core.type.TypeReference +import com.fasterxml.jackson.databind.ObjectMapper +import com.fasterxml.jackson.datatype.jdk8.Jdk8Module +import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule +import pl.allegro.tech.hermes.common.exception.InternalProcessingException +import pl.allegro.tech.hermes.infrastructure.zookeeper.ZookeeperPaths +import pl.allegro.tech.hermes.management.config.storage.DefaultZookeeperGroupRepositoryFactory +import pl.allegro.tech.hermes.management.domain.dc.MultiDatacenterRepositoryCommandExecutor +import pl.allegro.tech.hermes.management.domain.mode.ModeService +import pl.allegro.tech.hermes.management.infrastructure.detection.ZookeeperUnusedTopicsRepository +import pl.allegro.tech.hermes.management.infrastructure.zookeeper.ZookeeperClientManager +import pl.allegro.tech.hermes.management.infrastructure.zookeeper.ZookeeperRepositoryManager +import pl.allegro.tech.hermes.management.utils.MultiZookeeperIntegrationTest + +class UnusedTopicsServiceTest extends MultiZookeeperIntegrationTest { + + static UNUSED_TOPICS_PATH = '/hermes/unused-topics' + + ZookeeperClientManager manager + ZookeeperRepositoryManager repositoryManager + ModeService modeService + MultiDatacenterRepositoryCommandExecutor commandExecutor + UnusedTopicsService unusedTopicsService + UnusedTopicsRepository unusedTopicsRepository + + def objectMapper = new ObjectMapper().registerModule(new JavaTimeModule()).registerModule(new Jdk8Module()) + def paths = new ZookeeperPaths('/hermes') + + def setup() { + manager = buildZookeeperClientManager() + manager.start() + assertZookeeperClientsConnected(manager.clients) + unusedTopicsRepository = new ZookeeperUnusedTopicsRepository(manager.localClient.curatorFramework, objectMapper, paths) + repositoryManager = new ZookeeperRepositoryManager( + manager, new TestDatacenterNameProvider(DC_1_NAME), objectMapper, + paths, new DefaultZookeeperGroupRepositoryFactory()) + repositoryManager.start() + modeService = new ModeService() + commandExecutor = new MultiDatacenterRepositoryCommandExecutor(repositoryManager, true, modeService) + unusedTopicsService = new UnusedTopicsService(unusedTopicsRepository, commandExecutor) + } + + def cleanup() { + manager.stop() + } + + def TEST_UNUSED_TOPICS = [ + new UnusedTopic("group.topic1", 1730641716154L, Optional.of(1730641716250L), false), + new UnusedTopic("group.topic2", 1730641712371L, Optional.empty(), true), + ] + + def "should create node in all zk clusters if it doesn't exist when upserting"() { + when: + unusedTopicsService.markAsUnused(TEST_UNUSED_TOPICS) + + then: + assertNodesContain(TEST_UNUSED_TOPICS) + } + + def "should update existing node data in all zk clusters when upserting"() { + given: + setupNodes(TEST_UNUSED_TOPICS) + + and: + def newUnusedTopics = [ + TEST_UNUSED_TOPICS[0], + new UnusedTopic("group.topic2", 1730641712371L, Optional.of(1730641712678L), false), + new UnusedTopic("group.topic3", 1730641712706L, Optional.of(1730641712999L), false), + ] + + when: + unusedTopicsService.markAsUnused(newUnusedTopics) + + then: + assertNodesContain(newUnusedTopics) + } + + def "nodes should remain unchanged in case of unavailability when upserting"() { + given: + setupNodes(TEST_UNUSED_TOPICS) + + and: + zookeeper2.stop() + + when: + unusedTopicsService.markAsUnused([ + new UnusedTopic("group.topic3", 1730641656154L, Optional.empty(), false) + ]) + + then: + def e = thrown(InternalProcessingException) + e.message == "Execution of command 'MarkTopicsAsUnused(number of topics=1)' failed on DC 'dc2'." + + and: + nodeData(DC_1_NAME) == TEST_UNUSED_TOPICS + } + + def "should return empty list when node doesn't exist"() { + when: + def result = unusedTopicsService.readUnusedTopics() + + then: + result == [] + } + + def "should return empty list when node is empty"() { + given: + setupNodes([]) + + when: + def result = unusedTopicsService.readUnusedTopics() + + then: + result == [] + } + + def "should return list of unused topics"() { + given: + setupNodes(TEST_UNUSED_TOPICS) + + when: + def result = unusedTopicsService.readUnusedTopics() + + then: + result.sort() == TEST_UNUSED_TOPICS.sort() + } + + private def setupNodes(List unusedTopics) { + manager.clients.each { + it.curatorFramework.create() + .creatingParentsIfNeeded() + .forPath(UNUSED_TOPICS_PATH, objectMapper.writeValueAsBytes(unusedTopics)) + } + } + + private def assertNodesContain(List unusedTopics) { + manager.clients.each { + def data = it.curatorFramework.getData().forPath(UNUSED_TOPICS_PATH) + def foundUnusedTopics = objectMapper.readValue(data, new TypeReference>() {}) + assert unusedTopics.sort() == foundUnusedTopics.sort() + } + } + + private List nodeData(String dc) { + def zkClient = findClientByDc(manager.clients, dc) + def data = zkClient.curatorFramework.getData().forPath(UNUSED_TOPICS_PATH) + return objectMapper.readValue(data, new TypeReference>() {}) + } +} From b82f3ac2c06d9552216089c910d849fe03b01303 Mon Sep 17 00:00:00 2001 From: Michal Wisniewski Date: Mon, 4 Nov 2024 12:23:54 +0100 Subject: [PATCH 05/39] SKYEDEN-3234 | Add repo for last published message timestamp --- ...LastPublishedMessageMetricsRepository.java | 9 ++++++ ...LastPublishedMessageMetricsRepository.java | 28 +++++++++++++++++ .../metrics/SummedSharedCounter.java | 28 +++++++++++++++++ .../metrics/SummedSharedCounterTest.groovy | 31 ++++++++++++++++++- 4 files changed, 95 insertions(+), 1 deletion(-) create mode 100644 hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/LastPublishedMessageMetricsRepository.java create mode 100644 hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/ZookeeperLastPublishedMessageMetricsRepository.java diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/LastPublishedMessageMetricsRepository.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/LastPublishedMessageMetricsRepository.java new file mode 100644 index 0000000000..2d3a4f65ba --- /dev/null +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/LastPublishedMessageMetricsRepository.java @@ -0,0 +1,9 @@ +package pl.allegro.tech.hermes.management.domain.detection; + +import pl.allegro.tech.hermes.api.TopicName; + +import java.time.Instant; + +public interface LastPublishedMessageMetricsRepository { + Instant getLastPublishedMessageTimestamp(TopicName topicName); +} diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/ZookeeperLastPublishedMessageMetricsRepository.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/ZookeeperLastPublishedMessageMetricsRepository.java new file mode 100644 index 0000000000..135d34ea77 --- /dev/null +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/ZookeeperLastPublishedMessageMetricsRepository.java @@ -0,0 +1,28 @@ +package pl.allegro.tech.hermes.management.infrastructure.detection; + +import org.springframework.stereotype.Component; +import pl.allegro.tech.hermes.api.TopicName; +import pl.allegro.tech.hermes.infrastructure.zookeeper.ZookeeperPaths; +import pl.allegro.tech.hermes.management.domain.detection.LastPublishedMessageMetricsRepository; +import pl.allegro.tech.hermes.management.infrastructure.metrics.SummedSharedCounter; + +import java.time.Instant; + +@Component +public class ZookeeperLastPublishedMessageMetricsRepository + implements LastPublishedMessageMetricsRepository { + private final SummedSharedCounter summedSharedCounter; + private final ZookeeperPaths zookeeperPaths; + + public ZookeeperLastPublishedMessageMetricsRepository( + SummedSharedCounter summedSharedCounter, ZookeeperPaths zookeeperPaths) { + this.summedSharedCounter = summedSharedCounter; + this.zookeeperPaths = zookeeperPaths; + } + + @Override + public Instant getLastPublishedMessageTimestamp(TopicName topicName) { + return summedSharedCounter.getLastModified( + zookeeperPaths.topicMetricPath(topicName, "published")); + } +} diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/metrics/SummedSharedCounter.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/metrics/SummedSharedCounter.java index cbd8f4a317..43ef6229dc 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/metrics/SummedSharedCounter.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/metrics/SummedSharedCounter.java @@ -3,6 +3,8 @@ import com.google.common.cache.CacheBuilder; import com.google.common.cache.CacheLoader; import com.google.common.cache.LoadingCache; + +import java.time.Instant; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -10,6 +12,7 @@ import org.apache.curator.framework.CuratorFramework; import org.apache.curator.framework.recipes.atomic.DistributedAtomicLong; import org.apache.curator.retry.ExponentialBackoffRetry; +import org.apache.zookeeper.data.Stat; import pl.allegro.tech.hermes.infrastructure.zookeeper.counter.ZookeeperCounterException; import pl.allegro.tech.hermes.management.infrastructure.zookeeper.ZookeeperClient; @@ -37,6 +40,16 @@ public long getValue(String path) { } } + public Instant getLastModified(String path) { + try { + return counterAggregators.get(path).getLastModified(); + } catch (ZookeeperCounterException e) { + throw e; + } catch (Exception e) { + throw new ZookeeperCounterException(path, e); + } + } + private LoadingCache buildLoadingCache( List zookeeperClients, int expireAfter, @@ -89,6 +102,21 @@ long aggregate() throws Exception { return sum; } + Instant getLastModified() throws Exception { + Instant lastModified = null; + for (Map.Entry curatorClient : curatorPerDatacenter.entrySet()) { + ensureConnected(curatorClient.getKey()); + Stat stat = curatorClient.getValue().checkExists().forPath(counterName); + if (stat != null) { + Instant nodeLastModified = Instant.ofEpochMilli(stat.getMtime()); + if (lastModified == null || nodeLastModified.isAfter(lastModified)) { + lastModified = nodeLastModified; + } + } + } + return lastModified; + } + private void ensureConnected(String datacenter) { CuratorFramework curator = curatorPerDatacenter.get(datacenter); if (!curator.getZookeeperClient().isConnected()) { diff --git a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/infrastructure/metrics/SummedSharedCounterTest.groovy b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/infrastructure/metrics/SummedSharedCounterTest.groovy index 3515e821aa..3fcf3b4e55 100644 --- a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/infrastructure/metrics/SummedSharedCounterTest.groovy +++ b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/infrastructure/metrics/SummedSharedCounterTest.groovy @@ -30,7 +30,7 @@ class SummedSharedCounterTest extends MultiZookeeperIntegrationTest { def zkClientDc2 = findClientByDc(manager.clients, DC_2_NAME).curatorFramework sharedCounterDc2 = new SharedCounter(zkClientDc2, EXPIRE_AFTER, DISTRIBUTED_LEADER_BACKOFF, DISTRIBUTED_LEADER_RETRIES) - summedSharedCounter = new SummedSharedCounter(manager.clients, (int) EXPIRE_AFTER.toHours(), (int) DISTRIBUTED_LEADER_BACKOFF.toMillis(), DISTRIBUTED_LEADER_RETRIES) + summedSharedCounter = new SummedSharedCounter(manager.clients, (int) EXPIRE_AFTER.toHours(), (int) DISTRIBUTED_LEADER_BACKOFF.toMillis(), DISTRIBUTED_LEADER_RETRIES) } def cleanup() { @@ -45,4 +45,33 @@ class SummedSharedCounterTest extends MultiZookeeperIntegrationTest { expect: summedSharedCounter.getValue(COUNTER_PATH) == 2 } + + def "should return last modified time of shared counters"() { + when: + sharedCounterDc1.increment(COUNTER_PATH, 1) + def sharedCounterDc1Mtime = getMtime(DC_1_NAME, COUNTER_PATH) + + then: + summedSharedCounter.getLastModified(COUNTER_PATH).toEpochMilli() == sharedCounterDc1Mtime + + when: + sharedCounterDc2.increment(COUNTER_PATH, 1) + def sharedCounterDc2Mtime = getMtime(DC_2_NAME, COUNTER_PATH) + + then: + summedSharedCounter.getLastModified(COUNTER_PATH).toEpochMilli() == sharedCounterDc2Mtime + } + + def "should return null for last modified time of non existing counter"() { + expect: + summedSharedCounter.getLastModified("/does/not/exist") == null + } + + private def getMtime(String dc, String counterPath) { + return findClientByDc(manager.clients, dc) + .curatorFramework + .checkExists() + .forPath(counterPath) + .mtime + } } From 3f89e441926ccb2336def9a8e184abf9dc907c41 Mon Sep 17 00:00:00 2001 From: Michal Wisniewski Date: Mon, 4 Nov 2024 14:04:59 +0100 Subject: [PATCH 06/39] SKYEDEN-3234 | Rename from read to get --- .../management/domain/detection/UnusedTopicsService.java | 2 +- .../domain/detection/UnusedTopicsServiceTest.groovy | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsService.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsService.java index b27754cc31..97af49095e 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsService.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsService.java @@ -22,7 +22,7 @@ public void markAsUnused(List unusedTopics) { multiDcExecutor.execute(new MarkTopicsAsUnusedRepositoryCommand(unusedTopics)); } - public List readUnusedTopics() { + public List getUnusedTopics() { return unusedTopicsRepository.read(); } } diff --git a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsServiceTest.groovy b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsServiceTest.groovy index 1b9c67bb95..25a5d67943 100644 --- a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsServiceTest.groovy +++ b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsServiceTest.groovy @@ -99,7 +99,7 @@ class UnusedTopicsServiceTest extends MultiZookeeperIntegrationTest { def "should return empty list when node doesn't exist"() { when: - def result = unusedTopicsService.readUnusedTopics() + def result = unusedTopicsService.getUnusedTopics() then: result == [] @@ -110,7 +110,7 @@ class UnusedTopicsServiceTest extends MultiZookeeperIntegrationTest { setupNodes([]) when: - def result = unusedTopicsService.readUnusedTopics() + def result = unusedTopicsService.getUnusedTopics() then: result == [] @@ -121,7 +121,7 @@ class UnusedTopicsServiceTest extends MultiZookeeperIntegrationTest { setupNodes(TEST_UNUSED_TOPICS) when: - def result = unusedTopicsService.readUnusedTopics() + def result = unusedTopicsService.getUnusedTopics() then: result.sort() == TEST_UNUSED_TOPICS.sort() From 5f888d435361109c3f5c501fc8fbd47c52f6f113 Mon Sep 17 00:00:00 2001 From: Michal Wisniewski Date: Mon, 4 Nov 2024 14:11:04 +0100 Subject: [PATCH 07/39] SKYEDEN-3234 | Change last notified to list with timestamps --- .../management/domain/detection/UnusedTopic.java | 5 ++--- .../domain/detection/UnusedTopicsServiceTest.groovy | 13 +++++++------ 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopic.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopic.java index 453b155bf4..d34c5649a1 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopic.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopic.java @@ -1,11 +1,10 @@ package pl.allegro.tech.hermes.management.domain.detection; import com.fasterxml.jackson.annotation.JsonProperty; - -import java.util.Optional; +import java.util.List; public record UnusedTopic( @JsonProperty String qualifiedTopicName, @JsonProperty long lastPublishedMessageTimestampMs, - @JsonProperty Optional lastNotifiedTimestampMs, + @JsonProperty List notificationTimestampsMs, @JsonProperty boolean whitelisted) {} diff --git a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsServiceTest.groovy b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsServiceTest.groovy index 25a5d67943..3fa02efdf1 100644 --- a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsServiceTest.groovy +++ b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsServiceTest.groovy @@ -25,7 +25,7 @@ class UnusedTopicsServiceTest extends MultiZookeeperIntegrationTest { UnusedTopicsService unusedTopicsService UnusedTopicsRepository unusedTopicsRepository - def objectMapper = new ObjectMapper().registerModule(new JavaTimeModule()).registerModule(new Jdk8Module()) + def objectMapper = new ObjectMapper().registerModule(new JavaTimeModule()) def paths = new ZookeeperPaths('/hermes') def setup() { @@ -47,8 +47,9 @@ class UnusedTopicsServiceTest extends MultiZookeeperIntegrationTest { } def TEST_UNUSED_TOPICS = [ - new UnusedTopic("group.topic1", 1730641716154L, Optional.of(1730641716250L), false), - new UnusedTopic("group.topic2", 1730641712371L, Optional.empty(), true), + new UnusedTopic("group.topic1", 1730641716154L, [1730641716250L, 1730641716321], false), + new UnusedTopic("group.topic2", 1730641712371L, [1730641716250L], false), + new UnusedTopic("group.topic3", 1730641712371L, [], true), ] def "should create node in all zk clusters if it doesn't exist when upserting"() { @@ -66,8 +67,8 @@ class UnusedTopicsServiceTest extends MultiZookeeperIntegrationTest { and: def newUnusedTopics = [ TEST_UNUSED_TOPICS[0], - new UnusedTopic("group.topic2", 1730641712371L, Optional.of(1730641712678L), false), - new UnusedTopic("group.topic3", 1730641712706L, Optional.of(1730641712999L), false), + new UnusedTopic("group.topic3", 1730641712371L, [1730641712678L], false), + new UnusedTopic("group.topic4", 1730641712706L, [1730641712999L], false), ] when: @@ -86,7 +87,7 @@ class UnusedTopicsServiceTest extends MultiZookeeperIntegrationTest { when: unusedTopicsService.markAsUnused([ - new UnusedTopic("group.topic3", 1730641656154L, Optional.empty(), false) + new UnusedTopic("group.topic3", 1730641656154L, [], false) ]) then: From c5e8ad098fca9994dc54dd01268e44f696931a96 Mon Sep 17 00:00:00 2001 From: Michal Wisniewski Date: Mon, 4 Nov 2024 16:18:59 +0100 Subject: [PATCH 08/39] SKYEDEN-3234 | Implement unused topics detection job --- .../UnusedTopicsDetectionProperties.java | 12 +++ .../domain/detection/UnusedTopic.java | 15 +++- .../detection/UnusedTopicsDetectionJob.java | 79 +++++++++++++++++++ .../UnusedTopicsDetectionService.java | 75 ++++++++++++++++++ .../detection/UnusedTopicsNotifier.java | 7 ++ 5 files changed, 187 insertions(+), 1 deletion(-) create mode 100644 hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/detection/UnusedTopicsDetectionProperties.java create mode 100644 hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionJob.java create mode 100644 hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionService.java create mode 100644 hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsNotifier.java diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/detection/UnusedTopicsDetectionProperties.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/detection/UnusedTopicsDetectionProperties.java new file mode 100644 index 0000000000..b796ddc71c --- /dev/null +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/detection/UnusedTopicsDetectionProperties.java @@ -0,0 +1,12 @@ +package pl.allegro.tech.hermes.management.config.detection; + +import org.springframework.boot.context.properties.ConfigurationProperties; + +import java.time.Duration; +import java.util.Set; + +@ConfigurationProperties(prefix = "detection.unused-topics") +public record UnusedTopicsDetectionProperties( + Duration inactivityThreshold, + Duration nextNotificationThreshold, + Set whitelistedQualifiedTopicNames) {} diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopic.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopic.java index d34c5649a1..4d4e01d6b2 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopic.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopic.java @@ -1,10 +1,23 @@ package pl.allegro.tech.hermes.management.domain.detection; import com.fasterxml.jackson.annotation.JsonProperty; +import java.time.Instant; +import java.util.ArrayList; import java.util.List; public record UnusedTopic( @JsonProperty String qualifiedTopicName, @JsonProperty long lastPublishedMessageTimestampMs, @JsonProperty List notificationTimestampsMs, - @JsonProperty boolean whitelisted) {} + @JsonProperty boolean whitelisted) { + + UnusedTopic notificationSent(Instant timestamp) { + List newNotificationTimestampsMs = new ArrayList<>(notificationTimestampsMs); + newNotificationTimestampsMs.add(timestamp.toEpochMilli()); + return new UnusedTopic( + this.qualifiedTopicName, + this.lastPublishedMessageTimestampMs, + newNotificationTimestampsMs, + whitelisted); + } +} diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionJob.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionJob.java new file mode 100644 index 0000000000..348b118d58 --- /dev/null +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionJob.java @@ -0,0 +1,79 @@ +package pl.allegro.tech.hermes.management.domain.detection; + +import static java.util.stream.Collectors.groupingBy; + +import java.time.Clock; +import java.time.Instant; +import java.util.*; +import java.util.stream.Collectors; +import java.util.stream.Stream; +import org.springframework.boot.context.properties.EnableConfigurationProperties; +import org.springframework.stereotype.Component; +import pl.allegro.tech.hermes.api.Topic; +import pl.allegro.tech.hermes.management.config.detection.UnusedTopicsDetectionProperties; +import pl.allegro.tech.hermes.management.domain.topic.TopicService; + +@Component +@EnableConfigurationProperties({UnusedTopicsDetectionProperties.class}) +public class UnusedTopicsDetectionJob { + private final TopicService topicService; + private final UnusedTopicsService unusedTopicsService; + private final UnusedTopicsDetectionService unusedTopicsDetectionService; + private final UnusedTopicsNotifier notifier; + private final Clock clock; + + public UnusedTopicsDetectionJob( + TopicService topicService, + UnusedTopicsService unusedTopicsService, + UnusedTopicsDetectionService unusedTopicsDetectionService, + UnusedTopicsNotifier notifier, + Clock clock) { + this.topicService = topicService; + this.unusedTopicsService = unusedTopicsService; + this.unusedTopicsDetectionService = unusedTopicsDetectionService; + this.notifier = notifier; + this.clock = clock; + } + + public void detectAndNotify() { + List allTopics = topicService.getAllTopics(); + List historicalUnusedTopics = unusedTopicsService.getUnusedTopics(); + List foundUnusedTopics = detectUnusedTopics(allTopics, historicalUnusedTopics); + + Map> groupedByNeedOfNotification = + foundUnusedTopics.stream() + .collect(groupingBy(unusedTopicsDetectionService::shouldBeNotified)); + + notifier.notify(groupedByNeedOfNotification.get(true)); + + Instant now = clock.instant(); + List unusedTopicsToSave = + Stream.concat( + groupedByNeedOfNotification.get(true).stream() + .map(topic -> topic.notificationSent(now)), + groupedByNeedOfNotification.get(false).stream()) + .toList(); + + unusedTopicsService.markAsUnused(unusedTopicsToSave); + } + + private List detectUnusedTopics( + List allTopics, List historicalUnusedTopics) { + Map historicalUnusedTopicsByName = groupByName(historicalUnusedTopics); + return allTopics.stream() + .map( + topic -> + unusedTopicsDetectionService.detectUnusedTopic( + topic.getName(), + Optional.ofNullable( + historicalUnusedTopicsByName.get(topic.getQualifiedName())))) + .map(opt -> opt.orElse(null)) + .filter(Objects::nonNull) + .toList(); + } + + private Map groupByName(List unusedTopics) { + return unusedTopics.stream() + .collect(Collectors.toMap(UnusedTopic::qualifiedTopicName, v -> v, (v1, v2) -> v1)); + } +} diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionService.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionService.java new file mode 100644 index 0000000000..e1aaa15f54 --- /dev/null +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionService.java @@ -0,0 +1,75 @@ +package pl.allegro.tech.hermes.management.domain.detection; + +import java.time.Clock; +import java.time.Duration; +import java.time.Instant; +import java.util.Collections; +import java.util.Optional; +import org.springframework.stereotype.Service; +import pl.allegro.tech.hermes.api.TopicName; +import pl.allegro.tech.hermes.management.config.detection.UnusedTopicsDetectionProperties; + +@Service +public class UnusedTopicsDetectionService { + private final LastPublishedMessageMetricsRepository metricsRepository; + private final UnusedTopicsDetectionProperties properties; + private final Clock clock; + + public UnusedTopicsDetectionService( + LastPublishedMessageMetricsRepository metricsRepository, + UnusedTopicsDetectionProperties properties, + Clock clock) { + this.metricsRepository = metricsRepository; + this.properties = properties; + this.clock = clock; + } + + public Optional detectUnusedTopic( + TopicName topicName, Optional historicalUnusedTopic) { + Instant now = clock.instant(); + Instant lastUsed = metricsRepository.getLastPublishedMessageTimestamp(topicName); + boolean isUnused = + Duration.between(lastUsed, now).compareTo(properties.inactivityThreshold()) >= 0; + + if (isUnused) { + return Optional.of( + new UnusedTopic( + topicName.qualifiedName(), + lastUsed.toEpochMilli(), + historicalUnusedTopic + .map(UnusedTopic::notificationTimestampsMs) + .orElse(Collections.emptyList()), + properties.whitelistedQualifiedTopicNames().contains(topicName.qualifiedName()))); + } else { + return Optional.empty(); + } + } + + public boolean shouldBeNotified(UnusedTopic unusedTopic) { + Instant now = clock.instant(); + Instant lastUsed = Instant.ofEpochMilli(unusedTopic.lastPublishedMessageTimestampMs()); + Optional lastNotified = + unusedTopic.notificationTimestampsMs().stream() + .max(Long::compare) + .map(Instant::ofEpochMilli); + boolean isInactive = + Duration.between(lastUsed, now).compareTo(properties.inactivityThreshold()) >= 0; + + return isInactive + && !unusedTopic.whitelisted() + && lastNotified + .map( + instant -> + durationBetweenSurpassesThreshold( + instant, now, properties.nextNotificationThreshold())) + .orElseGet( + () -> + durationBetweenSurpassesThreshold( + lastUsed, now, properties.inactivityThreshold())); + } + + private boolean durationBetweenSurpassesThreshold( + Instant start, Instant end, Duration threshold) { + return Duration.between(start, end).compareTo(threshold) >= 0; + } +} diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsNotifier.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsNotifier.java new file mode 100644 index 0000000000..6b1dca8f6e --- /dev/null +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsNotifier.java @@ -0,0 +1,7 @@ +package pl.allegro.tech.hermes.management.domain.detection; + +import java.util.List; + +public interface UnusedTopicsNotifier { + void notify(List unusedTopics); +} From a850566782a00a08196ef61441b6ef3a491bac17 Mon Sep 17 00:00:00 2001 From: Michal Wisniewski Date: Tue, 5 Nov 2024 15:48:21 +0100 Subject: [PATCH 09/39] SKYEDEN-3234 | Add tests for unused topics detection --- .../UnusedTopicsDetectionServiceTest.groovy | 199 ++++++++++++++++++ 1 file changed, 199 insertions(+) create mode 100644 hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionServiceTest.groovy diff --git a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionServiceTest.groovy b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionServiceTest.groovy new file mode 100644 index 0000000000..4549c903b9 --- /dev/null +++ b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionServiceTest.groovy @@ -0,0 +1,199 @@ +package pl.allegro.tech.hermes.management.domain.detection + +import pl.allegro.tech.hermes.api.TopicName +import pl.allegro.tech.hermes.management.config.detection.UnusedTopicsDetectionProperties +import spock.lang.Specification + +import java.time.Clock +import java.time.Duration +import java.time.Instant +import java.time.temporal.ChronoUnit + +class UnusedTopicsDetectionServiceTest extends Specification { + + private static def TEST_TOPIC_NAME = "group.topic" + private static def WHITELISTED_TOPIC_NAME = "whitelisted.topic" + private static def LAST_PUBLISHED = Instant.ofEpochMilli(1630600266987L) + private static def INACTIVITY_THRESHOLD = 7 + private static def NEXT_NOTIFICATION_THRESHOLD = 14 + + UnusedTopicsDetectionProperties properties = new UnusedTopicsDetectionProperties( + Duration.ofDays(INACTIVITY_THRESHOLD), + Duration.ofDays(NEXT_NOTIFICATION_THRESHOLD), + [WHITELISTED_TOPIC_NAME] as Set + ) + + private def metricsRepositoryMock = Mock(LastPublishedMessageMetricsRepository) + private def clockMock = Mock(Clock) + private def service = new UnusedTopicsDetectionService(metricsRepositoryMock, properties, clockMock) + + def setup() { + metricsRepositoryMock.getLastPublishedMessageTimestamp(TopicName.fromQualifiedName(TEST_TOPIC_NAME)) >> LAST_PUBLISHED + metricsRepositoryMock.getLastPublishedMessageTimestamp(TopicName.fromQualifiedName(WHITELISTED_TOPIC_NAME)) >> LAST_PUBLISHED + } + + def "should detect unused topic when it surpasses inactivity threshold"() { + given: + clockMock.instant() >> now + + when: + def result = service.detectUnusedTopic( + TopicName.fromQualifiedName(TEST_TOPIC_NAME), + historicalUnusedTopic + ) + + then: + result.get() == new UnusedTopic( + TEST_TOPIC_NAME, + LAST_PUBLISHED.toEpochMilli(), + historicalUnusedTopic.map { it.notificationTimestampsMs() }.orElse([]), + false + ) + + where: + now | historicalUnusedTopic + plusDays(LAST_PUBLISHED, INACTIVITY_THRESHOLD) | Optional.of(unusedTopic(TEST_TOPIC_NAME, LAST_PUBLISHED, [now])) + plusDays(LAST_PUBLISHED, INACTIVITY_THRESHOLD) | Optional.empty() + plusDays(LAST_PUBLISHED, INACTIVITY_THRESHOLD + 3) | Optional.of(unusedTopic(TEST_TOPIC_NAME, LAST_PUBLISHED, [now])) + } + + def "should correctly detect unused topic as whitelisted or not"() { + given: + def now = plusDays(LAST_PUBLISHED, INACTIVITY_THRESHOLD) + def historicalUnusedTopic = unusedTopic(topicName, LAST_PUBLISHED, [now], historicallyWhitelisted) + + and: + clockMock.instant() >> now + + when: + def result = service.detectUnusedTopic( + TopicName.fromQualifiedName(topicName), + Optional.of(historicalUnusedTopic) + ) + + then: + result.get() == new UnusedTopic( + topicName, + LAST_PUBLISHED.toEpochMilli(), + historicalUnusedTopic.notificationTimestampsMs(), + whitelisted + ) + + where: + topicName | historicallyWhitelisted || whitelisted + TEST_TOPIC_NAME | false || false + TEST_TOPIC_NAME | true || false + WHITELISTED_TOPIC_NAME | false || true + WHITELISTED_TOPIC_NAME | true || true + } + + def "should not detect unused topic when it is within inactivity threshold"() { + given: + def now = plusDays(LAST_PUBLISHED, INACTIVITY_THRESHOLD - 1) + + and: + clockMock.instant() >> now + + when: + def result = service.detectUnusedTopic( + TopicName.fromQualifiedName(TEST_TOPIC_NAME), + historicalUnusedTopic + ) + + then: + result.isEmpty() + + where: + historicalUnusedTopic << [ + Optional.of(unusedTopic(TEST_TOPIC_NAME, LAST_PUBLISHED, [])), + Optional.empty(), + ] + } + + def "should be notified when inactive and no sent notifications"() { + given: + clockMock.instant() >> now + + and: + def unusedTopic = unusedTopic(TEST_TOPIC_NAME, LAST_PUBLISHED, []) + + expect: + service.shouldBeNotified(unusedTopic) + + where: + now << [plusDays(LAST_PUBLISHED, INACTIVITY_THRESHOLD), plusDays(LAST_PUBLISHED, 2 * INACTIVITY_THRESHOLD)] + } + + def "should be notified when inactive and enough time from last notification"() { + given: + clockMock.instant() >> now + + and: + def unusedTopic = unusedTopic(TEST_TOPIC_NAME, LAST_PUBLISHED, notificationTimestamps) + + expect: + service.shouldBeNotified(unusedTopic) + + where: + now | notificationTimestamps + plusDays(LAST_PUBLISHED, INACTIVITY_THRESHOLD + NEXT_NOTIFICATION_THRESHOLD) | [plusDays(LAST_PUBLISHED, INACTIVITY_THRESHOLD)] + plusDays(LAST_PUBLISHED, INACTIVITY_THRESHOLD + 2 * NEXT_NOTIFICATION_THRESHOLD) | [plusDays(LAST_PUBLISHED, INACTIVITY_THRESHOLD + NEXT_NOTIFICATION_THRESHOLD), plusDays(LAST_PUBLISHED, INACTIVITY_THRESHOLD)] + } + + def "should not be notified if not inactive"() { + given: + def now = plusDays(LAST_PUBLISHED, INACTIVITY_THRESHOLD - 1) + + and: + clockMock.instant() >> now + + and: + def unusedTopic = unusedTopic(TEST_TOPIC_NAME, LAST_PUBLISHED, []) + + expect: + !service.shouldBeNotified(unusedTopic) + } + + def "should not be notified when whitelisted"() { + given: + def now = plusDays(LAST_PUBLISHED, INACTIVITY_THRESHOLD) + + and: + clockMock.instant() >> now + + and: + def unusedTopic = unusedTopic(TEST_TOPIC_NAME, LAST_PUBLISHED, [], true) + + expect: + !service.shouldBeNotified(unusedTopic) + } + + def "should not be notified when not enough time from last notification"() { + given: + clockMock.instant() >> now + + and: + def unusedTopic = unusedTopic(TEST_TOPIC_NAME, LAST_PUBLISHED, notificationTimestamps) + + expect: + !service.shouldBeNotified(unusedTopic) + + where: + now | notificationTimestamps + plusDays(LAST_PUBLISHED, INACTIVITY_THRESHOLD) | [now] + plusDays(LAST_PUBLISHED, INACTIVITY_THRESHOLD + NEXT_NOTIFICATION_THRESHOLD + 1) | [plusDays(LAST_PUBLISHED, INACTIVITY_THRESHOLD + NEXT_NOTIFICATION_THRESHOLD), plusDays(LAST_PUBLISHED, INACTIVITY_THRESHOLD)] + } + + private static Instant plusDays(Instant instant, int days) { + return instant.plus(days, ChronoUnit.DAYS) + } + + private static UnusedTopic unusedTopic(String name, Instant lastPublished, List notificationTimestamps, boolean whitelisted = false) { + return new UnusedTopic( + name, + lastPublished.toEpochMilli(), + notificationTimestamps.collect { it.toEpochMilli() }, + whitelisted + ) + } +} From a6779b9d36c051e739ba91fc26b7f3b5f61b6633 Mon Sep 17 00:00:00 2001 From: Michal Wisniewski Date: Wed, 6 Nov 2024 15:35:10 +0100 Subject: [PATCH 10/39] SKYEDEN-3234 | Add detection job test --- .../detection/UnusedTopicsDetectionJob.java | 22 ++--- .../InMemoryUnusedTopicsNotifier.groovy | 18 ++++ .../UnusedTopicsDetectionJobTest.groovy | 89 +++++++++++++++++++ 3 files changed, 118 insertions(+), 11 deletions(-) create mode 100644 hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InMemoryUnusedTopicsNotifier.groovy create mode 100644 hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionJobTest.groovy diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionJob.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionJob.java index 348b118d58..6b56a71838 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionJob.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionJob.java @@ -9,7 +9,7 @@ import java.util.stream.Stream; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.stereotype.Component; -import pl.allegro.tech.hermes.api.Topic; +import pl.allegro.tech.hermes.api.TopicName; import pl.allegro.tech.hermes.management.config.detection.UnusedTopicsDetectionProperties; import pl.allegro.tech.hermes.management.domain.topic.TopicService; @@ -36,9 +36,10 @@ public UnusedTopicsDetectionJob( } public void detectAndNotify() { - List allTopics = topicService.getAllTopics(); + List qualifiedTopicNames = topicService.listQualifiedTopicNames(); List historicalUnusedTopics = unusedTopicsService.getUnusedTopics(); - List foundUnusedTopics = detectUnusedTopics(allTopics, historicalUnusedTopics); + List foundUnusedTopics = + detectUnusedTopics(qualifiedTopicNames, historicalUnusedTopics); Map> groupedByNeedOfNotification = foundUnusedTopics.stream() @@ -49,24 +50,23 @@ public void detectAndNotify() { Instant now = clock.instant(); List unusedTopicsToSave = Stream.concat( - groupedByNeedOfNotification.get(true).stream() + groupedByNeedOfNotification.getOrDefault(true, Collections.emptyList()).stream() .map(topic -> topic.notificationSent(now)), - groupedByNeedOfNotification.get(false).stream()) + groupedByNeedOfNotification.getOrDefault(false, Collections.emptyList()).stream()) .toList(); unusedTopicsService.markAsUnused(unusedTopicsToSave); } private List detectUnusedTopics( - List allTopics, List historicalUnusedTopics) { + List qualifiedTopicNames, List historicalUnusedTopics) { Map historicalUnusedTopicsByName = groupByName(historicalUnusedTopics); - return allTopics.stream() + return qualifiedTopicNames.stream() .map( - topic -> + qualifiedTopicName -> unusedTopicsDetectionService.detectUnusedTopic( - topic.getName(), - Optional.ofNullable( - historicalUnusedTopicsByName.get(topic.getQualifiedName())))) + TopicName.fromQualifiedName(qualifiedTopicName), + Optional.ofNullable(historicalUnusedTopicsByName.get(qualifiedTopicName)))) .map(opt -> opt.orElse(null)) .filter(Objects::nonNull) .toList(); diff --git a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InMemoryUnusedTopicsNotifier.groovy b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InMemoryUnusedTopicsNotifier.groovy new file mode 100644 index 0000000000..e436521a76 --- /dev/null +++ b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InMemoryUnusedTopicsNotifier.groovy @@ -0,0 +1,18 @@ +package pl.allegro.tech.hermes.management.domain.detection + +class InMemoryUnusedTopicsNotifier implements UnusedTopicsNotifier { + private Set notifiedTopics = new HashSet<>(); + + @Override + void notify(List unusedTopics) { + notifiedTopics.addAll(unusedTopics) + } + + void reset() { + notifiedTopics.clear() + } + + Set getNotifiedTopics() { + return notifiedTopics + } +} diff --git a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionJobTest.groovy b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionJobTest.groovy new file mode 100644 index 0000000000..4082bd0773 --- /dev/null +++ b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionJobTest.groovy @@ -0,0 +1,89 @@ +package pl.allegro.tech.hermes.management.domain.detection + + +import pl.allegro.tech.hermes.api.TopicName +import pl.allegro.tech.hermes.management.config.detection.UnusedTopicsDetectionProperties +import pl.allegro.tech.hermes.management.domain.topic.TopicService +import spock.lang.Specification + +import java.time.Clock +import java.time.Duration +import java.time.Instant + +import static java.time.temporal.ChronoUnit.DAYS + +class UnusedTopicsDetectionJobTest extends Specification { + + def topicServiceMock = Mock(TopicService) + def unusedTopicsServiceMock = Mock(UnusedTopicsService) + def metricsRepositoryMock = Mock(LastPublishedMessageMetricsRepository) + def clockMock = Mock(Clock) + def unusedTopicsNotifier = new InMemoryUnusedTopicsNotifier() + + def unusedTopicsDetectionProperties = new UnusedTopicsDetectionProperties( + Duration.ofDays(7), + Duration.ofDays(14), + ["group.topic3"] as Set + ) + + UnusedTopicsDetectionService detectionService = new UnusedTopicsDetectionService( + metricsRepositoryMock, + unusedTopicsDetectionProperties, + clockMock + ) + + UnusedTopicsDetectionJob detectionJob = new UnusedTopicsDetectionJob( + topicServiceMock, + unusedTopicsServiceMock, + detectionService, + unusedTopicsNotifier, + clockMock + ) + + def "should detect unused topics and notify when needed"() { + given: + def now = Instant.ofEpochMilli(1630600266987L) + def ago7days = now.minus(7, DAYS) + def ago14days = now.minus(14, DAYS) + def ago21days = now.minus(21, DAYS) + clockMock.instant() >> now + + and: + topicServiceMock.listQualifiedTopicNames() >> [ + "group.topic0", + "group.topic1", + "group.topic2", + "group.topic3", + "group.topic4", + ] + + and: "historically saved unused topics" + unusedTopicsServiceMock.getUnusedTopics() >> [ + new UnusedTopic("group.topic2", ago7days.toEpochMilli(), [], false), + new UnusedTopic("group.topic3", ago7days.toEpochMilli(), [], true), + new UnusedTopic("group.topic4", ago21days.toEpochMilli(), [ago14days.toEpochMilli()], false), + ] + + and: "current last published message timestamp" + metricsRepositoryMock.getLastPublishedMessageTimestamp(TopicName.fromQualifiedName("group.topic0")) >> now + metricsRepositoryMock.getLastPublishedMessageTimestamp(TopicName.fromQualifiedName("group.topic1")) >> ago7days + metricsRepositoryMock.getLastPublishedMessageTimestamp(TopicName.fromQualifiedName("group.topic2")) >> now + metricsRepositoryMock.getLastPublishedMessageTimestamp(TopicName.fromQualifiedName("group.topic3")) >> ago7days + metricsRepositoryMock.getLastPublishedMessageTimestamp(TopicName.fromQualifiedName("group.topic4")) >> ago21days + + when: + detectionJob.detectAndNotify() + + then: + unusedTopicsNotifier.getNotifiedTopics().toList() == [ + new UnusedTopic("group.topic1", ago7days.toEpochMilli(), [], false), + new UnusedTopic("group.topic4", ago21days.toEpochMilli(), [ago14days.toEpochMilli()], false) + ] + + 1 * unusedTopicsServiceMock.markAsUnused([ + new UnusedTopic("group.topic1", ago7days.toEpochMilli(), [now.toEpochMilli()], false), + new UnusedTopic("group.topic4", ago21days.toEpochMilli(), [ago14days.toEpochMilli(), now.toEpochMilli()], false), + new UnusedTopic("group.topic3", ago7days.toEpochMilli(), [], true) + ]) + } +} From 299cf80e4b5a70a915621056c3e3d57a2ed9d705 Mon Sep 17 00:00:00 2001 From: Michal Wisniewski Date: Thu, 7 Nov 2024 11:57:41 +0100 Subject: [PATCH 11/39] SKYEDEN-3234 | Refactor detection job test --- .../UnusedTopicsDetectionJobTest.groovy | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionJobTest.groovy b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionJobTest.groovy index 4082bd0773..d6da0ad37b 100644 --- a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionJobTest.groovy +++ b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionJobTest.groovy @@ -1,7 +1,6 @@ package pl.allegro.tech.hermes.management.domain.detection -import pl.allegro.tech.hermes.api.TopicName import pl.allegro.tech.hermes.management.config.detection.UnusedTopicsDetectionProperties import pl.allegro.tech.hermes.management.domain.topic.TopicService import spock.lang.Specification @@ -11,6 +10,7 @@ import java.time.Duration import java.time.Instant import static java.time.temporal.ChronoUnit.DAYS +import static pl.allegro.tech.hermes.api.TopicName.fromQualifiedName class UnusedTopicsDetectionJobTest extends Specification { @@ -48,7 +48,7 @@ class UnusedTopicsDetectionJobTest extends Specification { def ago21days = now.minus(21, DAYS) clockMock.instant() >> now - and: + and: "names of all topics" topicServiceMock.listQualifiedTopicNames() >> [ "group.topic0", "group.topic1", @@ -65,21 +65,22 @@ class UnusedTopicsDetectionJobTest extends Specification { ] and: "current last published message timestamp" - metricsRepositoryMock.getLastPublishedMessageTimestamp(TopicName.fromQualifiedName("group.topic0")) >> now - metricsRepositoryMock.getLastPublishedMessageTimestamp(TopicName.fromQualifiedName("group.topic1")) >> ago7days - metricsRepositoryMock.getLastPublishedMessageTimestamp(TopicName.fromQualifiedName("group.topic2")) >> now - metricsRepositoryMock.getLastPublishedMessageTimestamp(TopicName.fromQualifiedName("group.topic3")) >> ago7days - metricsRepositoryMock.getLastPublishedMessageTimestamp(TopicName.fromQualifiedName("group.topic4")) >> ago21days + metricsRepositoryMock.getLastPublishedMessageTimestamp(fromQualifiedName("group.topic0")) >> now + metricsRepositoryMock.getLastPublishedMessageTimestamp(fromQualifiedName("group.topic1")) >> ago7days + metricsRepositoryMock.getLastPublishedMessageTimestamp(fromQualifiedName("group.topic2")) >> now + metricsRepositoryMock.getLastPublishedMessageTimestamp(fromQualifiedName("group.topic3")) >> ago7days + metricsRepositoryMock.getLastPublishedMessageTimestamp(fromQualifiedName("group.topic4")) >> ago21days when: detectionJob.detectAndNotify() - then: + then: "notified are unused topics that are not whitelisted" unusedTopicsNotifier.getNotifiedTopics().toList() == [ new UnusedTopic("group.topic1", ago7days.toEpochMilli(), [], false), new UnusedTopic("group.topic4", ago21days.toEpochMilli(), [ago14days.toEpochMilli()], false) ] + and: "saved are all unused topics with updated notification timestamps" 1 * unusedTopicsServiceMock.markAsUnused([ new UnusedTopic("group.topic1", ago7days.toEpochMilli(), [now.toEpochMilli()], false), new UnusedTopic("group.topic4", ago21days.toEpochMilli(), [ago14days.toEpochMilli(), now.toEpochMilli()], false), From da49bace20d951beeaaa3b5ac4c4168c7ddf7a80 Mon Sep 17 00:00:00 2001 From: Michal Wisniewski Date: Thu, 7 Nov 2024 12:50:04 +0100 Subject: [PATCH 12/39] SKYEDEN-3234 | Make unused topics notifier bean optional --- .../detection/UnusedTopicsDetectionJob.java | 39 +++++++++++++------ .../UnusedTopicsDetectionJobTest.groovy | 22 ++++++++++- 2 files changed, 49 insertions(+), 12 deletions(-) diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionJob.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionJob.java index 6b56a71838..917f95ef6f 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionJob.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionJob.java @@ -7,6 +7,8 @@ import java.util.*; import java.util.stream.Collectors; import java.util.stream.Stream; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.stereotype.Component; import pl.allegro.tech.hermes.api.TopicName; @@ -19,20 +21,25 @@ public class UnusedTopicsDetectionJob { private final TopicService topicService; private final UnusedTopicsService unusedTopicsService; private final UnusedTopicsDetectionService unusedTopicsDetectionService; - private final UnusedTopicsNotifier notifier; + private final Optional notifier; private final Clock clock; + private static final Logger logger = LoggerFactory.getLogger(UnusedTopicsDetectionJob.class); + public UnusedTopicsDetectionJob( TopicService topicService, UnusedTopicsService unusedTopicsService, UnusedTopicsDetectionService unusedTopicsDetectionService, - UnusedTopicsNotifier notifier, + Optional notifier, Clock clock) { this.topicService = topicService; this.unusedTopicsService = unusedTopicsService; this.unusedTopicsDetectionService = unusedTopicsDetectionService; - this.notifier = notifier; this.clock = clock; + if (notifier.isEmpty()) { + logger.info("Unused topics notifier bean is absent"); + } + this.notifier = notifier; } public void detectAndNotify() { @@ -45,17 +52,17 @@ public void detectAndNotify() { foundUnusedTopics.stream() .collect(groupingBy(unusedTopicsDetectionService::shouldBeNotified)); - notifier.notify(groupedByNeedOfNotification.get(true)); + List topicsToNotify = groupedByNeedOfNotification.getOrDefault(true, List.of()); + List topicsToSkipNotification = + groupedByNeedOfNotification.getOrDefault(false, List.of()); + + notify(topicsToNotify); Instant now = clock.instant(); - List unusedTopicsToSave = - Stream.concat( - groupedByNeedOfNotification.getOrDefault(true, Collections.emptyList()).stream() - .map(topic -> topic.notificationSent(now)), - groupedByNeedOfNotification.getOrDefault(false, Collections.emptyList()).stream()) - .toList(); + List notifiedTopics = + topicsToNotify.stream().map(topic -> topic.notificationSent(now)).toList(); - unusedTopicsService.markAsUnused(unusedTopicsToSave); + saveUnusedTopics(notifiedTopics, topicsToSkipNotification); } private List detectUnusedTopics( @@ -76,4 +83,14 @@ private Map groupByName(List unusedTopics) { return unusedTopics.stream() .collect(Collectors.toMap(UnusedTopic::qualifiedTopicName, v -> v, (v1, v2) -> v1)); } + + private void notify(List unusedTopics) { + notifier.ifPresent(notifier -> notifier.notify(unusedTopics)); + } + + private void saveUnusedTopics( + List notifiedTopics, List skippedNotificationTopics) { + unusedTopicsService.markAsUnused( + Stream.concat(notifiedTopics.stream(), skippedNotificationTopics.stream()).toList()); + } } diff --git a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionJobTest.groovy b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionJobTest.groovy index d6da0ad37b..ace9fa0346 100644 --- a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionJobTest.groovy +++ b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionJobTest.groovy @@ -36,7 +36,7 @@ class UnusedTopicsDetectionJobTest extends Specification { topicServiceMock, unusedTopicsServiceMock, detectionService, - unusedTopicsNotifier, + Optional.of(unusedTopicsNotifier), clockMock ) @@ -87,4 +87,24 @@ class UnusedTopicsDetectionJobTest extends Specification { new UnusedTopic("group.topic3", ago7days.toEpochMilli(), [], true) ]) } + + def "should not notify if there are no unused topics"() { + given: + topicServiceMock.listQualifiedTopicNames() >> ["group.topic0"] + unusedTopicsServiceMock.getUnusedTopics() >> [] + + and: + def now = Instant.ofEpochMilli(1630600266987L) + clockMock.instant() >> now + metricsRepositoryMock.getLastPublishedMessageTimestamp(fromQualifiedName("group.topic0")) >> now + + when: + detectionJob.detectAndNotify() + + then: + unusedTopicsNotifier.getNotifiedTopics().toList() == [] + + and: + 1 * unusedTopicsServiceMock.markAsUnused([]) + } } From 3ffe9f84414861244125095a7c0f31fa60384d20 Mon Sep 17 00:00:00 2001 From: Michal Wisniewski Date: Tue, 12 Nov 2024 09:39:31 +0100 Subject: [PATCH 13/39] SKYEDEN-3234 | Add scheduling --- .../zookeeper/ZookeeperPaths.java | 4 + .../hermes/management/HermesManagement.java | 2 + .../UnusedTopicsDetectionProperties.java | 3 +- .../UnusedTopicsDetectionScheduler.java | 75 +++++++++++++++++++ 4 files changed, 83 insertions(+), 1 deletion(-) create mode 100644 hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/UnusedTopicsDetectionScheduler.java diff --git a/hermes-common/src/main/java/pl/allegro/tech/hermes/infrastructure/zookeeper/ZookeeperPaths.java b/hermes-common/src/main/java/pl/allegro/tech/hermes/infrastructure/zookeeper/ZookeeperPaths.java index 5592f9a3e5..010b4b5f23 100644 --- a/hermes-common/src/main/java/pl/allegro/tech/hermes/infrastructure/zookeeper/ZookeeperPaths.java +++ b/hermes-common/src/main/java/pl/allegro/tech/hermes/infrastructure/zookeeper/ZookeeperPaths.java @@ -187,6 +187,10 @@ public String unusedTopicsPath() { return Joiner.on(URL_SEPARATOR).join(basePath, UNUSED_TOPICS_PATH); } + public String unusedTopicsLeaderPath() { + return Joiner.on(URL_SEPARATOR).join(unusedTopicsPath(), "leader"); + } + public String join(String... parts) { return Joiner.on(URL_SEPARATOR).join(parts); } diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/HermesManagement.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/HermesManagement.java index 3372816d20..f5298fec0d 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/HermesManagement.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/HermesManagement.java @@ -2,8 +2,10 @@ import org.springframework.boot.SpringApplication; import org.springframework.boot.autoconfigure.SpringBootApplication; +import org.springframework.scheduling.annotation.EnableScheduling; @SpringBootApplication +@EnableScheduling public class HermesManagement { public static void main(String[] args) { diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/detection/UnusedTopicsDetectionProperties.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/detection/UnusedTopicsDetectionProperties.java index b796ddc71c..deb6809656 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/detection/UnusedTopicsDetectionProperties.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/detection/UnusedTopicsDetectionProperties.java @@ -9,4 +9,5 @@ public record UnusedTopicsDetectionProperties( Duration inactivityThreshold, Duration nextNotificationThreshold, - Set whitelistedQualifiedTopicNames) {} + Set whitelistedQualifiedTopicNames, + String leaderElectionZookeeperDc) {} diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/UnusedTopicsDetectionScheduler.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/UnusedTopicsDetectionScheduler.java new file mode 100644 index 0000000000..5b077484f1 --- /dev/null +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/UnusedTopicsDetectionScheduler.java @@ -0,0 +1,75 @@ +package pl.allegro.tech.hermes.management.infrastructure.detection; + +import jakarta.annotation.PostConstruct; +import java.util.Optional; +import org.apache.curator.framework.CuratorFramework; +import org.apache.curator.framework.recipes.leader.LeaderLatch; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.boot.context.properties.EnableConfigurationProperties; +import org.springframework.scheduling.annotation.Scheduled; +import org.springframework.stereotype.Component; +import pl.allegro.tech.hermes.infrastructure.zookeeper.ZookeeperPaths; +import pl.allegro.tech.hermes.management.config.detection.UnusedTopicsDetectionProperties; +import pl.allegro.tech.hermes.management.domain.detection.UnusedTopicsDetectionJob; +import pl.allegro.tech.hermes.management.infrastructure.zookeeper.ZookeeperClient; +import pl.allegro.tech.hermes.management.infrastructure.zookeeper.ZookeeperClientManager; + +@ConditionalOnProperty(value = "detection.unused-topics.enabled", havingValue = "true") +@Component +@EnableConfigurationProperties(UnusedTopicsDetectionProperties.class) +public class UnusedTopicsDetectionScheduler { + private final UnusedTopicsDetectionJob job; + private final String leaderElectionDc; + private final Optional leaderLatch; + + private static final Logger logger = + LoggerFactory.getLogger(UnusedTopicsDetectionScheduler.class); + + public UnusedTopicsDetectionScheduler( + UnusedTopicsDetectionJob job, + ZookeeperClientManager zookeeperClientManager, + UnusedTopicsDetectionProperties unusedTopicsDetectionProperties, + ZookeeperPaths zookeeperPaths) { + this.leaderElectionDc = unusedTopicsDetectionProperties.leaderElectionZookeeperDc(); + String leaderPath = zookeeperPaths.unusedTopicsLeaderPath(); + Optional leaderCuratorFramework = + zookeeperClientManager.getClients().stream() + .filter(it -> it.getDatacenterName().equals(leaderElectionDc)) + .findFirst() + .map(ZookeeperClient::getCuratorFramework); + this.leaderLatch = leaderCuratorFramework.map(it -> new LeaderLatch(it, leaderPath)); + if (leaderLatch.isEmpty()) { + logLeaderZookeeperClientNotFound(leaderElectionDc); + } + this.job = job; + } + + @PostConstruct + public void startListeningForLeadership() { + leaderLatch.ifPresent( + it -> { + try { + it.start(); + } catch (Exception e) { + throw new RuntimeException(e); + } + }); + } + + @Scheduled(cron = "${detection.unused-topics.cron}") + public void run() { + if (leaderLatch.isPresent()) { + if (leaderLatch.get().hasLeadership()) { + job.detectAndNotify(); + } + } else { + logLeaderZookeeperClientNotFound(leaderElectionDc); + } + } + + private void logLeaderZookeeperClientNotFound(String dc) { + logger.error("Cannot run unused topics detection - no zookeeper client for datacenter={}", dc); + } +} From ad86f37710088e24f4fe35defaa3ada3a504cfa3 Mon Sep 17 00:00:00 2001 From: Michal Wisniewski Date: Tue, 12 Nov 2024 09:49:31 +0100 Subject: [PATCH 14/39] SKYEDEN-3234 | Add properties with default values --- hermes-management/src/main/resources/application.yaml | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/hermes-management/src/main/resources/application.yaml b/hermes-management/src/main/resources/application.yaml index 585938789c..77a00dd993 100644 --- a/hermes-management/src/main/resources/application.yaml +++ b/hermes-management/src/main/resources/application.yaml @@ -73,4 +73,13 @@ console: subscription: showHeadersFilter: false - showFixedHeaders: false \ No newline at end of file + showFixedHeaders: false + +detection: + unused-topics: + enabled: false + inactivity-threshold: 60d + next-notification-threshold: 14d + whitelisted-qualified-topic-names: [] + leader-election-zookeeper-dc: dc + cron: "0 0 8 * * *" From f20743393149a3e2d2e697ad60393a65a070bdde Mon Sep 17 00:00:00 2001 From: Michal Wisniewski Date: Tue, 12 Nov 2024 10:15:09 +0100 Subject: [PATCH 15/39] SKYEDEN-3234 | Add more logging and refactor --- .../detection/UnusedTopicsDetectionJob.java | 17 +++++++----- .../UnusedTopicsDetectionService.java | 26 +++++++------------ ...e.java => UnusedTopicsStorageService.java} | 4 +-- .../UnusedTopicsDetectionJobTest.groovy | 15 ++++++----- .../UnusedTopicsDetectionServiceTest.groovy | 3 ++- ... => UnusedTopicsStorageServiceTest.groovy} | 19 +++++++------- 6 files changed, 42 insertions(+), 42 deletions(-) rename hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/{UnusedTopicsService.java => UnusedTopicsStorageService.java} (92%) rename hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/{UnusedTopicsServiceTest.groovy => UnusedTopicsStorageServiceTest.groovy} (88%) diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionJob.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionJob.java index 917f95ef6f..c0e765bc25 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionJob.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionJob.java @@ -19,7 +19,7 @@ @EnableConfigurationProperties({UnusedTopicsDetectionProperties.class}) public class UnusedTopicsDetectionJob { private final TopicService topicService; - private final UnusedTopicsService unusedTopicsService; + private final UnusedTopicsStorageService unusedTopicsStorageService; private final UnusedTopicsDetectionService unusedTopicsDetectionService; private final Optional notifier; private final Clock clock; @@ -28,12 +28,12 @@ public class UnusedTopicsDetectionJob { public UnusedTopicsDetectionJob( TopicService topicService, - UnusedTopicsService unusedTopicsService, + UnusedTopicsStorageService unusedTopicsStorageService, UnusedTopicsDetectionService unusedTopicsDetectionService, Optional notifier, Clock clock) { this.topicService = topicService; - this.unusedTopicsService = unusedTopicsService; + this.unusedTopicsStorageService = unusedTopicsStorageService; this.unusedTopicsDetectionService = unusedTopicsDetectionService; this.clock = clock; if (notifier.isEmpty()) { @@ -44,7 +44,7 @@ public UnusedTopicsDetectionJob( public void detectAndNotify() { List qualifiedTopicNames = topicService.listQualifiedTopicNames(); - List historicalUnusedTopics = unusedTopicsService.getUnusedTopics(); + List historicalUnusedTopics = unusedTopicsStorageService.getUnusedTopics(); List foundUnusedTopics = detectUnusedTopics(qualifiedTopicNames, historicalUnusedTopics); @@ -85,12 +85,17 @@ private Map groupByName(List unusedTopics) { } private void notify(List unusedTopics) { - notifier.ifPresent(notifier -> notifier.notify(unusedTopics)); + if (notifier.isPresent()) { + logger.info("Notifying {} unused topics", unusedTopics.size()); + notifier.get().notify(unusedTopics); + } else { + logger.info("Skipping notification of {} unused topics", unusedTopics.size()); + } } private void saveUnusedTopics( List notifiedTopics, List skippedNotificationTopics) { - unusedTopicsService.markAsUnused( + unusedTopicsStorageService.markAsUnused( Stream.concat(notifiedTopics.stream(), skippedNotificationTopics.stream()).toList()); } } diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionService.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionService.java index e1aaa15f54..56da197317 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionService.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionService.java @@ -28,8 +28,7 @@ public Optional detectUnusedTopic( TopicName topicName, Optional historicalUnusedTopic) { Instant now = clock.instant(); Instant lastUsed = metricsRepository.getLastPublishedMessageTimestamp(topicName); - boolean isUnused = - Duration.between(lastUsed, now).compareTo(properties.inactivityThreshold()) >= 0; + boolean isUnused = isInactive(lastUsed, now); if (isUnused) { return Optional.of( @@ -52,24 +51,19 @@ public boolean shouldBeNotified(UnusedTopic unusedTopic) { unusedTopic.notificationTimestampsMs().stream() .max(Long::compare) .map(Instant::ofEpochMilli); - boolean isInactive = - Duration.between(lastUsed, now).compareTo(properties.inactivityThreshold()) >= 0; + boolean isInactive = isInactive(lastUsed, now); return isInactive && !unusedTopic.whitelisted() - && lastNotified - .map( - instant -> - durationBetweenSurpassesThreshold( - instant, now, properties.nextNotificationThreshold())) - .orElseGet( - () -> - durationBetweenSurpassesThreshold( - lastUsed, now, properties.inactivityThreshold())); + && lastNotified.map(instant -> readyForNextNotification(instant, now)).orElse(true); } - private boolean durationBetweenSurpassesThreshold( - Instant start, Instant end, Duration threshold) { - return Duration.between(start, end).compareTo(threshold) >= 0; + private boolean isInactive(Instant lastUsed, Instant now) { + return Duration.between(lastUsed, now).compareTo(properties.inactivityThreshold()) >= 0; + } + + private boolean readyForNextNotification(Instant lastNotified, Instant now) { + return Duration.between(lastNotified, now).compareTo(properties.nextNotificationThreshold()) + >= 0; } } diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsService.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsStorageService.java similarity index 92% rename from hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsService.java rename to hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsStorageService.java index 97af49095e..6462b9214f 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsService.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsStorageService.java @@ -7,11 +7,11 @@ import java.util.List; @Service -public class UnusedTopicsService { +public class UnusedTopicsStorageService { private final UnusedTopicsRepository unusedTopicsRepository; private final MultiDatacenterRepositoryCommandExecutor multiDcExecutor; - public UnusedTopicsService( + public UnusedTopicsStorageService( UnusedTopicsRepository unusedTopicsRepository, MultiDatacenterRepositoryCommandExecutor multiDcExecutor) { this.unusedTopicsRepository = unusedTopicsRepository; diff --git a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionJobTest.groovy b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionJobTest.groovy index ace9fa0346..e61c3b8b29 100644 --- a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionJobTest.groovy +++ b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionJobTest.groovy @@ -15,7 +15,7 @@ import static pl.allegro.tech.hermes.api.TopicName.fromQualifiedName class UnusedTopicsDetectionJobTest extends Specification { def topicServiceMock = Mock(TopicService) - def unusedTopicsServiceMock = Mock(UnusedTopicsService) + def unusedTopicsStorageServiceMock = Mock(UnusedTopicsStorageService) def metricsRepositoryMock = Mock(LastPublishedMessageMetricsRepository) def clockMock = Mock(Clock) def unusedTopicsNotifier = new InMemoryUnusedTopicsNotifier() @@ -23,7 +23,8 @@ class UnusedTopicsDetectionJobTest extends Specification { def unusedTopicsDetectionProperties = new UnusedTopicsDetectionProperties( Duration.ofDays(7), Duration.ofDays(14), - ["group.topic3"] as Set + ["group.topic3"] as Set, + "dc" ) UnusedTopicsDetectionService detectionService = new UnusedTopicsDetectionService( @@ -34,7 +35,7 @@ class UnusedTopicsDetectionJobTest extends Specification { UnusedTopicsDetectionJob detectionJob = new UnusedTopicsDetectionJob( topicServiceMock, - unusedTopicsServiceMock, + unusedTopicsStorageServiceMock, detectionService, Optional.of(unusedTopicsNotifier), clockMock @@ -58,7 +59,7 @@ class UnusedTopicsDetectionJobTest extends Specification { ] and: "historically saved unused topics" - unusedTopicsServiceMock.getUnusedTopics() >> [ + unusedTopicsStorageServiceMock.getUnusedTopics() >> [ new UnusedTopic("group.topic2", ago7days.toEpochMilli(), [], false), new UnusedTopic("group.topic3", ago7days.toEpochMilli(), [], true), new UnusedTopic("group.topic4", ago21days.toEpochMilli(), [ago14days.toEpochMilli()], false), @@ -81,7 +82,7 @@ class UnusedTopicsDetectionJobTest extends Specification { ] and: "saved are all unused topics with updated notification timestamps" - 1 * unusedTopicsServiceMock.markAsUnused([ + 1 * unusedTopicsStorageServiceMock.markAsUnused([ new UnusedTopic("group.topic1", ago7days.toEpochMilli(), [now.toEpochMilli()], false), new UnusedTopic("group.topic4", ago21days.toEpochMilli(), [ago14days.toEpochMilli(), now.toEpochMilli()], false), new UnusedTopic("group.topic3", ago7days.toEpochMilli(), [], true) @@ -91,7 +92,7 @@ class UnusedTopicsDetectionJobTest extends Specification { def "should not notify if there are no unused topics"() { given: topicServiceMock.listQualifiedTopicNames() >> ["group.topic0"] - unusedTopicsServiceMock.getUnusedTopics() >> [] + unusedTopicsStorageServiceMock.getUnusedTopics() >> [] and: def now = Instant.ofEpochMilli(1630600266987L) @@ -105,6 +106,6 @@ class UnusedTopicsDetectionJobTest extends Specification { unusedTopicsNotifier.getNotifiedTopics().toList() == [] and: - 1 * unusedTopicsServiceMock.markAsUnused([]) + 1 * unusedTopicsStorageServiceMock.markAsUnused([]) } } diff --git a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionServiceTest.groovy b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionServiceTest.groovy index 4549c903b9..e2adaed7d8 100644 --- a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionServiceTest.groovy +++ b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionServiceTest.groovy @@ -20,7 +20,8 @@ class UnusedTopicsDetectionServiceTest extends Specification { UnusedTopicsDetectionProperties properties = new UnusedTopicsDetectionProperties( Duration.ofDays(INACTIVITY_THRESHOLD), Duration.ofDays(NEXT_NOTIFICATION_THRESHOLD), - [WHITELISTED_TOPIC_NAME] as Set + [WHITELISTED_TOPIC_NAME] as Set, + "dc" ) private def metricsRepositoryMock = Mock(LastPublishedMessageMetricsRepository) diff --git a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsServiceTest.groovy b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsStorageServiceTest.groovy similarity index 88% rename from hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsServiceTest.groovy rename to hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsStorageServiceTest.groovy index 3fa02efdf1..dafaa87eae 100644 --- a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsServiceTest.groovy +++ b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsStorageServiceTest.groovy @@ -2,7 +2,6 @@ package pl.allegro.tech.hermes.management.domain.detection import com.fasterxml.jackson.core.type.TypeReference import com.fasterxml.jackson.databind.ObjectMapper -import com.fasterxml.jackson.datatype.jdk8.Jdk8Module import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule import pl.allegro.tech.hermes.common.exception.InternalProcessingException import pl.allegro.tech.hermes.infrastructure.zookeeper.ZookeeperPaths @@ -14,7 +13,7 @@ import pl.allegro.tech.hermes.management.infrastructure.zookeeper.ZookeeperClien import pl.allegro.tech.hermes.management.infrastructure.zookeeper.ZookeeperRepositoryManager import pl.allegro.tech.hermes.management.utils.MultiZookeeperIntegrationTest -class UnusedTopicsServiceTest extends MultiZookeeperIntegrationTest { +class UnusedTopicsStorageServiceTest extends MultiZookeeperIntegrationTest { static UNUSED_TOPICS_PATH = '/hermes/unused-topics' @@ -22,7 +21,7 @@ class UnusedTopicsServiceTest extends MultiZookeeperIntegrationTest { ZookeeperRepositoryManager repositoryManager ModeService modeService MultiDatacenterRepositoryCommandExecutor commandExecutor - UnusedTopicsService unusedTopicsService + UnusedTopicsStorageService unusedTopicsStorageService UnusedTopicsRepository unusedTopicsRepository def objectMapper = new ObjectMapper().registerModule(new JavaTimeModule()) @@ -39,7 +38,7 @@ class UnusedTopicsServiceTest extends MultiZookeeperIntegrationTest { repositoryManager.start() modeService = new ModeService() commandExecutor = new MultiDatacenterRepositoryCommandExecutor(repositoryManager, true, modeService) - unusedTopicsService = new UnusedTopicsService(unusedTopicsRepository, commandExecutor) + unusedTopicsStorageService = new UnusedTopicsStorageService(unusedTopicsRepository, commandExecutor) } def cleanup() { @@ -54,7 +53,7 @@ class UnusedTopicsServiceTest extends MultiZookeeperIntegrationTest { def "should create node in all zk clusters if it doesn't exist when upserting"() { when: - unusedTopicsService.markAsUnused(TEST_UNUSED_TOPICS) + unusedTopicsStorageService.markAsUnused(TEST_UNUSED_TOPICS) then: assertNodesContain(TEST_UNUSED_TOPICS) @@ -72,7 +71,7 @@ class UnusedTopicsServiceTest extends MultiZookeeperIntegrationTest { ] when: - unusedTopicsService.markAsUnused(newUnusedTopics) + unusedTopicsStorageService.markAsUnused(newUnusedTopics) then: assertNodesContain(newUnusedTopics) @@ -86,7 +85,7 @@ class UnusedTopicsServiceTest extends MultiZookeeperIntegrationTest { zookeeper2.stop() when: - unusedTopicsService.markAsUnused([ + unusedTopicsStorageService.markAsUnused([ new UnusedTopic("group.topic3", 1730641656154L, [], false) ]) @@ -100,7 +99,7 @@ class UnusedTopicsServiceTest extends MultiZookeeperIntegrationTest { def "should return empty list when node doesn't exist"() { when: - def result = unusedTopicsService.getUnusedTopics() + def result = unusedTopicsStorageService.getUnusedTopics() then: result == [] @@ -111,7 +110,7 @@ class UnusedTopicsServiceTest extends MultiZookeeperIntegrationTest { setupNodes([]) when: - def result = unusedTopicsService.getUnusedTopics() + def result = unusedTopicsStorageService.getUnusedTopics() then: result == [] @@ -122,7 +121,7 @@ class UnusedTopicsServiceTest extends MultiZookeeperIntegrationTest { setupNodes(TEST_UNUSED_TOPICS) when: - def result = unusedTopicsService.getUnusedTopics() + def result = unusedTopicsStorageService.getUnusedTopics() then: result.sort() == TEST_UNUSED_TOPICS.sort() From 719b705b9342a6731527ba223e9bb95928e5dd2b Mon Sep 17 00:00:00 2001 From: Michal Wisniewski Date: Tue, 12 Nov 2024 10:36:16 +0100 Subject: [PATCH 16/39] SKYEDEN-3234 | Rename from unused to inactive --- .../zookeeper/ZookeeperPaths.java | 10 +- ...=> InactiveTopicsDetectionProperties.java} | 4 +- .../config/storage/StorageConfiguration.java | 8 +- .../{UnusedTopic.java => InactiveTopic.java} | 6 +- .../detection/InactiveTopicsDetectionJob.java | 102 ++++++++++++++++++ ...va => InactiveTopicsDetectionService.java} | 32 +++--- .../detection/InactiveTopicsNotifier.java | 7 ++ .../detection/InactiveTopicsRepository.java | 9 ++ .../InactiveTopicsStorageService.java | 27 +++++ .../detection/UnusedTopicsDetectionJob.java | 101 ----------------- .../detection/UnusedTopicsNotifier.java | 7 -- .../detection/UnusedTopicsRepository.java | 9 -- .../detection/UnusedTopicsStorageService.java | 28 ----- ...MarkTopicsAsInactiveRepositoryCommand.java | 44 ++++++++ .../MarkTopicsAsUnusedRepositoryCommand.java | 44 -------- ... => InactiveTopicsDetectionScheduler.java} | 28 ++--- ...=> ZookeeperInactiveTopicsRepository.java} | 32 +++--- .../zookeeper/ZookeeperRepositoryManager.java | 14 +-- .../src/main/resources/application.yaml | 2 +- .../InMemoryInactiveTopicsNotifier.groovy | 18 ++++ .../InMemoryUnusedTopicsNotifier.groovy | 18 ---- ... => InactiveTopicsDetectionJobTest.groovy} | 58 +++++----- ...InactiveTopicsDetectionServiceTest.groovy} | 70 ++++++------ ...> InactiveTopicsStorageServiceTest.groovy} | 80 +++++++------- 24 files changed, 378 insertions(+), 380 deletions(-) rename hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/detection/{UnusedTopicsDetectionProperties.java => InactiveTopicsDetectionProperties.java} (75%) rename hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/{UnusedTopic.java => InactiveTopic.java} (85%) create mode 100644 hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJob.java rename hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/{UnusedTopicsDetectionService.java => InactiveTopicsDetectionService.java} (65%) create mode 100644 hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsNotifier.java create mode 100644 hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsRepository.java create mode 100644 hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsStorageService.java delete mode 100644 hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionJob.java delete mode 100644 hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsNotifier.java delete mode 100644 hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsRepository.java delete mode 100644 hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsStorageService.java create mode 100644 hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/command/MarkTopicsAsInactiveRepositoryCommand.java delete mode 100644 hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/command/MarkTopicsAsUnusedRepositoryCommand.java rename hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/{UnusedTopicsDetectionScheduler.java => InactiveTopicsDetectionScheduler.java} (67%) rename hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/{ZookeeperUnusedTopicsRepository.java => ZookeeperInactiveTopicsRepository.java} (52%) create mode 100644 hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InMemoryInactiveTopicsNotifier.groovy delete mode 100644 hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InMemoryUnusedTopicsNotifier.groovy rename hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/{UnusedTopicsDetectionJobTest.groovy => InactiveTopicsDetectionJobTest.groovy} (51%) rename hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/{UnusedTopicsDetectionServiceTest.groovy => InactiveTopicsDetectionServiceTest.groovy} (68%) rename hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/{UnusedTopicsStorageServiceTest.groovy => InactiveTopicsStorageServiceTest.groovy} (52%) diff --git a/hermes-common/src/main/java/pl/allegro/tech/hermes/infrastructure/zookeeper/ZookeeperPaths.java b/hermes-common/src/main/java/pl/allegro/tech/hermes/infrastructure/zookeeper/ZookeeperPaths.java index 010b4b5f23..48cb079f46 100644 --- a/hermes-common/src/main/java/pl/allegro/tech/hermes/infrastructure/zookeeper/ZookeeperPaths.java +++ b/hermes-common/src/main/java/pl/allegro/tech/hermes/infrastructure/zookeeper/ZookeeperPaths.java @@ -30,7 +30,7 @@ public class ZookeeperPaths { public static final String DATACENTER_READINESS_PATH = "datacenter-readiness"; public static final String OFFLINE_RETRANSMISSION_PATH = "offline-retransmission"; public static final String OFFLINE_RETRANSMISSION_TASKS_PATH = "tasks"; - public static final String UNUSED_TOPICS_PATH = "unused-topics"; + public static final String INACTIVE_TOPICS_PATH = "inactive-topics"; private final String basePath; @@ -183,12 +183,12 @@ public String offlineRetransmissionPath(String taskId) { .join(basePath, OFFLINE_RETRANSMISSION_PATH, OFFLINE_RETRANSMISSION_TASKS_PATH, taskId); } - public String unusedTopicsPath() { - return Joiner.on(URL_SEPARATOR).join(basePath, UNUSED_TOPICS_PATH); + public String inactiveTopicsPath() { + return Joiner.on(URL_SEPARATOR).join(basePath, INACTIVE_TOPICS_PATH); } - public String unusedTopicsLeaderPath() { - return Joiner.on(URL_SEPARATOR).join(unusedTopicsPath(), "leader"); + public String inactiveTopicsLeaderPath() { + return Joiner.on(URL_SEPARATOR).join(inactiveTopicsPath(), "leader"); } public String join(String... parts) { diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/detection/UnusedTopicsDetectionProperties.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/detection/InactiveTopicsDetectionProperties.java similarity index 75% rename from hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/detection/UnusedTopicsDetectionProperties.java rename to hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/detection/InactiveTopicsDetectionProperties.java index deb6809656..9ae4b4e352 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/detection/UnusedTopicsDetectionProperties.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/detection/InactiveTopicsDetectionProperties.java @@ -5,8 +5,8 @@ import java.time.Duration; import java.util.Set; -@ConfigurationProperties(prefix = "detection.unused-topics") -public record UnusedTopicsDetectionProperties( +@ConfigurationProperties(prefix = "detection.inactive-topics") +public record InactiveTopicsDetectionProperties( Duration inactivityThreshold, Duration nextNotificationThreshold, Set whitelistedQualifiedTopicNames, diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/storage/StorageConfiguration.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/storage/StorageConfiguration.java index 32c65b414c..45c9fffad5 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/storage/StorageConfiguration.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/storage/StorageConfiguration.java @@ -31,12 +31,12 @@ import pl.allegro.tech.hermes.infrastructure.zookeeper.ZookeeperWorkloadConstraintsRepository; import pl.allegro.tech.hermes.management.domain.blacklist.TopicBlacklistRepository; import pl.allegro.tech.hermes.management.domain.dc.MultiDatacenterRepositoryCommandExecutor; -import pl.allegro.tech.hermes.management.domain.detection.UnusedTopicsRepository; +import pl.allegro.tech.hermes.management.domain.detection.InactiveTopicsRepository; import pl.allegro.tech.hermes.management.domain.mode.ModeService; import pl.allegro.tech.hermes.management.domain.readiness.DatacenterReadinessRepository; import pl.allegro.tech.hermes.management.domain.retransmit.OfflineRetransmissionRepository; import pl.allegro.tech.hermes.management.infrastructure.blacklist.ZookeeperTopicBlacklistRepository; -import pl.allegro.tech.hermes.management.infrastructure.detection.ZookeeperUnusedTopicsRepository; +import pl.allegro.tech.hermes.management.infrastructure.detection.ZookeeperInactiveTopicsRepository; import pl.allegro.tech.hermes.management.infrastructure.metrics.SummedSharedCounter; import pl.allegro.tech.hermes.management.infrastructure.readiness.ZookeeperDatacenterReadinessRepository; import pl.allegro.tech.hermes.management.infrastructure.retransmit.ZookeeperOfflineRetransmissionRepository; @@ -181,9 +181,9 @@ DatacenterReadinessRepository readinessRepository() { } @Bean - UnusedTopicsRepository unusedTopicsRepository() { + InactiveTopicsRepository inactiveTopicsRepository() { ZookeeperClient localClient = clientManager().getLocalClient(); - return new ZookeeperUnusedTopicsRepository( + return new ZookeeperInactiveTopicsRepository( localClient.getCuratorFramework(), objectMapper, zookeeperPaths()); } } diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopic.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopic.java similarity index 85% rename from hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopic.java rename to hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopic.java index 4d4e01d6b2..5b2e407f41 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopic.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopic.java @@ -5,16 +5,16 @@ import java.util.ArrayList; import java.util.List; -public record UnusedTopic( +public record InactiveTopic( @JsonProperty String qualifiedTopicName, @JsonProperty long lastPublishedMessageTimestampMs, @JsonProperty List notificationTimestampsMs, @JsonProperty boolean whitelisted) { - UnusedTopic notificationSent(Instant timestamp) { + InactiveTopic notificationSent(Instant timestamp) { List newNotificationTimestampsMs = new ArrayList<>(notificationTimestampsMs); newNotificationTimestampsMs.add(timestamp.toEpochMilli()); - return new UnusedTopic( + return new InactiveTopic( this.qualifiedTopicName, this.lastPublishedMessageTimestampMs, newNotificationTimestampsMs, diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJob.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJob.java new file mode 100644 index 0000000000..96fb402004 --- /dev/null +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJob.java @@ -0,0 +1,102 @@ +package pl.allegro.tech.hermes.management.domain.detection; + +import static java.util.stream.Collectors.groupingBy; + +import java.time.Clock; +import java.time.Instant; +import java.util.*; +import java.util.stream.Collectors; +import java.util.stream.Stream; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.boot.context.properties.EnableConfigurationProperties; +import org.springframework.stereotype.Component; +import pl.allegro.tech.hermes.api.TopicName; +import pl.allegro.tech.hermes.management.config.detection.InactiveTopicsDetectionProperties; +import pl.allegro.tech.hermes.management.domain.topic.TopicService; + +@Component +@EnableConfigurationProperties({InactiveTopicsDetectionProperties.class}) +public class InactiveTopicsDetectionJob { + private final TopicService topicService; + private final InactiveTopicsStorageService inactiveTopicsStorageService; + private final InactiveTopicsDetectionService inactiveTopicsDetectionService; + private final Optional notifier; + private final Clock clock; + + private static final Logger logger = LoggerFactory.getLogger(InactiveTopicsDetectionJob.class); + + public InactiveTopicsDetectionJob( + TopicService topicService, + InactiveTopicsStorageService inactiveTopicsStorageService, + InactiveTopicsDetectionService inactiveTopicsDetectionService, + Optional notifier, + Clock clock) { + this.topicService = topicService; + this.inactiveTopicsStorageService = inactiveTopicsStorageService; + this.inactiveTopicsDetectionService = inactiveTopicsDetectionService; + this.clock = clock; + if (notifier.isEmpty()) { + logger.info("Inactive topics notifier bean is absent"); + } + this.notifier = notifier; + } + + public void detectAndNotify() { + List qualifiedTopicNames = topicService.listQualifiedTopicNames(); + List historicalInactiveTopics = inactiveTopicsStorageService.getInactiveTopics(); + List foundInactiveTopics = + detectInactiveTopics(qualifiedTopicNames, historicalInactiveTopics); + + Map> groupedByNeedOfNotification = + foundInactiveTopics.stream() + .collect(groupingBy(inactiveTopicsDetectionService::shouldBeNotified)); + + List topicsToNotify = groupedByNeedOfNotification.getOrDefault(true, List.of()); + List topicsToSkipNotification = + groupedByNeedOfNotification.getOrDefault(false, List.of()); + + notify(topicsToNotify); + + Instant now = clock.instant(); + List notifiedTopics = + topicsToNotify.stream().map(topic -> topic.notificationSent(now)).toList(); + + saveInactiveTopics(notifiedTopics, topicsToSkipNotification); + } + + private List detectInactiveTopics( + List qualifiedTopicNames, List historicalInactiveTopics) { + Map historicalInactiveTopicsByName = + groupByName(historicalInactiveTopics); + return qualifiedTopicNames.stream() + .map( + qualifiedTopicName -> + inactiveTopicsDetectionService.detectInactiveTopic( + TopicName.fromQualifiedName(qualifiedTopicName), + Optional.ofNullable(historicalInactiveTopicsByName.get(qualifiedTopicName)))) + .map(opt -> opt.orElse(null)) + .filter(Objects::nonNull) + .toList(); + } + + private Map groupByName(List inactiveTopics) { + return inactiveTopics.stream() + .collect(Collectors.toMap(InactiveTopic::qualifiedTopicName, v -> v, (v1, v2) -> v1)); + } + + private void notify(List inactiveTopics) { + if (notifier.isPresent()) { + logger.info("Notifying {} inactive topics", inactiveTopics.size()); + notifier.get().notify(inactiveTopics); + } else { + logger.info("Skipping notification of {} inactive topics", inactiveTopics.size()); + } + } + + private void saveInactiveTopics( + List notifiedTopics, List skippedNotificationTopics) { + inactiveTopicsStorageService.markAsInactive( + Stream.concat(notifiedTopics.stream(), skippedNotificationTopics.stream()).toList()); + } +} diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionService.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionService.java similarity index 65% rename from hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionService.java rename to hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionService.java index 56da197317..7ac8bd05ae 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionService.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionService.java @@ -7,36 +7,36 @@ import java.util.Optional; import org.springframework.stereotype.Service; import pl.allegro.tech.hermes.api.TopicName; -import pl.allegro.tech.hermes.management.config.detection.UnusedTopicsDetectionProperties; +import pl.allegro.tech.hermes.management.config.detection.InactiveTopicsDetectionProperties; @Service -public class UnusedTopicsDetectionService { +public class InactiveTopicsDetectionService { private final LastPublishedMessageMetricsRepository metricsRepository; - private final UnusedTopicsDetectionProperties properties; + private final InactiveTopicsDetectionProperties properties; private final Clock clock; - public UnusedTopicsDetectionService( + public InactiveTopicsDetectionService( LastPublishedMessageMetricsRepository metricsRepository, - UnusedTopicsDetectionProperties properties, + InactiveTopicsDetectionProperties properties, Clock clock) { this.metricsRepository = metricsRepository; this.properties = properties; this.clock = clock; } - public Optional detectUnusedTopic( - TopicName topicName, Optional historicalUnusedTopic) { + public Optional detectInactiveTopic( + TopicName topicName, Optional historicalInactiveTopic) { Instant now = clock.instant(); Instant lastUsed = metricsRepository.getLastPublishedMessageTimestamp(topicName); - boolean isUnused = isInactive(lastUsed, now); + boolean isInactive = isInactive(lastUsed, now); - if (isUnused) { + if (isInactive) { return Optional.of( - new UnusedTopic( + new InactiveTopic( topicName.qualifiedName(), lastUsed.toEpochMilli(), - historicalUnusedTopic - .map(UnusedTopic::notificationTimestampsMs) + historicalInactiveTopic + .map(InactiveTopic::notificationTimestampsMs) .orElse(Collections.emptyList()), properties.whitelistedQualifiedTopicNames().contains(topicName.qualifiedName()))); } else { @@ -44,17 +44,17 @@ public Optional detectUnusedTopic( } } - public boolean shouldBeNotified(UnusedTopic unusedTopic) { + public boolean shouldBeNotified(InactiveTopic inactiveTopic) { Instant now = clock.instant(); - Instant lastUsed = Instant.ofEpochMilli(unusedTopic.lastPublishedMessageTimestampMs()); + Instant lastUsed = Instant.ofEpochMilli(inactiveTopic.lastPublishedMessageTimestampMs()); Optional lastNotified = - unusedTopic.notificationTimestampsMs().stream() + inactiveTopic.notificationTimestampsMs().stream() .max(Long::compare) .map(Instant::ofEpochMilli); boolean isInactive = isInactive(lastUsed, now); return isInactive - && !unusedTopic.whitelisted() + && !inactiveTopic.whitelisted() && lastNotified.map(instant -> readyForNextNotification(instant, now)).orElse(true); } diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsNotifier.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsNotifier.java new file mode 100644 index 0000000000..5747226e0c --- /dev/null +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsNotifier.java @@ -0,0 +1,7 @@ +package pl.allegro.tech.hermes.management.domain.detection; + +import java.util.List; + +public interface InactiveTopicsNotifier { + void notify(List inactiveTopics); +} diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsRepository.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsRepository.java new file mode 100644 index 0000000000..9ca855b52a --- /dev/null +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsRepository.java @@ -0,0 +1,9 @@ +package pl.allegro.tech.hermes.management.domain.detection; + +import java.util.List; + +public interface InactiveTopicsRepository { + void upsert(List inactiveTopics); + + List read(); +} diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsStorageService.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsStorageService.java new file mode 100644 index 0000000000..588a5a8494 --- /dev/null +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsStorageService.java @@ -0,0 +1,27 @@ +package pl.allegro.tech.hermes.management.domain.detection; + +import java.util.List; +import org.springframework.stereotype.Service; +import pl.allegro.tech.hermes.management.domain.dc.MultiDatacenterRepositoryCommandExecutor; +import pl.allegro.tech.hermes.management.domain.detection.command.MarkTopicsAsInactiveRepositoryCommand; + +@Service +public class InactiveTopicsStorageService { + private final InactiveTopicsRepository inactiveTopicsRepository; + private final MultiDatacenterRepositoryCommandExecutor multiDcExecutor; + + public InactiveTopicsStorageService( + InactiveTopicsRepository inactiveTopicsRepository, + MultiDatacenterRepositoryCommandExecutor multiDcExecutor) { + this.inactiveTopicsRepository = inactiveTopicsRepository; + this.multiDcExecutor = multiDcExecutor; + } + + public void markAsInactive(List inactiveTopics) { + multiDcExecutor.execute(new MarkTopicsAsInactiveRepositoryCommand(inactiveTopics)); + } + + public List getInactiveTopics() { + return inactiveTopicsRepository.read(); + } +} diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionJob.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionJob.java deleted file mode 100644 index c0e765bc25..0000000000 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionJob.java +++ /dev/null @@ -1,101 +0,0 @@ -package pl.allegro.tech.hermes.management.domain.detection; - -import static java.util.stream.Collectors.groupingBy; - -import java.time.Clock; -import java.time.Instant; -import java.util.*; -import java.util.stream.Collectors; -import java.util.stream.Stream; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import org.springframework.boot.context.properties.EnableConfigurationProperties; -import org.springframework.stereotype.Component; -import pl.allegro.tech.hermes.api.TopicName; -import pl.allegro.tech.hermes.management.config.detection.UnusedTopicsDetectionProperties; -import pl.allegro.tech.hermes.management.domain.topic.TopicService; - -@Component -@EnableConfigurationProperties({UnusedTopicsDetectionProperties.class}) -public class UnusedTopicsDetectionJob { - private final TopicService topicService; - private final UnusedTopicsStorageService unusedTopicsStorageService; - private final UnusedTopicsDetectionService unusedTopicsDetectionService; - private final Optional notifier; - private final Clock clock; - - private static final Logger logger = LoggerFactory.getLogger(UnusedTopicsDetectionJob.class); - - public UnusedTopicsDetectionJob( - TopicService topicService, - UnusedTopicsStorageService unusedTopicsStorageService, - UnusedTopicsDetectionService unusedTopicsDetectionService, - Optional notifier, - Clock clock) { - this.topicService = topicService; - this.unusedTopicsStorageService = unusedTopicsStorageService; - this.unusedTopicsDetectionService = unusedTopicsDetectionService; - this.clock = clock; - if (notifier.isEmpty()) { - logger.info("Unused topics notifier bean is absent"); - } - this.notifier = notifier; - } - - public void detectAndNotify() { - List qualifiedTopicNames = topicService.listQualifiedTopicNames(); - List historicalUnusedTopics = unusedTopicsStorageService.getUnusedTopics(); - List foundUnusedTopics = - detectUnusedTopics(qualifiedTopicNames, historicalUnusedTopics); - - Map> groupedByNeedOfNotification = - foundUnusedTopics.stream() - .collect(groupingBy(unusedTopicsDetectionService::shouldBeNotified)); - - List topicsToNotify = groupedByNeedOfNotification.getOrDefault(true, List.of()); - List topicsToSkipNotification = - groupedByNeedOfNotification.getOrDefault(false, List.of()); - - notify(topicsToNotify); - - Instant now = clock.instant(); - List notifiedTopics = - topicsToNotify.stream().map(topic -> topic.notificationSent(now)).toList(); - - saveUnusedTopics(notifiedTopics, topicsToSkipNotification); - } - - private List detectUnusedTopics( - List qualifiedTopicNames, List historicalUnusedTopics) { - Map historicalUnusedTopicsByName = groupByName(historicalUnusedTopics); - return qualifiedTopicNames.stream() - .map( - qualifiedTopicName -> - unusedTopicsDetectionService.detectUnusedTopic( - TopicName.fromQualifiedName(qualifiedTopicName), - Optional.ofNullable(historicalUnusedTopicsByName.get(qualifiedTopicName)))) - .map(opt -> opt.orElse(null)) - .filter(Objects::nonNull) - .toList(); - } - - private Map groupByName(List unusedTopics) { - return unusedTopics.stream() - .collect(Collectors.toMap(UnusedTopic::qualifiedTopicName, v -> v, (v1, v2) -> v1)); - } - - private void notify(List unusedTopics) { - if (notifier.isPresent()) { - logger.info("Notifying {} unused topics", unusedTopics.size()); - notifier.get().notify(unusedTopics); - } else { - logger.info("Skipping notification of {} unused topics", unusedTopics.size()); - } - } - - private void saveUnusedTopics( - List notifiedTopics, List skippedNotificationTopics) { - unusedTopicsStorageService.markAsUnused( - Stream.concat(notifiedTopics.stream(), skippedNotificationTopics.stream()).toList()); - } -} diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsNotifier.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsNotifier.java deleted file mode 100644 index 6b1dca8f6e..0000000000 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsNotifier.java +++ /dev/null @@ -1,7 +0,0 @@ -package pl.allegro.tech.hermes.management.domain.detection; - -import java.util.List; - -public interface UnusedTopicsNotifier { - void notify(List unusedTopics); -} diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsRepository.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsRepository.java deleted file mode 100644 index b1c9b5ab88..0000000000 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsRepository.java +++ /dev/null @@ -1,9 +0,0 @@ -package pl.allegro.tech.hermes.management.domain.detection; - -import java.util.List; - -public interface UnusedTopicsRepository { - void upsert(List unusedTopics); - - List read(); -} diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsStorageService.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsStorageService.java deleted file mode 100644 index 6462b9214f..0000000000 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsStorageService.java +++ /dev/null @@ -1,28 +0,0 @@ -package pl.allegro.tech.hermes.management.domain.detection; - -import org.springframework.stereotype.Service; -import pl.allegro.tech.hermes.management.domain.dc.MultiDatacenterRepositoryCommandExecutor; -import pl.allegro.tech.hermes.management.domain.detection.command.MarkTopicsAsUnusedRepositoryCommand; - -import java.util.List; - -@Service -public class UnusedTopicsStorageService { - private final UnusedTopicsRepository unusedTopicsRepository; - private final MultiDatacenterRepositoryCommandExecutor multiDcExecutor; - - public UnusedTopicsStorageService( - UnusedTopicsRepository unusedTopicsRepository, - MultiDatacenterRepositoryCommandExecutor multiDcExecutor) { - this.unusedTopicsRepository = unusedTopicsRepository; - this.multiDcExecutor = multiDcExecutor; - } - - public void markAsUnused(List unusedTopics) { - multiDcExecutor.execute(new MarkTopicsAsUnusedRepositoryCommand(unusedTopics)); - } - - public List getUnusedTopics() { - return unusedTopicsRepository.read(); - } -} diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/command/MarkTopicsAsInactiveRepositoryCommand.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/command/MarkTopicsAsInactiveRepositoryCommand.java new file mode 100644 index 0000000000..6a7a126844 --- /dev/null +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/command/MarkTopicsAsInactiveRepositoryCommand.java @@ -0,0 +1,44 @@ +package pl.allegro.tech.hermes.management.domain.detection.command; + +import java.util.List; +import pl.allegro.tech.hermes.management.domain.dc.DatacenterBoundRepositoryHolder; +import pl.allegro.tech.hermes.management.domain.dc.RepositoryCommand; +import pl.allegro.tech.hermes.management.domain.detection.InactiveTopic; +import pl.allegro.tech.hermes.management.domain.detection.InactiveTopicsRepository; + +public class MarkTopicsAsInactiveRepositoryCommand + extends RepositoryCommand { + + private final List inactiveTopics; + private List backup; + + public MarkTopicsAsInactiveRepositoryCommand(List inactiveTopics) { + this.inactiveTopics = inactiveTopics; + } + + @Override + public void backup(DatacenterBoundRepositoryHolder holder) { + backup = holder.getRepository().read(); + } + + @Override + public void execute(DatacenterBoundRepositoryHolder holder) { + holder.getRepository().upsert(inactiveTopics); + } + + @Override + public void rollback( + DatacenterBoundRepositoryHolder holder, Exception exception) { + holder.getRepository().upsert(backup); + } + + @Override + public Class getRepositoryType() { + return InactiveTopicsRepository.class; + } + + @Override + public String toString() { + return String.format("MarkTopicsAsInactive(number of topics=%d)", inactiveTopics.size()); + } +} diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/command/MarkTopicsAsUnusedRepositoryCommand.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/command/MarkTopicsAsUnusedRepositoryCommand.java deleted file mode 100644 index 726d5d7243..0000000000 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/command/MarkTopicsAsUnusedRepositoryCommand.java +++ /dev/null @@ -1,44 +0,0 @@ -package pl.allegro.tech.hermes.management.domain.detection.command; - -import pl.allegro.tech.hermes.management.domain.dc.DatacenterBoundRepositoryHolder; -import pl.allegro.tech.hermes.management.domain.dc.RepositoryCommand; -import pl.allegro.tech.hermes.management.domain.detection.UnusedTopic; -import pl.allegro.tech.hermes.management.domain.detection.UnusedTopicsRepository; - -import java.util.List; - -public class MarkTopicsAsUnusedRepositoryCommand extends RepositoryCommand { - - private final List unusedTopics; - private List backup; - - public MarkTopicsAsUnusedRepositoryCommand(List unusedTopics) { - this.unusedTopics = unusedTopics; - } - - @Override - public void backup(DatacenterBoundRepositoryHolder holder) { - backup = holder.getRepository().read(); - } - - @Override - public void execute(DatacenterBoundRepositoryHolder holder) { - holder.getRepository().upsert(unusedTopics); - } - - @Override - public void rollback( - DatacenterBoundRepositoryHolder holder, Exception exception) { - holder.getRepository().upsert(backup); - } - - @Override - public Class getRepositoryType() { - return UnusedTopicsRepository.class; - } - - @Override - public String toString() { - return String.format("MarkTopicsAsUnused(number of topics=%d)", unusedTopics.size()); - } -} diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/UnusedTopicsDetectionScheduler.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/InactiveTopicsDetectionScheduler.java similarity index 67% rename from hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/UnusedTopicsDetectionScheduler.java rename to hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/InactiveTopicsDetectionScheduler.java index 5b077484f1..a748ad3c15 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/UnusedTopicsDetectionScheduler.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/InactiveTopicsDetectionScheduler.java @@ -11,29 +11,29 @@ import org.springframework.scheduling.annotation.Scheduled; import org.springframework.stereotype.Component; import pl.allegro.tech.hermes.infrastructure.zookeeper.ZookeeperPaths; -import pl.allegro.tech.hermes.management.config.detection.UnusedTopicsDetectionProperties; -import pl.allegro.tech.hermes.management.domain.detection.UnusedTopicsDetectionJob; +import pl.allegro.tech.hermes.management.config.detection.InactiveTopicsDetectionProperties; +import pl.allegro.tech.hermes.management.domain.detection.InactiveTopicsDetectionJob; import pl.allegro.tech.hermes.management.infrastructure.zookeeper.ZookeeperClient; import pl.allegro.tech.hermes.management.infrastructure.zookeeper.ZookeeperClientManager; -@ConditionalOnProperty(value = "detection.unused-topics.enabled", havingValue = "true") +@ConditionalOnProperty(value = "detection.inactive-topics.enabled", havingValue = "true") @Component -@EnableConfigurationProperties(UnusedTopicsDetectionProperties.class) -public class UnusedTopicsDetectionScheduler { - private final UnusedTopicsDetectionJob job; +@EnableConfigurationProperties(InactiveTopicsDetectionProperties.class) +public class InactiveTopicsDetectionScheduler { + private final InactiveTopicsDetectionJob job; private final String leaderElectionDc; private final Optional leaderLatch; private static final Logger logger = - LoggerFactory.getLogger(UnusedTopicsDetectionScheduler.class); + LoggerFactory.getLogger(InactiveTopicsDetectionScheduler.class); - public UnusedTopicsDetectionScheduler( - UnusedTopicsDetectionJob job, + public InactiveTopicsDetectionScheduler( + InactiveTopicsDetectionJob job, ZookeeperClientManager zookeeperClientManager, - UnusedTopicsDetectionProperties unusedTopicsDetectionProperties, + InactiveTopicsDetectionProperties inactiveTopicsDetectionProperties, ZookeeperPaths zookeeperPaths) { - this.leaderElectionDc = unusedTopicsDetectionProperties.leaderElectionZookeeperDc(); - String leaderPath = zookeeperPaths.unusedTopicsLeaderPath(); + this.leaderElectionDc = inactiveTopicsDetectionProperties.leaderElectionZookeeperDc(); + String leaderPath = zookeeperPaths.inactiveTopicsLeaderPath(); Optional leaderCuratorFramework = zookeeperClientManager.getClients().stream() .filter(it -> it.getDatacenterName().equals(leaderElectionDc)) @@ -58,7 +58,7 @@ public void startListeningForLeadership() { }); } - @Scheduled(cron = "${detection.unused-topics.cron}") + @Scheduled(cron = "${detection.inactive-topics.cron}") public void run() { if (leaderLatch.isPresent()) { if (leaderLatch.get().hasLeadership()) { @@ -70,6 +70,6 @@ public void run() { } private void logLeaderZookeeperClientNotFound(String dc) { - logger.error("Cannot run unused topics detection - no zookeeper client for datacenter={}", dc); + logger.error("Cannot run inactive topics detection - no zookeeper client for datacenter={}", dc); } } diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/ZookeeperUnusedTopicsRepository.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/ZookeeperInactiveTopicsRepository.java similarity index 52% rename from hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/ZookeeperUnusedTopicsRepository.java rename to hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/ZookeeperInactiveTopicsRepository.java index 2413d6864a..87cd755c16 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/ZookeeperUnusedTopicsRepository.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/ZookeeperInactiveTopicsRepository.java @@ -10,31 +10,29 @@ import pl.allegro.tech.hermes.common.exception.InternalProcessingException; import pl.allegro.tech.hermes.infrastructure.zookeeper.ZookeeperBasedRepository; import pl.allegro.tech.hermes.infrastructure.zookeeper.ZookeeperPaths; -import pl.allegro.tech.hermes.management.domain.detection.UnusedTopic; -import pl.allegro.tech.hermes.management.domain.detection.UnusedTopicsRepository; +import pl.allegro.tech.hermes.management.domain.detection.InactiveTopic; +import pl.allegro.tech.hermes.management.domain.detection.InactiveTopicsRepository; -public class ZookeeperUnusedTopicsRepository extends ZookeeperBasedRepository - implements UnusedTopicsRepository { +public class ZookeeperInactiveTopicsRepository extends ZookeeperBasedRepository + implements InactiveTopicsRepository { private static final Logger logger = - LoggerFactory.getLogger(ZookeeperUnusedTopicsRepository.class); + LoggerFactory.getLogger(ZookeeperInactiveTopicsRepository.class); - public ZookeeperUnusedTopicsRepository( + public ZookeeperInactiveTopicsRepository( CuratorFramework curatorFramework, ObjectMapper objectMapper, ZookeeperPaths paths) { super(curatorFramework, objectMapper, paths); } @Override - public void upsert(List unusedTopics) { - logger.info( - "Saving unused topics metadata into zookeeper, number of unused topics: {}", - unusedTopics.size()); - String path = paths.unusedTopicsPath(); + public void upsert(List inactiveTopics) { + logger.info("Saving inactive topics metadata into zookeeper, count={}", inactiveTopics.size()); + String path = paths.inactiveTopicsPath(); try { if (pathExists(path)) { - overwrite(path, unusedTopics); + overwrite(path, inactiveTopics); } else { - createRecursively(path, unusedTopics); + createRecursively(path, inactiveTopics); } } catch (Exception e) { throw new InternalProcessingException(e); @@ -42,13 +40,13 @@ public void upsert(List unusedTopics) { } @Override - public List read() { - String path = paths.unusedTopicsPath(); + public List read() { + String path = paths.inactiveTopicsPath(); if (!pathExists(path)) { - logger.warn("Unused topics ZK node does not exist: {}", path); + logger.warn("Inactive topics ZK node does not exist: {}", path); return Collections.emptyList(); } - return readFrom(paths.unusedTopicsPath(), new TypeReference>() {}, true) + return readFrom(paths.inactiveTopicsPath(), new TypeReference>() {}, true) .orElse(Collections.emptyList()); } } diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/zookeeper/ZookeeperRepositoryManager.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/zookeeper/ZookeeperRepositoryManager.java index 8f5fe88f08..fa9b323dd9 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/zookeeper/ZookeeperRepositoryManager.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/zookeeper/ZookeeperRepositoryManager.java @@ -33,11 +33,11 @@ import pl.allegro.tech.hermes.management.domain.blacklist.TopicBlacklistRepository; import pl.allegro.tech.hermes.management.domain.dc.DatacenterBoundRepositoryHolder; import pl.allegro.tech.hermes.management.domain.dc.RepositoryManager; -import pl.allegro.tech.hermes.management.domain.detection.UnusedTopicsRepository; +import pl.allegro.tech.hermes.management.domain.detection.InactiveTopicsRepository; import pl.allegro.tech.hermes.management.domain.readiness.DatacenterReadinessRepository; import pl.allegro.tech.hermes.management.domain.retransmit.OfflineRetransmissionRepository; import pl.allegro.tech.hermes.management.infrastructure.blacklist.ZookeeperTopicBlacklistRepository; -import pl.allegro.tech.hermes.management.infrastructure.detection.ZookeeperUnusedTopicsRepository; +import pl.allegro.tech.hermes.management.infrastructure.detection.ZookeeperInactiveTopicsRepository; import pl.allegro.tech.hermes.management.infrastructure.readiness.ZookeeperDatacenterReadinessRepository; import pl.allegro.tech.hermes.management.infrastructure.retransmit.ZookeeperOfflineRetransmissionRepository; @@ -69,7 +69,7 @@ public class ZookeeperRepositoryManager implements RepositoryManager { new HashMap<>(); private final Map offlineRetransmissionRepositoriesByDc = new HashMap<>(); - private final Map unusedTopicsRepositoriesByDc = new HashMap<>(); + private final Map inactiveTopicsRepositoriesByDc = new HashMap<>(); private final ZookeeperGroupRepositoryFactory zookeeperGroupRepositoryFactory; public ZookeeperRepositoryManager( @@ -142,9 +142,9 @@ public void start() { new ZookeeperOfflineRetransmissionRepository(zookeeper, mapper, paths); offlineRetransmissionRepositoriesByDc.put(dcName, offlineRetransmissionRepository); - ZookeeperUnusedTopicsRepository unusedTopicsRepository = - new ZookeeperUnusedTopicsRepository(zookeeper, mapper, paths); - unusedTopicsRepositoriesByDc.put(dcName, unusedTopicsRepository); + ZookeeperInactiveTopicsRepository inactiveTopicsRepository = + new ZookeeperInactiveTopicsRepository(zookeeper, mapper, paths); + inactiveTopicsRepositoriesByDc.put(dcName, inactiveTopicsRepository); } } @@ -196,6 +196,6 @@ private void initRepositoryTypeMap() { repositoryByType.put(DatacenterReadinessRepository.class, readinessRepositoriesByDc); repositoryByType.put( OfflineRetransmissionRepository.class, offlineRetransmissionRepositoriesByDc); - repositoryByType.put(UnusedTopicsRepository.class, unusedTopicsRepositoriesByDc); + repositoryByType.put(InactiveTopicsRepository.class, inactiveTopicsRepositoriesByDc); } } diff --git a/hermes-management/src/main/resources/application.yaml b/hermes-management/src/main/resources/application.yaml index 77a00dd993..43d1cdfb84 100644 --- a/hermes-management/src/main/resources/application.yaml +++ b/hermes-management/src/main/resources/application.yaml @@ -76,7 +76,7 @@ console: showFixedHeaders: false detection: - unused-topics: + inactive-topics: enabled: false inactivity-threshold: 60d next-notification-threshold: 14d diff --git a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InMemoryInactiveTopicsNotifier.groovy b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InMemoryInactiveTopicsNotifier.groovy new file mode 100644 index 0000000000..efcccc9317 --- /dev/null +++ b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InMemoryInactiveTopicsNotifier.groovy @@ -0,0 +1,18 @@ +package pl.allegro.tech.hermes.management.domain.detection + +class InMemoryInactiveTopicsNotifier implements InactiveTopicsNotifier { + private Set notifiedTopics = new HashSet<>(); + + @Override + void notify(List inactiveTopics) { + notifiedTopics.addAll(inactiveTopics) + } + + void reset() { + notifiedTopics.clear() + } + + Set getNotifiedTopics() { + return notifiedTopics + } +} diff --git a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InMemoryUnusedTopicsNotifier.groovy b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InMemoryUnusedTopicsNotifier.groovy deleted file mode 100644 index e436521a76..0000000000 --- a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InMemoryUnusedTopicsNotifier.groovy +++ /dev/null @@ -1,18 +0,0 @@ -package pl.allegro.tech.hermes.management.domain.detection - -class InMemoryUnusedTopicsNotifier implements UnusedTopicsNotifier { - private Set notifiedTopics = new HashSet<>(); - - @Override - void notify(List unusedTopics) { - notifiedTopics.addAll(unusedTopics) - } - - void reset() { - notifiedTopics.clear() - } - - Set getNotifiedTopics() { - return notifiedTopics - } -} diff --git a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionJobTest.groovy b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJobTest.groovy similarity index 51% rename from hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionJobTest.groovy rename to hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJobTest.groovy index e61c3b8b29..a530aa7979 100644 --- a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionJobTest.groovy +++ b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJobTest.groovy @@ -1,7 +1,7 @@ package pl.allegro.tech.hermes.management.domain.detection -import pl.allegro.tech.hermes.management.config.detection.UnusedTopicsDetectionProperties +import pl.allegro.tech.hermes.management.config.detection.InactiveTopicsDetectionProperties import pl.allegro.tech.hermes.management.domain.topic.TopicService import spock.lang.Specification @@ -12,36 +12,36 @@ import java.time.Instant import static java.time.temporal.ChronoUnit.DAYS import static pl.allegro.tech.hermes.api.TopicName.fromQualifiedName -class UnusedTopicsDetectionJobTest extends Specification { +class InactiveTopicsDetectionJobTest extends Specification { def topicServiceMock = Mock(TopicService) - def unusedTopicsStorageServiceMock = Mock(UnusedTopicsStorageService) + def inactiveTopicsStorageServiceMock = Mock(InactiveTopicsStorageService) def metricsRepositoryMock = Mock(LastPublishedMessageMetricsRepository) def clockMock = Mock(Clock) - def unusedTopicsNotifier = new InMemoryUnusedTopicsNotifier() + def inactiveTopicsNotifier = new InMemoryInactiveTopicsNotifier() - def unusedTopicsDetectionProperties = new UnusedTopicsDetectionProperties( + def inactiveTopicsDetectionProperties = new InactiveTopicsDetectionProperties( Duration.ofDays(7), Duration.ofDays(14), ["group.topic3"] as Set, "dc" ) - UnusedTopicsDetectionService detectionService = new UnusedTopicsDetectionService( + InactiveTopicsDetectionService detectionService = new InactiveTopicsDetectionService( metricsRepositoryMock, - unusedTopicsDetectionProperties, + inactiveTopicsDetectionProperties, clockMock ) - UnusedTopicsDetectionJob detectionJob = new UnusedTopicsDetectionJob( + InactiveTopicsDetectionJob detectionJob = new InactiveTopicsDetectionJob( topicServiceMock, - unusedTopicsStorageServiceMock, + inactiveTopicsStorageServiceMock, detectionService, - Optional.of(unusedTopicsNotifier), + Optional.of(inactiveTopicsNotifier), clockMock ) - def "should detect unused topics and notify when needed"() { + def "should detect inactive topics and notify when needed"() { given: def now = Instant.ofEpochMilli(1630600266987L) def ago7days = now.minus(7, DAYS) @@ -58,11 +58,11 @@ class UnusedTopicsDetectionJobTest extends Specification { "group.topic4", ] - and: "historically saved unused topics" - unusedTopicsStorageServiceMock.getUnusedTopics() >> [ - new UnusedTopic("group.topic2", ago7days.toEpochMilli(), [], false), - new UnusedTopic("group.topic3", ago7days.toEpochMilli(), [], true), - new UnusedTopic("group.topic4", ago21days.toEpochMilli(), [ago14days.toEpochMilli()], false), + and: "historically saved inactive topics" + inactiveTopicsStorageServiceMock.getInactiveTopics() >> [ + new InactiveTopic("group.topic2", ago7days.toEpochMilli(), [], false), + new InactiveTopic("group.topic3", ago7days.toEpochMilli(), [], true), + new InactiveTopic("group.topic4", ago21days.toEpochMilli(), [ago14days.toEpochMilli()], false), ] and: "current last published message timestamp" @@ -75,24 +75,24 @@ class UnusedTopicsDetectionJobTest extends Specification { when: detectionJob.detectAndNotify() - then: "notified are unused topics that are not whitelisted" - unusedTopicsNotifier.getNotifiedTopics().toList() == [ - new UnusedTopic("group.topic1", ago7days.toEpochMilli(), [], false), - new UnusedTopic("group.topic4", ago21days.toEpochMilli(), [ago14days.toEpochMilli()], false) + then: "notified are inactive topics that are not whitelisted" + inactiveTopicsNotifier.getNotifiedTopics().toList() == [ + new InactiveTopic("group.topic1", ago7days.toEpochMilli(), [], false), + new InactiveTopic("group.topic4", ago21days.toEpochMilli(), [ago14days.toEpochMilli()], false) ] - and: "saved are all unused topics with updated notification timestamps" - 1 * unusedTopicsStorageServiceMock.markAsUnused([ - new UnusedTopic("group.topic1", ago7days.toEpochMilli(), [now.toEpochMilli()], false), - new UnusedTopic("group.topic4", ago21days.toEpochMilli(), [ago14days.toEpochMilli(), now.toEpochMilli()], false), - new UnusedTopic("group.topic3", ago7days.toEpochMilli(), [], true) + and: "saved are all inactive topics with updated notification timestamps" + 1 * inactiveTopicsStorageServiceMock.markAsInactive([ + new InactiveTopic("group.topic1", ago7days.toEpochMilli(), [now.toEpochMilli()], false), + new InactiveTopic("group.topic4", ago21days.toEpochMilli(), [ago14days.toEpochMilli(), now.toEpochMilli()], false), + new InactiveTopic("group.topic3", ago7days.toEpochMilli(), [], true) ]) } - def "should not notify if there are no unused topics"() { + def "should not notify if there are no inactive topics"() { given: topicServiceMock.listQualifiedTopicNames() >> ["group.topic0"] - unusedTopicsStorageServiceMock.getUnusedTopics() >> [] + inactiveTopicsStorageServiceMock.getInactiveTopics() >> [] and: def now = Instant.ofEpochMilli(1630600266987L) @@ -103,9 +103,9 @@ class UnusedTopicsDetectionJobTest extends Specification { detectionJob.detectAndNotify() then: - unusedTopicsNotifier.getNotifiedTopics().toList() == [] + inactiveTopicsNotifier.getNotifiedTopics().toList() == [] and: - 1 * unusedTopicsStorageServiceMock.markAsUnused([]) + 1 * inactiveTopicsStorageServiceMock.markAsInactive([]) } } diff --git a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionServiceTest.groovy b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionServiceTest.groovy similarity index 68% rename from hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionServiceTest.groovy rename to hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionServiceTest.groovy index e2adaed7d8..e1560a8ab8 100644 --- a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsDetectionServiceTest.groovy +++ b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionServiceTest.groovy @@ -1,7 +1,7 @@ package pl.allegro.tech.hermes.management.domain.detection import pl.allegro.tech.hermes.api.TopicName -import pl.allegro.tech.hermes.management.config.detection.UnusedTopicsDetectionProperties +import pl.allegro.tech.hermes.management.config.detection.InactiveTopicsDetectionProperties import spock.lang.Specification import java.time.Clock @@ -9,7 +9,7 @@ import java.time.Duration import java.time.Instant import java.time.temporal.ChronoUnit -class UnusedTopicsDetectionServiceTest extends Specification { +class InactiveTopicsDetectionServiceTest extends Specification { private static def TEST_TOPIC_NAME = "group.topic" private static def WHITELISTED_TOPIC_NAME = "whitelisted.topic" @@ -17,7 +17,7 @@ class UnusedTopicsDetectionServiceTest extends Specification { private static def INACTIVITY_THRESHOLD = 7 private static def NEXT_NOTIFICATION_THRESHOLD = 14 - UnusedTopicsDetectionProperties properties = new UnusedTopicsDetectionProperties( + InactiveTopicsDetectionProperties properties = new InactiveTopicsDetectionProperties( Duration.ofDays(INACTIVITY_THRESHOLD), Duration.ofDays(NEXT_NOTIFICATION_THRESHOLD), [WHITELISTED_TOPIC_NAME] as Set, @@ -26,57 +26,57 @@ class UnusedTopicsDetectionServiceTest extends Specification { private def metricsRepositoryMock = Mock(LastPublishedMessageMetricsRepository) private def clockMock = Mock(Clock) - private def service = new UnusedTopicsDetectionService(metricsRepositoryMock, properties, clockMock) + private def service = new InactiveTopicsDetectionService(metricsRepositoryMock, properties, clockMock) def setup() { metricsRepositoryMock.getLastPublishedMessageTimestamp(TopicName.fromQualifiedName(TEST_TOPIC_NAME)) >> LAST_PUBLISHED metricsRepositoryMock.getLastPublishedMessageTimestamp(TopicName.fromQualifiedName(WHITELISTED_TOPIC_NAME)) >> LAST_PUBLISHED } - def "should detect unused topic when it surpasses inactivity threshold"() { + def "should detect inactive topic when it surpasses inactivity threshold"() { given: clockMock.instant() >> now when: - def result = service.detectUnusedTopic( + def result = service.detectInactiveTopic( TopicName.fromQualifiedName(TEST_TOPIC_NAME), - historicalUnusedTopic + historicalInactiveTopic ) then: - result.get() == new UnusedTopic( + result.get() == new InactiveTopic( TEST_TOPIC_NAME, LAST_PUBLISHED.toEpochMilli(), - historicalUnusedTopic.map { it.notificationTimestampsMs() }.orElse([]), + historicalInactiveTopic.map { it.notificationTimestampsMs() }.orElse([]), false ) where: - now | historicalUnusedTopic - plusDays(LAST_PUBLISHED, INACTIVITY_THRESHOLD) | Optional.of(unusedTopic(TEST_TOPIC_NAME, LAST_PUBLISHED, [now])) + now | historicalInactiveTopic + plusDays(LAST_PUBLISHED, INACTIVITY_THRESHOLD) | Optional.of(inactiveTopic(TEST_TOPIC_NAME, LAST_PUBLISHED, [now])) plusDays(LAST_PUBLISHED, INACTIVITY_THRESHOLD) | Optional.empty() - plusDays(LAST_PUBLISHED, INACTIVITY_THRESHOLD + 3) | Optional.of(unusedTopic(TEST_TOPIC_NAME, LAST_PUBLISHED, [now])) + plusDays(LAST_PUBLISHED, INACTIVITY_THRESHOLD + 3) | Optional.of(inactiveTopic(TEST_TOPIC_NAME, LAST_PUBLISHED, [now])) } - def "should correctly detect unused topic as whitelisted or not"() { + def "should correctly detect inactive topic as whitelisted or not"() { given: def now = plusDays(LAST_PUBLISHED, INACTIVITY_THRESHOLD) - def historicalUnusedTopic = unusedTopic(topicName, LAST_PUBLISHED, [now], historicallyWhitelisted) + def historicalInactiveTopic = inactiveTopic(topicName, LAST_PUBLISHED, [now], historicallyWhitelisted) and: clockMock.instant() >> now when: - def result = service.detectUnusedTopic( + def result = service.detectInactiveTopic( TopicName.fromQualifiedName(topicName), - Optional.of(historicalUnusedTopic) + Optional.of(historicalInactiveTopic) ) then: - result.get() == new UnusedTopic( + result.get() == new InactiveTopic( topicName, LAST_PUBLISHED.toEpochMilli(), - historicalUnusedTopic.notificationTimestampsMs(), + historicalInactiveTopic.notificationTimestampsMs(), whitelisted ) @@ -88,7 +88,7 @@ class UnusedTopicsDetectionServiceTest extends Specification { WHITELISTED_TOPIC_NAME | true || true } - def "should not detect unused topic when it is within inactivity threshold"() { + def "should not detect inactive topic when it is within inactivity threshold"() { given: def now = plusDays(LAST_PUBLISHED, INACTIVITY_THRESHOLD - 1) @@ -96,17 +96,17 @@ class UnusedTopicsDetectionServiceTest extends Specification { clockMock.instant() >> now when: - def result = service.detectUnusedTopic( + def result = service.detectInactiveTopic( TopicName.fromQualifiedName(TEST_TOPIC_NAME), - historicalUnusedTopic + historicalInactiveTopic ) then: result.isEmpty() where: - historicalUnusedTopic << [ - Optional.of(unusedTopic(TEST_TOPIC_NAME, LAST_PUBLISHED, [])), + historicalInactiveTopic << [ + Optional.of(inactiveTopic(TEST_TOPIC_NAME, LAST_PUBLISHED, [])), Optional.empty(), ] } @@ -116,10 +116,10 @@ class UnusedTopicsDetectionServiceTest extends Specification { clockMock.instant() >> now and: - def unusedTopic = unusedTopic(TEST_TOPIC_NAME, LAST_PUBLISHED, []) + def inactiveTopic = inactiveTopic(TEST_TOPIC_NAME, LAST_PUBLISHED, []) expect: - service.shouldBeNotified(unusedTopic) + service.shouldBeNotified(inactiveTopic) where: now << [plusDays(LAST_PUBLISHED, INACTIVITY_THRESHOLD), plusDays(LAST_PUBLISHED, 2 * INACTIVITY_THRESHOLD)] @@ -130,10 +130,10 @@ class UnusedTopicsDetectionServiceTest extends Specification { clockMock.instant() >> now and: - def unusedTopic = unusedTopic(TEST_TOPIC_NAME, LAST_PUBLISHED, notificationTimestamps) + def inactiveTopic = inactiveTopic(TEST_TOPIC_NAME, LAST_PUBLISHED, notificationTimestamps) expect: - service.shouldBeNotified(unusedTopic) + service.shouldBeNotified(inactiveTopic) where: now | notificationTimestamps @@ -149,10 +149,10 @@ class UnusedTopicsDetectionServiceTest extends Specification { clockMock.instant() >> now and: - def unusedTopic = unusedTopic(TEST_TOPIC_NAME, LAST_PUBLISHED, []) + def inactiveTopic = inactiveTopic(TEST_TOPIC_NAME, LAST_PUBLISHED, []) expect: - !service.shouldBeNotified(unusedTopic) + !service.shouldBeNotified(inactiveTopic) } def "should not be notified when whitelisted"() { @@ -163,10 +163,10 @@ class UnusedTopicsDetectionServiceTest extends Specification { clockMock.instant() >> now and: - def unusedTopic = unusedTopic(TEST_TOPIC_NAME, LAST_PUBLISHED, [], true) + def inactiveTopic = inactiveTopic(TEST_TOPIC_NAME, LAST_PUBLISHED, [], true) expect: - !service.shouldBeNotified(unusedTopic) + !service.shouldBeNotified(inactiveTopic) } def "should not be notified when not enough time from last notification"() { @@ -174,10 +174,10 @@ class UnusedTopicsDetectionServiceTest extends Specification { clockMock.instant() >> now and: - def unusedTopic = unusedTopic(TEST_TOPIC_NAME, LAST_PUBLISHED, notificationTimestamps) + def inactiveTopic = inactiveTopic(TEST_TOPIC_NAME, LAST_PUBLISHED, notificationTimestamps) expect: - !service.shouldBeNotified(unusedTopic) + !service.shouldBeNotified(inactiveTopic) where: now | notificationTimestamps @@ -189,8 +189,8 @@ class UnusedTopicsDetectionServiceTest extends Specification { return instant.plus(days, ChronoUnit.DAYS) } - private static UnusedTopic unusedTopic(String name, Instant lastPublished, List notificationTimestamps, boolean whitelisted = false) { - return new UnusedTopic( + private static InactiveTopic inactiveTopic(String name, Instant lastPublished, List notificationTimestamps, boolean whitelisted = false) { + return new InactiveTopic( name, lastPublished.toEpochMilli(), notificationTimestamps.collect { it.toEpochMilli() }, diff --git a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsStorageServiceTest.groovy b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsStorageServiceTest.groovy similarity index 52% rename from hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsStorageServiceTest.groovy rename to hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsStorageServiceTest.groovy index dafaa87eae..5b19a1ef1a 100644 --- a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/UnusedTopicsStorageServiceTest.groovy +++ b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsStorageServiceTest.groovy @@ -8,21 +8,21 @@ import pl.allegro.tech.hermes.infrastructure.zookeeper.ZookeeperPaths import pl.allegro.tech.hermes.management.config.storage.DefaultZookeeperGroupRepositoryFactory import pl.allegro.tech.hermes.management.domain.dc.MultiDatacenterRepositoryCommandExecutor import pl.allegro.tech.hermes.management.domain.mode.ModeService -import pl.allegro.tech.hermes.management.infrastructure.detection.ZookeeperUnusedTopicsRepository +import pl.allegro.tech.hermes.management.infrastructure.detection.ZookeeperInactiveTopicsRepository import pl.allegro.tech.hermes.management.infrastructure.zookeeper.ZookeeperClientManager import pl.allegro.tech.hermes.management.infrastructure.zookeeper.ZookeeperRepositoryManager import pl.allegro.tech.hermes.management.utils.MultiZookeeperIntegrationTest -class UnusedTopicsStorageServiceTest extends MultiZookeeperIntegrationTest { +class InactiveTopicsStorageServiceTest extends MultiZookeeperIntegrationTest { - static UNUSED_TOPICS_PATH = '/hermes/unused-topics' + static INACTIVE_TOPICS_PATH = '/hermes/inactive-topics' ZookeeperClientManager manager ZookeeperRepositoryManager repositoryManager ModeService modeService MultiDatacenterRepositoryCommandExecutor commandExecutor - UnusedTopicsStorageService unusedTopicsStorageService - UnusedTopicsRepository unusedTopicsRepository + InactiveTopicsStorageService inactiveTopicsStorageService + InactiveTopicsRepository inactiveTopicsRepository def objectMapper = new ObjectMapper().registerModule(new JavaTimeModule()) def paths = new ZookeeperPaths('/hermes') @@ -31,75 +31,75 @@ class UnusedTopicsStorageServiceTest extends MultiZookeeperIntegrationTest { manager = buildZookeeperClientManager() manager.start() assertZookeeperClientsConnected(manager.clients) - unusedTopicsRepository = new ZookeeperUnusedTopicsRepository(manager.localClient.curatorFramework, objectMapper, paths) + inactiveTopicsRepository = new ZookeeperInactiveTopicsRepository(manager.localClient.curatorFramework, objectMapper, paths) repositoryManager = new ZookeeperRepositoryManager( manager, new TestDatacenterNameProvider(DC_1_NAME), objectMapper, paths, new DefaultZookeeperGroupRepositoryFactory()) repositoryManager.start() modeService = new ModeService() commandExecutor = new MultiDatacenterRepositoryCommandExecutor(repositoryManager, true, modeService) - unusedTopicsStorageService = new UnusedTopicsStorageService(unusedTopicsRepository, commandExecutor) + inactiveTopicsStorageService = new InactiveTopicsStorageService(inactiveTopicsRepository, commandExecutor) } def cleanup() { manager.stop() } - def TEST_UNUSED_TOPICS = [ - new UnusedTopic("group.topic1", 1730641716154L, [1730641716250L, 1730641716321], false), - new UnusedTopic("group.topic2", 1730641712371L, [1730641716250L], false), - new UnusedTopic("group.topic3", 1730641712371L, [], true), + def TEST_INACTIVE_TOPICS = [ + new InactiveTopic("group.topic1", 1730641716154L, [1730641716250L, 1730641716321], false), + new InactiveTopic("group.topic2", 1730641712371L, [1730641716250L], false), + new InactiveTopic("group.topic3", 1730641712371L, [], true), ] def "should create node in all zk clusters if it doesn't exist when upserting"() { when: - unusedTopicsStorageService.markAsUnused(TEST_UNUSED_TOPICS) + inactiveTopicsStorageService.markAsInactive(TEST_INACTIVE_TOPICS) then: - assertNodesContain(TEST_UNUSED_TOPICS) + assertNodesContain(TEST_INACTIVE_TOPICS) } def "should update existing node data in all zk clusters when upserting"() { given: - setupNodes(TEST_UNUSED_TOPICS) + setupNodes(TEST_INACTIVE_TOPICS) and: - def newUnusedTopics = [ - TEST_UNUSED_TOPICS[0], - new UnusedTopic("group.topic3", 1730641712371L, [1730641712678L], false), - new UnusedTopic("group.topic4", 1730641712706L, [1730641712999L], false), + def newInactiveTopics = [ + TEST_INACTIVE_TOPICS[0], + new InactiveTopic("group.topic3", 1730641712371L, [1730641712678L], false), + new InactiveTopic("group.topic4", 1730641712706L, [1730641712999L], false), ] when: - unusedTopicsStorageService.markAsUnused(newUnusedTopics) + inactiveTopicsStorageService.markAsInactive(newInactiveTopics) then: - assertNodesContain(newUnusedTopics) + assertNodesContain(newInactiveTopics) } def "nodes should remain unchanged in case of unavailability when upserting"() { given: - setupNodes(TEST_UNUSED_TOPICS) + setupNodes(TEST_INACTIVE_TOPICS) and: zookeeper2.stop() when: - unusedTopicsStorageService.markAsUnused([ - new UnusedTopic("group.topic3", 1730641656154L, [], false) + inactiveTopicsStorageService.markAsInactive([ + new InactiveTopic("group.topic3", 1730641656154L, [], false) ]) then: def e = thrown(InternalProcessingException) - e.message == "Execution of command 'MarkTopicsAsUnused(number of topics=1)' failed on DC 'dc2'." + e.message == "Execution of command 'MarkTopicsAsInactive(number of topics=1)' failed on DC 'dc2'." and: - nodeData(DC_1_NAME) == TEST_UNUSED_TOPICS + nodeData(DC_1_NAME) == TEST_INACTIVE_TOPICS } def "should return empty list when node doesn't exist"() { when: - def result = unusedTopicsStorageService.getUnusedTopics() + def result = inactiveTopicsStorageService.getInactiveTopics() then: result == [] @@ -110,42 +110,42 @@ class UnusedTopicsStorageServiceTest extends MultiZookeeperIntegrationTest { setupNodes([]) when: - def result = unusedTopicsStorageService.getUnusedTopics() + def result = inactiveTopicsStorageService.getInactiveTopics() then: result == [] } - def "should return list of unused topics"() { + def "should return list of inactive topics"() { given: - setupNodes(TEST_UNUSED_TOPICS) + setupNodes(TEST_INACTIVE_TOPICS) when: - def result = unusedTopicsStorageService.getUnusedTopics() + def result = inactiveTopicsStorageService.getInactiveTopics() then: - result.sort() == TEST_UNUSED_TOPICS.sort() + result.sort() == TEST_INACTIVE_TOPICS.sort() } - private def setupNodes(List unusedTopics) { + private def setupNodes(List inactiveTopics) { manager.clients.each { it.curatorFramework.create() .creatingParentsIfNeeded() - .forPath(UNUSED_TOPICS_PATH, objectMapper.writeValueAsBytes(unusedTopics)) + .forPath(INACTIVE_TOPICS_PATH, objectMapper.writeValueAsBytes(inactiveTopics)) } } - private def assertNodesContain(List unusedTopics) { + private def assertNodesContain(List inactiveTopics) { manager.clients.each { - def data = it.curatorFramework.getData().forPath(UNUSED_TOPICS_PATH) - def foundUnusedTopics = objectMapper.readValue(data, new TypeReference>() {}) - assert unusedTopics.sort() == foundUnusedTopics.sort() + def data = it.curatorFramework.getData().forPath(INACTIVE_TOPICS_PATH) + def foundInactiveTopics = objectMapper.readValue(data, new TypeReference>() {}) + assert inactiveTopics.sort() == foundInactiveTopics.sort() } } - private List nodeData(String dc) { + private List nodeData(String dc) { def zkClient = findClientByDc(manager.clients, dc) - def data = zkClient.curatorFramework.getData().forPath(UNUSED_TOPICS_PATH) - return objectMapper.readValue(data, new TypeReference>() {}) + def data = zkClient.curatorFramework.getData().forPath(INACTIVE_TOPICS_PATH) + return objectMapper.readValue(data, new TypeReference>() {}) } } From edbec332ef6e210cda2500c6102af37fe5e49cea Mon Sep 17 00:00:00 2001 From: Michal Wisniewski Date: Tue, 12 Nov 2024 11:59:33 +0100 Subject: [PATCH 17/39] SKYEDEN-3234 | Handle lack of last published message metrics --- .../InactiveTopicsDetectionService.java | 10 ++++++--- ...LastPublishedMessageMetricsRepository.java | 3 ++- ...LastPublishedMessageMetricsRepository.java | 3 ++- .../metrics/SummedSharedCounter.java | 7 +++--- .../InactiveTopicsDetectionJobTest.groovy | 16 +++++++++----- .../InactiveTopicsDetectionServiceTest.groovy | 22 +++++++++++++++++-- .../metrics/SummedSharedCounterTest.groovy | 8 +++---- 7 files changed, 49 insertions(+), 20 deletions(-) diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionService.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionService.java index 7ac8bd05ae..fe94f48246 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionService.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionService.java @@ -27,10 +27,14 @@ public InactiveTopicsDetectionService( public Optional detectInactiveTopic( TopicName topicName, Optional historicalInactiveTopic) { Instant now = clock.instant(); - Instant lastUsed = metricsRepository.getLastPublishedMessageTimestamp(topicName); - boolean isInactive = isInactive(lastUsed, now); + Optional lastUsedOptional = + metricsRepository.getLastPublishedMessageTimestamp(topicName); + if (lastUsedOptional.isEmpty()) { + return Optional.empty(); + } + Instant lastUsed = lastUsedOptional.get(); - if (isInactive) { + if (isInactive(lastUsed, now)) { return Optional.of( new InactiveTopic( topicName.qualifiedName(), diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/LastPublishedMessageMetricsRepository.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/LastPublishedMessageMetricsRepository.java index 2d3a4f65ba..10c0e4b5f3 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/LastPublishedMessageMetricsRepository.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/LastPublishedMessageMetricsRepository.java @@ -3,7 +3,8 @@ import pl.allegro.tech.hermes.api.TopicName; import java.time.Instant; +import java.util.Optional; public interface LastPublishedMessageMetricsRepository { - Instant getLastPublishedMessageTimestamp(TopicName topicName); + Optional getLastPublishedMessageTimestamp(TopicName topicName); } diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/ZookeeperLastPublishedMessageMetricsRepository.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/ZookeeperLastPublishedMessageMetricsRepository.java index 135d34ea77..64d750af81 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/ZookeeperLastPublishedMessageMetricsRepository.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/ZookeeperLastPublishedMessageMetricsRepository.java @@ -7,6 +7,7 @@ import pl.allegro.tech.hermes.management.infrastructure.metrics.SummedSharedCounter; import java.time.Instant; +import java.util.Optional; @Component public class ZookeeperLastPublishedMessageMetricsRepository @@ -21,7 +22,7 @@ public ZookeeperLastPublishedMessageMetricsRepository( } @Override - public Instant getLastPublishedMessageTimestamp(TopicName topicName) { + public Optional getLastPublishedMessageTimestamp(TopicName topicName) { return summedSharedCounter.getLastModified( zookeeperPaths.topicMetricPath(topicName, "published")); } diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/metrics/SummedSharedCounter.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/metrics/SummedSharedCounter.java index 43ef6229dc..6fd1fa2e27 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/metrics/SummedSharedCounter.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/metrics/SummedSharedCounter.java @@ -8,6 +8,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.concurrent.TimeUnit; import org.apache.curator.framework.CuratorFramework; import org.apache.curator.framework.recipes.atomic.DistributedAtomicLong; @@ -40,7 +41,7 @@ public long getValue(String path) { } } - public Instant getLastModified(String path) { + public Optional getLastModified(String path) { try { return counterAggregators.get(path).getLastModified(); } catch (ZookeeperCounterException e) { @@ -102,7 +103,7 @@ long aggregate() throws Exception { return sum; } - Instant getLastModified() throws Exception { + Optional getLastModified() throws Exception { Instant lastModified = null; for (Map.Entry curatorClient : curatorPerDatacenter.entrySet()) { ensureConnected(curatorClient.getKey()); @@ -114,7 +115,7 @@ Instant getLastModified() throws Exception { } } } - return lastModified; + return Optional.ofNullable(lastModified); } private void ensureConnected(String datacenter) { diff --git a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJobTest.groovy b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJobTest.groovy index a530aa7979..8cb97b96ff 100644 --- a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJobTest.groovy +++ b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJobTest.groovy @@ -66,11 +66,11 @@ class InactiveTopicsDetectionJobTest extends Specification { ] and: "current last published message timestamp" - metricsRepositoryMock.getLastPublishedMessageTimestamp(fromQualifiedName("group.topic0")) >> now - metricsRepositoryMock.getLastPublishedMessageTimestamp(fromQualifiedName("group.topic1")) >> ago7days - metricsRepositoryMock.getLastPublishedMessageTimestamp(fromQualifiedName("group.topic2")) >> now - metricsRepositoryMock.getLastPublishedMessageTimestamp(fromQualifiedName("group.topic3")) >> ago7days - metricsRepositoryMock.getLastPublishedMessageTimestamp(fromQualifiedName("group.topic4")) >> ago21days + mockLastPublishedMessageTimestamp("group.topic0", now) + mockLastPublishedMessageTimestamp("group.topic1", ago7days) + mockLastPublishedMessageTimestamp("group.topic2", now) + mockLastPublishedMessageTimestamp("group.topic3", ago7days) + mockLastPublishedMessageTimestamp("group.topic4", ago21days) when: detectionJob.detectAndNotify() @@ -97,7 +97,7 @@ class InactiveTopicsDetectionJobTest extends Specification { and: def now = Instant.ofEpochMilli(1630600266987L) clockMock.instant() >> now - metricsRepositoryMock.getLastPublishedMessageTimestamp(fromQualifiedName("group.topic0")) >> now + mockLastPublishedMessageTimestamp("group.topic0", now) when: detectionJob.detectAndNotify() @@ -108,4 +108,8 @@ class InactiveTopicsDetectionJobTest extends Specification { and: 1 * inactiveTopicsStorageServiceMock.markAsInactive([]) } + + private def mockLastPublishedMessageTimestamp(String topicName, Instant instant) { + metricsRepositoryMock.getLastPublishedMessageTimestamp(fromQualifiedName(topicName)) >> Optional.ofNullable(instant) + } } diff --git a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionServiceTest.groovy b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionServiceTest.groovy index e1560a8ab8..207fda35fd 100644 --- a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionServiceTest.groovy +++ b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionServiceTest.groovy @@ -29,8 +29,8 @@ class InactiveTopicsDetectionServiceTest extends Specification { private def service = new InactiveTopicsDetectionService(metricsRepositoryMock, properties, clockMock) def setup() { - metricsRepositoryMock.getLastPublishedMessageTimestamp(TopicName.fromQualifiedName(TEST_TOPIC_NAME)) >> LAST_PUBLISHED - metricsRepositoryMock.getLastPublishedMessageTimestamp(TopicName.fromQualifiedName(WHITELISTED_TOPIC_NAME)) >> LAST_PUBLISHED + metricsRepositoryMock.getLastPublishedMessageTimestamp(TopicName.fromQualifiedName(TEST_TOPIC_NAME)) >> Optional.of(LAST_PUBLISHED) + metricsRepositoryMock.getLastPublishedMessageTimestamp(TopicName.fromQualifiedName(WHITELISTED_TOPIC_NAME)) >> Optional.of(LAST_PUBLISHED) } def "should detect inactive topic when it surpasses inactivity threshold"() { @@ -111,6 +111,24 @@ class InactiveTopicsDetectionServiceTest extends Specification { ] } + def "should not detect inactive topic when last published message metrics are not available"() { + given: + String topicName = "group.topicWithoutMetrics" + + and: + clockMock.instant() >> plusDays(LAST_PUBLISHED, INACTIVITY_THRESHOLD) + metricsRepositoryMock.getLastPublishedMessageTimestamp(TopicName.fromQualifiedName(topicName)) >> Optional.empty() + + when: + def result = service.detectInactiveTopic( + TopicName.fromQualifiedName(topicName), + Optional.empty() + ) + + then: + result.isEmpty() + } + def "should be notified when inactive and no sent notifications"() { given: clockMock.instant() >> now diff --git a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/infrastructure/metrics/SummedSharedCounterTest.groovy b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/infrastructure/metrics/SummedSharedCounterTest.groovy index 3fcf3b4e55..62be1f965a 100644 --- a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/infrastructure/metrics/SummedSharedCounterTest.groovy +++ b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/infrastructure/metrics/SummedSharedCounterTest.groovy @@ -52,19 +52,19 @@ class SummedSharedCounterTest extends MultiZookeeperIntegrationTest { def sharedCounterDc1Mtime = getMtime(DC_1_NAME, COUNTER_PATH) then: - summedSharedCounter.getLastModified(COUNTER_PATH).toEpochMilli() == sharedCounterDc1Mtime + summedSharedCounter.getLastModified(COUNTER_PATH).get().toEpochMilli() == sharedCounterDc1Mtime when: sharedCounterDc2.increment(COUNTER_PATH, 1) def sharedCounterDc2Mtime = getMtime(DC_2_NAME, COUNTER_PATH) then: - summedSharedCounter.getLastModified(COUNTER_PATH).toEpochMilli() == sharedCounterDc2Mtime + summedSharedCounter.getLastModified(COUNTER_PATH).get().toEpochMilli() == sharedCounterDc2Mtime } - def "should return null for last modified time of non existing counter"() { + def "should return empty optional for last modified time of non existing counter"() { expect: - summedSharedCounter.getLastModified("/does/not/exist") == null + summedSharedCounter.getLastModified("/does/not/exist") == Optional.empty() } private def getMtime(String dc, String counterPath) { From bfd9acf0ef59246d32fb91a56ad362854ba97ec5 Mon Sep 17 00:00:00 2001 From: Michal Wisniewski Date: Tue, 12 Nov 2024 13:19:47 +0100 Subject: [PATCH 18/39] SKYEDEN-3234 | Do not call notifier when no inactive topics --- .../domain/detection/InactiveTopicsDetectionJob.java | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJob.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJob.java index 96fb402004..508150f9b1 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJob.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJob.java @@ -86,11 +86,13 @@ private Map groupByName(List inactiveTopic } private void notify(List inactiveTopics) { - if (notifier.isPresent()) { - logger.info("Notifying {} inactive topics", inactiveTopics.size()); - notifier.get().notify(inactiveTopics); - } else { - logger.info("Skipping notification of {} inactive topics", inactiveTopics.size()); + if (!inactiveTopics.isEmpty()) { + if (notifier.isPresent()) { + logger.info("Notifying {} inactive topics", inactiveTopics.size()); + notifier.get().notify(inactiveTopics); + } else { + logger.info("Skipping notification of {} inactive topics", inactiveTopics.size()); + } } } From e0af3ee58fb08eb4dac0074b1773a63511bf4265 Mon Sep 17 00:00:00 2001 From: Michal Wisniewski Date: Tue, 12 Nov 2024 14:05:12 +0100 Subject: [PATCH 19/39] SKYEDEN-3234 | Fix style --- .../detection/InactiveTopicsDetectionProperties.java | 3 +-- .../domain/detection/InactiveTopicsDetectionJob.java | 12 ++++++------ .../LastPublishedMessageMetricsRepository.java | 5 ++--- .../detection/InactiveTopicsDetectionScheduler.java | 3 ++- ...okeeperLastPublishedMessageMetricsRepository.java | 5 ++--- .../infrastructure/metrics/SummedSharedCounter.java | 1 - .../zookeeper/ZookeeperRepositoryManager.java | 3 ++- 7 files changed, 15 insertions(+), 17 deletions(-) diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/detection/InactiveTopicsDetectionProperties.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/detection/InactiveTopicsDetectionProperties.java index 9ae4b4e352..680628d153 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/detection/InactiveTopicsDetectionProperties.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/detection/InactiveTopicsDetectionProperties.java @@ -1,9 +1,8 @@ package pl.allegro.tech.hermes.management.config.detection; -import org.springframework.boot.context.properties.ConfigurationProperties; - import java.time.Duration; import java.util.Set; +import org.springframework.boot.context.properties.ConfigurationProperties; @ConfigurationProperties(prefix = "detection.inactive-topics") public record InactiveTopicsDetectionProperties( diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJob.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJob.java index 508150f9b1..053c6f6033 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJob.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJob.java @@ -87,12 +87,12 @@ private Map groupByName(List inactiveTopic private void notify(List inactiveTopics) { if (!inactiveTopics.isEmpty()) { - if (notifier.isPresent()) { - logger.info("Notifying {} inactive topics", inactiveTopics.size()); - notifier.get().notify(inactiveTopics); - } else { - logger.info("Skipping notification of {} inactive topics", inactiveTopics.size()); - } + if (notifier.isPresent()) { + logger.info("Notifying {} inactive topics", inactiveTopics.size()); + notifier.get().notify(inactiveTopics); + } else { + logger.info("Skipping notification of {} inactive topics", inactiveTopics.size()); + } } } diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/LastPublishedMessageMetricsRepository.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/LastPublishedMessageMetricsRepository.java index 10c0e4b5f3..c14423a045 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/LastPublishedMessageMetricsRepository.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/LastPublishedMessageMetricsRepository.java @@ -1,10 +1,9 @@ package pl.allegro.tech.hermes.management.domain.detection; -import pl.allegro.tech.hermes.api.TopicName; - import java.time.Instant; import java.util.Optional; +import pl.allegro.tech.hermes.api.TopicName; public interface LastPublishedMessageMetricsRepository { - Optional getLastPublishedMessageTimestamp(TopicName topicName); + Optional getLastPublishedMessageTimestamp(TopicName topicName); } diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/InactiveTopicsDetectionScheduler.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/InactiveTopicsDetectionScheduler.java index a748ad3c15..82b5427e39 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/InactiveTopicsDetectionScheduler.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/InactiveTopicsDetectionScheduler.java @@ -70,6 +70,7 @@ public void run() { } private void logLeaderZookeeperClientNotFound(String dc) { - logger.error("Cannot run inactive topics detection - no zookeeper client for datacenter={}", dc); + logger.error( + "Cannot run inactive topics detection - no zookeeper client for datacenter={}", dc); } } diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/ZookeeperLastPublishedMessageMetricsRepository.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/ZookeeperLastPublishedMessageMetricsRepository.java index 64d750af81..4c02e0d10d 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/ZookeeperLastPublishedMessageMetricsRepository.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/ZookeeperLastPublishedMessageMetricsRepository.java @@ -1,14 +1,13 @@ package pl.allegro.tech.hermes.management.infrastructure.detection; +import java.time.Instant; +import java.util.Optional; import org.springframework.stereotype.Component; import pl.allegro.tech.hermes.api.TopicName; import pl.allegro.tech.hermes.infrastructure.zookeeper.ZookeeperPaths; import pl.allegro.tech.hermes.management.domain.detection.LastPublishedMessageMetricsRepository; import pl.allegro.tech.hermes.management.infrastructure.metrics.SummedSharedCounter; -import java.time.Instant; -import java.util.Optional; - @Component public class ZookeeperLastPublishedMessageMetricsRepository implements LastPublishedMessageMetricsRepository { diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/metrics/SummedSharedCounter.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/metrics/SummedSharedCounter.java index 6fd1fa2e27..bf609d14c5 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/metrics/SummedSharedCounter.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/metrics/SummedSharedCounter.java @@ -3,7 +3,6 @@ import com.google.common.cache.CacheBuilder; import com.google.common.cache.CacheLoader; import com.google.common.cache.LoadingCache; - import java.time.Instant; import java.util.HashMap; import java.util.List; diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/zookeeper/ZookeeperRepositoryManager.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/zookeeper/ZookeeperRepositoryManager.java index fa9b323dd9..575a580746 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/zookeeper/ZookeeperRepositoryManager.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/zookeeper/ZookeeperRepositoryManager.java @@ -69,7 +69,8 @@ public class ZookeeperRepositoryManager implements RepositoryManager { new HashMap<>(); private final Map offlineRetransmissionRepositoriesByDc = new HashMap<>(); - private final Map inactiveTopicsRepositoriesByDc = new HashMap<>(); + private final Map inactiveTopicsRepositoriesByDc = + new HashMap<>(); private final ZookeeperGroupRepositoryFactory zookeeperGroupRepositoryFactory; public ZookeeperRepositoryManager( From c9f3bac0b960f6e9e8ec58d1eaac6d8dbe34ad1f Mon Sep 17 00:00:00 2001 From: Michal Wisniewski Date: Tue, 12 Nov 2024 14:05:34 +0100 Subject: [PATCH 20/39] SKYEDEN-3234 | Add log when detection starts --- .../detection/InactiveTopicsDetectionScheduler.java | 1 + 1 file changed, 1 insertion(+) diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/InactiveTopicsDetectionScheduler.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/InactiveTopicsDetectionScheduler.java index 82b5427e39..f92f4dac31 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/InactiveTopicsDetectionScheduler.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/InactiveTopicsDetectionScheduler.java @@ -62,6 +62,7 @@ public void startListeningForLeadership() { public void run() { if (leaderLatch.isPresent()) { if (leaderLatch.get().hasLeadership()) { + logger.info("Inactive topics detection started"); job.detectAndNotify(); } } else { From ebc15410fba1603e0bb3b818d58caa13e6596635 Mon Sep 17 00:00:00 2001 From: Michal Wisniewski Date: Tue, 12 Nov 2024 15:09:22 +0100 Subject: [PATCH 21/39] SKYEDEN-3234 | Fix style --- .../zookeeper/ZookeeperBasedRepository.java | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/hermes-common/src/main/java/pl/allegro/tech/hermes/infrastructure/zookeeper/ZookeeperBasedRepository.java b/hermes-common/src/main/java/pl/allegro/tech/hermes/infrastructure/zookeeper/ZookeeperBasedRepository.java index a70d6c3723..a5b8ace0e1 100644 --- a/hermes-common/src/main/java/pl/allegro/tech/hermes/infrastructure/zookeeper/ZookeeperBasedRepository.java +++ b/hermes-common/src/main/java/pl/allegro/tech/hermes/infrastructure/zookeeper/ZookeeperBasedRepository.java @@ -3,6 +3,13 @@ import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.JsonMappingException; import com.fasterxml.jackson.databind.ObjectMapper; +import java.io.IOException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Optional; +import java.util.function.BiConsumer; +import java.util.stream.Collectors; import org.apache.commons.lang3.ArrayUtils; import org.apache.curator.framework.CuratorFramework; import org.apache.curator.framework.api.transaction.CuratorOp; @@ -14,14 +21,6 @@ import pl.allegro.tech.hermes.common.exception.RepositoryNotAvailableException; import pl.allegro.tech.hermes.infrastructure.MalformedDataException; -import java.io.IOException; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; -import java.util.Optional; -import java.util.function.BiConsumer; -import java.util.stream.Collectors; - public abstract class ZookeeperBasedRepository { private static final Logger logger = LoggerFactory.getLogger(ZookeeperBasedRepository.class); @@ -162,10 +161,11 @@ protected void createRecursively(String path, Object value) throws Exception { protected void createInTransaction(String path, Object value, String childPath) throws Exception { ensureConnected(); - zookeeper.transaction().forOperations( + zookeeper + .transaction() + .forOperations( zookeeper.transactionOp().create().forPath(path, mapper.writeValueAsBytes(value)), - zookeeper.transactionOp().create().forPath(childPath) - ); + zookeeper.transactionOp().create().forPath(childPath)); } protected void deleteInTransaction(List paths) throws Exception { From a2d47ca1cd83816886a40452915e3b3e83c6ef45 Mon Sep 17 00:00:00 2001 From: Michal Wisniewski Date: Wed, 13 Nov 2024 15:08:14 +0100 Subject: [PATCH 22/39] SKYEDEN-3234 | Add more logging --- .../detection/InactiveTopicsDetectionScheduler.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/InactiveTopicsDetectionScheduler.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/InactiveTopicsDetectionScheduler.java index f92f4dac31..2c1bc410d6 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/InactiveTopicsDetectionScheduler.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/InactiveTopicsDetectionScheduler.java @@ -16,7 +16,7 @@ import pl.allegro.tech.hermes.management.infrastructure.zookeeper.ZookeeperClient; import pl.allegro.tech.hermes.management.infrastructure.zookeeper.ZookeeperClientManager; -@ConditionalOnProperty(value = "detection.inactive-topics.enabled", havingValue = "true") +@ConditionalOnProperty(prefix = "detection.inactive-topics", value = "enabled", havingValue = "true") @Component @EnableConfigurationProperties(InactiveTopicsDetectionProperties.class) public class InactiveTopicsDetectionScheduler { @@ -42,6 +42,8 @@ public InactiveTopicsDetectionScheduler( this.leaderLatch = leaderCuratorFramework.map(it -> new LeaderLatch(it, leaderPath)); if (leaderLatch.isEmpty()) { logLeaderZookeeperClientNotFound(leaderElectionDc); + } else { + logger.info("Found leader Zookeeper Client, leader latch created"); } this.job = job; } @@ -50,6 +52,7 @@ public InactiveTopicsDetectionScheduler( public void startListeningForLeadership() { leaderLatch.ifPresent( it -> { + logger.info("Starting listening for leadership"); try { it.start(); } catch (Exception e) { @@ -64,6 +67,8 @@ public void run() { if (leaderLatch.get().hasLeadership()) { logger.info("Inactive topics detection started"); job.detectAndNotify(); + } else { + logger.info("Inactive topics detection not started - not a leader"); } } else { logLeaderZookeeperClientNotFound(leaderElectionDc); From 26e70f438bdaa563f4c3cb9e26751db14b777058 Mon Sep 17 00:00:00 2001 From: Michal Wisniewski Date: Wed, 13 Nov 2024 17:20:05 +0100 Subject: [PATCH 23/39] SKYEDEN-3234 | Move leader and config to separate classes --- .../InactiveTopicsDetectionConfig.java | 39 +++++++++++ .../detection/InactiveTopicsDetectionJob.java | 3 - .../InactiveTopicsDetectionLeader.java | 57 ++++++++++++++++ .../InactiveTopicsDetectionScheduler.java | 66 ++----------------- 4 files changed, 103 insertions(+), 62 deletions(-) create mode 100644 hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/detection/InactiveTopicsDetectionConfig.java create mode 100644 hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/InactiveTopicsDetectionLeader.java diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/detection/InactiveTopicsDetectionConfig.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/detection/InactiveTopicsDetectionConfig.java new file mode 100644 index 0000000000..491fa1322c --- /dev/null +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/detection/InactiveTopicsDetectionConfig.java @@ -0,0 +1,39 @@ +package pl.allegro.tech.hermes.management.config.detection; + +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.boot.context.properties.EnableConfigurationProperties; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.scheduling.annotation.EnableScheduling; +import pl.allegro.tech.hermes.infrastructure.zookeeper.ZookeeperPaths; +import pl.allegro.tech.hermes.management.domain.detection.InactiveTopicsDetectionJob; +import pl.allegro.tech.hermes.management.infrastructure.detection.InactiveTopicsDetectionLeader; +import pl.allegro.tech.hermes.management.infrastructure.detection.InactiveTopicsDetectionScheduler; +import pl.allegro.tech.hermes.management.infrastructure.zookeeper.ZookeeperClientManager; + +@Configuration +@EnableConfigurationProperties(InactiveTopicsDetectionProperties.class) +@EnableScheduling +public class InactiveTopicsDetectionConfig { + @ConditionalOnProperty( + prefix = "detection.inactive-topics", + value = "enabled", + havingValue = "true") + @Bean + InactiveTopicsDetectionLeader inactiveTopicsDetectionLeader( + ZookeeperClientManager zookeeperClientManager, + InactiveTopicsDetectionProperties properties, + ZookeeperPaths zookeeperPaths) { + return new InactiveTopicsDetectionLeader(zookeeperClientManager, properties, zookeeperPaths); + } + + @ConditionalOnProperty( + prefix = "detection.inactive-topics", + value = "enabled", + havingValue = "true") + @Bean + InactiveTopicsDetectionScheduler inactiveTopicsDetectionScheduler( + InactiveTopicsDetectionJob job, InactiveTopicsDetectionLeader leader) { + return new InactiveTopicsDetectionScheduler(job, leader); + } +} diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJob.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJob.java index 053c6f6033..25b3198b61 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJob.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJob.java @@ -9,14 +9,11 @@ import java.util.stream.Stream; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.stereotype.Component; import pl.allegro.tech.hermes.api.TopicName; -import pl.allegro.tech.hermes.management.config.detection.InactiveTopicsDetectionProperties; import pl.allegro.tech.hermes.management.domain.topic.TopicService; @Component -@EnableConfigurationProperties({InactiveTopicsDetectionProperties.class}) public class InactiveTopicsDetectionJob { private final TopicService topicService; private final InactiveTopicsStorageService inactiveTopicsStorageService; diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/InactiveTopicsDetectionLeader.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/InactiveTopicsDetectionLeader.java new file mode 100644 index 0000000000..f384e72e6d --- /dev/null +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/InactiveTopicsDetectionLeader.java @@ -0,0 +1,57 @@ +package pl.allegro.tech.hermes.management.infrastructure.detection; + +import jakarta.annotation.PostConstruct; +import java.util.Optional; +import org.apache.curator.framework.CuratorFramework; +import org.apache.curator.framework.recipes.leader.LeaderLatch; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import pl.allegro.tech.hermes.infrastructure.zookeeper.ZookeeperPaths; +import pl.allegro.tech.hermes.management.config.detection.InactiveTopicsDetectionProperties; +import pl.allegro.tech.hermes.management.infrastructure.zookeeper.ZookeeperClient; +import pl.allegro.tech.hermes.management.infrastructure.zookeeper.ZookeeperClientManager; + +public class InactiveTopicsDetectionLeader { + + private final String leaderElectionDc; + private final Optional leaderLatch; + + private static final Logger logger = LoggerFactory.getLogger(InactiveTopicsDetectionLeader.class); + + public InactiveTopicsDetectionLeader( + ZookeeperClientManager zookeeperClientManager, + InactiveTopicsDetectionProperties inactiveTopicsDetectionProperties, + ZookeeperPaths zookeeperPaths) { + this.leaderElectionDc = inactiveTopicsDetectionProperties.leaderElectionZookeeperDc(); + Optional leaderCuratorFramework = + zookeeperClientManager.getClients().stream() + .filter(it -> it.getDatacenterName().equals(leaderElectionDc)) + .findFirst() + .map(ZookeeperClient::getCuratorFramework); + String leaderPath = zookeeperPaths.inactiveTopicsLeaderPath(); + this.leaderLatch = leaderCuratorFramework.map(it -> new LeaderLatch(it, leaderPath)); + } + + @PostConstruct + public void startListeningForLeadership() { + if (leaderLatch.isPresent()) { + logger.info("Starting listening for leadership"); + try { + leaderLatch.get().start(); + } catch (Exception e) { + throw new RuntimeException(e); + } + } else { + logLeaderZookeeperClientNotFound(leaderElectionDc); + } + } + + public boolean isLeader() { + return leaderLatch.map(LeaderLatch::hasLeadership).orElse(false); + } + + private void logLeaderZookeeperClientNotFound(String dc) { + logger.error( + "Cannot run inactive topics detection - no zookeeper client for datacenter={}", dc); + } +} diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/InactiveTopicsDetectionScheduler.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/InactiveTopicsDetectionScheduler.java index 2c1bc410d6..f7daa1ee77 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/InactiveTopicsDetectionScheduler.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/InactiveTopicsDetectionScheduler.java @@ -1,82 +1,30 @@ package pl.allegro.tech.hermes.management.infrastructure.detection; -import jakarta.annotation.PostConstruct; -import java.util.Optional; -import org.apache.curator.framework.CuratorFramework; -import org.apache.curator.framework.recipes.leader.LeaderLatch; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; -import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.scheduling.annotation.Scheduled; -import org.springframework.stereotype.Component; -import pl.allegro.tech.hermes.infrastructure.zookeeper.ZookeeperPaths; -import pl.allegro.tech.hermes.management.config.detection.InactiveTopicsDetectionProperties; import pl.allegro.tech.hermes.management.domain.detection.InactiveTopicsDetectionJob; -import pl.allegro.tech.hermes.management.infrastructure.zookeeper.ZookeeperClient; -import pl.allegro.tech.hermes.management.infrastructure.zookeeper.ZookeeperClientManager; -@ConditionalOnProperty(prefix = "detection.inactive-topics", value = "enabled", havingValue = "true") -@Component -@EnableConfigurationProperties(InactiveTopicsDetectionProperties.class) public class InactiveTopicsDetectionScheduler { private final InactiveTopicsDetectionJob job; - private final String leaderElectionDc; - private final Optional leaderLatch; + private final InactiveTopicsDetectionLeader leader; private static final Logger logger = LoggerFactory.getLogger(InactiveTopicsDetectionScheduler.class); public InactiveTopicsDetectionScheduler( - InactiveTopicsDetectionJob job, - ZookeeperClientManager zookeeperClientManager, - InactiveTopicsDetectionProperties inactiveTopicsDetectionProperties, - ZookeeperPaths zookeeperPaths) { - this.leaderElectionDc = inactiveTopicsDetectionProperties.leaderElectionZookeeperDc(); - String leaderPath = zookeeperPaths.inactiveTopicsLeaderPath(); - Optional leaderCuratorFramework = - zookeeperClientManager.getClients().stream() - .filter(it -> it.getDatacenterName().equals(leaderElectionDc)) - .findFirst() - .map(ZookeeperClient::getCuratorFramework); - this.leaderLatch = leaderCuratorFramework.map(it -> new LeaderLatch(it, leaderPath)); - if (leaderLatch.isEmpty()) { - logLeaderZookeeperClientNotFound(leaderElectionDc); - } else { - logger.info("Found leader Zookeeper Client, leader latch created"); - } + InactiveTopicsDetectionJob job, InactiveTopicsDetectionLeader leader) { + this.leader = leader; this.job = job; } - @PostConstruct - public void startListeningForLeadership() { - leaderLatch.ifPresent( - it -> { - logger.info("Starting listening for leadership"); - try { - it.start(); - } catch (Exception e) { - throw new RuntimeException(e); - } - }); - } - @Scheduled(cron = "${detection.inactive-topics.cron}") public void run() { - if (leaderLatch.isPresent()) { - if (leaderLatch.get().hasLeadership()) { - logger.info("Inactive topics detection started"); - job.detectAndNotify(); - } else { - logger.info("Inactive topics detection not started - not a leader"); - } + if (leader.isLeader()) { + logger.info("Inactive topics detection started"); + job.detectAndNotify(); } else { - logLeaderZookeeperClientNotFound(leaderElectionDc); + logger.info("Inactive topics detection not started - not a leader"); } } - - private void logLeaderZookeeperClientNotFound(String dc) { - logger.error( - "Cannot run inactive topics detection - no zookeeper client for datacenter={}", dc); - } } From 972c4884a65fe12aab99ceecd33b3b2fc1c26a33 Mon Sep 17 00:00:00 2001 From: Michal Wisniewski Date: Wed, 13 Nov 2024 17:21:20 +0100 Subject: [PATCH 24/39] SKYEDEN-3234 | Change log message --- .../infrastructure/detection/InactiveTopicsDetectionLeader.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/InactiveTopicsDetectionLeader.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/InactiveTopicsDetectionLeader.java index f384e72e6d..bd43663249 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/InactiveTopicsDetectionLeader.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/InactiveTopicsDetectionLeader.java @@ -52,6 +52,6 @@ public boolean isLeader() { private void logLeaderZookeeperClientNotFound(String dc) { logger.error( - "Cannot run inactive topics detection - no zookeeper client for datacenter={}", dc); + "Cannot start listening for leadership - no zookeeper client for datacenter={}", dc); } } From caf1ca58ca3a3bf6180d7194201aed7bef4b430f Mon Sep 17 00:00:00 2001 From: Michal Wisniewski Date: Wed, 13 Nov 2024 17:33:21 +0100 Subject: [PATCH 25/39] SKYEDEN-3234 | Remove unnecessary annotation --- .../pl/allegro/tech/hermes/management/HermesManagement.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/HermesManagement.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/HermesManagement.java index f5298fec0d..3372816d20 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/HermesManagement.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/HermesManagement.java @@ -2,10 +2,8 @@ import org.springframework.boot.SpringApplication; import org.springframework.boot.autoconfigure.SpringBootApplication; -import org.springframework.scheduling.annotation.EnableScheduling; @SpringBootApplication -@EnableScheduling public class HermesManagement { public static void main(String[] args) { From 2227be7532d482f68f08f8fab53d334c540a3b75 Mon Sep 17 00:00:00 2001 From: Michal Wisniewski Date: Wed, 13 Nov 2024 18:29:23 +0100 Subject: [PATCH 26/39] SKYEDEN-3234 | Do not update notification timestamps when notification skipped --- .../detection/InactiveTopicsDetectionJob.java | 27 ++++++++-------- .../InactiveTopicsDetectionJobTest.groovy | 31 +++++++++++++++++++ 2 files changed, 44 insertions(+), 14 deletions(-) diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJob.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJob.java index 25b3198b61..4f92b8c251 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJob.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJob.java @@ -52,12 +52,7 @@ public void detectAndNotify() { List topicsToNotify = groupedByNeedOfNotification.getOrDefault(true, List.of()); List topicsToSkipNotification = groupedByNeedOfNotification.getOrDefault(false, List.of()); - - notify(topicsToNotify); - - Instant now = clock.instant(); - List notifiedTopics = - topicsToNotify.stream().map(topic -> topic.notificationSent(now)).toList(); + List notifiedTopics = notify(topicsToNotify); saveInactiveTopics(notifiedTopics, topicsToSkipNotification); } @@ -82,14 +77,18 @@ private Map groupByName(List inactiveTopic .collect(Collectors.toMap(InactiveTopic::qualifiedTopicName, v -> v, (v1, v2) -> v1)); } - private void notify(List inactiveTopics) { - if (!inactiveTopics.isEmpty()) { - if (notifier.isPresent()) { - logger.info("Notifying {} inactive topics", inactiveTopics.size()); - notifier.get().notify(inactiveTopics); - } else { - logger.info("Skipping notification of {} inactive topics", inactiveTopics.size()); - } + private List notify(List inactiveTopics) { + if (inactiveTopics.isEmpty()) { + logger.info("No inactive topics to notify"); + return inactiveTopics; + } else if (notifier.isPresent()) { + logger.info("Notifying {} inactive topics", inactiveTopics.size()); + notifier.get().notify(inactiveTopics); + Instant now = clock.instant(); + return inactiveTopics.stream().map(topic -> topic.notificationSent(now)).toList(); + } else { + logger.info("Skipping notification of {} inactive topics", inactiveTopics.size()); + return inactiveTopics; } } diff --git a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJobTest.groovy b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJobTest.groovy index 8cb97b96ff..a8b3b892cf 100644 --- a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJobTest.groovy +++ b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJobTest.groovy @@ -109,6 +109,37 @@ class InactiveTopicsDetectionJobTest extends Specification { 1 * inactiveTopicsStorageServiceMock.markAsInactive([]) } + def "should only save inactive topics when notifier is not provided"() { + given: + def job = new InactiveTopicsDetectionJob( + topicServiceMock, + inactiveTopicsStorageServiceMock, + detectionService, + Optional.empty(), + clockMock + ) + + and: + topicServiceMock.listQualifiedTopicNames() >> ["group.topic0"] + inactiveTopicsStorageServiceMock.getInactiveTopics() >> [] + + and: + def now = Instant.ofEpochMilli(1630600266987L) + clockMock.instant() >> now + + and: + def ago7days = now.minus(7, DAYS) + mockLastPublishedMessageTimestamp("group.topic0", ago7days) + + when: + job.detectAndNotify() + + then: "inactive topic is saved but notification timestamp is not added" + 1 * inactiveTopicsStorageServiceMock.markAsInactive([ + new InactiveTopic("group.topic0", ago7days.toEpochMilli(), [], false) + ]) + } + private def mockLastPublishedMessageTimestamp(String topicName, Instant instant) { metricsRepositoryMock.getLastPublishedMessageTimestamp(fromQualifiedName(topicName)) >> Optional.ofNullable(instant) } From 4d52b3edb45001006d2b89abdd02085c264b2a7c Mon Sep 17 00:00:00 2001 From: Michal Wisniewski Date: Thu, 14 Nov 2024 17:07:31 +0100 Subject: [PATCH 27/39] SKYEDEN-3234 | Add docs for inactive topics detection --- .../inactive-topics-detection.md | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 docs/docs/configuration/inactive-topics-detection.md diff --git a/docs/docs/configuration/inactive-topics-detection.md b/docs/docs/configuration/inactive-topics-detection.md new file mode 100644 index 0000000000..c1816bdf4f --- /dev/null +++ b/docs/docs/configuration/inactive-topics-detection.md @@ -0,0 +1,20 @@ +# Inactive Topics Detection + +Hermes Management provides an optional feature to detect inactive topics and +notify about them. This feature is **disabled by default**. You can enable it +and configure other options in the Hermes Management configuration. + + Option | Description | Default value +-------------------------------------------------------------|----------------------------------------------------------------------------|--------------- + detection.inactive-topics.enabled | enable inactive topics detection | false + detection.inactive-topics.inactivity-threshold | duration after which a topic is considered inactive and first notified | 60d + detection.inactive-topics.next-notification-threshold | duration after previous notification after which a topic is notified again | 14d + detection.inactive-topics.whitelisted-qualified-topic-names | list of qualified topic names that will not be notified event if inactive | [] + detection.inactive-topics.leader-election-zookeeper-dc | datacenter of Zookeeper used for leader election for the detection job | dc + detection.inactive-topics.cron | cron expression for the detection job | 0 0 8 * * * + +The detection job runs on a single instance of Hermes Management that is a +leader based on the leader election Zookeeper instance. + +To make notifying work, you need to provide an implementation of +`pl.allegro.tech.hermes.management.domain.detection.InactiveTopicsNotifier` From 15a22d11c19d42ef89496571fc024d62f0c7f252 Mon Sep 17 00:00:00 2001 From: Michal Wisniewski Date: Mon, 18 Nov 2024 10:35:36 +0100 Subject: [PATCH 28/39] SKYEDEN-3234 | Add notification result as return type of notifier --- .../detection/InactiveTopicsDetectionJob.java | 11 ++++- .../detection/InactiveTopicsNotifier.java | 2 +- .../domain/detection/NotificationResult.java | 16 ++++++++ .../InMemoryInactiveTopicsNotifier.groovy | 5 ++- .../InactiveTopicsDetectionJobTest.groovy | 41 +++++++++++++++++-- 5 files changed, 68 insertions(+), 7 deletions(-) create mode 100644 hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/NotificationResult.java diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJob.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJob.java index 4f92b8c251..e388159aed 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJob.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJob.java @@ -83,9 +83,16 @@ private List notify(List inactiveTopics) { return inactiveTopics; } else if (notifier.isPresent()) { logger.info("Notifying {} inactive topics", inactiveTopics.size()); - notifier.get().notify(inactiveTopics); + NotificationResult result = notifier.get().notify(inactiveTopics); Instant now = clock.instant(); - return inactiveTopics.stream().map(topic -> topic.notificationSent(now)).toList(); + + return inactiveTopics.stream() + .map( + topic -> + result.isSuccess(topic.qualifiedTopicName()) + ? topic.notificationSent(now) + : topic) + .toList(); } else { logger.info("Skipping notification of {} inactive topics", inactiveTopics.size()); return inactiveTopics; diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsNotifier.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsNotifier.java index 5747226e0c..7ebc180389 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsNotifier.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsNotifier.java @@ -3,5 +3,5 @@ import java.util.List; public interface InactiveTopicsNotifier { - void notify(List inactiveTopics); + NotificationResult notify(List inactiveTopics); } diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/NotificationResult.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/NotificationResult.java new file mode 100644 index 0000000000..e72f11733a --- /dev/null +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/NotificationResult.java @@ -0,0 +1,16 @@ +package pl.allegro.tech.hermes.management.domain.detection; + +import java.util.Map; +import java.util.Optional; + +public class NotificationResult { + private final Map qualifiedTopicNameToSuccess; + + public NotificationResult(Map qualifiedTopicNameToSuccess) { + this.qualifiedTopicNameToSuccess = qualifiedTopicNameToSuccess; + } + + public boolean isSuccess(String qualifiedTopicName) { + return Optional.ofNullable(qualifiedTopicNameToSuccess.get(qualifiedTopicName)).orElse(false); + } +} diff --git a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InMemoryInactiveTopicsNotifier.groovy b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InMemoryInactiveTopicsNotifier.groovy index efcccc9317..c91a3369fe 100644 --- a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InMemoryInactiveTopicsNotifier.groovy +++ b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InMemoryInactiveTopicsNotifier.groovy @@ -4,8 +4,11 @@ class InMemoryInactiveTopicsNotifier implements InactiveTopicsNotifier { private Set notifiedTopics = new HashSet<>(); @Override - void notify(List inactiveTopics) { + NotificationResult notify(List inactiveTopics) { notifiedTopics.addAll(inactiveTopics) + Map result = new HashMap<>(); + inactiveTopics.stream().forEach { result.put(it.qualifiedTopicName(), true) } + return new NotificationResult(result) } void reset() { diff --git a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJobTest.groovy b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJobTest.groovy index a8b3b892cf..3d45ecd7d7 100644 --- a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJobTest.groovy +++ b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJobTest.groovy @@ -109,7 +109,7 @@ class InactiveTopicsDetectionJobTest extends Specification { 1 * inactiveTopicsStorageServiceMock.markAsInactive([]) } - def "should only save inactive topics when notifier is not provided"() { + def "should not save new notification timestamp when notifier is not provided"() { given: def job = new InactiveTopicsDetectionJob( topicServiceMock, @@ -125,19 +125,54 @@ class InactiveTopicsDetectionJobTest extends Specification { and: def now = Instant.ofEpochMilli(1630600266987L) + def ago7days = now.minus(7, DAYS) clockMock.instant() >> now + mockLastPublishedMessageTimestamp("group.topic0", ago7days) + + when: + job.detectAndNotify() + + then: "inactive topic is saved but notification timestamp is not added" + 1 * inactiveTopicsStorageServiceMock.markAsInactive([ + new InactiveTopic("group.topic0", ago7days.toEpochMilli(), [], false) + ]) + } + + def "should not save new notification timestamp when notification did not succeed"() { + given: + def notifierMock = Mock(InactiveTopicsNotifier) and: + def job = new InactiveTopicsDetectionJob( + topicServiceMock, + inactiveTopicsStorageServiceMock, + detectionService, + Optional.of(notifierMock), + clockMock + ) + + and: + topicServiceMock.listQualifiedTopicNames() >> ["group.topic0", "group.topic1"] + inactiveTopicsStorageServiceMock.getInactiveTopics() >> [] + notifierMock.notify(_) >> new NotificationResult(["group.topic0": true, "group.topic1": false]) + + and: + def now = Instant.ofEpochMilli(1630600266987L) def ago7days = now.minus(7, DAYS) + clockMock.instant() >> now mockLastPublishedMessageTimestamp("group.topic0", ago7days) + mockLastPublishedMessageTimestamp("group.topic1", ago7days) when: job.detectAndNotify() - then: "inactive topic is saved but notification timestamp is not added" + then: 1 * inactiveTopicsStorageServiceMock.markAsInactive([ - new InactiveTopic("group.topic0", ago7days.toEpochMilli(), [], false) + new InactiveTopic("group.topic0", ago7days.toEpochMilli(), [now.toEpochMilli()], false), + new InactiveTopic("group.topic1", ago7days.toEpochMilli(), [], false), ]) + + } private def mockLastPublishedMessageTimestamp(String topicName, Instant instant) { From fd2b590d5e4d5919fdaf008c14e0937bfe0a1504 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Wi=C5=9Bniewski?= Date: Tue, 26 Nov 2024 10:47:12 +0100 Subject: [PATCH 29/39] SKYEDEN-3234 | Add owner info to be used by notifier --- .../detection/InactiveTopicWithOwner.java | 5 ++++ .../detection/InactiveTopicsDetectionJob.java | 27 +++++++++++++++---- .../detection/InactiveTopicsNotifier.java | 2 +- .../InMemoryInactiveTopicsNotifier.groovy | 5 ++-- .../InactiveTopicsDetectionJobTest.groovy | 25 ++++++++++------- 5 files changed, 46 insertions(+), 18 deletions(-) create mode 100644 hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicWithOwner.java diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicWithOwner.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicWithOwner.java new file mode 100644 index 0000000000..f0448f48fb --- /dev/null +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicWithOwner.java @@ -0,0 +1,5 @@ +package pl.allegro.tech.hermes.management.domain.detection; + +import pl.allegro.tech.hermes.api.OwnerId; + +public record InactiveTopicWithOwner(InactiveTopic topic, OwnerId ownerId) {} diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJob.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJob.java index e388159aed..4e3d750e4a 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJob.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJob.java @@ -10,6 +10,8 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.stereotype.Component; +import pl.allegro.tech.hermes.api.OwnerId; +import pl.allegro.tech.hermes.api.Topic; import pl.allegro.tech.hermes.api.TopicName; import pl.allegro.tech.hermes.management.domain.topic.TopicService; @@ -40,7 +42,8 @@ public InactiveTopicsDetectionJob( } public void detectAndNotify() { - List qualifiedTopicNames = topicService.listQualifiedTopicNames(); + List topics = topicService.getAllTopics(); + List qualifiedTopicNames = topics.stream().map(Topic::getQualifiedName).toList(); List historicalInactiveTopics = inactiveTopicsStorageService.getInactiveTopics(); List foundInactiveTopics = detectInactiveTopics(qualifiedTopicNames, historicalInactiveTopics); @@ -52,7 +55,7 @@ public void detectAndNotify() { List topicsToNotify = groupedByNeedOfNotification.getOrDefault(true, List.of()); List topicsToSkipNotification = groupedByNeedOfNotification.getOrDefault(false, List.of()); - List notifiedTopics = notify(topicsToNotify); + List notifiedTopics = notify(enrichWithOwner(topicsToNotify, topics)); saveInactiveTopics(notifiedTopics, topicsToSkipNotification); } @@ -77,16 +80,30 @@ private Map groupByName(List inactiveTopic .collect(Collectors.toMap(InactiveTopic::qualifiedTopicName, v -> v, (v1, v2) -> v1)); } - private List notify(List inactiveTopics) { + private List enrichWithOwner( + List inactiveTopics, List topics) { + Map ownerByTopicName = new HashMap<>(); + topics.forEach(topic -> ownerByTopicName.put(topic.getQualifiedName(), topic.getOwner())); + + return inactiveTopics.stream() + .map( + inactiveTopic -> + new InactiveTopicWithOwner( + inactiveTopic, ownerByTopicName.get(inactiveTopic.qualifiedTopicName()))) + .toList(); + } + + private List notify(List inactiveTopics) { if (inactiveTopics.isEmpty()) { logger.info("No inactive topics to notify"); - return inactiveTopics; + return List.of(); } else if (notifier.isPresent()) { logger.info("Notifying {} inactive topics", inactiveTopics.size()); NotificationResult result = notifier.get().notify(inactiveTopics); Instant now = clock.instant(); return inactiveTopics.stream() + .map(InactiveTopicWithOwner::topic) .map( topic -> result.isSuccess(topic.qualifiedTopicName()) @@ -95,7 +112,7 @@ private List notify(List inactiveTopics) { .toList(); } else { logger.info("Skipping notification of {} inactive topics", inactiveTopics.size()); - return inactiveTopics; + return inactiveTopics.stream().map(InactiveTopicWithOwner::topic).toList(); } } diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsNotifier.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsNotifier.java index 7ebc180389..ed9ad4e1de 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsNotifier.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsNotifier.java @@ -3,5 +3,5 @@ import java.util.List; public interface InactiveTopicsNotifier { - NotificationResult notify(List inactiveTopics); + NotificationResult notify(List inactiveTopics); } diff --git a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InMemoryInactiveTopicsNotifier.groovy b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InMemoryInactiveTopicsNotifier.groovy index c91a3369fe..0a72e734af 100644 --- a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InMemoryInactiveTopicsNotifier.groovy +++ b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InMemoryInactiveTopicsNotifier.groovy @@ -1,10 +1,11 @@ package pl.allegro.tech.hermes.management.domain.detection class InMemoryInactiveTopicsNotifier implements InactiveTopicsNotifier { - private Set notifiedTopics = new HashSet<>(); + private Set notifiedTopics = new HashSet<>() @Override - NotificationResult notify(List inactiveTopics) { + NotificationResult notify(List inactiveTopicsWithOwner) { + def inactiveTopics = inactiveTopicsWithOwner.stream().map(InactiveTopicWithOwner::topic).toList() notifiedTopics.addAll(inactiveTopics) Map result = new HashMap<>(); inactiveTopics.stream().forEach { result.put(it.qualifiedTopicName(), true) } diff --git a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJobTest.groovy b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJobTest.groovy index 3d45ecd7d7..83e0df800c 100644 --- a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJobTest.groovy +++ b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJobTest.groovy @@ -1,8 +1,9 @@ package pl.allegro.tech.hermes.management.domain.detection - +import pl.allegro.tech.hermes.api.Topic import pl.allegro.tech.hermes.management.config.detection.InactiveTopicsDetectionProperties import pl.allegro.tech.hermes.management.domain.topic.TopicService +import pl.allegro.tech.hermes.test.helper.builder.TopicBuilder import spock.lang.Specification import java.time.Clock @@ -50,12 +51,12 @@ class InactiveTopicsDetectionJobTest extends Specification { clockMock.instant() >> now and: "names of all topics" - topicServiceMock.listQualifiedTopicNames() >> [ - "group.topic0", - "group.topic1", - "group.topic2", - "group.topic3", - "group.topic4", + topicServiceMock.getAllTopics() >> [ + topic("group.topic0"), + topic("group.topic1"), + topic("group.topic2"), + topic("group.topic3"), + topic("group.topic4"), ] and: "historically saved inactive topics" @@ -91,7 +92,7 @@ class InactiveTopicsDetectionJobTest extends Specification { def "should not notify if there are no inactive topics"() { given: - topicServiceMock.listQualifiedTopicNames() >> ["group.topic0"] + topicServiceMock.getAllTopics() >> [topic("group.topic0")] inactiveTopicsStorageServiceMock.getInactiveTopics() >> [] and: @@ -120,7 +121,7 @@ class InactiveTopicsDetectionJobTest extends Specification { ) and: - topicServiceMock.listQualifiedTopicNames() >> ["group.topic0"] + topicServiceMock.getAllTopics() >> [topic("group.topic0")] inactiveTopicsStorageServiceMock.getInactiveTopics() >> [] and: @@ -152,7 +153,7 @@ class InactiveTopicsDetectionJobTest extends Specification { ) and: - topicServiceMock.listQualifiedTopicNames() >> ["group.topic0", "group.topic1"] + topicServiceMock.getAllTopics() >> [topic("group.topic0"), topic("group.topic1")] inactiveTopicsStorageServiceMock.getInactiveTopics() >> [] notifierMock.notify(_) >> new NotificationResult(["group.topic0": true, "group.topic1": false]) @@ -178,4 +179,8 @@ class InactiveTopicsDetectionJobTest extends Specification { private def mockLastPublishedMessageTimestamp(String topicName, Instant instant) { metricsRepositoryMock.getLastPublishedMessageTimestamp(fromQualifiedName(topicName)) >> Optional.ofNullable(instant) } + + private static Topic topic(String name) { + return TopicBuilder.topic(name).build(); + } } From 9841f44b691392247754cbd6cf35483ae569ee30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Wi=C5=9Bniewski?= Date: Tue, 26 Nov 2024 11:35:58 +0100 Subject: [PATCH 30/39] SKYEDEN-3234 | Limit number of notification timestamps in history --- .../InactiveTopicsDetectionProperties.java | 3 +- .../domain/detection/InactiveTopic.java | 15 +++++- .../detection/InactiveTopicsDetectionJob.java | 11 ++++- .../src/main/resources/application.yaml | 1 + .../domain/detection/InactiveTopicTest.groovy | 49 +++++++++++++++++++ .../InactiveTopicsDetectionJobTest.groovy | 8 ++- .../InactiveTopicsDetectionServiceTest.groovy | 3 +- 7 files changed, 83 insertions(+), 7 deletions(-) create mode 100644 hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicTest.groovy diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/detection/InactiveTopicsDetectionProperties.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/detection/InactiveTopicsDetectionProperties.java index 680628d153..2751cfcbb8 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/detection/InactiveTopicsDetectionProperties.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/detection/InactiveTopicsDetectionProperties.java @@ -9,4 +9,5 @@ public record InactiveTopicsDetectionProperties( Duration inactivityThreshold, Duration nextNotificationThreshold, Set whitelistedQualifiedTopicNames, - String leaderElectionZookeeperDc) {} + String leaderElectionZookeeperDc, + int notificationsHistoryLimit) {} diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopic.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopic.java index 5b2e407f41..bec23c6359 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopic.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopic.java @@ -18,6 +18,19 @@ InactiveTopic notificationSent(Instant timestamp) { this.qualifiedTopicName, this.lastPublishedMessageTimestampMs, newNotificationTimestampsMs, - whitelisted); + this.whitelisted); + } + + InactiveTopic limitNotificationsHistory(int limit) { + List newNotificationTimestampsMs = + notificationTimestampsMs.stream() + .sorted((a, b) -> Long.compare(b, a)) + .limit(limit) + .toList(); + return new InactiveTopic( + this.qualifiedTopicName, + this.lastPublishedMessageTimestampMs, + newNotificationTimestampsMs, + this.whitelisted); } } diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJob.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJob.java index 4e3d750e4a..545cbb31ff 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJob.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJob.java @@ -13,6 +13,7 @@ import pl.allegro.tech.hermes.api.OwnerId; import pl.allegro.tech.hermes.api.Topic; import pl.allegro.tech.hermes.api.TopicName; +import pl.allegro.tech.hermes.management.config.detection.InactiveTopicsDetectionProperties; import pl.allegro.tech.hermes.management.domain.topic.TopicService; @Component @@ -21,6 +22,7 @@ public class InactiveTopicsDetectionJob { private final InactiveTopicsStorageService inactiveTopicsStorageService; private final InactiveTopicsDetectionService inactiveTopicsDetectionService; private final Optional notifier; + private final InactiveTopicsDetectionProperties properties; private final Clock clock; private static final Logger logger = LoggerFactory.getLogger(InactiveTopicsDetectionJob.class); @@ -30,10 +32,12 @@ public InactiveTopicsDetectionJob( InactiveTopicsStorageService inactiveTopicsStorageService, InactiveTopicsDetectionService inactiveTopicsDetectionService, Optional notifier, + InactiveTopicsDetectionProperties properties, Clock clock) { this.topicService = topicService; this.inactiveTopicsStorageService = inactiveTopicsStorageService; this.inactiveTopicsDetectionService = inactiveTopicsDetectionService; + this.properties = properties; this.clock = clock; if (notifier.isEmpty()) { logger.info("Inactive topics notifier bean is absent"); @@ -118,7 +122,10 @@ private List notify(List inactiveTopics) private void saveInactiveTopics( List notifiedTopics, List skippedNotificationTopics) { - inactiveTopicsStorageService.markAsInactive( - Stream.concat(notifiedTopics.stream(), skippedNotificationTopics.stream()).toList()); + List topicsToSave = + Stream.concat(notifiedTopics.stream(), skippedNotificationTopics.stream()) + .map(topic -> topic.limitNotificationsHistory(properties.notificationsHistoryLimit())) + .toList(); + inactiveTopicsStorageService.markAsInactive(topicsToSave); } } diff --git a/hermes-management/src/main/resources/application.yaml b/hermes-management/src/main/resources/application.yaml index 43d1cdfb84..a01e7d47c2 100644 --- a/hermes-management/src/main/resources/application.yaml +++ b/hermes-management/src/main/resources/application.yaml @@ -83,3 +83,4 @@ detection: whitelisted-qualified-topic-names: [] leader-election-zookeeper-dc: dc cron: "0 0 8 * * *" + notifications-history-limit: 5 diff --git a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicTest.groovy b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicTest.groovy new file mode 100644 index 0000000000..4190cbd01e --- /dev/null +++ b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicTest.groovy @@ -0,0 +1,49 @@ +package pl.allegro.tech.hermes.management.domain.detection + +import spock.lang.Specification + +import java.time.Instant + +class InactiveTopicTest extends Specification { + def "should update notification timestamps"() { + given: + def inactiveTopic = new InactiveTopic( + "group.topic", + 1732616853320L, + [1732616853325L], + false + ) + + when: + def result = inactiveTopic.notificationSent(Instant.ofEpochMilli(1732616853330L)) + + then: + result == new InactiveTopic( + "group.topic", + 1732616853320L, + [1732616853325L, 1732616853330L], + false + ) + } + + def "should limit notification timestamps size and leave only latest"() { + given: + def inactiveTopic = new InactiveTopic( + "group.topic", + 1732616853320L, + [1732616853330L, 1732616853325L, 1732616853335L], + false + ) + + when: + def result = inactiveTopic.limitNotificationsHistory(2) + + then: + result == new InactiveTopic( + "group.topic", + 1732616853320L, + [1732616853335L, 1732616853330L], + false + ) + } +} diff --git a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJobTest.groovy b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJobTest.groovy index 83e0df800c..7fa03107cc 100644 --- a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJobTest.groovy +++ b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJobTest.groovy @@ -25,7 +25,8 @@ class InactiveTopicsDetectionJobTest extends Specification { Duration.ofDays(7), Duration.ofDays(14), ["group.topic3"] as Set, - "dc" + "dc", + 5 ) InactiveTopicsDetectionService detectionService = new InactiveTopicsDetectionService( @@ -39,6 +40,7 @@ class InactiveTopicsDetectionJobTest extends Specification { inactiveTopicsStorageServiceMock, detectionService, Optional.of(inactiveTopicsNotifier), + inactiveTopicsDetectionProperties, clockMock ) @@ -85,7 +87,7 @@ class InactiveTopicsDetectionJobTest extends Specification { and: "saved are all inactive topics with updated notification timestamps" 1 * inactiveTopicsStorageServiceMock.markAsInactive([ new InactiveTopic("group.topic1", ago7days.toEpochMilli(), [now.toEpochMilli()], false), - new InactiveTopic("group.topic4", ago21days.toEpochMilli(), [ago14days.toEpochMilli(), now.toEpochMilli()], false), + new InactiveTopic("group.topic4", ago21days.toEpochMilli(), [now.toEpochMilli(), ago14days.toEpochMilli()], false), new InactiveTopic("group.topic3", ago7days.toEpochMilli(), [], true) ]) } @@ -117,6 +119,7 @@ class InactiveTopicsDetectionJobTest extends Specification { inactiveTopicsStorageServiceMock, detectionService, Optional.empty(), + inactiveTopicsDetectionProperties, clockMock ) @@ -149,6 +152,7 @@ class InactiveTopicsDetectionJobTest extends Specification { inactiveTopicsStorageServiceMock, detectionService, Optional.of(notifierMock), + inactiveTopicsDetectionProperties, clockMock ) diff --git a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionServiceTest.groovy b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionServiceTest.groovy index 207fda35fd..9a5e896081 100644 --- a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionServiceTest.groovy +++ b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionServiceTest.groovy @@ -21,7 +21,8 @@ class InactiveTopicsDetectionServiceTest extends Specification { Duration.ofDays(INACTIVITY_THRESHOLD), Duration.ofDays(NEXT_NOTIFICATION_THRESHOLD), [WHITELISTED_TOPIC_NAME] as Set, - "dc" + "dc", + 5 ) private def metricsRepositoryMock = Mock(LastPublishedMessageMetricsRepository) From ca4216f95bf74847eda642b666f421c130a58daf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Wi=C5=9Bniewski?= Date: Tue, 26 Nov 2024 11:38:54 +0100 Subject: [PATCH 31/39] SKYEDEN-3234 | Update docs --- docs/docs/configuration/inactive-topics-detection.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/docs/configuration/inactive-topics-detection.md b/docs/docs/configuration/inactive-topics-detection.md index c1816bdf4f..4df54961c6 100644 --- a/docs/docs/configuration/inactive-topics-detection.md +++ b/docs/docs/configuration/inactive-topics-detection.md @@ -12,6 +12,7 @@ and configure other options in the Hermes Management configuration. detection.inactive-topics.whitelisted-qualified-topic-names | list of qualified topic names that will not be notified event if inactive | [] detection.inactive-topics.leader-election-zookeeper-dc | datacenter of Zookeeper used for leader election for the detection job | dc detection.inactive-topics.cron | cron expression for the detection job | 0 0 8 * * * + detection.inactive-topics.notifications-history-limit | how many notification timestamps will be kept in history | 5 The detection job runs on a single instance of Hermes Management that is a leader based on the leader election Zookeeper instance. From 81943219f0c5d41fcfdcb0ac55497ad7d93370a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Wi=C5=9Bniewski?= Date: Wed, 27 Nov 2024 15:46:09 +0100 Subject: [PATCH 32/39] SKYEDEN-3234 | Specify shorter names to be saved in json --- .../hermes/management/domain/detection/InactiveTopic.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopic.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopic.java index bec23c6359..6c0962e3af 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopic.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopic.java @@ -6,10 +6,10 @@ import java.util.List; public record InactiveTopic( - @JsonProperty String qualifiedTopicName, - @JsonProperty long lastPublishedMessageTimestampMs, - @JsonProperty List notificationTimestampsMs, - @JsonProperty boolean whitelisted) { + @JsonProperty("topic") String qualifiedTopicName, + @JsonProperty("lastPublishedTsMs") long lastPublishedMessageTimestampMs, + @JsonProperty("notificationTsMs") List notificationTimestampsMs, + @JsonProperty("whitelisted") boolean whitelisted) { InactiveTopic notificationSent(Instant timestamp) { List newNotificationTimestampsMs = new ArrayList<>(notificationTimestampsMs); From 894f8bdb7b3ba2310678a5d91f50431261818fea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcin=20Bobi=C5=84ski?= <49727204+MarcinBobinski@users.noreply.github.com> Date: Tue, 3 Dec 2024 14:02:36 +0100 Subject: [PATCH 33/39] SKYEDEN-3271 |hermes management leader (#1934) * SKYEDEN-3271 |hermes management leader * SKYEDEN-3271 | cr changes --- .../configuration/inactive-topics-detection.md | 5 ++++- .../zookeeper/ZookeeperPaths.java | 6 ++++-- .../config/ManagementConfiguration.java | 14 +++++++++++++- .../InactiveTopicsDetectionConfig.java | 18 ++---------------- .../InactiveTopicsDetectionScheduler.java | 5 +++-- .../ManagementLeadership.java} | 15 +++++++-------- .../src/main/resources/application.yaml | 3 ++- 7 files changed, 35 insertions(+), 31 deletions(-) rename hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/{detection/InactiveTopicsDetectionLeader.java => leader/ManagementLeadership.java} (74%) diff --git a/docs/docs/configuration/inactive-topics-detection.md b/docs/docs/configuration/inactive-topics-detection.md index 4df54961c6..16bc3e6478 100644 --- a/docs/docs/configuration/inactive-topics-detection.md +++ b/docs/docs/configuration/inactive-topics-detection.md @@ -10,12 +10,15 @@ and configure other options in the Hermes Management configuration. detection.inactive-topics.inactivity-threshold | duration after which a topic is considered inactive and first notified | 60d detection.inactive-topics.next-notification-threshold | duration after previous notification after which a topic is notified again | 14d detection.inactive-topics.whitelisted-qualified-topic-names | list of qualified topic names that will not be notified event if inactive | [] - detection.inactive-topics.leader-election-zookeeper-dc | datacenter of Zookeeper used for leader election for the detection job | dc detection.inactive-topics.cron | cron expression for the detection job | 0 0 8 * * * detection.inactive-topics.notifications-history-limit | how many notification timestamps will be kept in history | 5 The detection job runs on a single instance of Hermes Management that is a leader based on the leader election Zookeeper instance. + Option | Description | Default Value +------------------------------------|-----------------------------------------------------------------------------|--------------- + management.leadership.zookeeper-dc | Specifies the datacenter of the Zookeeper instance used for leader election | dc + To make notifying work, you need to provide an implementation of `pl.allegro.tech.hermes.management.domain.detection.InactiveTopicsNotifier` diff --git a/hermes-common/src/main/java/pl/allegro/tech/hermes/infrastructure/zookeeper/ZookeeperPaths.java b/hermes-common/src/main/java/pl/allegro/tech/hermes/infrastructure/zookeeper/ZookeeperPaths.java index 48cb079f46..4279d3e140 100644 --- a/hermes-common/src/main/java/pl/allegro/tech/hermes/infrastructure/zookeeper/ZookeeperPaths.java +++ b/hermes-common/src/main/java/pl/allegro/tech/hermes/infrastructure/zookeeper/ZookeeperPaths.java @@ -31,6 +31,8 @@ public class ZookeeperPaths { public static final String OFFLINE_RETRANSMISSION_PATH = "offline-retransmission"; public static final String OFFLINE_RETRANSMISSION_TASKS_PATH = "tasks"; public static final String INACTIVE_TOPICS_PATH = "inactive-topics"; + public static final String MANAGEMENT_PATH = "management"; + public static final String MANAGEMENT_PATH_LEADER = "leader"; private final String basePath; @@ -187,8 +189,8 @@ public String inactiveTopicsPath() { return Joiner.on(URL_SEPARATOR).join(basePath, INACTIVE_TOPICS_PATH); } - public String inactiveTopicsLeaderPath() { - return Joiner.on(URL_SEPARATOR).join(inactiveTopicsPath(), "leader"); + public String managementLeaderPath() { + return Joiner.on(URL_SEPARATOR).join(MANAGEMENT_PATH, MANAGEMENT_PATH_LEADER); } public String join(String... parts) { diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/ManagementConfiguration.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/ManagementConfiguration.java index 93139d4803..736cb01a15 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/ManagementConfiguration.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/ManagementConfiguration.java @@ -10,6 +10,7 @@ import io.micrometer.core.instrument.MeterRegistry; import java.time.Clock; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Value; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.context.annotation.Bean; @@ -19,8 +20,11 @@ import pl.allegro.tech.hermes.common.metric.MetricsFacade; import pl.allegro.tech.hermes.common.util.InetAddressInstanceIdResolver; import pl.allegro.tech.hermes.common.util.InstanceIdResolver; +import pl.allegro.tech.hermes.infrastructure.zookeeper.ZookeeperPaths; import pl.allegro.tech.hermes.management.domain.subscription.SubscriptionLagSource; +import pl.allegro.tech.hermes.management.infrastructure.leader.ManagementLeadership; import pl.allegro.tech.hermes.management.infrastructure.metrics.NoOpSubscriptionLagSource; +import pl.allegro.tech.hermes.management.infrastructure.zookeeper.ZookeeperClientManager; import pl.allegro.tech.hermes.metrics.PathsCompiler; @Configuration @@ -29,7 +33,7 @@ HttpClientProperties.class, ConsistencyCheckerProperties.class, PrometheusProperties.class, - MicrometerRegistryProperties.class + MicrometerRegistryProperties.class, }) public class ManagementConfiguration { @@ -85,4 +89,12 @@ public SubscriptionLagSource consumerLagSource() { public Clock clock() { return new ClockFactory().provide(); } + + @Bean + ManagementLeadership managementLeadership( + ZookeeperClientManager zookeeperClientManager, + @Value("${management.leadership.zookeeper-dc}") String leaderElectionDc, + ZookeeperPaths zookeeperPaths) { + return new ManagementLeadership(zookeeperClientManager, leaderElectionDc, zookeeperPaths); + } } diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/detection/InactiveTopicsDetectionConfig.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/detection/InactiveTopicsDetectionConfig.java index 491fa1322c..be957928dd 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/detection/InactiveTopicsDetectionConfig.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/detection/InactiveTopicsDetectionConfig.java @@ -5,35 +5,21 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.scheduling.annotation.EnableScheduling; -import pl.allegro.tech.hermes.infrastructure.zookeeper.ZookeeperPaths; import pl.allegro.tech.hermes.management.domain.detection.InactiveTopicsDetectionJob; -import pl.allegro.tech.hermes.management.infrastructure.detection.InactiveTopicsDetectionLeader; import pl.allegro.tech.hermes.management.infrastructure.detection.InactiveTopicsDetectionScheduler; -import pl.allegro.tech.hermes.management.infrastructure.zookeeper.ZookeeperClientManager; +import pl.allegro.tech.hermes.management.infrastructure.leader.ManagementLeadership; @Configuration @EnableConfigurationProperties(InactiveTopicsDetectionProperties.class) @EnableScheduling public class InactiveTopicsDetectionConfig { - @ConditionalOnProperty( - prefix = "detection.inactive-topics", - value = "enabled", - havingValue = "true") - @Bean - InactiveTopicsDetectionLeader inactiveTopicsDetectionLeader( - ZookeeperClientManager zookeeperClientManager, - InactiveTopicsDetectionProperties properties, - ZookeeperPaths zookeeperPaths) { - return new InactiveTopicsDetectionLeader(zookeeperClientManager, properties, zookeeperPaths); - } - @ConditionalOnProperty( prefix = "detection.inactive-topics", value = "enabled", havingValue = "true") @Bean InactiveTopicsDetectionScheduler inactiveTopicsDetectionScheduler( - InactiveTopicsDetectionJob job, InactiveTopicsDetectionLeader leader) { + InactiveTopicsDetectionJob job, ManagementLeadership leader) { return new InactiveTopicsDetectionScheduler(job, leader); } } diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/InactiveTopicsDetectionScheduler.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/InactiveTopicsDetectionScheduler.java index f7daa1ee77..7ac7756093 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/InactiveTopicsDetectionScheduler.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/InactiveTopicsDetectionScheduler.java @@ -4,16 +4,17 @@ import org.slf4j.LoggerFactory; import org.springframework.scheduling.annotation.Scheduled; import pl.allegro.tech.hermes.management.domain.detection.InactiveTopicsDetectionJob; +import pl.allegro.tech.hermes.management.infrastructure.leader.ManagementLeadership; public class InactiveTopicsDetectionScheduler { private final InactiveTopicsDetectionJob job; - private final InactiveTopicsDetectionLeader leader; + private final ManagementLeadership leader; private static final Logger logger = LoggerFactory.getLogger(InactiveTopicsDetectionScheduler.class); public InactiveTopicsDetectionScheduler( - InactiveTopicsDetectionJob job, InactiveTopicsDetectionLeader leader) { + InactiveTopicsDetectionJob job, ManagementLeadership leader) { this.leader = leader; this.job = job; } diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/InactiveTopicsDetectionLeader.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/leader/ManagementLeadership.java similarity index 74% rename from hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/InactiveTopicsDetectionLeader.java rename to hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/leader/ManagementLeadership.java index bd43663249..fb539d3bd3 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/detection/InactiveTopicsDetectionLeader.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/infrastructure/leader/ManagementLeadership.java @@ -1,4 +1,4 @@ -package pl.allegro.tech.hermes.management.infrastructure.detection; +package pl.allegro.tech.hermes.management.infrastructure.leader; import jakarta.annotation.PostConstruct; import java.util.Optional; @@ -7,28 +7,27 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import pl.allegro.tech.hermes.infrastructure.zookeeper.ZookeeperPaths; -import pl.allegro.tech.hermes.management.config.detection.InactiveTopicsDetectionProperties; import pl.allegro.tech.hermes.management.infrastructure.zookeeper.ZookeeperClient; import pl.allegro.tech.hermes.management.infrastructure.zookeeper.ZookeeperClientManager; -public class InactiveTopicsDetectionLeader { +public class ManagementLeadership { private final String leaderElectionDc; private final Optional leaderLatch; - private static final Logger logger = LoggerFactory.getLogger(InactiveTopicsDetectionLeader.class); + private static final Logger logger = LoggerFactory.getLogger(ManagementLeadership.class); - public InactiveTopicsDetectionLeader( + public ManagementLeadership( ZookeeperClientManager zookeeperClientManager, - InactiveTopicsDetectionProperties inactiveTopicsDetectionProperties, + String leaderElectionDc, ZookeeperPaths zookeeperPaths) { - this.leaderElectionDc = inactiveTopicsDetectionProperties.leaderElectionZookeeperDc(); + this.leaderElectionDc = leaderElectionDc; Optional leaderCuratorFramework = zookeeperClientManager.getClients().stream() .filter(it -> it.getDatacenterName().equals(leaderElectionDc)) .findFirst() .map(ZookeeperClient::getCuratorFramework); - String leaderPath = zookeeperPaths.inactiveTopicsLeaderPath(); + String leaderPath = zookeeperPaths.managementLeaderPath(); this.leaderLatch = leaderCuratorFramework.map(it -> new LeaderLatch(it, leaderPath)); } diff --git a/hermes-management/src/main/resources/application.yaml b/hermes-management/src/main/resources/application.yaml index a01e7d47c2..feb61a2ce7 100644 --- a/hermes-management/src/main/resources/application.yaml +++ b/hermes-management/src/main/resources/application.yaml @@ -39,6 +39,8 @@ management: health: periodSeconds: 30 enabled: true + leadership: + zookeeper-dc: dc audit: isLoggingAuditEnabled: false @@ -81,6 +83,5 @@ detection: inactivity-threshold: 60d next-notification-threshold: 14d whitelisted-qualified-topic-names: [] - leader-election-zookeeper-dc: dc cron: "0 0 8 * * *" notifications-history-limit: 5 From 7726994bad3f1f441881790159126ea7b939e2d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Wi=C5=9Bniewski?= Date: Tue, 3 Dec 2024 14:16:48 +0100 Subject: [PATCH 34/39] SKYEDEN-3234 | Remove unused property --- .../config/detection/InactiveTopicsDetectionProperties.java | 1 - .../domain/detection/InactiveTopicsDetectionJobTest.groovy | 1 - .../domain/detection/InactiveTopicsDetectionServiceTest.groovy | 1 - 3 files changed, 3 deletions(-) diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/detection/InactiveTopicsDetectionProperties.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/detection/InactiveTopicsDetectionProperties.java index 2751cfcbb8..043da02b6e 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/detection/InactiveTopicsDetectionProperties.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/detection/InactiveTopicsDetectionProperties.java @@ -9,5 +9,4 @@ public record InactiveTopicsDetectionProperties( Duration inactivityThreshold, Duration nextNotificationThreshold, Set whitelistedQualifiedTopicNames, - String leaderElectionZookeeperDc, int notificationsHistoryLimit) {} diff --git a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJobTest.groovy b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJobTest.groovy index 7fa03107cc..605f4efb71 100644 --- a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJobTest.groovy +++ b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJobTest.groovy @@ -25,7 +25,6 @@ class InactiveTopicsDetectionJobTest extends Specification { Duration.ofDays(7), Duration.ofDays(14), ["group.topic3"] as Set, - "dc", 5 ) diff --git a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionServiceTest.groovy b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionServiceTest.groovy index 9a5e896081..5957e8c502 100644 --- a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionServiceTest.groovy +++ b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionServiceTest.groovy @@ -21,7 +21,6 @@ class InactiveTopicsDetectionServiceTest extends Specification { Duration.ofDays(INACTIVITY_THRESHOLD), Duration.ofDays(NEXT_NOTIFICATION_THRESHOLD), [WHITELISTED_TOPIC_NAME] as Set, - "dc", 5 ) From 12c809e1705f2907609d49c51ca3c0e2d8b5923c Mon Sep 17 00:00:00 2001 From: Marcin Bobinski Date: Tue, 3 Dec 2024 14:27:48 +0100 Subject: [PATCH 35/39] SKYEDEN-3271 | fix leader path --- .../tech/hermes/infrastructure/zookeeper/ZookeeperPaths.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hermes-common/src/main/java/pl/allegro/tech/hermes/infrastructure/zookeeper/ZookeeperPaths.java b/hermes-common/src/main/java/pl/allegro/tech/hermes/infrastructure/zookeeper/ZookeeperPaths.java index 4279d3e140..3e5d69567e 100644 --- a/hermes-common/src/main/java/pl/allegro/tech/hermes/infrastructure/zookeeper/ZookeeperPaths.java +++ b/hermes-common/src/main/java/pl/allegro/tech/hermes/infrastructure/zookeeper/ZookeeperPaths.java @@ -190,7 +190,7 @@ public String inactiveTopicsPath() { } public String managementLeaderPath() { - return Joiner.on(URL_SEPARATOR).join(MANAGEMENT_PATH, MANAGEMENT_PATH_LEADER); + return Joiner.on(URL_SEPARATOR).join(basePath, MANAGEMENT_PATH, MANAGEMENT_PATH_LEADER); } public String join(String... parts) { From cf17940838868a524ecee000dc2a88fe90b0c388 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Wi=C5=9Bniewski?= Date: Tue, 3 Dec 2024 16:16:30 +0100 Subject: [PATCH 36/39] Revert "Merge branch 'master' into SKYEDEN-3234-detect-unused-topics" This reverts commit 9545569e141b8bf0c154a87e29b0d04935ceeb53, reversing changes made to 81943219f0c5d41fcfdcb0ac55497ad7d93370a0. --- build.gradle | 2 +- .../di/factories/PrometheusMeterRegistryFactory.java | 4 ++-- .../tech/hermes/consumers/config/CommonConfiguration.java | 4 ++-- .../hermes/consumers/config/PrometheusConfigAdapter.java | 2 +- .../tech/hermes/consumers/config/ServerConfiguration.java | 2 +- .../tech/hermes/consumers/server/ConsumerHttpServer.java | 2 +- .../tech/hermes/frontend/config/CommonConfiguration.java | 4 ++-- .../frontend/config/FrontendServerConfiguration.java | 2 +- .../hermes/frontend/config/PrometheusConfigAdapter.java | 2 +- .../allegro/tech/hermes/frontend/server/HermesServer.java | 2 +- .../hermes/frontend/server/PrometheusMetricsHandler.java | 2 +- .../hermes/management/config/PrometheusConfigAdapter.java | 2 +- .../hermes/management/config/PrometheusConfiguration.java | 8 ++++---- 13 files changed, 19 insertions(+), 19 deletions(-) diff --git a/build.gradle b/build.gradle index 4a1159a0ac..ca229379c3 100644 --- a/build.gradle +++ b/build.gradle @@ -57,7 +57,7 @@ allprojects { jetty : '12.0.8', curator : '5.4.0', dropwizard_metrics: '4.2.25', - micrometer_metrics: '1.13.0', + micrometer_metrics: '1.12.5', wiremock : '3.9.0', spock : '2.4-M4-groovy-4.0', groovy : '4.0.21', diff --git a/hermes-common/src/main/java/pl/allegro/tech/hermes/common/di/factories/PrometheusMeterRegistryFactory.java b/hermes-common/src/main/java/pl/allegro/tech/hermes/common/di/factories/PrometheusMeterRegistryFactory.java index d5b5915845..7d46f2bba2 100644 --- a/hermes-common/src/main/java/pl/allegro/tech/hermes/common/di/factories/PrometheusMeterRegistryFactory.java +++ b/hermes-common/src/main/java/pl/allegro/tech/hermes/common/di/factories/PrometheusMeterRegistryFactory.java @@ -7,8 +7,8 @@ import io.micrometer.core.instrument.binder.jvm.JvmThreadMetrics; import io.micrometer.core.instrument.config.MeterFilter; import io.micrometer.core.instrument.distribution.DistributionStatisticConfig; -import io.micrometer.prometheusmetrics.PrometheusConfig; -import io.micrometer.prometheusmetrics.PrometheusMeterRegistry; +import io.micrometer.prometheus.PrometheusConfig; +import io.micrometer.prometheus.PrometheusMeterRegistry; import java.util.concurrent.TimeUnit; import pl.allegro.tech.hermes.common.metric.counter.CounterStorage; import pl.allegro.tech.hermes.common.metric.counter.zookeeper.ZookeeperCounterReporter; diff --git a/hermes-consumers/src/main/java/pl/allegro/tech/hermes/consumers/config/CommonConfiguration.java b/hermes-consumers/src/main/java/pl/allegro/tech/hermes/consumers/config/CommonConfiguration.java index cd762c659c..23a7c78965 100644 --- a/hermes-consumers/src/main/java/pl/allegro/tech/hermes/consumers/config/CommonConfiguration.java +++ b/hermes-consumers/src/main/java/pl/allegro/tech/hermes/consumers/config/CommonConfiguration.java @@ -6,8 +6,8 @@ import com.fasterxml.jackson.databind.ObjectMapper; import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.composite.CompositeMeterRegistry; -import io.micrometer.prometheusmetrics.PrometheusConfig; -import io.micrometer.prometheusmetrics.PrometheusMeterRegistry; +import io.micrometer.prometheus.PrometheusConfig; +import io.micrometer.prometheus.PrometheusMeterRegistry; import jakarta.inject.Named; import java.time.Clock; import java.util.Arrays; diff --git a/hermes-consumers/src/main/java/pl/allegro/tech/hermes/consumers/config/PrometheusConfigAdapter.java b/hermes-consumers/src/main/java/pl/allegro/tech/hermes/consumers/config/PrometheusConfigAdapter.java index 3bac713413..43c1e2d9e8 100644 --- a/hermes-consumers/src/main/java/pl/allegro/tech/hermes/consumers/config/PrometheusConfigAdapter.java +++ b/hermes-consumers/src/main/java/pl/allegro/tech/hermes/consumers/config/PrometheusConfigAdapter.java @@ -1,6 +1,6 @@ package pl.allegro.tech.hermes.consumers.config; -import io.micrometer.prometheusmetrics.PrometheusConfig; +import io.micrometer.prometheus.PrometheusConfig; import java.time.Duration; public class PrometheusConfigAdapter implements PrometheusConfig { diff --git a/hermes-consumers/src/main/java/pl/allegro/tech/hermes/consumers/config/ServerConfiguration.java b/hermes-consumers/src/main/java/pl/allegro/tech/hermes/consumers/config/ServerConfiguration.java index af6b03bb35..8e2bfbfc0c 100644 --- a/hermes-consumers/src/main/java/pl/allegro/tech/hermes/consumers/config/ServerConfiguration.java +++ b/hermes-consumers/src/main/java/pl/allegro/tech/hermes/consumers/config/ServerConfiguration.java @@ -1,7 +1,7 @@ package pl.allegro.tech.hermes.consumers.config; import com.fasterxml.jackson.databind.ObjectMapper; -import io.micrometer.prometheusmetrics.PrometheusMeterRegistry; +import io.micrometer.prometheus.PrometheusMeterRegistry; import java.io.IOException; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.context.annotation.Bean; diff --git a/hermes-consumers/src/main/java/pl/allegro/tech/hermes/consumers/server/ConsumerHttpServer.java b/hermes-consumers/src/main/java/pl/allegro/tech/hermes/consumers/server/ConsumerHttpServer.java index 9fac0b659b..722ecbc711 100644 --- a/hermes-consumers/src/main/java/pl/allegro/tech/hermes/consumers/server/ConsumerHttpServer.java +++ b/hermes-consumers/src/main/java/pl/allegro/tech/hermes/consumers/server/ConsumerHttpServer.java @@ -7,7 +7,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.sun.net.httpserver.HttpExchange; import com.sun.net.httpserver.HttpServer; -import io.micrometer.prometheusmetrics.PrometheusMeterRegistry; +import io.micrometer.prometheus.PrometheusMeterRegistry; import java.io.IOException; import java.io.OutputStream; import java.net.InetSocketAddress; diff --git a/hermes-frontend/src/main/java/pl/allegro/tech/hermes/frontend/config/CommonConfiguration.java b/hermes-frontend/src/main/java/pl/allegro/tech/hermes/frontend/config/CommonConfiguration.java index 705fee2062..dfc71aae5b 100644 --- a/hermes-frontend/src/main/java/pl/allegro/tech/hermes/frontend/config/CommonConfiguration.java +++ b/hermes-frontend/src/main/java/pl/allegro/tech/hermes/frontend/config/CommonConfiguration.java @@ -5,8 +5,8 @@ import com.fasterxml.jackson.databind.ObjectMapper; import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.composite.CompositeMeterRegistry; -import io.micrometer.prometheusmetrics.PrometheusConfig; -import io.micrometer.prometheusmetrics.PrometheusMeterRegistry; +import io.micrometer.prometheus.PrometheusConfig; +import io.micrometer.prometheus.PrometheusMeterRegistry; import java.time.Clock; import java.util.List; import org.apache.curator.framework.CuratorFramework; diff --git a/hermes-frontend/src/main/java/pl/allegro/tech/hermes/frontend/config/FrontendServerConfiguration.java b/hermes-frontend/src/main/java/pl/allegro/tech/hermes/frontend/config/FrontendServerConfiguration.java index 9f81e5dca1..8f707fbab8 100644 --- a/hermes-frontend/src/main/java/pl/allegro/tech/hermes/frontend/config/FrontendServerConfiguration.java +++ b/hermes-frontend/src/main/java/pl/allegro/tech/hermes/frontend/config/FrontendServerConfiguration.java @@ -1,6 +1,6 @@ package pl.allegro.tech.hermes.frontend.config; -import io.micrometer.prometheusmetrics.PrometheusMeterRegistry; +import io.micrometer.prometheus.PrometheusMeterRegistry; import io.undertow.server.HttpHandler; import java.util.Optional; import org.springframework.boot.context.properties.EnableConfigurationProperties; diff --git a/hermes-frontend/src/main/java/pl/allegro/tech/hermes/frontend/config/PrometheusConfigAdapter.java b/hermes-frontend/src/main/java/pl/allegro/tech/hermes/frontend/config/PrometheusConfigAdapter.java index caec91b185..cc57060c15 100644 --- a/hermes-frontend/src/main/java/pl/allegro/tech/hermes/frontend/config/PrometheusConfigAdapter.java +++ b/hermes-frontend/src/main/java/pl/allegro/tech/hermes/frontend/config/PrometheusConfigAdapter.java @@ -1,6 +1,6 @@ package pl.allegro.tech.hermes.frontend.config; -import io.micrometer.prometheusmetrics.PrometheusConfig; +import io.micrometer.prometheus.PrometheusConfig; import java.time.Duration; public class PrometheusConfigAdapter implements PrometheusConfig { diff --git a/hermes-frontend/src/main/java/pl/allegro/tech/hermes/frontend/server/HermesServer.java b/hermes-frontend/src/main/java/pl/allegro/tech/hermes/frontend/server/HermesServer.java index bd26580bd2..796c613eb4 100644 --- a/hermes-frontend/src/main/java/pl/allegro/tech/hermes/frontend/server/HermesServer.java +++ b/hermes-frontend/src/main/java/pl/allegro/tech/hermes/frontend/server/HermesServer.java @@ -11,7 +11,7 @@ import static org.xnio.Options.READ_TIMEOUT; import static org.xnio.Options.SSL_CLIENT_AUTH_MODE; -import io.micrometer.prometheusmetrics.PrometheusMeterRegistry; +import io.micrometer.prometheus.PrometheusMeterRegistry; import io.undertow.Undertow; import io.undertow.server.HttpHandler; import io.undertow.server.RoutingHandler; diff --git a/hermes-frontend/src/main/java/pl/allegro/tech/hermes/frontend/server/PrometheusMetricsHandler.java b/hermes-frontend/src/main/java/pl/allegro/tech/hermes/frontend/server/PrometheusMetricsHandler.java index 5d0be6dbcb..c941a6d587 100644 --- a/hermes-frontend/src/main/java/pl/allegro/tech/hermes/frontend/server/PrometheusMetricsHandler.java +++ b/hermes-frontend/src/main/java/pl/allegro/tech/hermes/frontend/server/PrometheusMetricsHandler.java @@ -2,7 +2,7 @@ import static io.undertow.util.StatusCodes.OK; -import io.micrometer.prometheusmetrics.PrometheusMeterRegistry; +import io.micrometer.prometheus.PrometheusMeterRegistry; import io.undertow.server.HttpHandler; import io.undertow.server.HttpServerExchange; diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/PrometheusConfigAdapter.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/PrometheusConfigAdapter.java index aab50d52fa..4d4eba794d 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/PrometheusConfigAdapter.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/PrometheusConfigAdapter.java @@ -1,6 +1,6 @@ package pl.allegro.tech.hermes.management.config; -import io.micrometer.prometheusmetrics.PrometheusConfig; +import io.micrometer.prometheus.PrometheusConfig; import java.time.Duration; public class PrometheusConfigAdapter implements PrometheusConfig { diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/PrometheusConfiguration.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/PrometheusConfiguration.java index 48aa812b26..38eca0a4f6 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/PrometheusConfiguration.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/PrometheusConfiguration.java @@ -3,8 +3,8 @@ import io.micrometer.core.instrument.Meter; import io.micrometer.core.instrument.config.MeterFilter; import io.micrometer.core.instrument.distribution.DistributionStatisticConfig; -import io.micrometer.prometheusmetrics.PrometheusConfig; -import io.micrometer.prometheusmetrics.PrometheusMeterRegistry; +import io.micrometer.prometheus.PrometheusConfig; +import io.micrometer.prometheus.PrometheusMeterRegistry; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.context.annotation.Bean; @@ -15,7 +15,7 @@ public class PrometheusConfiguration { @Bean - @ConditionalOnMissingBean(PrometheusMeterRegistry.class) + @ConditionalOnMissingBean public PrometheusMeterRegistry micrometerRegistry( MicrometerRegistryProperties properties, PrometheusConfig prometheusConfig) { return new PrometheusMeterRegistryFactory(properties, prometheusConfig, "hermes-management") @@ -23,7 +23,7 @@ public PrometheusMeterRegistry micrometerRegistry( } @Bean - @ConditionalOnMissingBean(PrometheusConfig.class) + @ConditionalOnMissingBean public PrometheusConfig prometheusConfig(PrometheusProperties properties) { return new PrometheusConfigAdapter(properties); } From e55faa7d57d2e7764f5218cce890b81e3e47bba7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Wi=C5=9Bniewski?= Date: Thu, 5 Dec 2024 14:25:40 +0100 Subject: [PATCH 37/39] Revert "Revert "Merge branch 'master' into SKYEDEN-3234-detect-unused-topics"" This reverts commit cf17940838868a524ecee000dc2a88fe90b0c388. --- build.gradle | 2 +- .../di/factories/PrometheusMeterRegistryFactory.java | 4 ++-- .../tech/hermes/consumers/config/CommonConfiguration.java | 4 ++-- .../hermes/consumers/config/PrometheusConfigAdapter.java | 2 +- .../tech/hermes/consumers/config/ServerConfiguration.java | 2 +- .../tech/hermes/consumers/server/ConsumerHttpServer.java | 2 +- .../tech/hermes/frontend/config/CommonConfiguration.java | 4 ++-- .../frontend/config/FrontendServerConfiguration.java | 2 +- .../hermes/frontend/config/PrometheusConfigAdapter.java | 2 +- .../allegro/tech/hermes/frontend/server/HermesServer.java | 2 +- .../hermes/frontend/server/PrometheusMetricsHandler.java | 2 +- .../hermes/management/config/PrometheusConfigAdapter.java | 2 +- .../hermes/management/config/PrometheusConfiguration.java | 8 ++++---- 13 files changed, 19 insertions(+), 19 deletions(-) diff --git a/build.gradle b/build.gradle index ca229379c3..4a1159a0ac 100644 --- a/build.gradle +++ b/build.gradle @@ -57,7 +57,7 @@ allprojects { jetty : '12.0.8', curator : '5.4.0', dropwizard_metrics: '4.2.25', - micrometer_metrics: '1.12.5', + micrometer_metrics: '1.13.0', wiremock : '3.9.0', spock : '2.4-M4-groovy-4.0', groovy : '4.0.21', diff --git a/hermes-common/src/main/java/pl/allegro/tech/hermes/common/di/factories/PrometheusMeterRegistryFactory.java b/hermes-common/src/main/java/pl/allegro/tech/hermes/common/di/factories/PrometheusMeterRegistryFactory.java index 7d46f2bba2..d5b5915845 100644 --- a/hermes-common/src/main/java/pl/allegro/tech/hermes/common/di/factories/PrometheusMeterRegistryFactory.java +++ b/hermes-common/src/main/java/pl/allegro/tech/hermes/common/di/factories/PrometheusMeterRegistryFactory.java @@ -7,8 +7,8 @@ import io.micrometer.core.instrument.binder.jvm.JvmThreadMetrics; import io.micrometer.core.instrument.config.MeterFilter; import io.micrometer.core.instrument.distribution.DistributionStatisticConfig; -import io.micrometer.prometheus.PrometheusConfig; -import io.micrometer.prometheus.PrometheusMeterRegistry; +import io.micrometer.prometheusmetrics.PrometheusConfig; +import io.micrometer.prometheusmetrics.PrometheusMeterRegistry; import java.util.concurrent.TimeUnit; import pl.allegro.tech.hermes.common.metric.counter.CounterStorage; import pl.allegro.tech.hermes.common.metric.counter.zookeeper.ZookeeperCounterReporter; diff --git a/hermes-consumers/src/main/java/pl/allegro/tech/hermes/consumers/config/CommonConfiguration.java b/hermes-consumers/src/main/java/pl/allegro/tech/hermes/consumers/config/CommonConfiguration.java index 23a7c78965..cd762c659c 100644 --- a/hermes-consumers/src/main/java/pl/allegro/tech/hermes/consumers/config/CommonConfiguration.java +++ b/hermes-consumers/src/main/java/pl/allegro/tech/hermes/consumers/config/CommonConfiguration.java @@ -6,8 +6,8 @@ import com.fasterxml.jackson.databind.ObjectMapper; import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.composite.CompositeMeterRegistry; -import io.micrometer.prometheus.PrometheusConfig; -import io.micrometer.prometheus.PrometheusMeterRegistry; +import io.micrometer.prometheusmetrics.PrometheusConfig; +import io.micrometer.prometheusmetrics.PrometheusMeterRegistry; import jakarta.inject.Named; import java.time.Clock; import java.util.Arrays; diff --git a/hermes-consumers/src/main/java/pl/allegro/tech/hermes/consumers/config/PrometheusConfigAdapter.java b/hermes-consumers/src/main/java/pl/allegro/tech/hermes/consumers/config/PrometheusConfigAdapter.java index 43c1e2d9e8..3bac713413 100644 --- a/hermes-consumers/src/main/java/pl/allegro/tech/hermes/consumers/config/PrometheusConfigAdapter.java +++ b/hermes-consumers/src/main/java/pl/allegro/tech/hermes/consumers/config/PrometheusConfigAdapter.java @@ -1,6 +1,6 @@ package pl.allegro.tech.hermes.consumers.config; -import io.micrometer.prometheus.PrometheusConfig; +import io.micrometer.prometheusmetrics.PrometheusConfig; import java.time.Duration; public class PrometheusConfigAdapter implements PrometheusConfig { diff --git a/hermes-consumers/src/main/java/pl/allegro/tech/hermes/consumers/config/ServerConfiguration.java b/hermes-consumers/src/main/java/pl/allegro/tech/hermes/consumers/config/ServerConfiguration.java index 8e2bfbfc0c..af6b03bb35 100644 --- a/hermes-consumers/src/main/java/pl/allegro/tech/hermes/consumers/config/ServerConfiguration.java +++ b/hermes-consumers/src/main/java/pl/allegro/tech/hermes/consumers/config/ServerConfiguration.java @@ -1,7 +1,7 @@ package pl.allegro.tech.hermes.consumers.config; import com.fasterxml.jackson.databind.ObjectMapper; -import io.micrometer.prometheus.PrometheusMeterRegistry; +import io.micrometer.prometheusmetrics.PrometheusMeterRegistry; import java.io.IOException; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.context.annotation.Bean; diff --git a/hermes-consumers/src/main/java/pl/allegro/tech/hermes/consumers/server/ConsumerHttpServer.java b/hermes-consumers/src/main/java/pl/allegro/tech/hermes/consumers/server/ConsumerHttpServer.java index 722ecbc711..9fac0b659b 100644 --- a/hermes-consumers/src/main/java/pl/allegro/tech/hermes/consumers/server/ConsumerHttpServer.java +++ b/hermes-consumers/src/main/java/pl/allegro/tech/hermes/consumers/server/ConsumerHttpServer.java @@ -7,7 +7,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.sun.net.httpserver.HttpExchange; import com.sun.net.httpserver.HttpServer; -import io.micrometer.prometheus.PrometheusMeterRegistry; +import io.micrometer.prometheusmetrics.PrometheusMeterRegistry; import java.io.IOException; import java.io.OutputStream; import java.net.InetSocketAddress; diff --git a/hermes-frontend/src/main/java/pl/allegro/tech/hermes/frontend/config/CommonConfiguration.java b/hermes-frontend/src/main/java/pl/allegro/tech/hermes/frontend/config/CommonConfiguration.java index dfc71aae5b..705fee2062 100644 --- a/hermes-frontend/src/main/java/pl/allegro/tech/hermes/frontend/config/CommonConfiguration.java +++ b/hermes-frontend/src/main/java/pl/allegro/tech/hermes/frontend/config/CommonConfiguration.java @@ -5,8 +5,8 @@ import com.fasterxml.jackson.databind.ObjectMapper; import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.composite.CompositeMeterRegistry; -import io.micrometer.prometheus.PrometheusConfig; -import io.micrometer.prometheus.PrometheusMeterRegistry; +import io.micrometer.prometheusmetrics.PrometheusConfig; +import io.micrometer.prometheusmetrics.PrometheusMeterRegistry; import java.time.Clock; import java.util.List; import org.apache.curator.framework.CuratorFramework; diff --git a/hermes-frontend/src/main/java/pl/allegro/tech/hermes/frontend/config/FrontendServerConfiguration.java b/hermes-frontend/src/main/java/pl/allegro/tech/hermes/frontend/config/FrontendServerConfiguration.java index 8f707fbab8..9f81e5dca1 100644 --- a/hermes-frontend/src/main/java/pl/allegro/tech/hermes/frontend/config/FrontendServerConfiguration.java +++ b/hermes-frontend/src/main/java/pl/allegro/tech/hermes/frontend/config/FrontendServerConfiguration.java @@ -1,6 +1,6 @@ package pl.allegro.tech.hermes.frontend.config; -import io.micrometer.prometheus.PrometheusMeterRegistry; +import io.micrometer.prometheusmetrics.PrometheusMeterRegistry; import io.undertow.server.HttpHandler; import java.util.Optional; import org.springframework.boot.context.properties.EnableConfigurationProperties; diff --git a/hermes-frontend/src/main/java/pl/allegro/tech/hermes/frontend/config/PrometheusConfigAdapter.java b/hermes-frontend/src/main/java/pl/allegro/tech/hermes/frontend/config/PrometheusConfigAdapter.java index cc57060c15..caec91b185 100644 --- a/hermes-frontend/src/main/java/pl/allegro/tech/hermes/frontend/config/PrometheusConfigAdapter.java +++ b/hermes-frontend/src/main/java/pl/allegro/tech/hermes/frontend/config/PrometheusConfigAdapter.java @@ -1,6 +1,6 @@ package pl.allegro.tech.hermes.frontend.config; -import io.micrometer.prometheus.PrometheusConfig; +import io.micrometer.prometheusmetrics.PrometheusConfig; import java.time.Duration; public class PrometheusConfigAdapter implements PrometheusConfig { diff --git a/hermes-frontend/src/main/java/pl/allegro/tech/hermes/frontend/server/HermesServer.java b/hermes-frontend/src/main/java/pl/allegro/tech/hermes/frontend/server/HermesServer.java index 796c613eb4..bd26580bd2 100644 --- a/hermes-frontend/src/main/java/pl/allegro/tech/hermes/frontend/server/HermesServer.java +++ b/hermes-frontend/src/main/java/pl/allegro/tech/hermes/frontend/server/HermesServer.java @@ -11,7 +11,7 @@ import static org.xnio.Options.READ_TIMEOUT; import static org.xnio.Options.SSL_CLIENT_AUTH_MODE; -import io.micrometer.prometheus.PrometheusMeterRegistry; +import io.micrometer.prometheusmetrics.PrometheusMeterRegistry; import io.undertow.Undertow; import io.undertow.server.HttpHandler; import io.undertow.server.RoutingHandler; diff --git a/hermes-frontend/src/main/java/pl/allegro/tech/hermes/frontend/server/PrometheusMetricsHandler.java b/hermes-frontend/src/main/java/pl/allegro/tech/hermes/frontend/server/PrometheusMetricsHandler.java index c941a6d587..5d0be6dbcb 100644 --- a/hermes-frontend/src/main/java/pl/allegro/tech/hermes/frontend/server/PrometheusMetricsHandler.java +++ b/hermes-frontend/src/main/java/pl/allegro/tech/hermes/frontend/server/PrometheusMetricsHandler.java @@ -2,7 +2,7 @@ import static io.undertow.util.StatusCodes.OK; -import io.micrometer.prometheus.PrometheusMeterRegistry; +import io.micrometer.prometheusmetrics.PrometheusMeterRegistry; import io.undertow.server.HttpHandler; import io.undertow.server.HttpServerExchange; diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/PrometheusConfigAdapter.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/PrometheusConfigAdapter.java index 4d4eba794d..aab50d52fa 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/PrometheusConfigAdapter.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/PrometheusConfigAdapter.java @@ -1,6 +1,6 @@ package pl.allegro.tech.hermes.management.config; -import io.micrometer.prometheus.PrometheusConfig; +import io.micrometer.prometheusmetrics.PrometheusConfig; import java.time.Duration; public class PrometheusConfigAdapter implements PrometheusConfig { diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/PrometheusConfiguration.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/PrometheusConfiguration.java index 38eca0a4f6..48aa812b26 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/PrometheusConfiguration.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/config/PrometheusConfiguration.java @@ -3,8 +3,8 @@ import io.micrometer.core.instrument.Meter; import io.micrometer.core.instrument.config.MeterFilter; import io.micrometer.core.instrument.distribution.DistributionStatisticConfig; -import io.micrometer.prometheus.PrometheusConfig; -import io.micrometer.prometheus.PrometheusMeterRegistry; +import io.micrometer.prometheusmetrics.PrometheusConfig; +import io.micrometer.prometheusmetrics.PrometheusMeterRegistry; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.context.annotation.Bean; @@ -15,7 +15,7 @@ public class PrometheusConfiguration { @Bean - @ConditionalOnMissingBean + @ConditionalOnMissingBean(PrometheusMeterRegistry.class) public PrometheusMeterRegistry micrometerRegistry( MicrometerRegistryProperties properties, PrometheusConfig prometheusConfig) { return new PrometheusMeterRegistryFactory(properties, prometheusConfig, "hermes-management") @@ -23,7 +23,7 @@ public PrometheusMeterRegistry micrometerRegistry( } @Bean - @ConditionalOnMissingBean + @ConditionalOnMissingBean(PrometheusConfig.class) public PrometheusConfig prometheusConfig(PrometheusProperties properties) { return new PrometheusConfigAdapter(properties); } From 65e54ec6e7cf145a786a95c59eb099bbd54f1fcb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Wi=C5=9Bniewski?= Date: Wed, 11 Dec 2024 14:38:37 +0100 Subject: [PATCH 38/39] SKYEDEN-3234 | Add metrics for number of inactive topics --- .../detection/InactiveTopicsDetectionJob.java | 36 ++++++++++++++----- .../InactiveTopicsDetectionJobTest.groovy | 27 +++++++++++--- 2 files changed, 50 insertions(+), 13 deletions(-) diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJob.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJob.java index 545cbb31ff..e100f03c89 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJob.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJob.java @@ -2,6 +2,8 @@ import static java.util.stream.Collectors.groupingBy; +import io.micrometer.core.instrument.MeterRegistry; +import io.micrometer.core.instrument.Tags; import java.time.Clock; import java.time.Instant; import java.util.*; @@ -24,6 +26,7 @@ public class InactiveTopicsDetectionJob { private final Optional notifier; private final InactiveTopicsDetectionProperties properties; private final Clock clock; + private final MeterRegistry meterRegistry; private static final Logger logger = LoggerFactory.getLogger(InactiveTopicsDetectionJob.class); @@ -33,12 +36,14 @@ public InactiveTopicsDetectionJob( InactiveTopicsDetectionService inactiveTopicsDetectionService, Optional notifier, InactiveTopicsDetectionProperties properties, - Clock clock) { + Clock clock, + MeterRegistry meterRegistry) { this.topicService = topicService; this.inactiveTopicsStorageService = inactiveTopicsStorageService; this.inactiveTopicsDetectionService = inactiveTopicsDetectionService; this.properties = properties; this.clock = clock; + this.meterRegistry = meterRegistry; if (notifier.isEmpty()) { logger.info("Inactive topics notifier bean is absent"); } @@ -61,7 +66,11 @@ public void detectAndNotify() { groupedByNeedOfNotification.getOrDefault(false, List.of()); List notifiedTopics = notify(enrichWithOwner(topicsToNotify, topics)); - saveInactiveTopics(notifiedTopics, topicsToSkipNotification); + List processedTopics = + limitHistory( + Stream.concat(notifiedTopics.stream(), topicsToSkipNotification.stream()).toList()); + measureInactiveTopics(processedTopics); + inactiveTopicsStorageService.markAsInactive(processedTopics); } private List detectInactiveTopics( @@ -120,12 +129,21 @@ private List notify(List inactiveTopics) } } - private void saveInactiveTopics( - List notifiedTopics, List skippedNotificationTopics) { - List topicsToSave = - Stream.concat(notifiedTopics.stream(), skippedNotificationTopics.stream()) - .map(topic -> topic.limitNotificationsHistory(properties.notificationsHistoryLimit())) - .toList(); - inactiveTopicsStorageService.markAsInactive(topicsToSave); + private List limitHistory(List inactiveTopics) { + return inactiveTopics.stream() + .map(topic -> topic.limitNotificationsHistory(properties.notificationsHistoryLimit())) + .toList(); + } + + private void measureInactiveTopics(List processedTopics) { + processedTopics.stream() + .collect( + Collectors.groupingBy( + topic -> topic.notificationTimestampsMs().size(), Collectors.counting())) + .forEach( + (notificationsCount, topicsCount) -> { + Tags tags = Tags.of("notifications", notificationsCount.toString()); + meterRegistry.gauge("inactive-topics", tags, topicsCount); + }); } } diff --git a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJobTest.groovy b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJobTest.groovy index 605f4efb71..7fe2fab366 100644 --- a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJobTest.groovy +++ b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJobTest.groovy @@ -1,5 +1,6 @@ package pl.allegro.tech.hermes.management.domain.detection +import io.micrometer.core.instrument.simple.SimpleMeterRegistry import pl.allegro.tech.hermes.api.Topic import pl.allegro.tech.hermes.management.config.detection.InactiveTopicsDetectionProperties import pl.allegro.tech.hermes.management.domain.topic.TopicService @@ -33,6 +34,7 @@ class InactiveTopicsDetectionJobTest extends Specification { inactiveTopicsDetectionProperties, clockMock ) + static def meterRegistry = new SimpleMeterRegistry() InactiveTopicsDetectionJob detectionJob = new InactiveTopicsDetectionJob( topicServiceMock, @@ -40,7 +42,8 @@ class InactiveTopicsDetectionJobTest extends Specification { detectionService, Optional.of(inactiveTopicsNotifier), inactiveTopicsDetectionProperties, - clockMock + clockMock, + meterRegistry ) def "should detect inactive topics and notify when needed"() { @@ -89,6 +92,11 @@ class InactiveTopicsDetectionJobTest extends Specification { new InactiveTopic("group.topic4", ago21days.toEpochMilli(), [now.toEpochMilli(), ago14days.toEpochMilli()], false), new InactiveTopic("group.topic3", ago7days.toEpochMilli(), [], true) ]) + + and: + inactiveTopicsGauge(0) == 1 + inactiveTopicsGauge(1) == 1 + inactiveTopicsGauge(2) == 1 } def "should not notify if there are no inactive topics"() { @@ -119,7 +127,8 @@ class InactiveTopicsDetectionJobTest extends Specification { detectionService, Optional.empty(), inactiveTopicsDetectionProperties, - clockMock + clockMock, + meterRegistry ) and: @@ -139,6 +148,9 @@ class InactiveTopicsDetectionJobTest extends Specification { 1 * inactiveTopicsStorageServiceMock.markAsInactive([ new InactiveTopic("group.topic0", ago7days.toEpochMilli(), [], false) ]) + + and: + inactiveTopicsGauge(0) == 1 } def "should not save new notification timestamp when notification did not succeed"() { @@ -152,7 +164,8 @@ class InactiveTopicsDetectionJobTest extends Specification { detectionService, Optional.of(notifierMock), inactiveTopicsDetectionProperties, - clockMock + clockMock, + meterRegistry ) and: @@ -176,7 +189,9 @@ class InactiveTopicsDetectionJobTest extends Specification { new InactiveTopic("group.topic1", ago7days.toEpochMilli(), [], false), ]) - + and: + inactiveTopicsGauge(0) == 1 + inactiveTopicsGauge(1) == 1 } private def mockLastPublishedMessageTimestamp(String topicName, Instant instant) { @@ -186,4 +201,8 @@ class InactiveTopicsDetectionJobTest extends Specification { private static Topic topic(String name) { return TopicBuilder.topic(name).build(); } + + private static def inactiveTopicsGauge(int notifications) { + return meterRegistry.find("inactive-topics").tags("notifications", notifications.toString()).gauge().value() + } } From f33ee4d04854ab5e27487760cfca8a036c4f0a03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Wi=C5=9Bniewski?= Date: Thu, 19 Dec 2024 15:07:12 +0100 Subject: [PATCH 39/39] SKYEDEN-3234 | Remove * import --- .../domain/detection/InactiveTopicsDetectionJob.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJob.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJob.java index e100f03c89..0e8498016a 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJob.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/detection/InactiveTopicsDetectionJob.java @@ -6,7 +6,11 @@ import io.micrometer.core.instrument.Tags; import java.time.Clock; import java.time.Instant; -import java.util.*; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Optional; import java.util.stream.Collectors; import java.util.stream.Stream; import org.slf4j.Logger;