From 8211d3fb85cd1939312b5be180dc0ca4520486a5 Mon Sep 17 00:00:00 2001 From: David Sondermann Date: Tue, 26 Aug 2025 14:35:34 +0200 Subject: [PATCH] improve: Make MicrometerMetrics extendable Signed-off-by: David Sondermann --- .../micrometer/MicrometerMetrics.java | 75 ++++++++++++++----- .../AbstractMicrometerMetricsTestFixture.java | 2 +- .../micrometer/CleanerBuilderTest.java | 73 ++++++++++++++++++ .../micrometer/DefaultBehaviorIT.java | 1 + ...ultBehaviorWithCustomImplementationIT.java | 18 +++++ .../DelayedMetricsCleaningOnDeleteIT.java | 2 +- ...ingOnDeleteWithCustomImplementationIT.java | 25 +++++++ .../micrometer/NoPerResourceCollectionIT.java | 1 + ...eCollectionWithCustomImplementationIT.java | 18 +++++ 9 files changed, 194 insertions(+), 21 deletions(-) create mode 100644 micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/CleanerBuilderTest.java create mode 100644 micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/DefaultBehaviorWithCustomImplementationIT.java create mode 100644 micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/DelayedMetricsCleaningOnDeleteWithCustomImplementationIT.java create mode 100644 micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/NoPerResourceCollectionWithCustomImplementationIT.java diff --git a/micrometer-support/src/main/java/io/javaoperatorsdk/operator/monitoring/micrometer/MicrometerMetrics.java b/micrometer-support/src/main/java/io/javaoperatorsdk/operator/monitoring/micrometer/MicrometerMetrics.java index 26f149249b..88ffe3f8e0 100644 --- a/micrometer-support/src/main/java/io/javaoperatorsdk/operator/monitoring/micrometer/MicrometerMetrics.java +++ b/micrometer-support/src/main/java/io/javaoperatorsdk/operator/monitoring/micrometer/MicrometerMetrics.java @@ -96,14 +96,13 @@ public static MicrometerMetricsBuilder newMicrometerMetricsBuilder(MeterRegistry /** * Creates a micrometer-based Metrics implementation that cleans up {@link Meter}s associated with - * deleted resources as specified by the (possibly {@code null}) provided {@link Cleaner} - * instance. + * deleted resources as specified by the provided {@link Cleaner} instance. * * @param registry the {@link MeterRegistry} instance to use for metrics recording * @param cleaner the {@link Cleaner} to use * @param collectingPerResourceMetrics whether to collect per resource metrics */ - private MicrometerMetrics( + protected MicrometerMetrics( MeterRegistry registry, Cleaner cleaner, boolean collectingPerResourceMetrics) { this.registry = registry; this.cleaner = cleaner; @@ -315,11 +314,9 @@ protected Set recordedMeterIdsFor(ResourceID resourceID) { public static class PerResourceCollectingMicrometerMetricsBuilder extends MicrometerMetricsBuilder { - private int cleaningThreadsNumber; - private int cleanUpDelayInSeconds; - private PerResourceCollectingMicrometerMetricsBuilder(MeterRegistry registry) { super(registry); + collectingPerResourceMetrics = true; } /** @@ -329,7 +326,7 @@ private PerResourceCollectingMicrometerMetricsBuilder(MeterRegistry registry) { */ public PerResourceCollectingMicrometerMetricsBuilder withCleaningThreadNumber( int cleaningThreadsNumber) { - this.cleaningThreadsNumber = cleaningThreadsNumber <= 0 ? 1 : cleaningThreadsNumber; + cleanerBuilder.withCleaningThreadNumber(cleaningThreadsNumber); return this; } @@ -343,30 +340,24 @@ public PerResourceCollectingMicrometerMetricsBuilder withCleaningThreadNumber( */ public PerResourceCollectingMicrometerMetricsBuilder withCleanUpDelayInSeconds( int cleanUpDelayInSeconds) { - this.cleanUpDelayInSeconds = Math.max(cleanUpDelayInSeconds, 1); + cleanerBuilder.withCleanUpDelayInSeconds(cleanUpDelayInSeconds); return this; } - - @Override - public MicrometerMetrics build() { - final var cleaner = - new DelayedCleaner(registry, cleanUpDelayInSeconds, cleaningThreadsNumber); - return new MicrometerMetrics(registry, cleaner, true); - } } public static class MicrometerMetricsBuilder { protected final MeterRegistry registry; - private boolean collectingPerResourceMetrics = true; + protected final CleanerBuilder cleanerBuilder; + protected boolean collectingPerResourceMetrics = true; private MicrometerMetricsBuilder(MeterRegistry registry) { this.registry = registry; + this.cleanerBuilder = new CleanerBuilder(registry); } /** Configures the instance to collect metrics on a per-resource basis. */ @SuppressWarnings("unused") public PerResourceCollectingMicrometerMetricsBuilder collectingMetricsPerResource() { - collectingPerResourceMetrics = true; return new PerResourceCollectingMicrometerMetricsBuilder(registry); } @@ -381,11 +372,52 @@ public MicrometerMetricsBuilder notCollectingMetricsPerResource() { } public MicrometerMetrics build() { - return new MicrometerMetrics(registry, Cleaner.NOOP, collectingPerResourceMetrics); + return new MicrometerMetrics(registry, cleanerBuilder.build(), collectingPerResourceMetrics); + } + } + + public static class CleanerBuilder { + + private final MeterRegistry registry; + private int cleaningThreadsNumber = 0; + private int cleanUpDelayInSeconds = 0; + + public CleanerBuilder(MeterRegistry registry) { + this.registry = registry; + } + + /** + * @param cleaningThreadsNumber the maximal number of threads that can be assigned to the + * removal of {@link Meter}s associated with deleted resources, defaults to 1 if not + * specified or if the provided number is lesser or equal to 0 + */ + public CleanerBuilder withCleaningThreadNumber(int cleaningThreadsNumber) { + this.cleaningThreadsNumber = cleaningThreadsNumber <= 0 ? 1 : cleaningThreadsNumber; + return this; + } + + /** + * @param cleanUpDelayInSeconds the number of seconds to wait before {@link Meter}s are removed + * for deleted resources, defaults to 1 (meaning meters will be removed one second after the + * associated resource is deleted) if not specified or if the provided number is lesser than + * 0. Threading and the general interaction model of interacting with the API server means + * that it's not possible to ensure that meters are immediately deleted in all cases so a + * minimal delay of one second is always enforced + */ + public CleanerBuilder withCleanUpDelayInSeconds(int cleanUpDelayInSeconds) { + this.cleanUpDelayInSeconds = Math.max(cleanUpDelayInSeconds, 1); + return this; + } + + public Cleaner build() { + if (cleanUpDelayInSeconds > 0 || cleaningThreadsNumber > 0) { + return new DelayedCleaner(registry, cleanUpDelayInSeconds, cleaningThreadsNumber); + } + return Cleaner.NOOP; } } - interface Cleaner { + public interface Cleaner { Cleaner NOOP = new Cleaner() {}; default void removeMetersFor(ResourceID resourceID) {} @@ -444,5 +476,10 @@ public void removeMetersFor(ResourceID resourceID) { metersCleaner.schedule( () -> super.removeMetersFor(resourceID), cleanUpDelayInSeconds, TimeUnit.SECONDS); } + + // for testing purposes only + int getCleanUpDelayInSeconds() { + return cleanUpDelayInSeconds; + } } } diff --git a/micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/AbstractMicrometerMetricsTestFixture.java b/micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/AbstractMicrometerMetricsTestFixture.java index 788253e5e8..c011324ffb 100644 --- a/micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/AbstractMicrometerMetricsTestFixture.java +++ b/micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/AbstractMicrometerMetricsTestFixture.java @@ -87,7 +87,7 @@ public DeleteControl cleanup(ConfigMap resource, Context context) { } } - static class TestSimpleMeterRegistry extends SimpleMeterRegistry { + protected static class TestSimpleMeterRegistry extends SimpleMeterRegistry { private final Set removed = new HashSet<>(); @Override diff --git a/micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/CleanerBuilderTest.java b/micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/CleanerBuilderTest.java new file mode 100644 index 0000000000..0bb49a0cce --- /dev/null +++ b/micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/CleanerBuilderTest.java @@ -0,0 +1,73 @@ +package io.javaoperatorsdk.operator.monitoring.micrometer; + +import org.junit.jupiter.api.Test; + +import io.javaoperatorsdk.operator.monitoring.micrometer.MicrometerMetrics.CleanerBuilder; +import io.micrometer.core.instrument.simple.SimpleMeterRegistry; + +import static org.assertj.core.api.Assertions.assertThat; + +class CleanerBuilderTest { + + private final SimpleMeterRegistry registry = new SimpleMeterRegistry(); + + @Test + void test_defaultsToNoopCleaner() { + final var cleaner = new CleanerBuilder(registry).build(); + assertThat(cleaner).isSameAs(MicrometerMetrics.Cleaner.NOOP); + } + + @Test + void test_withCleaningThreadNumber() { + final var cleaner = new CleanerBuilder(registry).withCleaningThreadNumber(42).build(); + assertThat(cleaner).isInstanceOf(MicrometerMetrics.DelayedCleaner.class); + if (cleaner instanceof MicrometerMetrics.DelayedCleaner delayedCleaner) { + assertThat(delayedCleaner.getCleanUpDelayInSeconds()).isEqualTo(0); + } + } + + @Test + void test_withCleaningThreadNumber_whenNegative_thenApplyDefault() { + final var cleaner = new CleanerBuilder(registry).withCleaningThreadNumber(42).build(); + assertThat(cleaner).isInstanceOf(MicrometerMetrics.DelayedCleaner.class); + if (cleaner instanceof MicrometerMetrics.DelayedCleaner delayedCleaner) { + assertThat(delayedCleaner.getCleanUpDelayInSeconds()).isEqualTo(0); + } + } + + @Test + void test_withCleaningThreadNumber_whenZero_thenApplyDefault() { + final var cleaner = new CleanerBuilder(registry).withCleaningThreadNumber(0).build(); + assertThat(cleaner).isInstanceOf(MicrometerMetrics.DelayedCleaner.class); + if (cleaner instanceof MicrometerMetrics.DelayedCleaner delayedCleaner) { + assertThat(delayedCleaner.getCleanUpDelayInSeconds()).isEqualTo(0); + } + } + + @Test + void test_withCleanUpDelayInSeconds() { + final var cleaner = new CleanerBuilder(registry).withCleanUpDelayInSeconds(23).build(); + assertThat(cleaner).isInstanceOf(MicrometerMetrics.DelayedCleaner.class); + if (cleaner instanceof MicrometerMetrics.DelayedCleaner delayedCleaner) { + assertThat(delayedCleaner.getCleanUpDelayInSeconds()).isEqualTo(23); + } + } + + @Test + void test_withCleanUpDelayInSeconds_whenNegative_thenApplyDefault() { + final var cleaner = new CleanerBuilder(registry).withCleanUpDelayInSeconds(-23).build(); + assertThat(cleaner).isInstanceOf(MicrometerMetrics.DelayedCleaner.class); + if (cleaner instanceof MicrometerMetrics.DelayedCleaner delayedCleaner) { + assertThat(delayedCleaner.getCleanUpDelayInSeconds()).isEqualTo(1); + } + } + + @Test + void test_withCleanUpDelayInSeconds_whenZero_thenApplyDefault() { + final var cleaner = new CleanerBuilder(registry).withCleanUpDelayInSeconds(0).build(); + assertThat(cleaner).isInstanceOf(MicrometerMetrics.DelayedCleaner.class); + if (cleaner instanceof MicrometerMetrics.DelayedCleaner delayedCleaner) { + assertThat(delayedCleaner.getCleanUpDelayInSeconds()).isEqualTo(1); + } + } +} diff --git a/micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/DefaultBehaviorIT.java b/micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/DefaultBehaviorIT.java index 6f0388c49b..4dadef62f6 100644 --- a/micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/DefaultBehaviorIT.java +++ b/micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/DefaultBehaviorIT.java @@ -9,6 +9,7 @@ import static org.assertj.core.api.Assertions.assertThat; public class DefaultBehaviorIT extends AbstractMicrometerMetricsTestFixture { + @Override protected MicrometerMetrics getMetrics() { return MicrometerMetrics.newMicrometerMetricsBuilder(registry).build(); diff --git a/micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/DefaultBehaviorWithCustomImplementationIT.java b/micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/DefaultBehaviorWithCustomImplementationIT.java new file mode 100644 index 0000000000..040b7584d9 --- /dev/null +++ b/micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/DefaultBehaviorWithCustomImplementationIT.java @@ -0,0 +1,18 @@ +package io.javaoperatorsdk.operator.monitoring.micrometer; + +import io.micrometer.core.instrument.MeterRegistry; + +public class DefaultBehaviorWithCustomImplementationIT extends DefaultBehaviorIT { + + @Override + protected MicrometerMetrics getMetrics() { + return new TestMetrics(registry); + } + + private static class TestMetrics extends MicrometerMetrics { + + private TestMetrics(MeterRegistry registry) { + super(registry, new CleanerBuilder(registry).build(), true); + } + } +} diff --git a/micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/DelayedMetricsCleaningOnDeleteIT.java b/micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/DelayedMetricsCleaningOnDeleteIT.java index 92929d5ddb..977830beec 100644 --- a/micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/DelayedMetricsCleaningOnDeleteIT.java +++ b/micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/DelayedMetricsCleaningOnDeleteIT.java @@ -10,7 +10,7 @@ public class DelayedMetricsCleaningOnDeleteIT extends AbstractMicrometerMetricsTestFixture { - private static final int testDelay = 1; + protected static final int testDelay = 1; @Override protected MicrometerMetrics getMetrics() { diff --git a/micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/DelayedMetricsCleaningOnDeleteWithCustomImplementationIT.java b/micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/DelayedMetricsCleaningOnDeleteWithCustomImplementationIT.java new file mode 100644 index 0000000000..690848634a --- /dev/null +++ b/micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/DelayedMetricsCleaningOnDeleteWithCustomImplementationIT.java @@ -0,0 +1,25 @@ +package io.javaoperatorsdk.operator.monitoring.micrometer; + +import io.micrometer.core.instrument.MeterRegistry; + +public class DelayedMetricsCleaningOnDeleteWithCustomImplementationIT + extends DelayedMetricsCleaningOnDeleteIT { + + @Override + protected MicrometerMetrics getMetrics() { + return new TestMetrics(registry); + } + + private static class TestMetrics extends MicrometerMetrics { + + private TestMetrics(MeterRegistry registry) { + super( + registry, + new CleanerBuilder(registry) + .withCleanUpDelayInSeconds(testDelay) + .withCleaningThreadNumber(2) + .build(), + true); + } + } +} diff --git a/micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/NoPerResourceCollectionIT.java b/micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/NoPerResourceCollectionIT.java index ac35347697..c8a4c7d1f5 100644 --- a/micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/NoPerResourceCollectionIT.java +++ b/micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/NoPerResourceCollectionIT.java @@ -9,6 +9,7 @@ import static org.assertj.core.api.Assertions.assertThat; public class NoPerResourceCollectionIT extends AbstractMicrometerMetricsTestFixture { + @Override protected MicrometerMetrics getMetrics() { return MicrometerMetrics.withoutPerResourceMetrics(registry); diff --git a/micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/NoPerResourceCollectionWithCustomImplementationIT.java b/micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/NoPerResourceCollectionWithCustomImplementationIT.java new file mode 100644 index 0000000000..8d49cb7542 --- /dev/null +++ b/micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/NoPerResourceCollectionWithCustomImplementationIT.java @@ -0,0 +1,18 @@ +package io.javaoperatorsdk.operator.monitoring.micrometer; + +import io.micrometer.core.instrument.MeterRegistry; + +public class NoPerResourceCollectionWithCustomImplementationIT extends NoPerResourceCollectionIT { + + @Override + protected MicrometerMetrics getMetrics() { + return new TestMetrics(registry); + } + + private static class TestMetrics extends MicrometerMetrics { + + private TestMetrics(MeterRegistry registry) { + super(registry, new CleanerBuilder(registry).build(), false); + } + } +}