8354508: JFR: Strengthen metadata checks for labels

Reviewed-by: shade

Author: egahlin

src/hotspot/share/jfr/metadata/metadata.xml

@@ -453,10 +453,10 @@
     <Field type="ulong" contentType="bytes" name="regionEndWaste" label="Region End Wasted" description="Total memory wasted at the end of regions due to refill" />
     <Field type="uint" name="regionsRefilled" label="Region Refills" description="Number of regions refilled" />
     <Field type="ulong" name="numPlabsFilled" label="PLAB Fills" description="Number of PLABs filled" />
-    <Field type="ulong" contentType="bytes" name="directAllocated" label="Allocated (direct)" description="Total memory allocated using direct allocation outside of PLABs" />
-    <Field type="ulong" name="numDirectAllocated" label="Direct allocations" description="Number of direct allocations" />
-    <Field type="ulong" contentType="bytes" name="failureUsed" label="Used (failure)" description="Total memory occupied by objects in regions where evacuation failed" />
-    <Field type="ulong" contentType="bytes" name="failureWaste" label="Wasted (failure)" description="Total memory left unused in regions where evacuation failed" />
+    <Field type="ulong" contentType="bytes" name="directAllocated" label="Allocated (Direct)" description="Total memory allocated using direct allocation outside of PLABs" />
+    <Field type="ulong" name="numDirectAllocated" label="Direct Allocations" description="Number of direct allocations" />
+    <Field type="ulong" contentType="bytes" name="failureUsed" label="Used (Failure)" description="Total memory occupied by objects in regions where evacuation failed" />
+    <Field type="ulong" contentType="bytes" name="failureWaste" label="Wasted (Failure)" description="Total memory left unused in regions where evacuation failed" />
   </Type>
 
   <Event name="G1EvacuationYoungStatistics" category="Java Virtual Machine, GC, Detailed" label="G1 Evacuation Statistics for Young" startTime="false"
@@ -523,7 +523,7 @@
   <Event name="PromotionFailed" category="Java Virtual Machine, GC, Detailed" label="Promotion Failed" startTime="false" description="Promotion of an object failed">
     <Field type="uint" name="gcId" label="GC Identifier" relation="GcId" />
     <Field type="CopyFailed" struct="true" name="promotionFailed" label="Promotion Failed Data" />
-    <Field type="Thread" name="thread" label="Running thread" />
+    <Field type="Thread" name="thread" label="Running Thread" />
   </Event>
 
   <Event name="EvacuationFailed" category="Java Virtual Machine, GC, Detailed" label="Evacuation Failed" startTime="false" description="Evacuation of an object failed">
@@ -1007,7 +1007,7 @@
     <Field type="ulong" contentType="bytes" name="nmethodsSize" label="Compilation Resulting Size" />
     <Field type="ulong" contentType="bytes" name="nmethodCodeSize" label="Compilation Resulting Code Size" />
     <Field type="long" contentType="millis" name="peakTimeSpent" label="Peak Time" />
-    <Field type="long" contentType="millis" name="totalTimeSpent" label="Total time" />
+    <Field type="long" contentType="millis" name="totalTimeSpent" label="Total Time" />
   </Event>
 
   <Event name="CompilerConfiguration" category="Java Virtual Machine, Compiler" label="Compiler Configuration" thread="false" period="endChunk" startTime="false">
@@ -1030,10 +1030,10 @@
   <Event name="CodeCacheConfiguration" category="Java Virtual Machine, Code Cache" label="Code Cache Configuration" thread="false" period="endChunk" startTime="false">
     <Field type="ulong" contentType="bytes" name="initialSize" label="Initial Size" />
     <Field type="ulong" contentType="bytes" name="reservedSize" label="Reserved Size" />
-    <Field type="ulong" contentType="bytes" name="nonNMethodSize" label="Non-nmethod Size" />
+    <Field type="ulong" contentType="bytes" name="nonNMethodSize" label="Non-Nmethod Size" />
     <Field type="ulong" contentType="bytes" name="profiledSize" label="Profiled Size" />
