RFR: 8257967: JFR: Event for loaded agents

[David Holmes]

On Tue, 7 Feb 2023 18:43:32 GMT, Erik Gahlin <egahlin at openjdk.org> wrote:

> 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

@egahlin  this PR targets JDK-8257967, but that was already fixed by https://github.com/openjdk/jdk/pull/12923. I think that PR used the wrong bug id! @mgronlun

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

PR Comment: https://git.openjdk.org/jdk/pull/12460#issuecomment-1515643848