jmc 7455: Add support for jolokia JMX service connection

Setting back for review. The azure problem requires a fix in Jolokia.

Make use of support for JMX service connection in jolokia 1.7.0 and later to connect to JVMs over this protocol.

Progress

[x] Commit message must refer to an issue
[ ] Change must be properly reviewed (1 review required, with at least 1 Committer)

Issue

JMC-7455: Add support for jolokia JMX service connection

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jmc pull/332/head:pull/332
$ git checkout pull/332

Update a local copy of the PR:
$ git checkout pull/332
$ git pull https://git.openjdk.org/jmc pull/332/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 332

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jmc/pull/332.diff

Oct 26 '21 19:10 skarsaune

Hi @skarsaune, welcome to this OpenJDK project and thanks for contributing!

We do not recognize you as Contributor and need to ensure you have signed the Oracle Contributor Agreement (OCA). If you have not signed the OCA, please follow the instructions. Please fill in your GitHub username in the "Username" field of the application. Once you have signed the OCA, please let us know by writing /signed in a comment in this pull request.

If you already are an OpenJDK Author, Committer or Reviewer, please click here to open a new issue so that we can record that fact. Please use "Add GitHub user skarsaune" as summary for the issue.

If you are contributing this work on behalf of your employer and your employer has signed the OCA, please let us know by writing /covered in a comment in this pull request.

Oct 26 '21 19:10 bridgekeeper[bot]

@skarsaune do you have an JMC issue already or do you need help creating one?

Nov 22 '21 15:11 reinhapa

@reinhapa : Please help creating an issue. I do not have access.

Nov 23 '21 08:11 skarsaune

@skarsaune created JMC-7455 change the title to: 7455: Add support for jolokia JMX service connection

Nov 25 '21 21:11 reinhapa

Webrevs

Nov 26 '21 06:11 mlbridge[bot]

I noticed the failed test run. Have used spotbugs to reformat. Re-run please.

Dec 14 '21 20:12 skarsaune

I have corrected the last formatting and rebased with master. When I run the shell scripts from the github action definition on my mac, the first 3 run fine, however in scripts/runagenttests.sh there is a test failure (test-permissions). Should not be related to this PR however.

Dec 15 '21 06:12 skarsaune

Fails on some platforms: "Unrecognized option: -XstartOnFirstThread" , will fix

Dec 15 '21 14:12 skarsaune

⚠️ @skarsaune This pull request contains merges that bring in commits not present in the target repository. Since this is not a "merge style" pull request, these changes will be squashed when this pull request in integrated. If this is your intention, then please ignore this message. If you want to preserve the commit structure, you must change the title of this pull request to Merge <project>:<branch> where <project> is the name of another project in the OpenJDK organization (for example Merge jdk:master).

Jan 09 '22 21:01 openjdk[bot]

It would be good to get a new CI run with the new way of handling OSGi dependencies

Jan 12 '22 16:01 skarsaune

Hi again! Have been missing in action for a bit due to very critical phase at client, but have taken some time off my holiday to figure out a way to test the kubernetes support by mocking. If I could please have a CI test run to see that it works fine in the pipeline. Also rebased and updated to 8.3. cc: @thegreystone

Jul 24 '22 07:07 skarsaune

Yeah, whenever I think I am done done, I think of something more to fix .... Set to draft now. Will probably draw the line in not too many days.

Jul 29 '22 05:07 skarsaune

Hi @skarsaune! The master branch is now JMC 9, and now would be a great opportunity to get this in. Is there anything else you'd like to fix before I look at it again?

Oct 26 '22 17:10 thegreystone

Great @thegreystone . Some actions I must take:

Align with current master
Do some manual testing in jmc after updating
Ideally I would like to see if I can get a new release of Jolokia with some improvements

Edit: The kubernetes test is broken after I rebased onto master, and I haven't found out why yet. Tycho surefire complains that the test class is not found, however the compile stage reports 1 file compiled and I see it both (!) under target/classes AND target/test-classes. Anyone experienced anything like this before?

Oct 27 '22 19:10 skarsaune

Manual tests look good. Solved a problem with stored credentials. Have requested merges and a release on the jolokia side. Still problems running the test though. Would it be possible to request a CI run on this PR?

