From 305ebd3dad78f74320e508704a8b84acb770a1be Mon Sep 17 00:00:00 2001 From: Tommy Ludwig <8924140+shakuzen@users.noreply.github.com> Date: Fri, 26 Nov 2021 17:40:38 +0900 Subject: [PATCH 1/2] NPE when allocation pool name not set With the OpenJ9 non-generational collectors `optavgpause` and `optthruput`, a NPE was happening in the notification listener which caused some code in it to not be executed. Namely, setting the live and max data size was being effectively skipped for these collectors. Now we check for `null` and only the allocated bytes counter is skipped when no allocation pool name is set. --- .../instrument/binder/jvm/JvmGcMetrics.java | 5 ++++- .../binder/jvm/JvmGcMetricsTest.java | 18 +++++++++++------- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/jvm/JvmGcMetrics.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/jvm/JvmGcMetrics.java index fcef1ad043..9223de36c3 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/jvm/JvmGcMetrics.java +++ b/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/jvm/JvmGcMetrics.java @@ -195,7 +195,10 @@ private boolean isGenerationalGcConfigured() { } private void countPoolSizeDelta(Map before, Map after, Counter counter, - AtomicLong previousPoolSize, String poolName) { + AtomicLong previousPoolSize, @Nullable String poolName) { + if (poolName == null) { + return; + } final long beforeBytes = before.get(poolName).getUsed(); final long afterBytes = after.get(poolName).getUsed(); final long delta = beforeBytes - previousPoolSize.get(); diff --git a/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/jvm/JvmGcMetricsTest.java b/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/jvm/JvmGcMetricsTest.java index 5eac525a87..e699f49c64 100644 --- a/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/jvm/JvmGcMetricsTest.java +++ b/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/jvm/JvmGcMetricsTest.java @@ -34,7 +34,6 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.within; import static org.awaitility.Awaitility.await; -import static org.junit.jupiter.api.Assumptions.assumeTrue; /** * Tests for {@link JvmGcMetrics}. @@ -65,13 +64,18 @@ void noJvmImplementationSpecificApiSignatures() { } @Test - void metersAreBound() { - assertThat(registry.find("jvm.gc.live.data.size").gauge()).isNotNull(); - assertThat(registry.find("jvm.gc.memory.allocated").counter()).isNotNull(); - assertThat(registry.find("jvm.gc.max.data.size").gauge().value()).isGreaterThan(0); + void gcMetricsAvailableAfterGc() { + System.gc(); + await().timeout(200, TimeUnit.MILLISECONDS).alias("NotificationListener takes time after GC") + .untilAsserted(() -> + assertThat(registry.find("jvm.gc.live.data.size").gauge().value()).isPositive()); + assertThat(registry.find("jvm.gc.memory.allocated").counter().count()).isPositive(); + assertThat(registry.find("jvm.gc.max.data.size").gauge().value()).isPositive(); - assumeTrue(binder.isGenerationalGc); - assertThat(registry.find("jvm.gc.memory.promoted").counter()).isNotNull(); + if (!binder.isGenerationalGc) { + return; + } + assertThat(registry.find("jvm.gc.memory.promoted").counter().count()).isPositive(); } @Test From c86842c26f1dab6f4097fdebcc7446e766a3003c Mon Sep 17 00:00:00 2001 From: Tommy Ludwig <8924140+shakuzen@users.noreply.github.com> Date: Fri, 26 Nov 2021 18:09:31 +0900 Subject: [PATCH 2/2] Forcing GC does not guarantee object promotion in gen gc --- .../core/instrument/binder/jvm/JvmGcMetricsTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/jvm/JvmGcMetricsTest.java b/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/jvm/JvmGcMetricsTest.java index e699f49c64..cfc74ae10a 100644 --- a/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/jvm/JvmGcMetricsTest.java +++ b/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/jvm/JvmGcMetricsTest.java @@ -75,7 +75,8 @@ void gcMetricsAvailableAfterGc() { if (!binder.isGenerationalGc) { return; } - assertThat(registry.find("jvm.gc.memory.promoted").counter().count()).isPositive(); + // cannot guarantee an object was promoted, so cannot check for positive count + assertThat(registry.find("jvm.gc.memory.promoted").counter()).isNotNull(); } @Test