[pinpoint-apm#4044] Fix divide by zero error in ApdexScore

emeroad · Feb 15, 2022 · 57095f6 · 57095f6
1 parent bf66b71
commit 57095f6
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 17 deletions.
diff --git a/web/src/main/java/com/navercorp/pinpoint/web/applicationmap/histogram/ApdexScore.java b/web/src/main/java/com/navercorp/pinpoint/web/applicationmap/histogram/ApdexScore.java
@@ -6,7 +6,13 @@
 import java.math.RoundingMode;
 import java.util.Objects;
 
+/**
+ * https://en.wikipedia.org/wiki/Apdex
+ */
 public class ApdexScore {
+
+    private static final BigDecimal TWO = BigDecimal.valueOf(2);
+
     private final double apdexScore;
 
     public static ApdexScore newApdexScore(Histogram histogram) {
@@ -18,16 +24,24 @@ public static ApdexScore newApdexScore(Histogram histogram) {
         return new ApdexScore(satisfiedCount, toleratingCount, totalCount);
     }
 
-    public ApdexScore(long satisfiedCount, long toleratingCount, long totalCount) {
-        this.apdexScore = calculateApdexScore(satisfiedCount, toleratingCount, totalCount);
+    public ApdexScore(long satisfiedCount, long toleratingCount, long totalSamples) {
+        this.apdexScore = calculateApdexScore(satisfiedCount, toleratingCount, totalSamples);
     }
 
-    private double calculateApdexScore(long satisfiedCount, long toleratingCount, long totalCount) {
-        BigDecimal satisfied = new BigDecimal(satisfiedCount);
-        BigDecimal tolerating = new BigDecimal(toleratingCount).multiply(BigDecimal.valueOf(0.5));
-        BigDecimal total = new BigDecimal(totalCount);
-        BigDecimal numerator = satisfied.add(tolerating);
+    private double calculateApdexScore(long satisfiedCount, long toleratingCount, long totalSamples) {
+        // divide by zero
+        if (totalSamples == 0) {
+            return 0;
+        }
+        BigDecimal satisfied = BigDecimal.valueOf(satisfiedCount);
+        BigDecimal tolerating = BigDecimal.valueOf(toleratingCount);
+        BigDecimal total = BigDecimal.valueOf(totalSamples);
+
+
+        BigDecimal toleratingScore = tolerating.divide(TWO, RoundingMode.FLOOR);
+        BigDecimal numerator = satisfied.add(toleratingScore);
         BigDecimal score = numerator.divide(total, 3, RoundingMode.FLOOR);
+
         return score.doubleValue();
     }
 

diff --git a/web/src/test/java/com/navercorp/pinpoint/web/applicationmap/histogram/ApdexScoreTest.java b/web/src/test/java/com/navercorp/pinpoint/web/applicationmap/histogram/ApdexScoreTest.java
@@ -1,7 +1,5 @@
 package com.navercorp.pinpoint.web.applicationmap.histogram;
 
-import com.fasterxml.jackson.core.JsonFactory;
-import com.fasterxml.jackson.core.JsonGenerator;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
@@ -12,29 +10,42 @@
 import org.skyscreamer.jsonassert.JSONCompareMode;
 
 import java.io.IOException;
-import java.io.StringWriter;
-import java.io.Writer;
 
 public class ApdexScoreTest {
 
-    private static final ObjectMapper MAPPER = new ObjectMapper();
     private final Logger logger = LogManager.getLogger(this.getClass());
 
+    private final ObjectMapper MAPPER = new ObjectMapper();
+
     @Test
     public void getApdexScore() {
-        Assert.assertEquals(1.00, new ApdexScore(100, 0, 100).getApdexScore(), 0.001);
-        Assert.assertEquals(0.50, new ApdexScore(100, 0, 200).getApdexScore(), 0.001);
+        ApdexScore apdexScore = new ApdexScore(100, 0, 100);
+        Assert.assertEquals(1.00, apdexScore.getApdexScore(), 0.001);
+
+        ApdexScore apdexScore1 = new ApdexScore(100, 0, 200);
+        Assert.assertEquals(0.50, apdexScore1.getApdexScore(), 0.001);
+
+        ApdexScore apdexScore2 = new ApdexScore(60, 30, 100);
+        Assert.assertEquals(0.75, apdexScore2.getApdexScore(), 0.001);
     }
 
     @Test
     public void getApdexScore_floatingPoint() {
-        Assert.assertEquals(0.999, new ApdexScore(Long.MAX_VALUE - 1, 0, Long.MAX_VALUE).getApdexScore(), 0.001);
+        ApdexScore apdexScore = new ApdexScore(Long.MAX_VALUE - 1, 0, Long.MAX_VALUE);
+        Assert.assertEquals(0.999, apdexScore.getApdexScore(), 0.001);
+    }
+
+    @Test
+    public void getApdexScore_divide_by_zero() {
+        ApdexScore apdexScore = new ApdexScore(0, 0, 0);
+        Assert.assertEquals(0, apdexScore.getApdexScore(), 0.001);
     }
 
     @Test
     public void getApdexScore_format() throws IOException, JSONException {
-        ObjectMapper objectMapper = new ObjectMapper();
-        final String actualStr = objectMapper.writeValueAsString(new ApdexScore(100, 0, 100));
+
+        final String actualStr = MAPPER.writeValueAsString(new ApdexScore(100, 0, 100));
+
         JSONAssert.assertEquals("{\"apdexScore\":1.0}", actualStr, JSONCompareMode.LENIENT);
     }