cli: Fix for display of name column

Author: rob-bygrave

cli/src/main/java/org/ebean/monitor/cli/Charts.java

@@ -42,12 +42,9 @@ static String unit(TopCommand.Sort sort) {
     };
   }
 
-  /** A row's display label: the group value, falling back to the label tag. */
+  /** A row's single-line chart identity: {@code name:label}, with fallbacks. */
   private static String rowLabel(TopGroup r) {
-    if (r.group() != null) {
-      return r.group();
-    }
-    return r.label() == null ? "" : r.label();
+    return Display.chartLabel(r);
   }
 
   /**

cli/src/main/java/org/ebean/monitor/cli/Display.java

@@ -0,0 +1,37 @@
+package org.ebean.monitor.cli;
+
+import org.ebean.monitor.v1.model.TopGroup;
+
+/**
+ * Shared rendering of a metric's display identity.
+ *
+ * <p>In the v2 metric model the family {@code name} is the primary identity
+ * (e.g. {@code ebean.txn}, {@code ebean.query}) and {@code label} is the
+ * secondary discriminator (e.g. {@code readonly}, {@code DMessage.findMessages}).
+ * Tables render these as separate {@code NAME} and {@code LABEL} columns; where a
+ * single column is required (a chart bar) they are joined as {@code name:label}.
+ */
+final class Display {
+
+  private Display() {
+  }
+
+  /** Single-line identity for charts: {@code name:label}, with sensible fallbacks. */
+  static String chartLabel(TopGroup r) {
+    return join(r.name(), r.label(), r.group());
+  }
+
+  /** Join a metric's {@code name} and {@code label} into a single {@code name:label} line. */
+  static String join(String name, String label, String fallback) {
+    if (name == null || name.isBlank()) {
+      if (label != null && !label.isBlank()) {
+        return label;
+      }
+      return fallback == null ? "" : fallback;
+    }
+    if (label == null || label.isBlank() || label.equals(name)) {
+      return name;
+    }
+    return name + ":" + label;
+  }
+}

cli/src/main/java/org/ebean/monitor/cli/Interactive.java

