7167: Agent Plugin #226

Josh-Matsuoka · 2021-03-10T22:58:07Z

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 :)

Progress

Commit message must refer to an issue
Change must be properly reviewed

Issue

JMC-7167: Agent Plugin

Reviewers

Marcus Hirt (@thegreystone - Reviewer)
@bric3 (no known github.com user name / role)
Jie Kang (@jiekang - Author)

Contributors

Alex Macdonald <aptmac@openjdk.org>
Jessye Coleman-Shapiro <jescolem@openjdk.org>
Kangcheng Xu <kxu@openjdk.org>

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jmc pull/226/head:pull/226
$ git checkout pull/226

Update a local copy of the PR:
$ git checkout pull/226
$ git pull https://git.openjdk.java.net/jmc pull/226/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 226

View PR using the GUI difftool:
$ git pr show -t 226

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jmc/pull/226.diff

bridgekeeper · 2021-03-10T22:59:06Z

👋 Welcome back jmatsuoka! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

Josh-Matsuoka · 2021-03-10T23:01:33Z

/contributor add aptmac
/contributor add jescolem
/contributor add kxu

openjdk · 2021-03-10T23:01:56Z

@Josh-Matsuoka
Contributor Alex Macdonald <aptmac@openjdk.org> successfully added.

openjdk · 2021-03-10T23:02:12Z

@Josh-Matsuoka
Contributor Jessye Coleman-Shapiro <jescolem@openjdk.org> successfully added.

openjdk · 2021-03-10T23:02:54Z

@Josh-Matsuoka
Contributor Kangcheng Xu <kxu@openjdk.org> successfully added.

application/org.openjdk.jmc.console.ext.agent/plugin.xml

thegreystone · 2021-03-12T12:09:54Z

New plug-ins must have both 16x16 and 32x32 pixel versions of icons, and preferably also either a vector version, or a higher resolution variant checked into jmc-graphics. Let me know if you need help inventing something. :)

Josh-Matsuoka · 2021-03-12T15:46:51Z

I see. Do you have any ideas/preferences for what the icon should look like for the agent plugin?

thegreystone · 2021-03-15T14:56:49Z

I see. Do you have any ideas/preferences for what the icon should look like for the agent plugin?

Hm. We've been talking about extending the agent with other stuff (i.e. not directly related to the JFR), so something JMC-ish? Rocket with sunglasses (since all agents have sunglasses)? Or just an agent icon (hat + sunglasses?).

mlbridge · 2021-03-29T18:36:39Z

Webrevs

thegreystone · 2021-03-31T17:15:59Z

/reviewers 2

openjdk · 2021-03-31T17:16:35Z

@thegreystone
The number of required reviews for this PR is now set to 2 (with at least 1 of role committers).

application/org.openjdk.jmc.console.ext.agent/META-INF/MANIFEST.MF

thegreystone · 2021-04-19T17:42:07Z

Since this is part of the core rcp distribution (not an optional downloadable plug-in), I think you can skip the "ext".

thegreystone · 2021-04-27T20:19:52Z

Failing validation checks.

Josh-Matsuoka · 2021-06-15T17:09:28Z

@mirage22 @bric3 I've updated the PR, there were a couple of spots that got missed by eclipse when I was renaming everything and removing the exts. For whatever reason my local build worked but making a fresh build didn't, so I missed that, apologies. It's been fixed now and should work as expected

Josh-Matsuoka · 2021-06-15T17:10:33Z

I've a seems replacing the current agent didn't work, Is this desired behaviour ?

This sounds like a possible agent bug, I'll take a look and get back to you on that

mirage22 · 2021-06-15T20:22:30Z

@Josh-Matsuoka seems build works nicelly ! Thank you but the "org.openjdk.jmc.console.agen" module is still not properly displayed in side the Eclipse as Java project, sadly, there is something some where missing.

btw: agent definition on void methods without inputs ()V , works properly, I've done a mistake.

bric3 · 2021-06-17T09:24:53Z

@Josh-Matsuoka I will look at it by the end of this week.

…apturing

bric3

So I build upon you latest change ( 8f8398e ), and indeed this fixes most of the issues.

I have noted a few additional things :

For example if I open the agent on JMC itself

The help button on the dialog with the question mark does nothing.

If I only give the agent jar path eg : agent/target/org.openjdk.jmc.agent-1.0.0-SNAPSHOT.jar

Then the agent editor is empty:

