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

[Martin Skarsaune]

On Tue, 30 Apr 2024 12:43:18 GMT, Alex Macdonald <aptmac 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?
>> 
>> 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.
>
>> > 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 .

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

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