RFR: 8257967: JFR: Event for loaded agents

Erik Gahlin egahlin at openjdk.org
Fri Feb 10 11:21:30 UTC 2023


Could I have a review of an event for native and Java agents. 

Testing: 
- tier1
- tier2
- jdk/jdk/jfr/*
- 100 * TestLoadedAgent.java

Rationale for event fields:
- name: to identify problematic third party agents
- options: to identify problematic options, such as too generic filters or conflicting port numbers, that could impact application behavior
- dynamic: if an agent was loaded by a user using jcmd, which could explain why problem only occur on some server instances
- java: if the agent is native, it could explain crashes
- loadTime: to understand if application problem correlates with the time the agent was loaded

Alternatives:
- I considered making a non-periodic event that is emitted when the agent is loaded, but agents loaded at startup are then unlikely to make it into the recording.
- If it is a JPLIS agent, the jar name is used instead of "instrument". This is likely what users expect when using VirtualMachine::loadAgent(name, options) API or -javaagent:name=options
- I considered making all accesses to the agentLibrary list protected by a mutex, but it could potentially lead to deadlocks when non-JFR code iterates over the list. I think this is better fixed as a separate issue, if deemed necessary. So far so good. JFR iterates the list at every chunk rotation and not from the attach thread, so the risk for an unsafe access is real and synchronization is needed.
- When a JPLIS agent was loaded by attach, it wasn't detected properly, so I added a name check so I could pass true to the constructor and make TestLoadedAgent pass. It would possible to make all detection of JPLIS using the name "instrument" and not rely on a boolean in the constructor, but I deemed it outside the scope for the enhancement.
- I considered using Ticks::now() instead of os::javaTimeMillis() as time source, but TSC is not initialized this early. It could perhaps be fixed by moving the initialization earlier, but it might have other side effects, and is better done outside this enhancement. 


Thanks
Erik

-------------

Commit messages:
 - Change description
 - Change description
 - Use os::javaTimeMillis()
 - Use jio_snprintf
 - Remove duration
 - Remove printf
 - Remove test of smiley
 - Remove smiley test as it doesn't work on Windows
 - Cleanup
 - Rename test
 - ... and 2 more: https://git.openjdk.org/jdk/compare/27126157...ec990b5d

Changes: https://git.openjdk.org/jdk/pull/12460/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=12460&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8257967
  Stats: 290 lines in 13 files changed: 283 ins; 0 del; 7 mod
  Patch: https://git.openjdk.org/jdk/pull/12460.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/12460/head:pull/12460

PR: https://git.openjdk.org/jdk/pull/12460


More information about the hotspot-dev mailing list