Integration tests for JMX Scraper

[robsunday]

No description provided.

[breedx-splk]

I'm a bit confused by this. Unless I'm missing something, you shouldn't have to find an ephemeral/open/unused port on the box running the tests -- just use a fixed port for JMX, use that fixed port in the jvm args for both rmi and jmx, and then docker itself should assign an ephemeral port on the outside surface of the container.

That's what the getMappedPort() then does for you -- it lets you know what external port docker assigned to the fixed internal port. Make sense?

With that, I think you can just delete the PortSelector class.

[SylvainJuge]

Here we have multiple challenges, and using a fixed port mapping is a way to work-around the JMX/RMI implementation details, this is clearly not ideal and I don't know if we have a better option to test JMX with RMI. I haven't investigated deeper but it seems that using jmxmp instead of rmi protocol instead would be a better option for most integration tests.

In short:

• setting com.sun.management.jmxremote.port=9999 will make the target JVM listen on port 9999
• testcontainers map the 9999 JMX port to a random port, let's say 12345.
• we build a JMX URI with localhost:12345 which allows to connect to the mapped port on the host
• when connecting to localhost:12345 the server-side JMX implementation replies that a different host:port which in my case in Linux host is something like 172.20.0.xx:9999.
• This host:port combination can be controlled with com.sun.management.jmxremote.rmi.port and java.rmi.server.hostname properties on the target JVM
• this behavior is related to the RMI protocol itself and is problematic with NAT as it forces to use only a single host:port to be used at once.

Maybe here another option could also be to make sure that we have a network that is always accessible from the host like it does on Linux could be a simpler option.

[trask]

just in case it's helpful/relevant here: https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/instrumentation/rmi/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/rmi/RmiTest.java

[SylvainJuge]

After failing to make things working with jmxmp, I think using a fixed port is an acceptable work-around for now:

• it's only needed to test the JmxConnectorBuilder class with host-to-container communication, packaging the test and running it in a container would be way overkill and harder to debug in the future.
• there is no impact on container-to-container communication for other integration tests.
• it's simpler to have the same implementation for both mac and linux, the fact it works on Linux and in CI by default is almost "by accident"

I've opened a PR on this PR to simplify even further and reuse an available port implementation already in classpath: robsunday#1

[SylvainJuge]

[for reviewer] this part had to go anyway as it's now replaced by actual integration tests in #1485 , here it only checked the connection from the host as a proxy of "target JVM is working with JMX settings".

[trask]

I'd suggest either remove or include a code comment explaining why you may want to uncomment

Suggested change

	// properties.put("com.sun.management.jmxremote.local.only", "false");
	// properties.put("java.rmi.server.logCalls", "true");
	//