@@ -34,20 +34,27 @@
 final class Interactive {
 
   private static final int BAR_WIDTH = 24;
+  private static final int NAME_MAX = 40;
   private static final int LABEL_MAX = 70;
   private static final int HASH_SHORT = 12;
 
   /** Which ranked list is being shown — drives the per-mode columns. */
   enum Mode { TOP, MISSING }
 
-  /** A flattened ranked row: enough to render and to drive the row actions. */
-  record Row(String app, String hash, String label, double value, String unit,
+  /**
+   * A flattened ranked row: enough to render and to drive the row actions.
+   *
+   * <p>{@code name} is the v2 metric family (display only). {@code label} carries
+   * the aggregation dimension value and doubles as the drill-down filter key, so it
+   * must stay the raw value (not a name-decorated display string).
+   */
+  record Row(String app, String hash, String name, String label, double value, String unit,
              long count, long total, long mean, long max,
              boolean planCapable, Long captureCount, String lastCaptured) {
 
     /** Compact constructor for tests that only need the identity + ranked value. */
     Row(String app, String hash, String label, double value, String unit) {
-      this(app, hash, label, value, unit, 0, 0, 0, 0, false, null, null);
+      this(app, hash, null, label, value, unit, 0, 0, 0, 0, false, null, null);
     }
   }
 
@@ -212,7 +219,7 @@ private static List<Row> toTopRows(List<TopGroup> src, By by) {
     List<Row> out = new ArrayList<>(src.size());
     for (TopGroup r : src) {
       String label = r.label() != null ? r.label() : r.group();
-      out.add(new Row(r.app(), r.key(), label, measure(r, by), unit,
+      out.add(new Row(r.app(), r.key(), r.name(), label, measure(r, by), unit,
           nz(r.count()), nz(r.totalMicros()), nz(r.meanMicros()), nz(r.maxMicros()),
           Boolean.TRUE.equals(r.planCapable()), null, null));
     }
@@ -223,7 +230,7 @@ private static List<Row> toMissingRows(List<MissingPlanMetric> src, By by) {
     String unit = by.unit();
     List<Row> out = new ArrayList<>(src.size());
     for (MissingPlanMetric m : src) {
-      out.add(new Row(m.app(), m.key(), m.label(), measure(m, by), unit,
+      out.add(new Row(m.app(), m.key(), null, m.label(), measure(m, by), unit,
           m.count(), m.totalMicros(), m.meanMicros(), m.maxMicros(),
           false, m.captureCount(), m.lastCapturedAt() == null ? "never" : m.lastCapturedAt().toString()));
     }
@@ -512,7 +519,7 @@ private boolean drillGroup(Row group) {
         return true;
       }
       if (!hashRows.isEmpty()) {
-        return drillLoop(group.label(), toTopRows(hashRows, by));
+        return drillLoop(Display.join(group.name(), group.label(), group.label()), toTopRows(hashRows, by));
       }
     }
     // No windowed activity (or no drill fetcher): list the catalog so the
@@ -531,9 +538,9 @@ private boolean drillGroup(Row group) {
     final List<Row> rows = new ArrayList<>(metrics.size());
     for (AppMetric m : metrics) {
       final String lbl = m.label() != null ? m.label() : m.name();
-      rows.add(new Row(app, m.key(), lbl, 0, "us", 0, 0, 0, 0, true, null, null));
+      rows.add(new Row(app, m.key(), m.name(), lbl, 0, "us", 0, 0, 0, 0, true, null, null));
     }
-    return drillLoop(group.label(), rows);
+    return drillLoop(Display.join(group.name(), group.label(), group.label()), rows);
   }
 
   /** A nested list of the individual queries under a drilled group. */
@@ -990,6 +997,7 @@ private List<Col> preColumns() {
     if (showApp) {
       cols.add(new Col("APP", false, r -> r.app() == null ? "" : r.app()));
     }
+    cols.add(new Col("NAME", false, r -> r.name() == null ? "" : tail(r.name(), NAME_MAX)));
     cols.add(new Col("LABEL", false, r -> tail(r.label(), LABEL_MAX)));
     cols.add(new Col("HASH", false, r -> shortHash(r.hash())));
     cols.add(new Col("COUNT", true, r -> num(r.count())));

cli/src/main/java/org/ebean/monitor/cli/TopCommand.java

@@ -1,6 +1,7 @@
 package org.ebean.monitor.cli;
 
 import java.util.List;
+import java.util.Locale;
 import java.util.concurrent.Callable;
 
 import org.ebean.monitor.v1.model.TopGroup;
@@ -115,7 +116,6 @@ public Integer call() {
     if (env == null) {
       env = ConfigDefaults.envOrNull();
     }
-    final boolean byHash = "hash".equalsIgnoreCase(by);
     final boolean gauge = sort == Sort.value;
     try (Insight insight = Insight.open(conn)) {
       java.util.function.Function<String, List<TopGroup>> fetch = ob -> (app == null)
@@ -140,57 +140,88 @@ public Integer call() {
         Charts.printPareto(rows, sort);
         return 0;
       }
-      printTable(rows, byHash, gauge);
+      printTable(rows, gauge);
       return 0;
     }
   }
 
-  private void printTable(List<TopGroup> rows, boolean byHash, boolean gauge) {
+  private void printTable(List<TopGroup> rows, boolean gauge) {
+    final boolean byHash = "hash".equalsIgnoreCase(by);
+    final boolean byName = "name".equalsIgnoreCase(by);
+    // A "tag" dimension is any grouping other than name/label/hash (e.g. type, kind):
+    // NAME and LABEL aren't single-valued, so the tag value gets its own column.
+    final boolean tagDim = !byHash && !byName && !"label".equalsIgnoreCase(by);
+    final String tagHeader = by.toUpperCase(Locale.ROOT);
+
     int appWidth = "APP".length();
-    int groupWidth = (byHash ? "LABEL" : "GROUP").length();
+    int nameWidth = "NAME".length();
+    int labelWidth = "LABEL".length();
+    int tagWidth = tagHeader.length();
     for (TopGroup r : rows) {
       if (r.app() != null) {
         appWidth = Math.max(appWidth, r.app().length());
       }
-      String groupValue = byHash ? (r.label() == null ? r.group() : r.label()) : r.group();
-      if (groupValue != null) {
-        groupWidth = Math.max(groupWidth, groupValue.length());
+      nameWidth = Math.max(nameWidth, nv(r.name()).length());
+      labelWidth = Math.max(labelWidth, nv(r.label()).length());
+      if (tagDim) {
+        tagWidth = Math.max(tagWidth, nv(r.group()).length());
       }
     }
+
+    String idFmt = "%-" + appWidth + "s  %-" + nameWidth + "s  %-" + labelWidth + "s"
+        + (tagDim ? "  %-" + tagWidth + "s" : "");
+
     if (gauge) {
-      String headFmt = "%-" + appWidth + "s  %-" + groupWidth + "s  %16s  %8s  %6s%n";
-      String rowFmt = "%-" + appWidth + "s  %-" + groupWidth + "s  %16s  %8d  %6d%n";
-      System.out.printf(headFmt, "APP", "GROUP", "VALUE", "WINDOW", "HASHES");
+      System.out.printf(idFmt + "  %16s  %8s  %6s%n", idHead(tagDim, tagHeader, "VALUE", "WINDOW", "HASHES"));
       for (TopGroup r : rows) {
-        System.out.printf(rowFmt, nv(r.app()), nv(r.group()),
-            r.value() == null ? "" : String.format("%,.0f", r.value()),
-            r.windowMinutes(), r.hashCount());
+        System.out.printf(idFmt + "  %16s  %8d  %6d%n", idRow(r, tagDim,
+            r.value() == null ? "" : String.format("%,.0f", r.value()), r.windowMinutes(), r.hashCount()));
       }
       return;
     }
     if (byHash) {
-      String headFmt = "%-" + appWidth + "s  %-" + groupWidth + "s  %10s  %16s  %14s  %14s  %8s  %5s  %s%n";
-      String rowFmt = "%-" + appWidth + "s  %-" + groupWidth + "s  %10d  %16d  %14d  %14d  %8d  %5s  %s%n";
-      System.out.printf(headFmt,
-          "APP", "LABEL", "COUNT", "TOTAL(us)", "MEAN(us)", "MAX(us)", "WINDOW", "PLAN", "HASH");
+      System.out.printf(idFmt + "  %10s  %16s  %14s  %14s  %8s  %5s  %s%n",
+          idHead(false, tagHeader, "COUNT", "TOTAL(us)", "MEAN(us)", "MAX(us)", "WINDOW", "PLAN", "HASH"));
       for (TopGroup r : rows) {
-        System.out.printf(rowFmt,
-            nv(r.app()), nv(r.label() == null ? r.group() : r.label()),
+        System.out.printf(idFmt + "  %10d  %16d  %14d  %14d  %8d  %5s  %s%n", idRow(r, false,
             z(r.count()), z(r.totalMicros()), z(r.meanMicros()), z(r.maxMicros()),
-            r.windowMinutes(), Boolean.TRUE.equals(r.planCapable()) ? "yes" : "no", nv(r.key()));
+            r.windowMinutes(), Boolean.TRUE.equals(r.planCapable()) ? "yes" : "no", nv(r.key())));
       }
       return;
     }
-    String headFmt = "%-" + appWidth + "s  %-" + groupWidth + "s  %10s  %16s  %14s  %14s  %8s  %6s%n";
-    String rowFmt = "%-" + appWidth + "s  %-" + groupWidth + "s  %10d  %16d  %14d  %14d  %8d  %6d%n";
-    System.out.printf(headFmt,
-        "APP", "GROUP", "COUNT", "TOTAL(us)", "MEAN(us)", "MAX(us)", "WINDOW", "HASHES");
+    System.out.printf(idFmt + "  %10s  %16s  %14s  %14s  %8s  %6s%n",
+        idHead(tagDim, tagHeader, "COUNT", "TOTAL(us)", "MEAN(us)", "MAX(us)", "WINDOW", "HASHES"));
     for (TopGroup r : rows) {
-      System.out.printf(rowFmt,
-          nv(r.app()), nv(r.group()),
+      System.out.printf(idFmt + "  %10d  %16d  %14d  %14d  %8d  %6d%n", idRow(r, tagDim,
           z(r.count()), z(r.totalMicros()), z(r.meanMicros()), z(r.maxMicros()),
-          r.windowMinutes(), r.hashCount());
+          r.windowMinutes(), r.hashCount()));
+    }
+  }
+
+  /** Header cells: APP, NAME, LABEL, [tag], then the supplied measure headers. */
+  private static Object[] idHead(boolean tagDim, String tagHeader, Object... rest) {
+    List<Object> cells = new java.util.ArrayList<>();
+    cells.add("APP");
+    cells.add("NAME");
+    cells.add("LABEL");
+    if (tagDim) {
+      cells.add(tagHeader);
+    }
+    java.util.Collections.addAll(cells, rest);
+    return cells.toArray();
+  }
+
+  /** Row cells: app, name, label, [tag value], then the supplied measure values. */
+  private static Object[] idRow(TopGroup r, boolean tagDim, Object... rest) {
+    List<Object> cells = new java.util.ArrayList<>();
+    cells.add(nv(r.app()));
+    cells.add(nv(r.name()));
+    cells.add(nv(r.label()));
+    if (tagDim) {
+      cells.add(nv(r.group()));
     }
+    java.util.Collections.addAll(cells, rest);
+    return cells.toArray();
   }
 
   private static String nv(@Nullable String s) {

cli/src/test/java/org/ebean/monitor/cli/InteractiveRenderTest.java

@@ -14,23 +14,24 @@ class InteractiveRenderTest {
 
   private static final String HASH = "a2e2082df04620910f8fa034561b3346";
 
-  private static Interactive.Row top(String app, String hash, String label, long mean) {
-    return new Interactive.Row(app, hash, label, mean, "us", 1000, 5_000_000, mean, 120_000, true, null, null);
+  private static Interactive.Row top(String app, String hash, String name, String label, long mean) {
+    return new Interactive.Row(app, hash, name, label, mean, "us", 1000, 5_000_000, mean, 120_000, true, null, null);
   }
 
   @Test
   void renderList_topMode_showsCoreColumnsValuesAndShortHash() {
     var i = Interactive.forRender(Interactive.Mode.TOP, true);
     List<Interactive.Row> rows = List.of(
-        top("central-access", HASH, "orm.A.find", 93203),
-        top("central-access", "h2", "orm.B.find", 12612));
+        top("central-access", HASH, "ebean.query", "orm.A.find", 93203),
+        top("central-access", "h2", "ebean.query", "orm.B.find", 12612));
 
     String out = i.renderList("Top 2 by mean", rows);
     String header = headerLine(out.split("\n"));
 
-    assertThat(header).contains("#").contains("APP").contains("LABEL")
+    assertThat(header).contains("#").contains("APP").contains("NAME").contains("LABEL")
         .contains("COUNT").contains("TOTAL(us)").contains("MEAN(us)").contains("MAX(us)")
         .contains("PLAN").contains("HASH").contains("chart");
+    assertThat(out).contains("ebean.query");   // v2 family name column
     assertThat(out).contains("orm.A.find");
     assertThat(out).contains("93,203");        // grouped digits, no unit suffix in columns
     assertThat(out).contains("a2e2082df046");  // 12-char short hash
@@ -41,14 +42,14 @@ void renderList_topMode_showsCoreColumnsValuesAndShortHash() {
   @Test
   void renderList_singleApp_hidesAppColumn() {
     var i = Interactive.forRender(Interactive.Mode.TOP, false);
-    String out = i.renderList("Top 1 by mean", List.of(top("central-access", HASH, "orm.A.find", 5)));
+    String out = i.renderList("Top 1 by mean", List.of(top("central-access", HASH, "ebean.query", "orm.A.find", 5)));
     assertThat(headerLine(out.split("\n"))).doesNotContain("APP");
   }
 
   @Test
   void renderList_missingMode_showsCapturesAndCaptured() {
     var i = Interactive.forRender(Interactive.Mode.MISSING, false);
-    Interactive.Row r = new Interactive.Row("app", HASH, "orm.CDriver.driver_all_since",
+    Interactive.Row r = new Interactive.Row("app", HASH, null, "orm.CDriver.driver_all_since",
         0, "us", 0, 0, 0, 0, false, 0L, null);
 
     String out = i.renderList("Missing plans 1 by total", List.of(r));