RFR: 7455: Add support for jolokia JMX service connection [v16]

Alex Macdonald aptmac at openjdk.org
Mon Jun 3 17:45:13 UTC 2024


On Mon, 6 May 2024 13:45:23 GMT, Martin Skarsaune <duke at openjdk.org> wrote:

>>> > Overall this is looking nice. The tests pass and I was able to connect over Jolokia to the test jvm and verify that the mbean browser & recordings work as expected. To test I added this to the test class:
>>> > ```
>>> > diff --git a/application/tests/org.openjdk.jmc.jolokia.test/src/test/java/org/openjdk/jmc/jolokia/JolokiaTest.java b/application/tests/org.openjdk.jmc.jolokia.test/src/test/java/org/openjdk/jmc/jolokia/JolokiaTest.java
>>> > index 7b65e835..a245f4b1 100644
>>> > --- a/application/tests/org.openjdk.jmc.jolokia.test/src/test/java/org/openjdk/jmc/jolokia/JolokiaTest.java
>>> > +++ b/application/tests/org.openjdk.jmc.jolokia.test/src/test/java/org/openjdk/jmc/jolokia/JolokiaTest.java
>>> > @@ -81,7 +81,11 @@ public class JolokiaTest {
>>> >  		Awaitility.await().atMost(Duration.ofSeconds(15))//Note: hard code property to avoid module dependency on agent
>>> >  				.until(() -> (jolokiaUrl = System.getProperty("jolokia.agent")) != null);
>>> >  		jolokiaConnection = getJolokiaMBeanConnector();
>>> > -
>>> > +		try {
>>> > +			Thread.sleep(500000000);
>>> > +		} catch (Exception e) {
>>> > +			// do nothing, let's take a look at the jolokia ..
>>> > +		}
>>> >  	}
>>> >  
>>> >  	@Test
>>> > ```
>>> > 
>>> > 
>>> >     
>>> >       
>>> >     
>>> > 
>>> >       
>>> >     
>>> > 
>>> >     
>>> >   
>>> > and then added a new connection and connected using the custom jmx service url.
>>> > What is your current plans for this PR? Did you want to try and get the JVM Discovery working before turning it from draft to ready for review? Or is the current functionality what you're currently hoping to have integrated?
>>> 
>>> Just to be sure I understood the code change: That was in order to manually test, right?
>> 
>> Yeah, just incase anyone coming in wants to try it out/review without doing configuration locally. It was just a hack on my side to test this out a bit faster.
>> 
>>> It seems some people would be very happy to have Jolokia support, so I suggest we push ahead with the connectivity. Discovery is more of a niche feature that I can continue working on (ran into some issues with the new 2.x modules there). I will mark as ready for review once I sorted the jolokia version issue you noticed and added coverage.
>> 
>> Sounds good to me, and just the connectivity would be enough to satisfy the description of JMC-7455, we can make another jira ticket to track the discover-ability afterwards.
>
> @aptmac : Marked it as ready for review. You may see from the last commits that when I tried to get test coverage it broke the regular test run. Hence I reverted back. I see you are working on that in #562 .

> @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!

Bah, sorry for the delay in the review. Trying to run JMC through the Eclipse IDE has been a dreadful experience the last little while, but looks to be fixed with: https://github.com/openjdk/jmc/pull/566

Once that is in I should have a much better time testing out this PR.

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

PR Comment: https://git.openjdk.org/jmc/pull/548#issuecomment-2145780878


More information about the jmc-dev mailing list