From f55502432483e9f7e921fef77d2f7a3c4c3f1f2a Mon Sep 17 00:00:00 2001 From: David Grieve Date: Tue, 13 Aug 2024 08:30:38 -0400 Subject: [PATCH 1/7] Recording close should not throw exception --- ...htRecorderDiagnosticCommandConnection.java | 37 +++++-------------- .../contrib/jfr/connection/Recording.java | 8 +++- 2 files changed, 16 insertions(+), 29 deletions(-) diff --git a/jfr-connection/src/main/java/io/opentelemetry/contrib/jfr/connection/FlightRecorderDiagnosticCommandConnection.java b/jfr-connection/src/main/java/io/opentelemetry/contrib/jfr/connection/FlightRecorderDiagnosticCommandConnection.java index 4d1b052dc..6005fe7f7 100644 --- a/jfr-connection/src/main/java/io/opentelemetry/contrib/jfr/connection/FlightRecorderDiagnosticCommandConnection.java +++ b/jfr-connection/src/main/java/io/opentelemetry/contrib/jfr/connection/FlightRecorderDiagnosticCommandConnection.java @@ -156,14 +156,6 @@ private static Object[] formOptions( return mkParamsArray(params); } - /** - * Stop a recording. This method is called from the {@link Recording#stop()} method. - * - * @param id The id of the recording. - * @throws JfrConnectionException Wraps an {@code javax.management.InstanceNotFoundException}, a - * {@code javax.management.MBeanException} or a {@code javax.management.ReflectionException} - * and indicates an issue with the FlightRecorderMXBean in the JVM. - */ @Override public void stopRecording(long id) throws JfrConnectionException { try { @@ -174,14 +166,6 @@ public void stopRecording(long id) throws JfrConnectionException { } } - /** - * Writes recording data to the specified file. The recording must be started, but not necessarily - * stopped. The {@code outputFile} argument is relevant to the machine where the JVM is running. - * - * @param id The id of the recording. - * @param outputFile the system-dependent file name where data is written, not {@code null} - * @throws JfrConnectionException Wraps a {@code javax.management.JMException}. - */ @Override public void dumpRecording(long id, String outputFile) throws IOException, JfrConnectionException { try { @@ -192,28 +176,25 @@ public void dumpRecording(long id, String outputFile) throws IOException, JfrCon } } - /** - * Not supported on Java 8 - * - * @param id The id of the recording being cloned. - * @param stop Whether to stop the cloned recording. - * @return id of the recording - */ + /** Not available through the DiagnosticCommand MBean. {@inheritDoc} */ @Override public long cloneRecording(long id, boolean stop) { - throw new UnsupportedOperationException("Clone not supported on Java 8"); + throw new UnsupportedOperationException( + "cloneRecording not available through the DiagnosticCommand connection"); } - /** Not supported on Java 8 */ + /** Not available through the DiagnosticCommand MBean. {@inheritDoc} */ @Override public InputStream getStream(long id, Instant startTime, Instant endTime, long blockSize) { - throw new UnsupportedOperationException("getStream not supported on Java 8"); + throw new UnsupportedOperationException( + "getStream not available through the DiagnosticCommand connection"); } - /** Not supported on Java 8 */ + /** Not available through the DiagnosticCommand MBean. {@inheritDoc} */ @Override public void closeRecording(long id) { - throw new UnsupportedOperationException("closeRecording not supported on Java 8"); + throw new UnsupportedOperationException( + "closeRecording not available through the DiagnosticCommand connection"); } // Do this check separate from assertCommercialFeatures because reliance diff --git a/jfr-connection/src/main/java/io/opentelemetry/contrib/jfr/connection/Recording.java b/jfr-connection/src/main/java/io/opentelemetry/contrib/jfr/connection/Recording.java index 06c643059..ba63aebf1 100644 --- a/jfr-connection/src/main/java/io/opentelemetry/contrib/jfr/connection/Recording.java +++ b/jfr-connection/src/main/java/io/opentelemetry/contrib/jfr/connection/Recording.java @@ -270,8 +270,14 @@ public void close() throws IOException, JfrConnectionException { connection.stopRecording(id); } catch (IOException | JfrConnectionException ignored) { // Stopping the recording is best-effort - } finally { + } + } + if (oldState == State.STOPPED || oldState == State.RECORDING) { + try { connection.closeRecording(id); + } catch (IOException | JfrConnectionException | UnsupportedOperationException ignored) { + // Closing the recording is best-effort + // FlightRecorderDiagnosticCommandConnection close throws UnsupportedOperationException } } } From bbd883273e7a094e7d7c325a2cc1c5dab76f0be3 Mon Sep 17 00:00:00 2001 From: David Grieve Date: Thu, 10 Oct 2024 08:53:18 -0600 Subject: [PATCH 2/7] Fix for #1491 --- ...htRecorderDiagnosticCommandConnection.java | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/jfr-connection/src/main/java/io/opentelemetry/contrib/jfr/connection/FlightRecorderDiagnosticCommandConnection.java b/jfr-connection/src/main/java/io/opentelemetry/contrib/jfr/connection/FlightRecorderDiagnosticCommandConnection.java index 6005fe7f7..edd3c32f3 100644 --- a/jfr-connection/src/main/java/io/opentelemetry/contrib/jfr/connection/FlightRecorderDiagnosticCommandConnection.java +++ b/jfr-connection/src/main/java/io/opentelemetry/contrib/jfr/connection/FlightRecorderDiagnosticCommandConnection.java @@ -123,21 +123,22 @@ public long startRecording( Object[] params = formOptions(recordingOptions, recordingConfiguration); // jfrStart returns "Started recording 2." and some more stuff, but all we care about is the - // name of the recording. + // id of the recording. + String jfrStart; try { - String jfrStart = - (String) mBeanServerConnection.invoke(objectName, "jfrStart", params, signature); - String name; + jfrStart = (String) mBeanServerConnection.invoke(objectName, "jfrStart", params, signature); Matcher matcher = JFR_START_PATTERN.matcher(jfrStart); if (matcher.find()) { - name = matcher.group(1); - return Long.parseLong(name); + String id = matcher.group(1); + return Long.parseLong(id); } } catch (InstanceNotFoundException | ReflectionException | MBeanException e) { throw JfrConnectionException.canonicalJfrConnectionException(getClass(), "startRecording", e); } throw JfrConnectionException.canonicalJfrConnectionException( - getClass(), "startRecording", new IllegalStateException("Failed to parse jfrStart output")); + getClass(), + "startRecording", + new IllegalStateException("Failed to parse: '" + jfrStart + "'")); } private static Object[] formOptions( @@ -159,7 +160,7 @@ private static Object[] formOptions( @Override public void stopRecording(long id) throws JfrConnectionException { try { - Object[] params = mkParams("name=" + id); + Object[] params = mkParams("recording=" + id); mBeanServerConnection.invoke(objectName, "jfrStop", params, signature); } catch (InstanceNotFoundException | MBeanException | ReflectionException | IOException e) { throw JfrConnectionException.canonicalJfrConnectionException(getClass(), "stopRecording", e); @@ -169,7 +170,7 @@ public void stopRecording(long id) throws JfrConnectionException { @Override public void dumpRecording(long id, String outputFile) throws IOException, JfrConnectionException { try { - Object[] params = mkParams("filename=" + outputFile, "name=" + id); + Object[] params = mkParams("filename=" + outputFile, "recording=" + id); mBeanServerConnection.invoke(objectName, "jfrDump", params, signature); } catch (InstanceNotFoundException | MBeanException | ReflectionException e) { throw JfrConnectionException.canonicalJfrConnectionException(getClass(), "dumpRecording", e); From 421c00b6e24a0608493237ee72f1dd08c88d4d29 Mon Sep 17 00:00:00 2001 From: David Grieve Date: Fri, 11 Oct 2024 13:15:53 -0600 Subject: [PATCH 3/7] use JFR.check to determine recording id parameter --- ...htRecorderDiagnosticCommandConnection.java | 31 +++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/jfr-connection/src/main/java/io/opentelemetry/contrib/jfr/connection/FlightRecorderDiagnosticCommandConnection.java b/jfr-connection/src/main/java/io/opentelemetry/contrib/jfr/connection/FlightRecorderDiagnosticCommandConnection.java index edd3c32f3..2a1cfc00b 100644 --- a/jfr-connection/src/main/java/io/opentelemetry/contrib/jfr/connection/FlightRecorderDiagnosticCommandConnection.java +++ b/jfr-connection/src/main/java/io/opentelemetry/contrib/jfr/connection/FlightRecorderDiagnosticCommandConnection.java @@ -37,6 +37,8 @@ final class FlightRecorderDiagnosticCommandConnection implements FlightRecorderC "com.sun.management:type=DiagnosticCommand"; private static final String JFR_START_REGEX = "Started recording (\\d+?)\\."; private static final Pattern JFR_START_PATTERN = Pattern.compile(JFR_START_REGEX, Pattern.DOTALL); + private static final String JFR_CHECK_REGEX = "(recording|name)=(\\d+?)"; + private static final Pattern JFR_CHECK_PATTERN = Pattern.compile(JFR_CHECK_REGEX, Pattern.DOTALL); // All JFR commands take String[] parameters private static final String[] signature = new String[] {"[Ljava.lang.String;"}; @@ -157,10 +159,35 @@ private static Object[] formOptions( return mkParamsArray(params); } + // + // Whether to use the 'name' or 'recording' parameter depends on the JVM. + // Use JFR.check to determine which one to use. + // + private String getRecordingParam(long recordingId) throws JfrConnectionException, IOException { + try { + Object[] params = new String[]{}; + String jfrCheck = (String) mBeanServerConnection.invoke(objectName, "jfrCheck", params, signature); + Matcher matcher = JFR_CHECK_PATTERN.matcher(jfrCheck); + while (matcher.find()) { + String id = matcher.group(2); + if (id.equals(Long.toString(recordingId))) { + return matcher.group(0); + } + } + } catch (InstanceNotFoundException | MBeanException | ReflectionException e) { + throw JfrConnectionException.canonicalJfrConnectionException(getClass(), "jfrCheck", e); + } + throw JfrConnectionException.canonicalJfrConnectionException( + getClass(), + "jfrCheck", + new IllegalStateException("No recording found for id: '" + recordingId + "'")); + + } + @Override public void stopRecording(long id) throws JfrConnectionException { try { - Object[] params = mkParams("recording=" + id); + Object[] params = mkParams(getRecordingParam(id)); mBeanServerConnection.invoke(objectName, "jfrStop", params, signature); } catch (InstanceNotFoundException | MBeanException | ReflectionException | IOException e) { throw JfrConnectionException.canonicalJfrConnectionException(getClass(), "stopRecording", e); @@ -170,7 +197,7 @@ public void stopRecording(long id) throws JfrConnectionException { @Override public void dumpRecording(long id, String outputFile) throws IOException, JfrConnectionException { try { - Object[] params = mkParams("filename=" + outputFile, "recording=" + id); + Object[] params = mkParams("filename=" + outputFile, getRecordingParam(id)); mBeanServerConnection.invoke(objectName, "jfrDump", params, signature); } catch (InstanceNotFoundException | MBeanException | ReflectionException e) { throw JfrConnectionException.canonicalJfrConnectionException(getClass(), "dumpRecording", e); From 40c764fcf82993ccd66f8e8f403d2f29df5c77a5 Mon Sep 17 00:00:00 2001 From: David Grieve Date: Fri, 11 Oct 2024 13:23:11 -0600 Subject: [PATCH 4/7] fix formatting --- .../FlightRecorderDiagnosticCommandConnection.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/jfr-connection/src/main/java/io/opentelemetry/contrib/jfr/connection/FlightRecorderDiagnosticCommandConnection.java b/jfr-connection/src/main/java/io/opentelemetry/contrib/jfr/connection/FlightRecorderDiagnosticCommandConnection.java index 2a1cfc00b..1d290aeb0 100644 --- a/jfr-connection/src/main/java/io/opentelemetry/contrib/jfr/connection/FlightRecorderDiagnosticCommandConnection.java +++ b/jfr-connection/src/main/java/io/opentelemetry/contrib/jfr/connection/FlightRecorderDiagnosticCommandConnection.java @@ -165,8 +165,9 @@ private static Object[] formOptions( // private String getRecordingParam(long recordingId) throws JfrConnectionException, IOException { try { - Object[] params = new String[]{}; - String jfrCheck = (String) mBeanServerConnection.invoke(objectName, "jfrCheck", params, signature); + Object[] params = new String[] {}; + String jfrCheck = + (String) mBeanServerConnection.invoke(objectName, "jfrCheck", params, signature); Matcher matcher = JFR_CHECK_PATTERN.matcher(jfrCheck); while (matcher.find()) { String id = matcher.group(2); @@ -181,7 +182,6 @@ private String getRecordingParam(long recordingId) throws JfrConnectionException getClass(), "jfrCheck", new IllegalStateException("No recording found for id: '" + recordingId + "'")); - } @Override From cc9265f1b34698141cdc9cbd47ee7fbf9a335d1d Mon Sep 17 00:00:00 2001 From: David Grieve Date: Mon, 14 Oct 2024 12:30:05 -0600 Subject: [PATCH 5/7] use jfrCheck to find correct jfrDump parameter --- ...htRecorderDiagnosticCommandConnection.java | 81 +++++++++---------- ...corderDiagnosticCommandConnectionTest.java | 57 +------------ 2 files changed, 43 insertions(+), 95 deletions(-) diff --git a/jfr-connection/src/main/java/io/opentelemetry/contrib/jfr/connection/FlightRecorderDiagnosticCommandConnection.java b/jfr-connection/src/main/java/io/opentelemetry/contrib/jfr/connection/FlightRecorderDiagnosticCommandConnection.java index 1d290aeb0..a0a27467f 100644 --- a/jfr-connection/src/main/java/io/opentelemetry/contrib/jfr/connection/FlightRecorderDiagnosticCommandConnection.java +++ b/jfr-connection/src/main/java/io/opentelemetry/contrib/jfr/connection/FlightRecorderDiagnosticCommandConnection.java @@ -7,18 +7,21 @@ import java.io.IOException; import java.io.InputStream; -import java.lang.management.ManagementFactory; -import java.lang.management.RuntimeMXBean; import java.time.Instant; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Objects; +import java.util.Optional; import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; import javax.management.InstanceNotFoundException; +import javax.management.IntrospectionException; import javax.management.MBeanException; +import javax.management.MBeanInfo; +import javax.management.MBeanOperationInfo; import javax.management.MBeanServerConnection; import javax.management.MalformedObjectNameException; import javax.management.ObjectInstance; @@ -37,7 +40,7 @@ final class FlightRecorderDiagnosticCommandConnection implements FlightRecorderC "com.sun.management:type=DiagnosticCommand"; private static final String JFR_START_REGEX = "Started recording (\\d+?)\\."; private static final Pattern JFR_START_PATTERN = Pattern.compile(JFR_START_REGEX, Pattern.DOTALL); - private static final String JFR_CHECK_REGEX = "(recording|name)=(\\d+?)"; + private static final String JFR_CHECK_REGEX = "(?:recording|name)=(\\d+)"; private static final Pattern JFR_CHECK_PATTERN = Pattern.compile(JFR_CHECK_REGEX, Pattern.DOTALL); // All JFR commands take String[] parameters @@ -61,9 +64,7 @@ static FlightRecorderConnection connect(MBeanServerConnection mBeanServerConnect mBeanServerConnection.getObjectInstance(new ObjectName(DIAGNOSTIC_COMMAND_OBJECT_NAME)); ObjectName objectName = objectInstance.getObjectName(); - if (jdkHasUnlockCommercialFeatures(mBeanServerConnection)) { - assertCommercialFeaturesUnlocked(mBeanServerConnection, objectName); - } + assertCommercialFeaturesUnlocked(mBeanServerConnection, objectName); return new FlightRecorderDiagnosticCommandConnection( mBeanServerConnection, objectInstance.getObjectName()); @@ -164,13 +165,13 @@ private static Object[] formOptions( // Use JFR.check to determine which one to use. // private String getRecordingParam(long recordingId) throws JfrConnectionException, IOException { + String jfrCheck; try { - Object[] params = new String[] {}; - String jfrCheck = - (String) mBeanServerConnection.invoke(objectName, "jfrCheck", params, signature); + Object[] params = new Object[] {new String[] {}}; + jfrCheck = (String) mBeanServerConnection.invoke(objectName, "jfrCheck", params, signature); Matcher matcher = JFR_CHECK_PATTERN.matcher(jfrCheck); while (matcher.find()) { - String id = matcher.group(2); + String id = matcher.group(1); if (id.equals(Long.toString(recordingId))) { return matcher.group(0); } @@ -179,9 +180,7 @@ private String getRecordingParam(long recordingId) throws JfrConnectionException throw JfrConnectionException.canonicalJfrConnectionException(getClass(), "jfrCheck", e); } throw JfrConnectionException.canonicalJfrConnectionException( - getClass(), - "jfrCheck", - new IllegalStateException("No recording found for id: '" + recordingId + "'")); + getClass(), "jfrCheck", new IllegalStateException("Failed to parse: '" + jfrCheck + "'")); } @Override @@ -225,41 +224,41 @@ public void closeRecording(long id) { "closeRecording not available through the DiagnosticCommand connection"); } - // Do this check separate from assertCommercialFeatures because reliance - // on System properties makes it difficult to test. - static boolean jdkHasUnlockCommercialFeatures(MBeanServerConnection mBeanServerConnection) { - try { - RuntimeMXBean runtimeMxBean = - ManagementFactory.getPlatformMXBean(mBeanServerConnection, RuntimeMXBean.class); - String javaVmVendor = runtimeMxBean.getVmVendor(); - String javaVersion = runtimeMxBean.getVmVersion(); - return javaVmVendor.contains("Oracle Corporation") - && javaVersion.matches("(?:^1\\.8|9|10).*"); - } catch (IOException e) { - return false; - } - } - // visible for testing static void assertCommercialFeaturesUnlocked( MBeanServerConnection mBeanServerConnection, ObjectName objectName) throws IOException, JfrConnectionException { try { - Object unlockedMessage = - mBeanServerConnection.invoke(objectName, "vmCheckCommercialFeatures", null, null); - if (unlockedMessage instanceof String) { - boolean unlocked = ((String) unlockedMessage).contains("unlocked"); - if (!unlocked) { - throw JfrConnectionException.canonicalJfrConnectionException( - FlightRecorderDiagnosticCommandConnection.class, - "assertCommercialFeaturesUnlocked", - new UnsupportedOperationException( - "Unlocking commercial features may be required. This must be explicitly enabled by adding -XX:+UnlockCommercialFeatures")); - } + Object[] params = new Object[] {new String[] {}}; + MBeanInfo mBeanInfo = mBeanServerConnection.getMBeanInfo(objectName); + if (mBeanInfo == null) { + throw JfrConnectionException.canonicalJfrConnectionException( + FlightRecorderDiagnosticCommandConnection.class, + "assertCommercialFeaturesUnlocked", + new NullPointerException("Could not get MBeanInfo for " + objectName)); + } + Optional operation = + Arrays.stream(mBeanInfo.getOperations()) + .filter(it -> "vmUnlockCommercialFeatures".equals(it.getName())) + .findFirst(); + + if (operation.isPresent()) { + mBeanServerConnection.invoke(objectName, "vmUnlockCommercialFeatures", params, signature); + } else { + // No vmUnlockCommercialFeatures command. + return; } - } catch (InstanceNotFoundException | MBeanException | ReflectionException ignored) { - // If the MBean doesn't have the vmCheckCommercialFeatures method, then we can't check it. + } catch (InstanceNotFoundException + | IntrospectionException + | MBeanException + | ReflectionException e) { + throw JfrConnectionException.canonicalJfrConnectionException( + FlightRecorderDiagnosticCommandConnection.class, + "assertCommercialFeaturesUnlocked", + new UnsupportedOperationException( + "Unlocking commercial features may be required. This must be explicitly enabled by adding -XX:+UnlockCommercialFeatures", + e)); } } diff --git a/jfr-connection/src/test/java/io/opentelemetry/contrib/jfr/connection/FlightRecorderDiagnosticCommandConnectionTest.java b/jfr-connection/src/test/java/io/opentelemetry/contrib/jfr/connection/FlightRecorderDiagnosticCommandConnectionTest.java index a8313582b..636e88ccf 100644 --- a/jfr-connection/src/test/java/io/opentelemetry/contrib/jfr/connection/FlightRecorderDiagnosticCommandConnectionTest.java +++ b/jfr-connection/src/test/java/io/opentelemetry/contrib/jfr/connection/FlightRecorderDiagnosticCommandConnectionTest.java @@ -10,71 +10,20 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.mockStatic; import static org.mockito.Mockito.when; -import com.google.errorprone.annotations.Keep; import java.lang.management.ManagementFactory; -import java.lang.management.RuntimeMXBean; -import java.util.stream.Stream; +import javax.management.MBeanServer; import javax.management.MBeanServerConnection; import javax.management.ObjectName; import org.junit.jupiter.api.Test; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.Arguments; -import org.junit.jupiter.params.provider.MethodSource; -import org.mockito.MockedStatic; -import org.mockito.invocation.InvocationOnMock; -import org.mockito.stubbing.Answer; class FlightRecorderDiagnosticCommandConnectionTest { - @Keep - static Stream assertJdkHasUnlockCommercialFeatures() { - return Stream.of( - Arguments.of("Oracle Corporation", "1.8.0_401", true), - Arguments.of("AdoptOpenJDK", "1.8.0_282", false), - Arguments.of("Oracle Corporation", "10.0.2", true), - Arguments.of("Oracle Corporation", "9.0.4", true), - Arguments.of("Oracle Corporation", "11.0.22", false), - Arguments.of("Microsoft", "11.0.13", false), - Arguments.of("Microsoft", "17.0.3", false), - Arguments.of("Oracle Corporation", "21.0.3", false)); - } - - @ParameterizedTest - @MethodSource - void assertJdkHasUnlockCommercialFeatures(String vmVendor, String vmVersion, boolean expected) - throws Exception { - - MBeanServerConnection mBeanServerConnection = mock(MBeanServerConnection.class); - - try (MockedStatic mockedStatic = mockStatic(ManagementFactory.class)) { - mockedStatic - .when( - () -> ManagementFactory.getPlatformMXBean(mBeanServerConnection, RuntimeMXBean.class)) - .thenAnswer( - new Answer() { - @Override - public RuntimeMXBean answer(InvocationOnMock invocation) { - RuntimeMXBean mockedRuntimeMxBean = mock(RuntimeMXBean.class); - when(mockedRuntimeMxBean.getVmVendor()).thenReturn(vmVendor); - when(mockedRuntimeMxBean.getVmVersion()).thenReturn(vmVersion); - return mockedRuntimeMxBean; - } - }); - - boolean actual = - FlightRecorderDiagnosticCommandConnection.jdkHasUnlockCommercialFeatures( - mBeanServerConnection); - assertEquals(expected, actual, "Expected " + expected + " for " + vmVendor + " " + vmVersion); - } - } - @Test void assertCommercialFeaturesUnlocked() throws Exception { - ObjectName objectName = mock(ObjectName.class); - MBeanServerConnection mBeanServerConnection = mockMbeanServer(objectName, "unlocked"); + MBeanServer mBeanServerConnection = ManagementFactory.getPlatformMBeanServer(); + ObjectName objectName = new ObjectName("com.sun.management:type=DiagnosticCommand"); FlightRecorderDiagnosticCommandConnection.assertCommercialFeaturesUnlocked( mBeanServerConnection, objectName); } From dc92f02d781a6d9663c4c93a3dd5a52a16e14999 Mon Sep 17 00:00:00 2001 From: David Grieve Date: Tue, 15 Oct 2024 08:47:36 -0600 Subject: [PATCH 6/7] address comments from PR 1492 --- .../FlightRecorderDiagnosticCommandConnection.java | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/jfr-connection/src/main/java/io/opentelemetry/contrib/jfr/connection/FlightRecorderDiagnosticCommandConnection.java b/jfr-connection/src/main/java/io/opentelemetry/contrib/jfr/connection/FlightRecorderDiagnosticCommandConnection.java index a0a27467f..62709c827 100644 --- a/jfr-connection/src/main/java/io/opentelemetry/contrib/jfr/connection/FlightRecorderDiagnosticCommandConnection.java +++ b/jfr-connection/src/main/java/io/opentelemetry/contrib/jfr/connection/FlightRecorderDiagnosticCommandConnection.java @@ -245,20 +245,13 @@ static void assertCommercialFeaturesUnlocked( if (operation.isPresent()) { mBeanServerConnection.invoke(objectName, "vmUnlockCommercialFeatures", params, signature); - } else { - // No vmUnlockCommercialFeatures command. - return; } } catch (InstanceNotFoundException | IntrospectionException | MBeanException | ReflectionException e) { throw JfrConnectionException.canonicalJfrConnectionException( - FlightRecorderDiagnosticCommandConnection.class, - "assertCommercialFeaturesUnlocked", - new UnsupportedOperationException( - "Unlocking commercial features may be required. This must be explicitly enabled by adding -XX:+UnlockCommercialFeatures", - e)); + FlightRecorderDiagnosticCommandConnection.class, "assertCommercialFeaturesUnlocked", e); } } From 1facf59d329bd7ee680e9edd144c9f8d6d122a0c Mon Sep 17 00:00:00 2001 From: David Grieve Date: Tue, 15 Oct 2024 14:33:17 -0600 Subject: [PATCH 7/7] add end-to-end diagnostic command test --- ...corderDiagnosticCommandConnectionTest.java | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/jfr-connection/src/test/java/io/opentelemetry/contrib/jfr/connection/FlightRecorderDiagnosticCommandConnectionTest.java b/jfr-connection/src/test/java/io/opentelemetry/contrib/jfr/connection/FlightRecorderDiagnosticCommandConnectionTest.java index 636e88ccf..8cf7f06e1 100644 --- a/jfr-connection/src/test/java/io/opentelemetry/contrib/jfr/connection/FlightRecorderDiagnosticCommandConnectionTest.java +++ b/jfr-connection/src/test/java/io/opentelemetry/contrib/jfr/connection/FlightRecorderDiagnosticCommandConnectionTest.java @@ -7,12 +7,15 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.fail; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import java.lang.management.ManagementFactory; +import java.nio.file.Files; +import java.nio.file.Path; import javax.management.MBeanServer; import javax.management.MBeanServerConnection; import javax.management.ObjectName; @@ -73,6 +76,36 @@ void startRecordingParsesIdCorrectly() throws Exception { assertEquals(id, 99); } + @Test + void endToEndTest() throws Exception { + + MBeanServerConnection mBeanServer = ManagementFactory.getPlatformMBeanServer(); + FlightRecorderConnection flightRecorderConnection = + FlightRecorderDiagnosticCommandConnection.connect(mBeanServer); + RecordingOptions recordingOptions = + new RecordingOptions.Builder().disk("true").duration("5s").build(); + RecordingConfiguration recordingConfiguration = RecordingConfiguration.PROFILE_CONFIGURATION; + Path tempFile = Files.createTempFile("recording", ".jfr"); + + try (Recording recording = + flightRecorderConnection.newRecording(recordingOptions, recordingConfiguration)) { + + recording.start(); + try { + Thread.sleep(1000); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + recording.dump(tempFile.toString()); + recording.stop(); + } finally { + if (!Files.exists(tempFile)) { + fail("Recording file not found"); + } + Files.deleteIfExists(tempFile); + } + } + MBeanServerConnection mockMbeanServer( ObjectName objectName, String vmCheckCommercialFeaturesResponse) throws Exception { MBeanServerConnection mBeanServerConnection = mock(MBeanServerConnection.class);