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