Then doing something silly I clicked on the save preset and SAX error when saving as the preset is empty.

org.xml.sax.SAXParseException; lineNumber: 1; columnNumber: 1; Premature end of file.
	at java.xml/com.sun.org.apache.xerces.internal.jaxp.validation.Util.toSAXParseException(Util.java:75)
	at java.xml/com.sun.org.apache.xerces.internal.jaxp.validation.StreamValidatorHelper.validate(StreamValidatorHelper.java:178)
	at java.xml/com.sun.org.apache.xerces.internal.jaxp.validation.ValidatorImpl.validate(ValidatorImpl.java:115)
	at org.openjdk.jmc.console.agent.utils.ProbeValidator.validate(ProbeValidator.java:86)
	at java.xml/javax.xml.validation.Validator.validate(Validator.java:124)
	at org.openjdk.jmc.console.agent.manager.model.Preset.deserialize(Preset.java:147)
	at org.openjdk.jmc.console.agent.editor.AgentEditorUi.savePreset(AgentEditorUi.java:185)
	at org.openjdk.jmc.console.agent.editor.AgentEditorAction.run(AgentEditorAction.java:57)
	at org.eclipse.jface.action.Action.runWithEvent(Action.java:474)
	at org.eclipse.jface.action.ActionContributionItem.handleWidgetSelection(ActionContributionItem.java:579)
       ...

Is there no way to stop a running agent and restart the agent, so it's possible to give a different configuration. I didn't have the time to explore the agent at this time.

...gent/src/main/java/org/openjdk/jmc/console/agent/raweditor/internal/XmlDocumentProvider.java

Josh-Matsuoka · 2021-06-21T15:56:36Z

The help button on the dialog with the question mark does nothing.

That help button should just be hidden, the textbox hints already tell the user exactly what's needed

If I only give the agent jar path eg : agent/target/org.openjdk.jmc.agent-1.0.0-SNAPSHOT.jar

Then the agent editor is empty:

This is intended behaviour, there are no probes inserted, so there's nothing to display

Then doing something silly I clicked on the save preset and SAX error when saving as the preset is empty.

We should have a more meaningful error if the preset is empty, I'll fix that

Is there no way to stop a running agent and restart the agent, so it's possible to give a different configuration. I didn't have the time to explore the agent at this time.

Not to stop/restart a running agent, but defineEventProbes already reverts existing instrumentation and inserts new probes (i.e. it will override the current configuration).

…help button

Josh-Matsuoka · 2021-06-25T17:49:53Z

@bric3 PR has been updated with the requested changes, let me know what you think when you get some time :)

bric3

Yes it looks good to me.

bric3 · 2021-06-25T21:16:44Z

...njdk.jmc.console.agent/src/main/java/org/openjdk/jmc/console/agent/editor/AgentEditorUi.java

+				IWorkbenchWindow window = PlatformUI.getWorkbench().getActiveWorkbenchWindow();
+				DialogToolkit.showWarning(window.getShell(), Messages.AgentEditorUI_MESSAGE_EMPTY_PRESET_TITLE,
+						Messages.AgentEditorUI_MESSAGE_EMPTY_PRESET);
+			}


bric3 · 2021-06-25T21:23:02Z

....jmc.console.agent/src/main/java/org/openjdk/jmc/console/agent/wizards/StartAgentWizard.java

@@ -61,6 +62,8 @@
 	public StartAgentWizard(AgentJmxHelper helper) {
 		this.helper = helper;
 		startAgentWizardPage = new StartAgentWizardPage(helper);
+		this.setHelpAvailable(false);
+		WizardDialog.setDialogHelpAvailable(false);


Josh-Matsuoka · 2021-06-28T21:33:56Z

@Gunde mind taking another look when you get some time? :)

openjdk · 2021-06-30T20:48:11Z

@Josh-Matsuoka This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

7167: Agent Plugin

Co-authored-by: Alex Macdonald <aptmac@openjdk.org>
Co-authored-by: Jessye Coleman-Shapiro <jescolem@openjdk.org>
Co-authored-by: Kangcheng Xu <kxu@openjdk.org>
Reviewed-by: hirt, jkang

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 43 new commits pushed to the master branch:

