Repository: deepjavalibrary/djl
Pull request #1614 (2022-04-29 21:28:56)

[api] Fixes metric dimension
Change-Id: Idb7048e33c22e4cee3cc8bd5eac5d91cf9b94e01

## Description ##

Brief description of what this PR is about

- If this change is a backward incompatible change, why must this change be made?
- Interesting edge cases to note here



Patch:
diff --git a/api/src/main/java/ai/djl/metric/Metric.java b/api/src/main/java/ai/djl/metric/Metric.java
index 1a8c44894..eb1d8fcab 100644
--- a/api/src/main/java/ai/djl/metric/Metric.java
+++ b/api/src/main/java/ai/djl/metric/Metric.java
@@ -15,8 +15,6 @@ package ai.djl.metric;
 import com.google.gson.annotations.SerializedName;
 import java.net.InetAddress;
 import java.net.UnknownHostException;
-import java.util.ArrayList;
-import java.util.List;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -29,9 +27,9 @@ public class Metric {
 
     private static final Pattern PATTERN =
             Pattern.compile(
-                    "\\s*([\\w\\s]+)\\.([\\w\\s]+):([0-9\\-,.e]+)\\|#([^|]*)\\|#hostname:([^,]+),([^,]+)(,.*)?");
+                    "\\s*([\\w\\s]+)\\.([\\w\\s]+):([0-9\\-,.e]+)(?>\\|#([^|]*))?(?>\\|(\\d+))?");
 
-    private static final String LOCALHOST = getLocalHostName();
+    private static final Dimension HOST = new Dimension("Host", getLocalHostName());
 
     @SerializedName("MetricName")
     private String metricName;
