Fix memory pool metrics (#2075)

extensions/runtime-metrics/src/main/java/io/opentelemetry/extension/metrics/runtime/MemoryPools.java

@@ -45,6 +45,18 @@ public final class MemoryPools {
   private static final String HEAP = "heap";
   private static final String NON_HEAP = "non_heap";
 
+  private static final Labels COMMITTED_HEAP =
+      Labels.of(TYPE_LABEL_KEY, COMMITTED, AREA_LABEL_KEY, HEAP);
+  private static final Labels USED_HEAP = Labels.of(TYPE_LABEL_KEY, USED, AREA_LABEL_KEY, HEAP);
+  private static final Labels MAX_HEAP = Labels.of(TYPE_LABEL_KEY, MAX, AREA_LABEL_KEY, HEAP);
+
+  private static final Labels COMMITTED_NON_HEAP =
+      Labels.of(TYPE_LABEL_KEY, COMMITTED, AREA_LABEL_KEY, NON_HEAP);
+  private static final Labels USED_NON_HEAP =
+      Labels.of(TYPE_LABEL_KEY, USED, AREA_LABEL_KEY, NON_HEAP);
+  private static final Labels MAX_NON_HEAP =
+      Labels.of(TYPE_LABEL_KEY, MAX, AREA_LABEL_KEY, NON_HEAP);
+
   private final MemoryMXBean memoryBean;
   private final List<MemoryPoolMXBean> poolBeans;
   private final Meter meter;
@@ -64,25 +76,12 @@ public final class MemoryPools {
             .setDescription("Bytes of a given JVM memory area.")
             .setUnit("By")
             .build();
-    final Labels usedHeap = Labels.of(TYPE_LABEL_KEY, USED, AREA_LABEL_KEY, HEAP);
-    final Labels usedNonHeap = Labels.of(TYPE_LABEL_KEY, USED, AREA_LABEL_KEY, NON_HEAP);
-    final Labels committedHeap = Labels.of(TYPE_LABEL_KEY, COMMITTED, AREA_LABEL_KEY, HEAP);
-    final Labels committedNonHeap = Labels.of(TYPE_LABEL_KEY, COMMITTED, AREA_LABEL_KEY, NON_HEAP);
-    // TODO: Decide if max is needed or not. May be derived with some approximation from max(used).
-    final Labels maxHeap = Labels.of(TYPE_LABEL_KEY, MAX, AREA_LABEL_KEY, HEAP);
-    final Labels maxNonHeap = Labels.of(TYPE_LABEL_KEY, MAX, AREA_LABEL_KEY, NON_HEAP);
     areaMetric.setCallback(
         new AsynchronousInstrument.Callback<LongResult>() {
           @Override
           public void update(LongResult resultLongObserver) {
-            MemoryUsage heapUsage = memoryBean.getHeapMemoryUsage();
+            observeHeap(resultLongObserver, memoryBean.getHeapMemoryUsage());
-            MemoryUsage nonHeapUsage = memoryBean.getNonHeapMemoryUsage();
+            observeNonHeap(resultLongObserver, memoryBean.getNonHeapMemoryUsage());
-            resultLongObserver.observe(heapUsage.getUsed(), usedHeap);
-            resultLongObserver.observe(nonHeapUsage.getUsed(), usedNonHeap);
-            resultLongObserver.observe(heapUsage.getUsed(), committedHeap);
-            resultLongObserver.observe(nonHeapUsage.getUsed(), committedNonHeap);
-            resultLongObserver.observe(heapUsage.getUsed(), maxHeap);
-            resultLongObserver.observe(nonHeapUsage.getUsed(), maxNonHeap);
           }
         });
   }
@@ -109,11 +108,14 @@ public final class MemoryPools {
           public void update(LongResult resultLongObserver) {
             for (int i = 0; i < poolBeans.size(); i++) {
               MemoryUsage poolUsage = poolBeans.get(i).getUsage();
-              resultLongObserver.observe(poolUsage.getUsed(), usedLabelSets.get(i));
+              if (poolUsage != null) {
-              resultLongObserver.observe(poolUsage.getCommitted(), committedLabelSets.get(i));
+                observe(
-              // TODO: Decide if max is needed or not. May be derived with some approximation from
+                    resultLongObserver,
-              //  max(used).
+                    poolUsage,
-              resultLongObserver.observe(poolUsage.getMax(), maxLabelSets.get(i));
+                    usedLabelSets.get(i),
+                    committedLabelSets.get(i),
+                    maxLabelSets.get(i));
+              }
             }
           }
         });
@@ -124,4 +126,30 @@ public final class MemoryPools {
     exportMemoryAreaMetric();
     exportMemoryPoolMetric();
   }
+
+  static void observeHeap(LongResult observer, MemoryUsage usage) {
+    observe(observer, usage, USED_HEAP, COMMITTED_HEAP, MAX_HEAP);
+  }
+
+  static void observeNonHeap(LongResult observer, MemoryUsage usage) {
+    observe(observer, usage, USED_NON_HEAP, COMMITTED_NON_HEAP, MAX_NON_HEAP);
+  }
+
+  private static void observe(
+      LongResult observer,
+      MemoryUsage usage,
+      Labels usedLabels,
+      Labels committedLabels,
+      Labels maxLabels) {
+    // TODO: Decide if init is needed or not. It is a constant that can be queried once on startup.
+    // if (usage.getInit() != -1) {
+    //  observer.observe(usage.getInit(), ...);
+    // }
+    observer.observe(usage.getUsed(), usedLabels);
+    observer.observe(usage.getCommitted(), committedLabels);
+    // TODO: Decide if max is needed or not. It is a constant that can be queried once on startup.
+    if (usage.getMax() != -1) {
+      observer.observe(usage.getMax(), maxLabels);
+    }
+  }
 }

