Repository: pinpoint-apm/pinpoint
Issue: Fix long division when calculating detailed memory usage
Labels: bug
module:agent

#### Description
Detailed memory usage metric calculation truncates decimal values due to long division. Need a cast to double on one of it's operands.
```return (double) (long / long)``` -> ```return long / (double) long```

Issue comments:

Pull request review comments:

Patch that resolved the issue:
diff --git a/profiler/src/main/java/com/navercorp/pinpoint/profiler/monitor/metric/memory/DefaultDetailedMemoryMetric.java b/profiler/src/main/java/com/navercorp/pinpoint/profiler/monitor/metric/memory/DefaultDetailedMemoryMetric.java
index c5054703402f..e80e6eb1573a 100644
--- a/profiler/src/main/java/com/navercorp/pinpoint/profiler/monitor/metric/memory/DefaultDetailedMemoryMetric.java
+++ b/profiler/src/main/java/com/navercorp/pinpoint/profiler/monitor/metric/memory/DefaultDetailedMemoryMetric.java
@@ -82,13 +82,10 @@ private double calculateUsage(MemoryUsage memoryUsage) {
             return UNCOLLECTED_USAGE;
         }
         long max = memoryUsage.getMax() == -1 ? memoryUsage.getCommitted() : memoryUsage.getMax();