81344ce: 7042: Bring back summary screen for Statistics on Object Allocation outside TLAB.
e838a74: 7304: JMC 8.0.1 fix formatting failure
66685ca: 7188: JMC fails to dump file and gets stuck when flightrecording is attempted on jmxremote connection
ef101c2: 7172: fix spell mistake in secure store class
a3fa120: 6920: UI improvements Reviewed-by: cries, bbanathur, schaturvedi
5f23f54: 6336: Remove Triple DES Cipher in Secure store
ee428c5: 6398: Better JNDI Usage
e34323d: 6141: JMC logs warnings when creating recordings
7f38790: 6967: Add Percentage Column w.r.t 'Profiling Samples' in Thread table
6723a70: 7299: Accessibility Issue - Too low contrast on rule result values
... and 33 more: https://git.openjdk.java.net/jmc/compare/1c008c304e6c6bd707fa1a895f83b948c3730713...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

aptmac · 2021-07-07T18:09:05Z

Thanks for all the reviews everyone, I'll integrate this shortly on behalf of Josh

aptmac · 2021-07-07T18:09:23Z

/integrate

openjdk · 2021-07-07T18:10:11Z

@aptmac Only the author (@Josh-Matsuoka) is allowed to issue the integrate command.

aptmac · 2021-07-07T18:10:39Z

@aptmac Only the author (@Josh-Matsuoka) is allowed to issue the integrate command.

Jokes on me, I'll send him a ping to do so

Josh-Matsuoka · 2021-07-07T18:14:23Z

/integrate

openjdk · 2021-07-07T18:15:39Z

Going to push as commit 2e8a251.
Since your change was applied there have been 44 commits pushed to the master branch:

1d7397e: 6204: Flight recorder launcher tab bugs out
81344ce: 7042: Bring back summary screen for Statistics on Object Allocation outside TLAB.
e838a74: 7304: JMC 8.0.1 fix formatting failure
66685ca: 7188: JMC fails to dump file and gets stuck when flightrecording is attempted on jmxremote connection
ef101c2: 7172: fix spell mistake in secure store class
a3fa120: 6920: UI improvements Reviewed-by: cries, bbanathur, schaturvedi
5f23f54: 6336: Remove Triple DES Cipher in Secure store
ee428c5: 6398: Better JNDI Usage
e34323d: 6141: JMC logs warnings when creating recordings
7f38790: 6967: Add Percentage Column w.r.t 'Profiling Samples' in Thread table
... and 34 more: https://git.openjdk.java.net/jmc/compare/1c008c304e6c6bd707fa1a895f83b948c3730713...master

Your commit was automatically rebased without conflicts.

openjdk · 2021-07-07T18:15:58Z

@Josh-Matsuoka Pushed as commit 2e8a251.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

7167: Agent Plugin

968bb1d

thegreystone reviewed Mar 12, 2021

View reviewed changes

application/org.openjdk.jmc.console.ext.agent/plugin.xml Outdated Show resolved Hide resolved

Josh-Matsuoka added 4 commits March 29, 2021 13:57

Clean up spotbugs excludes

614abee

Updating copyright headers

6113db5

Merge remote-tracking branch 'upstream/master' into agent-plugin

404e75b

Update with jmc-graphics icon for agent/agent plugn

789298d

Josh-Matsuoka marked this pull request as ready for review March 29, 2021 18:32

openjdk bot added the rfr label Mar 29, 2021

thegreystone requested review from Gunde and guruhb April 9, 2021 14:40

thegreystone reviewed Apr 19, 2021

View reviewed changes

application/org.openjdk.jmc.console.ext.agent/META-INF/MANIFEST.MF Outdated Show resolved Hide resolved

Josh-Matsuoka added 3 commits April 27, 2021 09:19

Removing ext from agent plugin identifier, fixing bundle version

a87cbb7

Fixing build issues, cleanup

11b1fb1

cleanup

cebd5e4

Josh-Matsuoka added 2 commits April 28, 2021 13:23

Fix missed renames

e785217

Fix rcp feature to use new agent plugin name

411dc18

thegreystone approved these changes May 20, 2021

View reviewed changes

Josh-Matsuoka added 2 commits June 15, 2021 12:44

Bring Preset logging in line with rest of agent plugin

c1afb84

Fixing formatting

558713d

Resolving an issue with generated XML for field capturing/parameter c…

8f8398e

…apturing

bric3 reviewed Jun 20, 2021

View reviewed changes

...gent/src/main/java/org/openjdk/jmc/console/agent/raweditor/internal/XmlDocumentProvider.java Outdated Show resolved Hide resolved

Josh-Matsuoka added 2 commits June 22, 2021 18:03

Adding warning when attempting to save empty preset, removing unused …