extensions/runtime-metrics/src/test/java/io/opentelemetry/extension/metrics/runtime/MemoryPoolsTest.java

@@ -0,0 +1,56 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.extension.metrics.runtime;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
+
+import io.opentelemetry.api.common.Labels;
+import io.opentelemetry.api.metrics.AsynchronousInstrument;
+import java.lang.management.MemoryUsage;
+import org.junit.jupiter.api.Test;
+
+class MemoryPoolsTest {
+
+  @Test
+  void observeHeap() {
+    AsynchronousInstrument.LongResult observer = mock(AsynchronousInstrument.LongResult.class);
+    MemoryPools.observeHeap(observer, new MemoryUsage(-1, 1, 2, 3));
+    verify(observer).observe(1, Labels.of("type", "used", "area", "heap"));
+    verify(observer).observe(2, Labels.of("type", "committed", "area", "heap"));
+    verify(observer).observe(3, Labels.of("type", "max", "area", "heap"));
+    verifyNoMoreInteractions(observer);
+  }
+
+  @Test
+  void observeHeapNoMax() {
+    AsynchronousInstrument.LongResult observer = mock(AsynchronousInstrument.LongResult.class);
+    MemoryPools.observeHeap(observer, new MemoryUsage(-1, 1, 2, -1));
+    verify(observer).observe(1, Labels.of("type", "used", "area", "heap"));
+    verify(observer).observe(2, Labels.of("type", "committed", "area", "heap"));
+    verifyNoMoreInteractions(observer);
+  }
+
+  @Test
+  void observeNonHeap() {
+    AsynchronousInstrument.LongResult observer = mock(AsynchronousInstrument.LongResult.class);
+    MemoryPools.observeNonHeap(observer, new MemoryUsage(-1, 4, 5, 6));
+    verify(observer).observe(4, Labels.of("type", "used", "area", "non_heap"));
+    verify(observer).observe(5, Labels.of("type", "committed", "area", "non_heap"));
+    verify(observer).observe(6, Labels.of("type", "max", "area", "non_heap"));
+    verifyNoMoreInteractions(observer);
+  }
+
+  @Test
+  void observeNonHeapNoMax() {
+    AsynchronousInstrument.LongResult observer = mock(AsynchronousInstrument.LongResult.class);
+    MemoryPools.observeNonHeap(observer, new MemoryUsage(-1, 4, 5, -1));
+    verify(observer).observe(4, Labels.of("type", "used", "area", "non_heap"));
+    verify(observer).observe(5, Labels.of("type", "committed", "area", "non_heap"));
+    verifyNoMoreInteractions(observer);
+  }
+}