-        if (max == -1) {
+        if (max == -1 || max == 0) {
             return UNCOLLECTED_USAGE;
         }
-        if (Double.isNaN(max) || Double.isInfinite(max) || max == 0) {
-            return UNCOLLECTED_USAGE;
-        }
-        return memoryUsage.getUsed() / max;
+        return memoryUsage.getUsed() / (double) max;
     }
 
     private interface MemoryPoolMXBeanWrapper {


Test patch:
diff --git a/profiler/src/test/java/com/navercorp/pinpoint/profiler/monitor/metric/memory/DefaultDetailedMemoryMetricTest.java b/profiler/src/test/java/com/navercorp/pinpoint/profiler/monitor/metric/memory/DefaultDetailedMemoryMetricTest.java
index e4c3e153a48e..6f8870492628 100644
--- a/profiler/src/test/java/com/navercorp/pinpoint/profiler/monitor/metric/memory/DefaultDetailedMemoryMetricTest.java
+++ b/profiler/src/test/java/com/navercorp/pinpoint/profiler/monitor/metric/memory/DefaultDetailedMemoryMetricTest.java
@@ -20,6 +20,12 @@
 import org.junit.Assert;
 import org.junit.Test;
 
+import java.lang.management.MemoryPoolMXBean;
+import java.lang.management.MemoryUsage;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
 /**
  * @author HyunGil Jeong
  */
@@ -60,4 +66,64 @@ public void testNullMemoryPoolMXBeans() {
         Assert.assertTrue(DetailedMemoryMetric.UNCOLLECTED_USAGE == snapshot.getPermGenUsage());
         Assert.assertTrue(DetailedMemoryMetric.UNCOLLECTED_USAGE == snapshot.getMetaspaceUsage());
     }
+
+    @Test
+    public void testNullMemoryUsage() {
+        // Given
+        MemoryPoolMXBean mockMXBean = mock(MemoryPoolMXBean.class);
+        MemoryUsage nullMemoryUsage = null;
+        when(mockMXBean.getUsage()).thenReturn(nullMemoryUsage);
+        DetailedMemoryMetric detailedMemoryMetric = new DefaultDetailedMemoryMetric(MemoryPoolType.CMS, mockMXBean, mockMXBean, mockMXBean, mockMXBean, mockMXBean, mockMXBean);
+        // When
+        DetailedMemoryMetricSnapshot snapshot = detailedMemoryMetric.getSnapshot();
+        // Then
+        Assert.assertTrue(DetailedMemoryMetric.UNCOLLECTED_USAGE == snapshot.getNewGenUsage());
+        Assert.assertTrue(DetailedMemoryMetric.UNCOLLECTED_USAGE == snapshot.getOldGenUsage());
+        Assert.assertTrue(DetailedMemoryMetric.UNCOLLECTED_USAGE == snapshot.getSurvivorSpaceUsage());
+        Assert.assertTrue(DetailedMemoryMetric.UNCOLLECTED_USAGE == snapshot.getCodeCacheUsage());
+        Assert.assertTrue(DetailedMemoryMetric.UNCOLLECTED_USAGE == snapshot.getPermGenUsage());
+        Assert.assertTrue(DetailedMemoryMetric.UNCOLLECTED_USAGE == snapshot.getMetaspaceUsage());
+    }
+
+    @Test
+    public void testUnknownMax() {
+        // Given
+        MemoryPoolMXBean mockMXBean = mock(MemoryPoolMXBean.class);
+        MemoryUsage mockUsage = mock(MemoryUsage.class);
+        when(mockMXBean.getUsage()).thenReturn(mockUsage);
+        when(mockUsage.getMax()).thenReturn(-1L);
+        DetailedMemoryMetric detailedMemoryMetric = new DefaultDetailedMemoryMetric(MemoryPoolType.CMS, mockMXBean, mockMXBean, mockMXBean, mockMXBean, mockMXBean, mockMXBean);
+        // When
+        DetailedMemoryMetricSnapshot snapshot = detailedMemoryMetric.getSnapshot();
+        // Then
+        Assert.assertTrue(DetailedMemoryMetric.UNCOLLECTED_USAGE == snapshot.getNewGenUsage());
+        Assert.assertTrue(DetailedMemoryMetric.UNCOLLECTED_USAGE == snapshot.getOldGenUsage());
+        Assert.assertTrue(DetailedMemoryMetric.UNCOLLECTED_USAGE == snapshot.getSurvivorSpaceUsage());
+        Assert.assertTrue(DetailedMemoryMetric.UNCOLLECTED_USAGE == snapshot.getCodeCacheUsage());
+        Assert.assertTrue(DetailedMemoryMetric.UNCOLLECTED_USAGE == snapshot.getPermGenUsage());
+        Assert.assertTrue(DetailedMemoryMetric.UNCOLLECTED_USAGE == snapshot.getMetaspaceUsage());
+    }
+
+    @Test
+    public void testValidMax() {
+        // Given
+        long givenUsed = 50L;
+        long givenMax = 100L;
+        double expectedUsage = givenUsed / (double) givenMax;
+        MemoryPoolMXBean mockMXBean = mock(MemoryPoolMXBean.class);
+        MemoryUsage mockUsage = mock(MemoryUsage.class);
+        when(mockMXBean.getUsage()).thenReturn(mockUsage);
+        when(mockUsage.getUsed()).thenReturn(50L);
+        when(mockUsage.getMax()).thenReturn(100L);
+        DetailedMemoryMetric detailedMemoryMetric = new DefaultDetailedMemoryMetric(MemoryPoolType.CMS, mockMXBean, mockMXBean, mockMXBean, mockMXBean, mockMXBean, mockMXBean);
+        // When
+        DetailedMemoryMetricSnapshot snapshot = detailedMemoryMetric.getSnapshot();
+        // Then
+        Assert.assertEquals(expectedUsage, snapshot.getNewGenUsage(), 0.01);
+        Assert.assertEquals(expectedUsage, snapshot.getOldGenUsage(), 0.01);
+        Assert.assertEquals(expectedUsage, snapshot.getSurvivorSpaceUsage(), 0.01);
+        Assert.assertEquals(expectedUsage, snapshot.getCodeCacheUsage(), 0.01);
+        Assert.assertEquals(expectedUsage, snapshot.getPermGenUsage(), 0.01);
+        Assert.assertEquals(expectedUsage, snapshot.getMetaspaceUsage(), 0.01);
+    }
 }