Dec 06 '22 20:12 skarsaune

Hi @skarsaune! Are you ready to make this PR ready for review?

Regarding the tests, typically the tests would be run in your fork, on commits to your branch. I can try running them on a few OSes locally here though.

Dec 19 '22 22:12 thegreystone

Great if you can run the tests @thegreystone ! The jolokia tests run , but the Kubernetes tests not. They should be set up the same way, but it here is probably a silly error somewhere.
Thanks for checking in ☺️. I just asked for a release of jolokia. I hope to conclude that soon.

Dec 20 '22 21:12 skarsaune

@skarsaune this pull request can not be integrated into master due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:

git checkout jolokia-support
git fetch https://git.openjdk.org/jmc master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push

Dec 26 '22 21:12 openjdk[bot]

Jolokia is updated now. Merged with master and marking ready for review. PR is ready for slaughter 😉 Kuternetes tests still fail when I run ./build.sh --test I need some assistance in understanding that one.

Dec 26 '22 22:12 skarsaune

Found a functional problem with the Kubernetes test after upgrading to jolokia 1.7.2. will investigate ASAP

Edit: fixed https://github.com/openjdk/jmc/pull/332/commits/b33f15be2161c2eeae21890e61359387a1bdbb33

Jan 03 '23 22:01 skarsaune

@thegreystone : I believe the previous review should be addressed. You can have a new look when you have time, and any assistance on running the test would be appreciated.

Jan 14 '23 19:01 skarsaune

@skarsaune Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. See OpenJDK Developers’ Guide for more information.

Oct 06 '23 18:10 openjdk[bot]

@skarsaune This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

Dec 05 '23 01:12 bridgekeeper[bot]

Still troubleshooting Kubernetes tests. Now they run , but fail with class not found on Apache commons http classes

Dec 24 '23 12:12 skarsaune

Hi Martin! We're getting closer to the JMC release date. Is this something you're aiming to get into JMC 9?

Jan 03 '24 18:01 thegreystone

Really interested by the jolokia support in JMC. Will be amazing to have it in the next release ;)

Jan 23 '24 05:01 RoiSoleil

Sorry, missed this email. I'm still struggling with the classloading of the kubernetes tests, and it has been hard to prioritze lately. It should be reproducible running ./build.sh --test on both linux and macos.

On Wed, Jan 3, 2024 at 7:26 PM Marcus Hirt @.***> wrote:

Hi Martin! We're getting closer to the JMC release date. Is this something you're aiming to get into JMC 9?

— Reply to this email directly, view it on GitHub https://github.com/openjdk/jmc/pull/332#issuecomment-1875787578, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABQB2GSKG654E3LAYEABNSLYMWPDHAVCNFSM5GYUGXXKU5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TCOBXGU3TQNZVG44A . You are receiving this because you were mentioned.Message ID: @.***>

Jan 23 '24 06:01 skarsaune

Wondering if it is easiest to simply rip the PR in 2. Then we could merge joloka and struggle on with Kubernetes 😅

On Tue, Jan 23, 2024 at 7:25 AM Martin Skarsaune @.***> wrote:

Sorry, missed this email. I'm still struggling with the classloading of the kubernetes tests, and it has been hard to prioritze lately. It should be reproducible running ./build.sh --test on both linux and macos.

On Wed, Jan 3, 2024 at 7:26 PM Marcus Hirt @.***> wrote:

Hi Martin! We're getting closer to the JMC release date. Is this something you're aiming to get into JMC 9?

— Reply to this email directly, view it on GitHub https://github.com/openjdk/jmc/pull/332#issuecomment-1875787578, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABQB2GSKG654E3LAYEABNSLYMWPDHAVCNFSM5GYUGXXKU5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TCOBXGU3TQNZVG44A . You are receiving this because you were mentioned.Message ID: @.***>

Jan 24 '24 07:01 skarsaune

replaced by https://github.com/openjdk/jmc/pull/548

Jan 25 '24 17:01 skarsaune

jmc jmc copied to clipboard

7455: Add support for jolokia JMX service connection

Progress

Issue

Reviewing

Webrevs

jmc
jmc copied to clipboard