diff --git a/micrometer-support/pom.xml b/micrometer-support/pom.xml index 733bc4528c..71d9f61d95 100644 --- a/micrometer-support/pom.xml +++ b/micrometer-support/pom.xml @@ -51,6 +51,11 @@ assertj-core test + + org.mockito + mockito-core + test + org.awaitility awaitility 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 0c102e52c4..f49aaf31d9 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 @@ -22,6 +22,7 @@ import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicLong; import io.fabric8.kubernetes.api.model.HasMetadata; import io.javaoperatorsdk.operator.OperatorException; @@ -78,6 +79,7 @@ public class MicrometerMetrics implements Metrics { private final boolean collectPerResourceMetrics; private final MeterRegistry registry; private final Map gauges = new ConcurrentHashMap<>(); + private final Map longGauges = new ConcurrentHashMap<>(); private final Cleaner cleaner; /** @@ -153,10 +155,19 @@ public void controllerRegistered(Controller controller) { public void eventProcessingStarted(Controller controller) { final var configuration = controller.getConfiguration(); final var name = configuration.getName(); - final var tags = new ArrayList(3); - addGVKTags(GroupVersionKind.gvkFor(configuration.getResourceClass()), tags, false); - registry.gauge( - PROCESSING_STARTED_LATENCY + name, tags, ManagementFactory.getRuntimeMXBean().getUptime()); + // the registry only holds a weak reference to the gauged object, so it has to be kept alive + // here, otherwise the gauge reports NaN as soon as the value is garbage collected + longGauges + .computeIfAbsent( + PROCESSING_STARTED_LATENCY + name, + key -> { + final var tags = new ArrayList(3); + addGVKTags(GroupVersionKind.gvkFor(configuration.getResourceClass()), tags, false); + var holder = new AtomicLong(); + registry.gauge(key, tags, holder); + return holder; + }) + .set(ManagementFactory.getRuntimeMXBean().getUptime()); } @Override diff --git a/micrometer-support/src/main/java/io/javaoperatorsdk/operator/monitoring/micrometer/MicrometerMetricsV2.java b/micrometer-support/src/main/java/io/javaoperatorsdk/operator/monitoring/micrometer/MicrometerMetricsV2.java index b342061720..6b4b446830 100644 --- a/micrometer-support/src/main/java/io/javaoperatorsdk/operator/monitoring/micrometer/MicrometerMetricsV2.java +++ b/micrometer-support/src/main/java/io/javaoperatorsdk/operator/monitoring/micrometer/MicrometerMetricsV2.java @@ -20,6 +20,7 @@ import java.util.*; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicLong; import java.util.function.Consumer; import java.util.function.Function; @@ -69,6 +70,7 @@ public class MicrometerMetricsV2 implements Metrics { private final MeterRegistry registry; private final Map gauges = new ConcurrentHashMap<>(); + private final Map longGauges = new ConcurrentHashMap<>(); private final Map executionTimers = new ConcurrentHashMap<>(); private final Function timerConfig; private final boolean includeNamespaceTag; @@ -150,10 +152,19 @@ public void controllerRegistered(Controller controller) { @Override public void eventProcessingStarted(Controller controller) { final var name = controller.getConfiguration().getName(); - final var tags = new ArrayList(); - addControllerNameTag(name, tags); - registry.gauge( - PROCESSING_STARTED_LATENCY_GAUGE, tags, ManagementFactory.getRuntimeMXBean().getUptime()); + // the registry only holds a weak reference to the gauged object, so it has to be kept alive + // here, otherwise the gauge reports NaN as soon as the value is garbage collected + longGauges + .computeIfAbsent( + processingStartedLatencyGaugeRefKey(name), + key -> { + final var tags = new ArrayList(); + addControllerNameTag(name, tags); + var holder = new AtomicLong(); + registry.gauge(PROCESSING_STARTED_LATENCY_GAUGE, tags, holder); + return holder; + }) + .set(ManagementFactory.getRuntimeMXBean().getUptime()); } private String numberOfResourcesRefName(String name) { @@ -289,6 +300,10 @@ private static String controllerQueueSizeGaugeRefKey(String controllerName) { return RECONCILIATIONS_QUEUE_SIZE_GAUGE + "." + controllerName; } + private static String processingStartedLatencyGaugeRefKey(String controllerName) { + return PROCESSING_STARTED_LATENCY_GAUGE + "." + controllerName; + } + public static String getControllerName(Map metadata) { return (String) metadata.get(Constants.CONTROLLER_NAME); } diff --git a/micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/ProcessingStartedLatencyGaugeTest.java b/micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/ProcessingStartedLatencyGaugeTest.java new file mode 100644 index 0000000000..0f969d2fec --- /dev/null +++ b/micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/ProcessingStartedLatencyGaugeTest.java @@ -0,0 +1,76 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.monitoring.micrometer; + +import org.junit.jupiter.api.Test; + +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; +import io.javaoperatorsdk.operator.processing.Controller; +import io.micrometer.core.instrument.simple.SimpleMeterRegistry; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +class ProcessingStartedLatencyGaugeTest { + + private static final String CONTROLLER_NAME = "testcontroller"; + + @Test + void latencyGaugeKeepsItsValue() { + var registry = new SimpleMeterRegistry(); + var metrics = MicrometerMetricsV2.newBuilder(registry).build(); + + metrics.eventProcessingStarted(controller()); + + var gauge = registry.find(MicrometerMetricsV2.PROCESSING_STARTED_LATENCY_GAUGE).gauge(); + assertThat(gauge).isNotNull(); + assertThat(gauge.value()).isNotNaN().isPositive(); + + // the registry holds only a weak reference to the gauged object + forceGarbageCollection(); + + assertThat(gauge.value()).isNotNaN().isPositive(); + } + + @Test + void repeatedCallsUpdateTheSameGauge() { + var registry = new SimpleMeterRegistry(); + var metrics = MicrometerMetricsV2.newBuilder(registry).build(); + + metrics.eventProcessingStarted(controller()); + metrics.eventProcessingStarted(controller()); + + assertThat(registry.find(MicrometerMetricsV2.PROCESSING_STARTED_LATENCY_GAUGE).gauges()) + .hasSize(1); + } + + @SuppressWarnings("unchecked") + private static Controller controller() { + Controller controller = mock(Controller.class); + ControllerConfiguration configuration = mock(ControllerConfiguration.class); + when(controller.getConfiguration()).thenReturn(configuration); + when(configuration.getName()).thenReturn(CONTROLLER_NAME); + return controller; + } + + private static void forceGarbageCollection() { + for (int i = 0; i < 5; i++) { + System.gc(); + } + } +}