Skip to content

8354508: JFR: Strengthen metadata checks for labels #24616

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
egahlin wants to merge 2 commits into from
Closed

8354508: JFR: Strengthen metadata checks for labels #24616

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cf1c0a5
Initial
Apr 14, 2025
5dd5653
Fixes
Apr 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions src/hotspot/share/jfr/metadata/metadata.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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">
Expand Down Expand Up @@ -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">
Expand All @@ -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" />
Expand Down
85 changes: 63 additions & 22 deletions test/jdk/jdk/jfr/event/metadata/TestEventMetadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
*
Expand All @@ -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
* -----
Expand All @@ -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 {
Expand Down Expand Up @@ -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());
Expand All @@ -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");
Expand All @@ -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) {
Expand Down