RFR: 7167: Agent Plugin [v6]

Jie Kang jkang at openjdk.java.net
Tue Jun 1 20:15:12 UTC 2021


On Wed, 28 Apr 2021 17:57:17 GMT, Joshua Matsuoka <jmatsuoka at openjdk.org> wrote:

>> This PR adds the Agent Plugin to JMC. This is a plugin for controlling the jmc agent and provides a way to attach it and add probes to any JVMs in the JVM browser, as well as providing a preset manager for creating/modifying probe presets.
>> 
>> There is still a small amount of cleanup to be done so I'll mark it as a draft for now but it would be great to get a review :)
>
> Joshua Matsuoka has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fix rcp feature to use new agent plugin name

application/org.openjdk.jmc.console.agent/plugin.xml line 46:

> 44:             icon="icons/agent-16.png"
> 45:             id="org.openjdk.jmc.console.ext.agent"
> 46:             description="The JMC Agent Plugin can be used to add JFR instrumentation declaratively to a running program"

Super minor suggestion:

s/add... declaratively/declaratively add .../

application/org.openjdk.jmc.console.agent/pom.xml line 68:

> 66: 							<groupId>sun.jdk</groupId>
> 67: 							<artifactId>tools</artifactId>
> 68: 							<version>1.8.0</version>

There's a `java.version` property defined above in this pom that doesn't seem to be used. Is it meant to be here instead of this magic version number for `sun.jdk tools`?

application/org.openjdk.jmc.console.agent/src/main/java/org/openjdk/jmc/console/agent/actions/AgentEditorOpener.java line 100:

> 98: 				return ret;
> 99: 			} catch (ConnectionException e) {
> 100: 				// FIXME: Show stacktrace? (Need to show our own ExceptionDialog in that case, or maybe create our own DetailsAreaProvider, see WorkbenchStatusDialogManager.setDetailsAreaProvider)

Personally not a fan of PRs that add FIXMEs; can you provide some justification for having this? What's preventing this being done sooner rather than later?

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

PR: https://git.openjdk.java.net/jmc/pull/226


More information about the jmc-dev mailing list