@@ -43,14 +41,11 @@ public class Metric {
     private String unit;
 
     @SerializedName("Dimensions")
-    private List<Dimension> dimensions;
+    private Dimension[] dimensions;
 
     @SerializedName("Timestamp")
     private String timestamp;
 
-    @SerializedName("HostName")
-    private String hostName;
-
     /**
      * Constructs a {@code Metric} instance with the specified {@code metricName} and <code>
      * value</code>.
@@ -71,28 +66,42 @@ public class Metric {
      * @param unit the metric unit
      */
     public Metric(String metricName, Number value, Unit unit) {
-        this.metricName = metricName;
-        this.value = value.toString();
-        this.unit = unit.getValue();
-        this.hostName = LOCALHOST;
+        this(metricName, value.toString(), unit.getValue(), null, HOST);
     }
 
     /**
-     * Constructs a new {@code Metric} instance.
+     * Constructs a {@code Metric} instance with the specified {@code metricName}, <code>value
+     * </code>, and {@code unit}.
      *
      * @param metricName the metric name
+     * @param value the metric value
      * @param unit the metric unit
+     * @param dimensions the metric dimensions
+     */
+    public Metric(String metricName, Number value, Unit unit, Dimension... dimensions) {
+        this(metricName, value.toString(), unit.getValue(), null, dimensions);
+    }
+
+    /**
+     * Constructs a new {@code Metric} instance.
+     *
+     * @param metricName the metric name
      * @param value the metric value
-     * @param hostName the host name
+     * @param unit the metric unit
      * @param timestamp the metric timestamp
+     * @param dimensions the metric dimensions
      */
     private Metric(
-            String metricName, String unit, String value, String hostName, String timestamp) {
+            String metricName,
+            String value,
+            String unit,
+            String timestamp,
+            Dimension... dimensions) {
         this.metricName = metricName;
         this.unit = unit;
         this.value = value;
-        this.hostName = hostName;
         this.timestamp = timestamp;
+        this.dimensions = dimensions;
     }
 
     /**
@@ -132,12 +141,12 @@ public class Metric {
     }
 
     /**
-     * Returns the timestamp of the {@code Metric}.
+     * Returns the metric dimensions.
      *
-     * @return the metric timestamp
+     * @return the metric dimensions
      */
-    public String getHostName() {
-        return hostName;
+    public Dimension[] getDimensions() {
+        return dimensions;
     }
 
     /**
@@ -147,45 +156,44 @@ public class Metric {
      * @return a {@code Metric} object
      */
     public static Metric parse(String line) {
-        // DiskAvailable.Gigabytes:311|#Level:Host|#hostname:localhost,1650953744320,request_id
+        // DiskAvailable.Gigabytes:311|#Host:localhost|1650953744320
         Matcher matcher = PATTERN.matcher(line);
         if (!matcher.matches()) {
             return null;
         }
 
-        Metric metric =
-                new Metric(
-                        matcher.group(1),
-                        matcher.group(2),
-                        matcher.group(3),
-                        matcher.group(5),
-                        matcher.group(6));
-
-        String dimensions = matcher.group(4);
-        if (dimensions != null) {
-            String[] dimension = dimensions.split(",");
-            List<Dimension> list = new ArrayList<>(dimension.length);
-            for (String dime : dimension) {
-                String[] pair = dime.split(":");
+        String metricName = matcher.group(1);
+        String unit = matcher.group(2);
+        String value = matcher.group(3);
+        String dimension = matcher.group(4);
+        String timestamp = matcher.group(5);
+
+        Dimension[] dimensions = null;
+        if (dimension != null) {
+            String[] dims = dimension.split(",");
+            dimensions = new Dimension[dims.length];
+            int index = 0;
+            for (String dim : dims) {
+                String[] pair = dim.split(":");
                 if (pair.length == 2) {
-                    list.add(new Dimension(pair[0], pair[1]));
+                    dimensions[index++] = new Dimension(pair[0], pair[1]);
                 }
             }
-            metric.dimensions = list;
         }
 
-        return metric;
+        return new Metric(metricName, value, unit, timestamp, dimensions);
     }
 
     /** {@inheritDoc} */
     @Override
     public String toString() {
         StringBuilder sb = new StringBuilder(128);
-        sb.append(metricName).append('.').append(unit).append(':').append(value).append("|#");
+        sb.append(metricName).append('.').append(unit).append(':').append(value);
         if (dimensions != null) {
             boolean first = true;
             for (Dimension dimension : dimensions) {
                 if (first) {
+                    sb.append("|#");
                     first = false;
                 } else {
                     sb.append(',');
@@ -193,8 +201,9 @@ public class Metric {
                 sb.append(dimension.getName()).append(':').append(dimension.getValue());
             }
         }
-        sb.append("|#hostname:").append(hostName);
-        sb.append(',').append(timestamp);
+        if (timestamp != null) {
+            sb.append('|').append(timestamp);
+        }
         return sb.toString();
     }
 


Test patch:
diff --git a/api/src/test/java/ai/djl/metric/MetricsTest.java b/api/src/test/java/ai/djl/metric/MetricsTest.java
index 6fec58d51..8c2e9e03e 100644
--- a/api/src/test/java/ai/djl/metric/MetricsTest.java
+++ b/api/src/test/java/ai/djl/metric/MetricsTest.java
@@ -34,7 +34,7 @@ public class MetricsTest {
         p50 = metrics.percentile("m2", 50);
         Assert.assertEquals(p50.getValue().floatValue(), 2f);
 
-        metrics.addMetric(new Metric("m3", 1d));
+        metrics.addMetric(new Metric("m3", 1d, Unit.NONE, new Dimension("model", "mlp")));
         metrics.addMetric("m3", 3d, Unit.COUNT);
         metrics.addMetric("m3", 2d);
         p50 = metrics.percentile("m3", 50);
@@ -59,17 +59,18 @@ public class MetricsTest {
 
     @Test
     public void testParseMetrics() {
-        String line = "Disk.Gigabytes:311|#Level:Host|#hostname:localhost,1650953744320";
+        String line = "Disk.Gigabytes:311|#Host:localhost,Model:resnet|1650953744320";
         Metric metric = Metric.parse(line);
         Assert.assertNotNull(metric);
         Assert.assertEquals(metric.getValue().intValue(), 311);
         Assert.assertEquals(metric.getTimestamp(), "1650953744320");
         Assert.assertEquals(metric.getUnit(), Unit.GIGABYTES);
-        Assert.assertEquals(metric.getHostName(), "localhost");
+        Assert.assertEquals(metric.getDimensions()[0].getName(), "Host");
+        Assert.assertEquals(metric.getDimensions()[0].getValue(), "localhost");
         Assert.assertEquals(line, metric.toString());
 
-        Assert.assertNull(Metric.parse("DiskAvailable.Gigabytes:311"));
-        Metric invalid = Metric.parse("Disk.InvalidUnits:311|#Level:Host|#hostname:localhost,1,X");
+        Assert.assertNull(Metric.parse("DiskAvailable.Gigabytes:311#"));
+        Metric invalid = Metric.parse("Disk.InvalidUnits:311");
         Assert.assertNotNull(invalid);
         Assert.expectThrows(IllegalArgumentException.class, invalid::getUnit);
     }