-    <Field type="ulong" contentType="bytes" name="nonProfiledSize" label="Non-profiled Size" />
-    <Field type="ulong" contentType="bytes" name="expansionSize" label="Expansion size" />
+    <Field type="ulong" contentType="bytes" name="nonProfiledSize" label="Non-Profiled Size" />
+    <Field type="ulong" contentType="bytes" name="expansionSize" label="Expansion Size" />
     <Field type="ulong" contentType="bytes" name="minBlockLength" label="Minimum Block Length" />
     <Field type="ulong" contentType="address" name="startAddress" label="Start Address" />
     <Field type="ulong" contentType="address" name="reservedTopAddress" label="Reserved Top" />

test/jdk/jdk/jfr/event/metadata/TestEventMetadata.java

@@ -23,6 +23,7 @@
 
 package jdk.jfr.event.metadata;
 
+import java.util.Arrays;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
@@ -51,7 +52,7 @@ public class TestEventMetadata {
      * ----
      *
      * Symbolic name that is used to identify an event, or a field. Referred to
-     * as "id" and "field" in trace.xml-files and @Name in the Java API. If it is
+     * as "name" in metadata.xml and @Name in the Java API. If it is
      * the name of an event, the name should be prefixed "jdk.", which
      * happens automatically for native events.
      *
@@ -61,10 +62,10 @@ public class TestEventMetadata {
      * "allocationRate" for a field. Do not use "_" and don't add the word
      * "Event" to the event name.
      *
-     * Abbreviations should be avoided, but may be acceptable if the name
-     * becomes long, or if it is a well established acronym. Write whole words,
-     * i.e. "allocation" instead of "alloc". The name should not be a reserved
-     * Java keyword, i.e "void" or "class".
+     * Abbreviations, such as info, alloc, num, gen, conf, stat, and evac, should
+     * be avoided. For example, use "allocation" instead of "alloc". Acronyms should be
+     * avoided unless they are well-established. The name should not be a reserved
+     * Java keyword, such as "void" or "class".
      *
      * Label
      * -----
@@ -84,8 +85,8 @@ public class TestEventMetadata {
      * period should not be included.
      *
      *
-     * Do not forget to set proper units for fields, i.e "NANOS", "MILLS",
-     * "TICKSPAN" ,"BYETS", "PECENTAGE" etc. in native and @Timespan, @Timespan
+     * Do not forget to set proper units for fields, such as "NANOS", "MILLIS",
+     * "TICKSPAN", "BYTES", and "PERCENTAGE", in native and @Timespan, @Timespan
      * etc. in Java.
      */
     public static void main(String[] args) throws Exception {
@@ -161,18 +162,63 @@ private static void verifyName(String name) {
     }
 
     private static void verifyLabel(String label) {
+        System.out.println("Verifying label: " + label);
         Asserts.assertNotEquals(label, null, "Label not allowed to be null");
-        Asserts.assertTrue(label.length() > 1, "Name must be at least two characters");
-        Asserts.assertTrue(label.length() < 45, "Label should not exceed 45 characters, use description to explain " + label);
-        Asserts.assertTrue(label.length() == label.trim().length(), "Label should not have trim character at start and end");
-        Asserts.assertTrue(Character.isUpperCase(label.charAt(0)), "Label should start with upper case letter");
-        for (int i = 0; i < label.length(); i++) {
-            char c = label.charAt(i);
-            Asserts.assertTrue(Character.isDigit(c) || Character.isAlphabetic(label.charAt(i)) || c == ' ' || c == '(' || c == ')' || c == '-', "Label should only consist of letters or space, found '" + label.charAt(i)
-                    + "'");
+        Asserts.assertTrue(label.length() > 1, "Label must be at least two characters");
+        Asserts.assertTrue(label.length() <= 45, "Label should not exceed 45 characters, use description to explain");
+        Asserts.assertTrue(label.length() == label.trim().length(), "Label should not have superfluous whitespace at start or end");
+
+        String[] words = label.split(" ");
+        String[] middleWords = words.length > 2 ? Arrays.copyOfRange(words, 1, words.length - 1) : new String[0];
+        String firstWord = words[0];
+        String lastWord = words[words.length - 1];
+        Asserts.assertTrue(isCapitalized(firstWord), "Label should capitalize first word");
+
+        // The isNumeric check is a workaround so "GC Phase Pause Level 1" doesn't fail.
+        if (!isNumeric(lastWord)) {
+            Asserts.assertTrue(isCapitalized(lastWord), "Label should capitalize last word");
+        }
+        for (String word : words) {
+            Asserts.assertFalse(word.endsWith("-") || word.startsWith("-"), "Word in label should not start or end with hyphen");
+            Asserts.assertTrue(word.length() != 0, "Label should not contain superfluous whitespace");
+            if (isCapitalized(word)) {
+                for (String w : word.split("-")) {
+                    Asserts.assertTrue(isCapitalized(w), "Label should capitalize all words in a hyphenated word");
+                }
+            }
+        }
+        for (String word : middleWords) {
+            if (isShortCommonPreposition(word)) {
+                Asserts.assertFalse(isCapitalized(word), "Preposition in label should be lower case, unless first and last word");
+            }
+        }
+        for (char c : label.toCharArray()) {
+            Asserts.assertTrue(isAllowedCharacter(c), "Label should only consist of letters, numbers, hyphens, parentheses or whitespace, found '" + c + "'");
         }
     }
 
+    private static boolean isAllowedCharacter(char c) {
+        return Character.isDigit(c) || Character.isAlphabetic(c) || c == ' ' || c == '(' || c == ')' || c == '-';
+    }
+
+    private static boolean isCapitalized(String word) {
+        String w = word.replace("(", "").replace(")", "");
+        return !w.isEmpty() && Character.isUpperCase(w.charAt(0));
+    }
+
+    private static boolean isNumeric(String word) {
+        return word.chars().allMatch(Character::isDigit);
+    }
+
+    private static boolean isShortCommonPreposition(String word) {
+        String[] prepositions = { "in", "on", "at", "by", "to", "of" };
+        return containsWord(prepositions, word);
+    }
+
+    private static boolean containsWord(String[] words, String match) {
+        return Arrays.asList(words).contains(match);
+    }
+
     private static void verifyEventType(EventType eventType) {
         System.out.println("Verifying event: " + eventType.getName());
         verifyDescription(eventType.getDescription());
@@ -182,7 +228,7 @@ private static void verifyEventType(EventType eventType) {
         String name = eventType.getName().substring(EventNames.PREFIX.length());
         Asserts.assertFalse(isReservedKeyword(name),"Name must not be reserved keyword in the Java language (" + name + ")");
         checkCommonAbbreviations(name);
-          char firstChar = name.charAt(0);
+        char firstChar = name.charAt(0);
         Asserts.assertFalse(name.contains("ID"), "'ID' should not be used in name, consider using 'Id'");
         Asserts.assertTrue(Character.isAlphabetic(firstChar), "Name " + name + " must start with a character");
         Asserts.assertTrue(Character.isUpperCase(firstChar), "Name " + name + " must start with upper case letter");
@@ -198,12 +244,7 @@ static boolean isReservedKeyword(String s) {
                 "abstract", "assert", "boolean", "break", "byte", "case", "catch", "char", "class", "const", "continue", "default", "do", "double", "else", "enum",
                 "extends", "false", "final", "finally", "float", "for", "goto", "if", "implements", "import", "instanceof", "int", "interface", "long", "native", "new", "null", "package", "private",
                 "protected", "public", "return", "short", "static", "strictfp", "super", "switch", "synchronized", "this", "throw", "throws", "transient", "true", "try", "void", "volatile", "while" };
-        for (int i = 0; i < keywords.length; i++) {
-            if (s.equals(keywords[i])) {
-                return true;
-            }
-        }
-        return false;
+        return containsWord(keywords, s);
     }
 
     private static void checkCommonAbbreviations(String name) {