open-telemetry / opentelemetry-java-contrib

https://opentelemetry.io
Apache License 2.0
144 stars 117 forks source link

jfr-connection: fixes for using diagnostic command to start a recording #1352

Open dsgrieve opened 1 week ago

dsgrieve commented 1 week ago

Description: Time based recording options (time and age) are sent with a space between the value and units, e.g. "6000 ms", which may cause an issue with Java 8/Diagnostic Command.

In the course of testing, I found an issue with the code that checks to see if -XX:+UnlockCommercialFeatures needs to be set. The issue was that this check was applied always and would erroneously report that the user needed to UnlockCommercialFeatures even though the JVM was not an Oracle JVM. I addressed this by putting an additional check to see if the VM vendor is "Oracle Corporation" and the VM version is 1.8, 9, or 10. Oracle removed UnlockCommercialFeatures in JDK 11.

Existing Issue(s): Fixes 1338

Testing: I tested locally with Oracle JVMs version 1.8, 11, 17 and 21, and also with Adoptium and Microsoft JVMs. Modified existing unit tests to accept <value><units> as oppsed to <value> <units> Added unit test that starts a recording for a specified duration where the duration configured in the RecordingOptions as 5s or 5 s. This test is run with both the diagnostic command connection and with the MBean connection to ensure there are no differences. This test does not use Mock as do others.

Documentation: One place in the documentation had an example duration with a space between the value and units. I eliminated the space, although the code normalizes the string by eliminating any space.

Outstanding items:

< Anything that these changes are intentionally missing that will be needed or can be helpful in the future. >