-
Notifications
You must be signed in to change notification settings - Fork 6.3k
8365400: enhance JFR to emit file and module metadata for class loading #27656
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.
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
Conversation
👋 Welcome back larry-cable! A progress list of the required criteria for merging this PR into |
❗ This change is not yet ready to be integrated. |
@larry-cable The following labels will be automatically applied to this pull request:
When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command. |
Can you please a new test for the added functionality? |
I have not reviewed SecureClassLoader, as this is better done by people who are more knowledgeable in the area. That said, toExternalForm() appears to be a somewhat expensive function, so you may want to add a guard. Also check for null.
Could you also add the event to profile.jfc? Please remove "Name" from the label "Defined Class Name," as the whole class is serialized, and add a test to test/jdk/jdk/jfr/event/runtime. |
sounds good, will do so!
thx
…On 10/6/25 3:33 PM, Erik Gahlin wrote:
*egahlin* left a comment (openjdk/jdk#27656)
<https://urldefense.com/v3/__https://github.com/openjdk/jdk/pull/27656*issuecomment-3374488908__;Iw!!ACWV5N9M2RV99hQ!LsYP65VL4wbG0567H5u0fobZGwD-AhgqyFUXDwC7LTnVDAop8HdvYCAzZLBn4QogqPSZA-LReG2opO6WMvYfxMeLZw$>
I have not reviewed SecureClassLoader, as this is better done by
people who are more knowledgeable in the area. That said,
toExternalForm() appears to be a somewhat expensive function, so you
may want to add a guard. Also check for null.
|ClassFileDefineEvent evt = new ClassFileDefineEvent(); if
(event.shouldCommit()) { event.definedClass = cls; if (cs != null) {
URL location = cs.getLocation(); if (location != null) { event =
location.toExternalForm(); } } event.commit(); } |
Could you also add the event to profile.jfc? Please remove "Name" from
the label "Defined Class Name," as the whole class is serialized, and
add a test to test/jdk/jdk/jfr/event/runtime.
—
Reply to this email directly, view it on GitHub
<https://urldefense.com/v3/__https://github.com/openjdk/jdk/pull/27656*issuecomment-3374488908__;Iw!!ACWV5N9M2RV99hQ!LsYP65VL4wbG0567H5u0fobZGwD-AhgqyFUXDwC7LTnVDAop8HdvYCAzZLBn4QogqPSZA-LReG2opO6WMvYfxMeLZw$>,
or unsubscribe
<https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/ANTA67UO5YZDJZINJIH73T33WLU5PAVCNFSM6AAAAACIN4BNMWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTGNZUGQ4DQOJQHA__;!!ACWV5N9M2RV99hQ!LsYP65VL4wbG0567H5u0fobZGwD-AhgqyFUXDwC7LTnVDAop8HdvYCAzZLBn4QogqPSZA-LReG2opO6WMvY6rHOFuw$>.
You are receiving this because you were mentioned.Message ID:
***@***.***>
--------------H8Nh870r73JozTO7kRSnjvm6
Content-Type: text/html; charset=UTF-8
Content-Transfer-Encoding: 8bit
<!DOCTYPE html><html><head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body>
sounds good, will do so!<br>
<br>
thx<br>
<br>
<br>
<div class="moz-cite-prefix">On 10/6/25 3:33 PM, Erik Gahlin wrote:<br>
</div>
<blockquote type="cite" ***@***.***">
<div style="display: flex; flex-wrap: wrap; white-space: pre-wrap; align-items: center; "><img height="20" width="20" style="border-radius:50%; margin-right: 4px;" decoding="async" src="https://avatars.githubusercontent.com/u/43493071?s=20&v=4" moz-do-not-send="true"><strong>egahlin</strong> left a comment <a href="https://urldefense.com/v3/__https://github.com/openjdk/jdk/pull/27656*issuecomment-3374488908__;Iw!!ACWV5N9M2RV99hQ!LsYP65VL4wbG0567H5u0fobZGwD-AhgqyFUXDwC7LTnVDAop8HdvYCAzZLBn4QogqPSZA-LReG2opO6WMvYfxMeLZw$" moz-do-not-send="true">(openjdk/jdk#27656)</a></div>
<p dir="auto">I have not reviewed SecureClassLoader, as this is
better done by people who are more knowledgeable in the area.
That said, toExternalForm() appears to be a somewhat expensive
function, so you may want to add a guard. Also check for null.</p>
<pre class="notranslate"><code class="notranslate"> ClassFileDefineEvent evt = new ClassFileDefineEvent();
if (event.shouldCommit()) {
event.definedClass = cls;
if (cs != null) {
URL location = cs.getLocation();
if (location != null) {
event = location.toExternalForm();
}
}
event.commit();
}
</code></pre>
<p dir="auto">Could you also add the event to profile.jfc? Please
remove "Name" from the label "Defined Class Name," as the whole
class is serialized, and add a test to
test/jdk/jdk/jfr/event/runtime.</p>
<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br>
Reply to this email directly, <a href="https://urldefense.com/v3/__https://github.com/openjdk/jdk/pull/27656*issuecomment-3374488908__;Iw!!ACWV5N9M2RV99hQ!LsYP65VL4wbG0567H5u0fobZGwD-AhgqyFUXDwC7LTnVDAop8HdvYCAzZLBn4QogqPSZA-LReG2opO6WMvYfxMeLZw$" moz-do-not-send="true">view it on GitHub</a>, or <a href="https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/ANTA67UO5YZDJZINJIH73T33WLU5PAVCNFSM6AAAAACIN4BNMWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTGNZUGQ4DQOJQHA__;!!ACWV5N9M2RV99hQ!LsYP65VL4wbG0567H5u0fobZGwD-AhgqyFUXDwC7LTnVDAop8HdvYCAzZLBn4QogqPSZA-LReG2opO6WMvY6rHOFuw$" moz-do-not-send="true">unsubscribe</a>.<br>
You are receiving this because you were mentioned.<img src="https://github.com/notifications/beacon/ANTA67R5OH4OJJA2F6QE6NL3WLU5PA5CNFSM6AAAAACIN4BNMWWGG33NNVSW45C7OR4XAZNMJFZXG5LFINXW23LFNZ2KUY3PNVWWK3TUL5UWJTWJEKOUY.gif" height="1" width="1" alt="" moz-do-not-send="true"><span style="color: transparent; font-size: 0; display: none; visibility: hidden; overflow: hidden; opacity: 0; width: 0; height: 0; max-width: 0; max-height: 0; mso-hide: all">Message
ID: <span><openjdk/jdk/pull/27656/c3374488908</span><span>@</span><span>github</span><span>.</span><span>com></span></span></p>
<script type="application/ld+json">[
{
***@***.***": "http://schema.org",
***@***.***": "EmailMessage",
"potentialAction": {
***@***.***": "ViewAction",
"target": "#27656 (comment)",
"url": "#27656 (comment)",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
***@***.***": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]</script>
</blockquote>
<br>
</body>
</html>
--------------H8Nh870r73JozTO7kRSnjvm6--
|
What is the scope of the event? I assume it will not work for all classes. Could more be added, or could the limitation be stated somewhere, e.g. Label("Secure Class File Define")? The event name can still be generic. |
@larry-cable would it be possible to provide a summary on why the existing ClassLoad/ClassDefine events can't be updated to add the additional fields? I think it would also be useful to understand why SecureClassLoader has been chosen as the place to record this event. The class loading part of the boot loader is in the VM, not Java. The modified code paths will get used for the platform and app class loaders but it's coin toss with custom class loaders as they might extend ClassLoader directly. Have you looked at the defineClass implementation in the base class (java.lang.ClassLoader)? It already extracts the location from the protection domain so that the VM has the code source. For classes loaded by the boot loader, the VM will generate a code source that is the string representation of a "jrt" or "file" URL. That code source is mostly used by logging (e.g. -Xlog:class+load) and CDS/AOT but I suspect what you are looking for it to get at these in the VM when recording an event. |
while existing class loading JFR events exist, none of these provide the path (if available) from which a class is loaded/defined, nor are they easily modified to do so from a compatibility standpoint.
therefore this ER/PR adds a simple JFR event that encapsulates a tuple of class and path that can be enabled in order to provide an audit/debug trail of locations (path, if available) from which a particular class is loaded/defined.
this association can be used for various "applications" such as basic auditing etc
Progress
Warning
8365400: enhance JFR to emit file and module metadata for class loading
Issue
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/27656/head:pull/27656
$ git checkout pull/27656
Update a local copy of the PR:
$ git checkout pull/27656
$ git pull https://git.openjdk.org/jdk.git pull/27656/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 27656
View PR using the GUI difftool:
$ git pr show -t 27656
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/27656.diff
Using Webrev
Link to Webrev Comment