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

[Martin Skarsaune]

On Mon, 29 Apr 2024 19:02:36 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.

> application/tests/org.openjdk.jmc.jolokia.test/pom.xml line 51:
> 
>> 49: 	</properties>
>> 50: 
>> 51: 	<dependencies>
> 
> Tried running the tests but was missing this dependency. The root pom has `jolokia.version` as 1.7.2, and the releng/third-party pom has it as 2.0.2, did you mean to mix the two versions?
> 
> diff --git a/application/tests/org.openjdk.jmc.jolokia.test/pom.xml b/application/tests/org.openjdk.jmc.jolokia.test/pom.xml
> index 79fc76f4..f46397fa 100644
> --- a/application/tests/org.openjdk.jmc.jolokia.test/pom.xml
> +++ b/application/tests/org.openjdk.jmc.jolokia.test/pom.xml
> @@ -54,6 +54,11 @@
>  			<artifactId>awaitility</artifactId>
>  			<version>4.0.0</version>
>  		</dependency>
> +		<dependency>
> +			<groupId>org.jolokia</groupId>
> +			<artifactId>jolokia-jvm</artifactId>
> +			<version>${jolokia.version}</version>
> +		</dependency>
>  	</dependencies>
>  
>  	<build>

Nice catch, thanks. I will fix the root POM. However for the test I may have to use the 1.7.2 agent as I am struggling with the 2.0.2. Will have a look. 👍

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

PR Comment: https://git.openjdk.org/jmc/pull/548#issuecomment-2084614003
PR Review Comment: https://git.openjdk.org/jmc/pull/548#discussion_r1584285351