Skip to content

Commit

Permalink
CORDA-2254: Replaced JmxPolicy.httpPort with the original PortAllocat…
Browse files Browse the repository at this point in the history
…ion (corda#4345)

The httpPort parameter is fundamentally wrong as it gives the same monitoring port to each node!

Added JmxPolicy.defaultEnabled() to workaround the (incorrect) default false value of startJmxHttpServer.
  • Loading branch information
shamsasari authored Dec 3, 2018
1 parent 45e4a85 commit c03ad5a
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 60 deletions.
4 changes: 2 additions & 2 deletions .ci/api-current.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5976,12 +5976,12 @@ public final class net.corda.testing.driver.JmxPolicy extends java.lang.Object
public <init>()
public <init>(boolean, net.corda.testing.driver.PortAllocation)
public final boolean component1()
@Nullable
@NotNull
public final net.corda.testing.driver.PortAllocation component2()
@NotNull
public final net.corda.testing.driver.JmxPolicy copy(boolean, net.corda.testing.driver.PortAllocation)
public boolean equals(Object)
@Nullable
@NotNull
public final net.corda.testing.driver.PortAllocation getJmxHttpServerPortAllocation()
public final boolean getStartJmxHttpServer()
public int hashCode()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import net.corda.testing.common.internal.ProjectStructure.projectRootDir
import net.corda.testing.core.BOB_NAME
import net.corda.testing.core.DUMMY_BANK_A_NAME
import net.corda.testing.core.DUMMY_BANK_B_NAME
import net.corda.testing.driver.internal.incrementalPortAllocation
import net.corda.testing.http.HttpApi
import net.corda.testing.node.internal.addressMustBeBound
import net.corda.testing.node.internal.addressMustNotBeBound
Expand Down Expand Up @@ -101,8 +100,7 @@ class DriverTests {

@Test
fun `monitoring mode enables jolokia exporting of JMX metrics via HTTP JSON`() {
val portAllocation = incrementalPortAllocation(7006)
driver(DriverParameters(jmxPolicy = JmxPolicy(portAllocation.nextPort()), startNodesInProcess = false, notarySpecs = emptyList())) {
driver(DriverParameters(jmxPolicy = JmxPolicy.defaultEnabled(), startNodesInProcess = false, notarySpecs = emptyList())) {
val node = startNode(providedName = DUMMY_REGULATOR_NAME).getOrThrow()
// request access to some JMX metrics via Jolokia HTTP/JSON
val api = HttpApi.fromHostAndPort(node.jmxAddress!!, "/jolokia/")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,34 @@ abstract class PortAllocation {
}
}

/**
* 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 from port 7005. Use [NodeHandle.jmxAddress] to get the assigned address.
*/
@Suppress("DEPRECATION")
data class JmxPolicy
@Deprecated("Use the constructor that just takes in the jmxHttpServerPortAllocation or use JmxPolicy.defaultEnabled()")
constructor(
val startJmxHttpServer: Boolean = false,
val jmxHttpServerPortAllocation: PortAllocation = incrementalPortAllocation(7005)
) {
@Deprecated("The default constructor does not turn on monitoring. Simply leave the jmxPolicy parameter unspecified if you wish to not have monitoring turned on.")
constructor() : this(false)

/** Create a [JmxPolicy] that turns on monitoring using the given [PortAllocation]. */
constructor(jmxHttpServerPortAllocation: PortAllocation) : this(true, jmxHttpServerPortAllocation)

companion object {
/** Returns a default [JmxPolicy] that turns on monitoring. */
@JvmStatic
fun defaultEnabled(): JmxPolicy = JmxPolicy(true)
}
}

/**
* [driver] allows one to start up nodes like this:
* driver {
Expand All @@ -136,7 +164,7 @@ abstract class PortAllocation {
*
* @param defaultParameters The default parameters for the driver. Allows the driver to be configured in builder style
* when called from Java code.
* @property dsl The dsl itself.
* @param dsl The dsl itself.
* @return The value returned in the [dsl] closure.
*/
fun <A> driver(defaultParameters: DriverParameters = DriverParameters(), dsl: DriverDSL.() -> A): A {
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -221,9 +221,9 @@ class DriverDSLImpl(
"networkServices.networkMapURL" to compatibilityZone.networkMapURL().toString())
}

@Suppress("DEPRECATION")
val jmxConfig = if (jmxPolicy.startJmxHttpServer) {
mapOf(NodeConfiguration::jmxMonitoringHttpPort.name to jmxPolicy.httpPort)
val jmxPort = if (jmxPolicy.startJmxHttpServer) jmxPolicy.jmxHttpServerPortAllocation.nextPort() else null
val jmxConfig = if (jmxPort != null) {
mapOf(NodeConfiguration::jmxMonitoringHttpPort.name to jmxPort)
} else {
emptyMap()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ class ExplorerSimulation(private val options: OptionSet) {
portAllocation = portAllocation,
cordappsForAllNodes = listOf(FINANCE_CORDAPP),
waitForAllNodesToFinish = true,
jmxPolicy = JmxPolicy(7006)
jmxPolicy = JmxPolicy.defaultEnabled()
)) {
// TODO : Supported flow should be exposed somehow from the node instead of set of ServiceInfo.
val alice = startNode(providedName = ALICE_NAME, rpcUsers = listOf(user))
Expand Down

0 comments on commit c03ad5a

Please sign in to comment.