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

Marcus Hirt hirt at openjdk.java.net
Mon Dec 20 10:45:35 UTC 2021


On Wed, 15 Dec 2021 15:27:35 GMT, Martin Skarsaune <duke at openjdk.java.net> wrote:

>> 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.
>
> Martin Skarsaune has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Removed -XstartOnFirstThread as it is not valid for some JVMs

I took a quick pass on (mainly) the javadocs.

application/org.openjdk.jmc.jolokia/src/main/java/org/openjdk/jmc/jolokia/AbstractCachedDescriptorProvider.java line 45:

> 43: 
> 44: /**
> 45:  * I cache a list if identified JVMs that can be refreshed in the background by some means of

"I" personal pronomen. "if" identified. 

Better to keep it impersonal and describe what the class does. E.g. "Keeps a list of identified JVMs..."
"The {@code AbstractCachedDescriptorProvider} keeps a list of identified JVMs..." would be even better.

application/org.openjdk.jmc.jolokia/src/main/java/org/openjdk/jmc/jolokia/JmcJolokiaJmxConnection.java line 128:

> 126: 
> 127: 	/**
> 128: 	 * build / reverse engineer MBeanOperationInfo by using the local one if it is a match or try to

Punctuation.

application/org.openjdk.jmc.jolokia/src/main/java/org/openjdk/jmc/jolokia/JmcJolokiaJmxConnection.java line 137:

> 135: 	 */
> 136: 	private MBeanOperationInfo stealOrBuildOperationInfo(MBeanOperationInfo original, MBeanInfo localInfo) {
> 137: 		// first attempt to get descriptor from local copy

Might want to break out into individual methods at the comment lines.

application/org.openjdk.jmc.jolokia/src/main/java/org/openjdk/jmc/jolokia/JolokiaAgentDescriptor.java line 60:

> 58: import org.openjdk.jmc.ui.common.jvm.JVMType;
> 59: 
> 60: public class JolokiaAgentDescriptor implements ServerConnectionDescriptor {

Public class. Would prefer a class comment.

application/org.openjdk.jmc.jolokia/src/main/java/org/openjdk/jmc/jolokia/JolokiaAgentDescriptor.java line 99:

> 97: 	/**
> 98: 	 * Best effort to extract JVM information from a connection if everything works. Can be adjusted
> 99: 	 * to support different flavors of JVM

Punctuation, last sentence.

application/org.openjdk.jmc.jolokia/src/main/java/org/openjdk/jmc/jolokia/JolokiaDiscoveryListener.java line 48:

> 46: import org.openjdk.jmc.jolokia.preferences.PreferenceConstants;
> 47: 
> 48: public class JolokiaDiscoveryListener extends AbstractCachedDescriptorProvider implements PreferenceConstants {

Class comment.

application/org.openjdk.jmc.jolokia/src/main/java/org/openjdk/jmc/jolokia/ServerConnectionDescriptor.java line 42:

> 40: 
> 41: /**
> 42:  * Describes the JVM and how to connect to it

Punctuation.

application/org.openjdk.jmc.jolokia/src/main/java/org/openjdk/jmc/jolokia/preferences/PreferenceConstants.java line 37:

> 35: 
> 36: /**
> 37:  * Constant definitions for plug-in preferences

Punctuation.

application/org.openjdk.jmc.kubernetes/src/main/java/org/openjdk/jmc/kubernetes/JmcKubernetesJmxConnection.java line 50:

> 48: import org.openjdk.jmc.rjmx.ConnectionException;
> 49: 
> 50: public class JmcKubernetesJmxConnection extends JmcJolokiaJmxConnection {

Class comment.

application/org.openjdk.jmc.kubernetes/src/main/java/org/openjdk/jmc/kubernetes/JmcKubernetesJmxConnectionProvider.java line 44:

> 42: import javax.management.remote.JMXServiceURL;
> 43: 
> 44: public class JmcKubernetesJmxConnectionProvider implements JMXConnectorProvider {

Class comment. E.g. "This {@code JMXConnectionProvider} verifies that the protocol is kubernetes."

application/org.openjdk.jmc.kubernetes/src/main/java/org/openjdk/jmc/kubernetes/KubernetesDiscoveryListener.java line 81:

> 79: import io.fabric8.kubernetes.client.utils.Utils;
> 80: 
> 81: public class KubernetesDiscoveryListener extends AbstractCachedDescriptorProvider {

Class comment.

application/org.openjdk.jmc.kubernetes/src/main/java/org/openjdk/jmc/kubernetes/preferences/JmcKubernetesPreferenceForm.java line 57:

> 55:  * preference store.
> 56:  */
> 57: 

Line 57 is superfluous.

application/org.openjdk.jmc.kubernetes/src/main/java/org/openjdk/jmc/kubernetes/preferences/PreferenceConstants.java line 37:

> 35: 
> 36: /**
> 37:  * Constant definitions for plug-in preferences

Punctuation.

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

Changes requested by hirt (Lead).

PR: https://git.openjdk.java.net/jmc/pull/332


More information about the jmc-dev mailing list