CORDA-2254: JmxPolicy fixed so that it turns on monitoring via the jmxMonitoringHttpPort config (#4298)

* CORDA-2254: JmxPolicy fixed so that it turns on monitoring via the jmxMonitoringHttpPort config

To make the API easier to use, httpPort was introduced and both startJmxHttpServer and jmxHttpServerPortAllocation deprecated.

testing/node-driver/src/integration-test/kotlin/net/corda/testing/driver/DriverTests.kt

@@ -106,13 +106,10 @@ class DriverTests {
 
     @Test
     fun `monitoring mode enables jolokia exporting of JMX metrics via HTTP JSON`() {
-        driver(DriverParameters(startNodesInProcess = false, notarySpecs = emptyList())) {
+        driver(DriverParameters(jmxPolicy = JmxPolicy(7006), startNodesInProcess = false, notarySpecs = emptyList())) {
-            // start another node so we gain access to node JMX metrics
+            startNode(providedName = DUMMY_REGULATOR_NAME).getOrThrow()
-            val webAddress = NetworkHostAndPort("localhost", 7006)
-            startNode(providedName = DUMMY_REGULATOR_NAME,
-                      customOverrides = mapOf("jmxMonitoringHttpPort" to webAddress.port)).getOrThrow()
             // request access to some JMX metrics via Jolokia HTTP/JSON
-            val api = HttpApi.fromHostAndPort(webAddress, "/jolokia/")
+            val api = HttpApi.fromHostAndPort(NetworkHostAndPort("localhost", 7006), "/jolokia/")
             val versionAsJson = api.getJson<JSONObject>("/jolokia/version/")
             assertThat(versionAsJson.getValue("status")).isEqualTo(200)
         }

testing/node-driver/src/main/kotlin/net/corda/testing/driver/Driver.kt

@@ -297,18 +297,6 @@ data class NodeParameters(
     fun withDeleteExistingCordappsDirectory(regenerateCordappsOnStart: Boolean): NodeParameters = copy(regenerateCordappsOnStart = regenerateCordappsOnStart)
 }
 
-/**
- * A class containing configuration information for Jolokia JMX, to be used when creating a node via the [driver]
- *
- * @property startJmxHttpServer Indicates whether the spawned nodes should start with a Jolokia JMX agent to enable remote
- * JMX monitoring using HTTP/JSON
- * @property jmxHttpServerPortAllocation The port allocation strategy to use for remote Jolokia/JMX monitoring over HTTP.
- * Defaults to incremental.
- */
-data class JmxPolicy(val startJmxHttpServer: Boolean = false,
-                     val jmxHttpServerPortAllocation: PortAllocation? =
-                             if (startJmxHttpServer) incrementalPortAllocation(7005) else null)
-
 /**
  * [driver] allows one to start up nodes like this:
  *   driver {
@@ -396,7 +384,7 @@ data class DriverParameters(
         val waitForAllNodesToFinish: Boolean = false,
         val notarySpecs: List<NotarySpec> = listOf(NotarySpec(DUMMY_NOTARY_NAME)),
         val extraCordappPackagesToScan: List<String> = emptyList(),
-        val jmxPolicy: JmxPolicy = JmxPolicy(),
+        @Suppress("DEPRECATION") val jmxPolicy: JmxPolicy = JmxPolicy(),
         val networkParameters: NetworkParameters = testNetworkParameters(notaries = emptyList()),
         val notaryCustomOverrides: Map<String, Any?> = emptyMap(),
         val initialiseSerialization: Boolean = true,
@@ -414,7 +402,7 @@ data class DriverParameters(
             waitForAllNodesToFinish: Boolean = false,
             notarySpecs: List<NotarySpec> = listOf(NotarySpec(DUMMY_NOTARY_NAME)),
             extraCordappPackagesToScan: List<String> = emptyList(),
-            jmxPolicy: JmxPolicy = JmxPolicy(),
+            @Suppress("DEPRECATION") jmxPolicy: JmxPolicy = JmxPolicy(),
             networkParameters: NetworkParameters = testNetworkParameters(notaries = emptyList()),
             notaryCustomOverrides: Map<String, Any?> = emptyMap(),
             initialiseSerialization: Boolean = true,

testing/node-driver/src/main/kotlin/net/corda/testing/driver/JmxPolicy.kt

@@ -0,0 +1,50 @@
+package net.corda.testing.driver
+
+/**
+ * A class containing configuration information for Jolokia JMX, to be used when creating a node via the [driver].
+ *
+ * @property httpPort The port to use for remote Jolokia/JMX monitoring over HTTP. Defaults to 7006.
+ */
+@Suppress("DEPRECATION")
+class JmxPolicy private constructor(
+        @Deprecated("This is no longer needed to turn on monitoring.")
+        val startJmxHttpServer: Boolean,
+        @Deprecated("This has been replaced by httpPort which makes it clear to the calling code which port will be used.")
+        val jmxHttpServerPortAllocation: PortAllocation?,
+        val httpPort: Int
+) {
+    /** Create a JmxPolicy that turns on monitoring on the given [httpPort]. */
+    constructor(httpPort: Int) : this(true, null, httpPort)
+
+    override fun equals(other: Any?): Boolean {
+        if (this === other) return true
+        if (other !is JmxPolicy) return false
+        return this.httpPort == other.httpPort
+    }
+
+    override fun hashCode(): Int {
+        var result = httpPort.hashCode()
+        result = 31 * result + httpPort
+        return result
+    }
+
+    override fun toString(): String = "JmxPolicy(httpPort=$httpPort)"
+
+    // The below cannot be removed as they're already part of the public API, so it's deprecated instead
+
+    @Deprecated("The default constructor does not turn on monitoring. Simply leave the jmxPolicy parameter unspecified.")
+    constructor() : this(false, null, 7006)
+    @Deprecated("Use constructor that takes in the httpPort")
+    constructor(startJmxHttpServer: Boolean = false, jmxHttpServerPortAllocation: PortAllocation? = null) : this(startJmxHttpServer, jmxHttpServerPortAllocation, 7006)
+
+    @Deprecated("startJmxHttpServer is deprecated as it's no longer needed to turn on monitoring.")
+    operator fun component1(): Boolean = startJmxHttpServer
+    @Deprecated("jmxHttpServerPortAllocation is deprecated and no longer does anything. Use httpPort instead.")
+    operator fun component2(): PortAllocation? = jmxHttpServerPortAllocation
+
+    @Deprecated("startJmxHttpServer and jmxHttpServerPortAllocation are both deprecated.")
+    fun copy(startJmxHttpServer: Boolean = this.startJmxHttpServer,
+             jmxHttpServerPortAllocation: PortAllocation? = this.jmxHttpServerPortAllocation): JmxPolicy {
+        return JmxPolicy(startJmxHttpServer, jmxHttpServerPortAllocation)
+    }
+}

testing/node-driver/src/main/kotlin/net/corda/testing/node/internal/DriverDSLImpl.kt

@@ -130,8 +130,6 @@ class DriverDSLImpl(
     //TODO: remove this once we can bundle quasar properly.
     private val quasarJarPath: String by lazy { resolveJar(".*quasar.*\\.jar$") }
 
-    private val jolokiaJarPath: String by lazy { resolveJar(".*jolokia-jvm-.*-agent\\.jar$") }
-
     private fun NodeConfig.checkAndOverrideForInMemoryDB(): NodeConfig = this.run {
         if (inMemoryDB && corda.dataSourceProperties.getProperty("dataSource.url").startsWith("jdbc:h2:")) {
             val jdbcUrl = "jdbc:h2:mem:persistence${inMemoryCounter.getAndIncrement()};DB_CLOSE_ON_EXIT=FALSE;LOCK_TIMEOUT=10000;WRITE_DELAY=100"
@@ -265,7 +263,14 @@ class DriverDSLImpl(
                         "networkServices.networkMapURL" to compatibilityZone.networkMapURL().toString())
         }
 
-        val flowOverrideConfig = flowOverrides.entries.map { FlowOverride(it.key.canonicalName, it.value.canonicalName) }.let { FlowOverrideConfig(it) }
+        @Suppress("DEPRECATION")
+        val jmxConfig = if (jmxPolicy.startJmxHttpServer) {
+            mapOf(NodeConfiguration::jmxMonitoringHttpPort.name to jmxPolicy.httpPort)
+        } else {
+            emptyMap()
+        }
+
+        val flowOverrideConfig = FlowOverrideConfig(flowOverrides.entries.map { FlowOverride(it.key.canonicalName, it.value.canonicalName) })
         val overrides = configOf(
                 NodeConfiguration::myLegalName.name to name.toString(),
                 NodeConfiguration::p2pAddress.name to p2pAddress.toString(),
@@ -275,7 +280,7 @@ class DriverDSLImpl(
                 NodeConfiguration::rpcUsers.name to if (users.isEmpty()) defaultRpcUserList else users.map { it.toConfig().root().unwrapped() },
                 NodeConfiguration::verifierType.name to verifierType.name,
                 NodeConfiguration::flowOverrides.name to flowOverrideConfig.toConfig().root().unwrapped()
-        ) + czUrlConfig + customOverrides
+        ) + czUrlConfig + jmxConfig + customOverrides
         val config = NodeConfig(ConfigHelper.loadConfig(
                 baseDirectory = baseDirectory(name),
                 allowMissingConfig = true,
@@ -335,12 +340,6 @@ class DriverDSLImpl(
         }
     }
 
-    private enum class ClusterType(val validating: Boolean, val clusterName: CordaX500Name) {
-        VALIDATING_RAFT(true, CordaX500Name("Raft", "Zurich", "CH")),
-        NON_VALIDATING_RAFT(false, CordaX500Name("Raft", "Zurich", "CH")),
-        NON_VALIDATING_BFT(false, CordaX500Name("BFT", "Zurich", "CH"))
-    }
-
     @Suppress("DEPRECATION")
     private fun queryWebserver(handle: NodeHandle, process: Process): WebserverHandle {
         val protocol = if ((handle as NodeHandleInternal).useHTTPS) "https://" else "http://"
@@ -564,13 +563,10 @@ class DriverDSLImpl(
      */
     private fun startOutOfProcessMiniNode(config: NodeConfig, vararg extraCmdLineFlag: String): CordaFuture<Unit> {
         val debugPort = if (isDebug) debugPortAllocation.nextPort() else null
-        val monitorPort = if (jmxPolicy.startJmxHttpServer) jmxPolicy.jmxHttpServerPortAllocation?.nextPort() else null
         val process = startOutOfProcessNode(
                 config,
                 quasarJarPath,
                 debugPort,
-                jolokiaJarPath,
-                monitorPort,
                 systemProperties,
                 "512m",
                 *extraCmdLineFlag
@@ -645,8 +641,7 @@ class DriverDSLImpl(
             return nodeFuture
         } else {
             val debugPort = if (isDebug) debugPortAllocation.nextPort() else null
-            val monitorPort = if (jmxPolicy.startJmxHttpServer) jmxPolicy.jmxHttpServerPortAllocation?.nextPort() else null
+            val process = startOutOfProcessNode(config, quasarJarPath, debugPort, systemProperties, maximumHeapSize)
-            val process = startOutOfProcessNode(config, quasarJarPath, debugPort, jolokiaJarPath, monitorPort, systemProperties, maximumHeapSize)
 
             // Destroy the child process when the parent exits.This is needed even when `waitForAllNodesToFinish` is
             // true because we don't want orphaned processes in the case that the parent process is terminated by the
@@ -776,16 +771,11 @@ class DriverDSLImpl(
                 config: NodeConfig,
                 quasarJarPath: String,
                 debugPort: Int?,
-                jolokiaJarPath: String,
-                monitorPort: Int?,
                 overriddenSystemProperties: Map<String, String>,
                 maximumHeapSize: String,
                 vararg extraCmdLineFlag: String
         ): Process {
-
+            log.info("Starting out-of-process Node ${config.corda.myLegalName.organisation}, debug port is " + (debugPort ?: "not enabled"))
-            log.info("Starting out-of-process Node ${config.corda.myLegalName.organisation}, " +
-                    "debug port is " + (debugPort ?: "not enabled") + ", " +
-                    "jolokia monitoring port is " + (monitorPort ?: "not enabled"))
             // Write node.conf
             writeConfig(config.corda.baseDirectory, "node.conf", config.typesafe.toNodeOnly())
 
@@ -811,7 +801,6 @@ class DriverDSLImpl(
                     "org.objenesis**;org.slf4j**;org.w3c**;org.xml**;org.yaml**;reflectasm**;rx**;org.jolokia**;)"
             val extraJvmArguments = systemProperties.removeResolvedClasspath().map { "-D${it.key}=${it.value}" } +
                     "-javaagent:$quasarJarPath=$excludePattern"
-            val jolokiaAgent = monitorPort?.let { "-javaagent:$jolokiaJarPath=port=$monitorPort,host=localhost" }
             val loggingLevel = if (debugPort == null) "INFO" else "DEBUG"
 
             val arguments = mutableListOf(
@@ -825,7 +814,7 @@ class DriverDSLImpl(
                     className = "net.corda.node.Corda", // cannot directly get class for this, so just use string
                     arguments = arguments,
                     jdwpPort = debugPort,
-                    extraJvmArguments = extraJvmArguments + listOfNotNull(jolokiaAgent),
+                    extraJvmArguments = extraJvmArguments,
                     workingDirectory = config.corda.baseDirectory,
                     maximumHeapSize = maximumHeapSize
             )
@@ -955,7 +944,7 @@ private class NetworkVisibilityController {
                         // Nothing to do here but better being exhaustive.
                     }
                 }
-            }, { _ ->
+            }, {
                 // Nothing to do on errors here.
             })
             return future

testing/node-driver/src/main/kotlin/net/corda/testing/node/internal/RPCDriver.kt

@@ -117,7 +117,7 @@ fun <A> rpcDriver(
         waitForNodesToFinish: Boolean = false,
         notarySpecs: List<NotarySpec> = emptyList(),
         externalTrace: Trace? = null,
-        jmxPolicy: JmxPolicy = JmxPolicy(),
+        @Suppress("DEPRECATION") jmxPolicy: JmxPolicy = JmxPolicy(),
         networkParameters: NetworkParameters = testNetworkParameters(),
         notaryCustomOverrides: Map<String, Any?> = emptyMap(),
         inMemoryDB: Boolean = true,

tools/explorer/src/main/kotlin/net/corda/explorer/ExplorerSimulation.kt

@@ -73,7 +73,7 @@ class ExplorerSimulation(private val options: OptionSet) {
                 portAllocation = portAllocation,
                 cordappsForAllNodes = listOf(FINANCE_CORDAPP),
                 waitForAllNodesToFinish = true,
-                jmxPolicy = JmxPolicy(true)
+                jmxPolicy = JmxPolicy(7006)
         )) {
             // TODO : Supported flow should be exposed somehow from the node instead of set of ServiceInfo.
             val alice = startNode(providedName = ALICE_NAME, rpcUsers = listOf(user))