6204: Flight recorder launcher tab bugs out

[mirage22]

Adding check box to the launcher tab to select Oracle JDK < 11.

---

Progress

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

Issue

• JMC-6204: Flight Recorder launcher tab bugs out

Reviewers

• Alex Macdonald (@aptmac - Committer)
• @bric3 (no known github.com user name / role) ⚠️ Review applies to 09e329f

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 265

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

Using diff file

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

[bridgekeeper]

👋 Welcome back mwengner! 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.

[mlbridge]

Webrevs

• 08: Full - Incremental (9be40714)
• 07: Full - Incremental (c3624160)
• 06: Full - Incremental (e576aa74)
• 05: Full - Incremental (b4ee87cf)
• 04: Full - Incremental (09e329f9)
• 03: Full - Incremental (bf742ee0)
• 02: Full - Incremental (35e5fa73)
• 01: Full - Incremental (309f3ec4)
• 00: Full (bc3f3fe5)

[aptmac]

There were a few grammatical errors, and I made a comment wondering if ConnectionToolkit.isOarcle() could be of any help in automating the process as I see you've added a //TODO to JfrLaunchModel to get information dynamically. If you're adding a TODO here maybe open a JMC bug that can be used to track progress on fixing it.

Is there an easy and reproduce-able way to verify the bug and this fix?

[aptmac]

It took me longer to figure out how to get to this page than I had originally thought. For anyone else curious, here's what it looks like:

Steps:

1. Run as JMC-Eclipse IDE
2. Import a project
3. Highlight project in the package explorer and select "Run as" -> "Run as Java Application with Flight Recorder"
4. Go back to the context menu under the package explorer, but this time go "Run as" -> "Run Configurations.."
5. Find the Launcher under "Java Application", then use the "Flight Recorder" tab at the top

At first observation, compared to master these changes allow for the "Run as Java Application with Flight Recorder" to be used, otherwise I was experiencing and error trying to enable flight recorder features, so that's nice.

However using this changeset I'm hitting a NPE at RecordingWizardPage line 222: https://github.com/openjdk/jmc/blob/master/application/org.openjdk.jmc.flightrecorder.controlpanel.ui/src/main/java/org/openjdk/jmc/flightrecorder/controlpanel/ui/wizards/RecordingWizardPage.java#L222

Running through with the debugger I found that the shell is actually null, so this blows up trying to layout(). Simply removing this line looks to fix it.

Is this what the bug is? I'm trying to select a template in the template manager but nothing seems to be working:

[mirage22]

@bric3 : Thank you ! I've moved them properly to the bundle

[mirage22]

@aptmac it also doesn't allow you to select a template, even when you have created one in previous recording. This is bug and should be IMHO solved in another ticket, what do you think ?

[aptmac]

@aptmac it also doesn't allow you to select a template, even when you have created one in previous recording. This is bug and should be IMHO solved in another ticket, what do you think ?

To be honest I thought from reading the bug description on JMC-6204 that this was the actual issue needing to be solved. It reads:

Usually the template combo box will contain the templates available in the template manager. However, it seems to bug out, especially for JDK 11 templates. Perhaps if the previously used template is no longer available too? Or if trying to select a template using the Template Manager button?

So I see this PR as addressing a separate issue where the addition of commercial features flags was preventing the launcher tab from opening, but JMC-6204 is concerned with the template manager within the launcher. @thegreystone could you confirm?

If you still wanted to go the separate bug route then a new bug could be opened for tracking this PR, and leave 6204 for the template manager work.

For Linux there still needs to be a resolution for this line: https://github.com/openjdk/jmc/blob/master/application/org.openjdk.jmc.flightrecorder.controlpanel.ui/src/main/java/org/openjdk/jmc/flightrecorder/controlpanel/ui/wizards/RecordingWizardPage.java#L222. I'm not sure if that issue reported for Oracle JDK 7.1 is still valid, but to be safe there could also just be a quick check on getShell() to make sure it's not null. As it stands now it's crashing from a NPE and not providing any helpful diagnostic information in the dialog, here's what the error looks like:

[mirage22]

@aptmac I've fixed the commercial flag issue, but seems your are right.
I'll take a look at the templates that should be available in template manager, this should be part of this bugfix

[mirage22]

It took me longer to figure out how to get to this page than I had originally thought. For anyone else curious, here's what it looks like:

Steps:

1. Run as JMC-Eclipse IDE
2. Import a project
3. Highlight project in the package explorer and select "Run as" -> "Run as Java Application with Flight Recorder"
4. Go back to the context menu under the package explorer, but this time go "Run as" -> "Run Configurations.."
5. Find the Launcher under "Java Application", then use the "Flight Recorder" tab at the top

At first observation, compared to master these changes allow for the "Run as Java Application with Flight Recorder" to be used, otherwise I was experiencing and error trying to enable flight recorder features, so that's nice.

However using this changeset I'm hitting a NPE at RecordingWizardPage line 222: https://github.com/openjdk/jmc/blob/master/application/org.openjdk.jmc.flightrecorder.controlpanel.ui/src/main/java/org/openjdk/jmc/flightrecorder/controlpanel/ui/wizards/RecordingWizardPage.java#L222

Running through with the debugger I found that the shell is actually null, so this blows up trying to layout(). Simply removing this line looks to fix it.

Is this what the bug is? I'm trying to select a template in the template manager but nothing seems to be working:

this behavior is caused by wrong schema version (1.0) which comes from JfrLaunchModel, I've changed it now to JDK_11. which should allows at least more JDKs. issue is again Oracle JDK. Trying to figure out how to update properly the configuration. I need to find a way how to override the schema configuration when the RecordingWizard is open. Currently all works when you just configure JFR and push Apply for Oracle JDK 1.8

[mirage22]

I'll open a bug according to the issue with Oracle JDK 1.8 and Template Manager. @aptmac

[mirage22]

Seem this bug is fixed for most of use-cases

[openjdk]

@mirage22 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:

6204: Flight recorder launcher tab bugs out

Reviewed-by: aptmac

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 8 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

Please see this link for an up-to-date comparison between the source branch of this pull request and the master branch.
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.

[mirage22]

/integrate

[openjdk]

Going to push as commit 1d7397e.
Since your change was applied there have been 8 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

Your commit was automatically rebased without conflicts.

[openjdk]

@mirage22 Pushed as commit 1d7397e.

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