7772c2f

…help button

Decorating outputstreamWriter with a bufferedWriter

dfcfad1

bric3 approved these changes Jun 25, 2021

View reviewed changes

jiekang approved these changes Jun 30, 2021

View reviewed changes

openjdk bot added the ready label Jun 30, 2021

thegreystone self-requested a review July 7, 2021 17:07

thegreystone approved these changes Jul 7, 2021

View reviewed changes

openjdk bot closed this Jul 7, 2021

openjdk bot added integrated and removed ready rfr labels Jul 7, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

7167: Agent Plugin #226

7167: Agent Plugin #226

Josh-Matsuoka commented Mar 10, 2021 •

edited by openjdk bot

Loading

bridgekeeper bot commented Mar 10, 2021

Josh-Matsuoka commented Mar 10, 2021

openjdk bot commented Mar 10, 2021

openjdk bot commented Mar 10, 2021

openjdk bot commented Mar 10, 2021

thegreystone commented Mar 12, 2021 •

edited

Loading

Josh-Matsuoka commented Mar 12, 2021

thegreystone commented Mar 15, 2021

mlbridge bot commented Mar 29, 2021 •

edited

Loading

thegreystone commented Mar 31, 2021

openjdk bot commented Mar 31, 2021

thegreystone commented Apr 19, 2021

thegreystone commented Apr 27, 2021

Josh-Matsuoka commented Jun 15, 2021

Josh-Matsuoka commented Jun 15, 2021

mirage22 commented Jun 15, 2021 •

edited

Loading

bric3 commented Jun 17, 2021

bric3 left a comment

Josh-Matsuoka commented Jun 21, 2021

Josh-Matsuoka commented Jun 25, 2021

bric3 left a comment

bric3 Jun 25, 2021

bric3 Jun 25, 2021

Josh-Matsuoka commented Jun 28, 2021

openjdk bot commented Jun 30, 2021 •

edited

Loading

aptmac commented Jul 7, 2021

aptmac commented Jul 7, 2021

openjdk bot commented Jul 7, 2021

aptmac commented Jul 7, 2021

Josh-Matsuoka commented Jul 7, 2021

openjdk bot commented Jul 7, 2021

openjdk bot commented Jul 7, 2021

7167: Agent Plugin #226

7167: Agent Plugin #226

Conversation

Josh-Matsuoka commented Mar 10, 2021 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Contributors

Reviewing

bridgekeeper bot commented Mar 10, 2021

Josh-Matsuoka commented Mar 10, 2021

openjdk bot commented Mar 10, 2021

openjdk bot commented Mar 10, 2021

openjdk bot commented Mar 10, 2021

thegreystone commented Mar 12, 2021 • edited Loading

Josh-Matsuoka commented Mar 12, 2021

thegreystone commented Mar 15, 2021

mlbridge bot commented Mar 29, 2021 • edited Loading

Webrevs

thegreystone commented Mar 31, 2021

openjdk bot commented Mar 31, 2021

thegreystone commented Apr 19, 2021

thegreystone commented Apr 27, 2021

Josh-Matsuoka commented Jun 15, 2021

Josh-Matsuoka commented Jun 15, 2021

mirage22 commented Jun 15, 2021 • edited Loading

bric3 commented Jun 17, 2021

bric3 left a comment

Choose a reason for hiding this comment

Josh-Matsuoka commented Jun 21, 2021

Josh-Matsuoka commented Jun 25, 2021

bric3 left a comment

Choose a reason for hiding this comment

bric3 Jun 25, 2021

Choose a reason for hiding this comment

bric3 Jun 25, 2021

Choose a reason for hiding this comment

Josh-Matsuoka commented Jun 28, 2021

openjdk bot commented Jun 30, 2021 • edited Loading

aptmac commented Jul 7, 2021

aptmac commented Jul 7, 2021

openjdk bot commented Jul 7, 2021

aptmac commented Jul 7, 2021

Josh-Matsuoka commented Jul 7, 2021

openjdk bot commented Jul 7, 2021

openjdk bot commented Jul 7, 2021

Josh-Matsuoka commented Mar 10, 2021 •

edited by openjdk bot

Loading

thegreystone commented Mar 12, 2021 •

edited

Loading

mlbridge bot commented Mar 29, 2021 •

edited

Loading

mirage22 commented Jun 15, 2021 •

edited

Loading

openjdk bot commented Jun 30, 2021 •

edited

Loading