grafana / pyroscope-java

pyroscope java integration
Apache License 2.0
72 stars 31 forks source link

Jfr profiler support #136

Open kcrimson opened 6 months ago

kcrimson commented 6 months ago

This change adds support for JFR recordings, it allows to run agents on platforms that don't support async profiler (like Windows). We have introduced ProfilerDelegate abstraction, allowing us to use different implementations to collect profiler recordings. This requires running JFR commands with specific settings, which only collect events supported by the JFR parser on the pyroscope server side. This configuration can be found pyroscope.jfc file. Any changes/suggestions are more than welcomed.

CLAassistant commented 6 months ago

CLA assistant check
All committers have signed the CLA.

korniltsev commented 6 months ago

Thanks for the PR. Look forward to merge it. I will take a look soon. Quick question: Does ingestion to pyroscope work?. Lat time I've checked the previous version of jfr-parser was unable to parse jcmd produced JFR files (https://github.com/grafana/jfr-parser/issues/20 ) . The parser was fully rewritten so it may or may not work, idk, I will do some tests. Nevermind just read the PR description

korniltsev commented 6 months ago

I also noticed LabelsWrapper does not work. I think we should make it not crash and maybe print a warning once.

With some little changes (mostly for debugging purposes) to workaround issues I encounted the profiling seem to start.

diff --git a/agent/src/main/java/io/pyroscope/javaagent/CurrentPidProvider.java b/agent/src/main/java/io/pyroscope/javaagent/CurrentPidProvider.java
index 089651e..60ba240 100644
--- a/agent/src/main/java/io/pyroscope/javaagent/CurrentPidProvider.java
+++ b/agent/src/main/java/io/pyroscope/javaagent/CurrentPidProvider.java
@@ -10,19 +10,6 @@ import java.lang.reflect.Method;

 public class CurrentPidProvider {
     public static int getCurrentProcessId() {
-        RuntimeMXBean runtime = ManagementFactory.getRuntimeMXBean();
-        Field jvm = null;
-        try {
-            jvm = runtime.getClass().getDeclaredField("jvm");
-            jvm.setAccessible(true);
-
-            VMManagement management = (VMManagement) jvm.get(runtime);
-            Method method = management.getClass().getDeclaredMethod("getProcessId");
-            method.setAccessible(true);
-
-            return (Integer) method.invoke(management);
-        } catch (NoSuchFieldException | InvocationTargetException | IllegalAccessException | NoSuchMethodException e) {
-            throw new RuntimeException(e);
-        }
+        return (int) ProcessHandle.current().pid();
     }
 }
diff --git a/agent/src/main/java/io/pyroscope/javaagent/JFRProfilerDelegate.java b/agent/src/main/java/io/pyroscope/javaagent/JFRProfilerDelegate.java
index aefc735..2557b4d 100644
--- a/agent/src/main/java/io/pyroscope/javaagent/JFRProfilerDelegate.java
+++ b/agent/src/main/java/io/pyroscope/javaagent/JFRProfilerDelegate.java
@@ -30,6 +30,7 @@ public final class JFRProfilerDelegate implements ProfilerDelegate {
             // flight recorder is built on top of a file descriptor, so we need a file.
             tempJFRFile = File.createTempFile("pyroscope", ".jfr");
             tempJFRFile.deleteOnExit();
+            System.out.println(tempJFRFile);
         } catch (IOException e) {
             throw new IllegalStateException(e);
         }
@@ -46,9 +47,11 @@ public final class JFRProfilerDelegate implements ProfilerDelegate {
             commands.add("JFR.start");
             commands.add("name=" + RECORDING_NAME);
             commands.add("filename=" + tempJFRFile.getAbsolutePath());
-            commands.add("settings=pyroscope");
+            commands.add("settings=C:\\Users\\huihu\\pyro\\pyroscope-java\\agent\\build\\resources\\main\\pyroscope.jfc");
             ProcessBuilder processBuilder = new ProcessBuilder(commands);
             Process process = processBuilder.start();
+            System.out.println(new String(process.getInputStream().readAllBytes()));
+            System.out.println(new String(process.getErrorStream().readAllBytes()));
             int exitCode = process.waitFor();
             if (exitCode != 0) {
                 throw new RuntimeException("Invalid exit code: " + exitCode);
@@ -72,6 +75,8 @@ public final class JFRProfilerDelegate implements ProfilerDelegate {
             commands.add("name=" + RECORDING_NAME);
             ProcessBuilder processBuilder = new ProcessBuilder(commands);
             Process process = processBuilder.start();
+            System.out.println(new String(process.getInputStream().readAllBytes()));
+            System.out.println(new String(process.getErrorStream().readAllBytes()));
             int exitCode = process.waitFor();
             if (exitCode != 0) {
                 throw new RuntimeException("Invalid exit code: " + exitCode);
@@ -114,7 +119,7 @@ public final class JFRProfilerDelegate implements ProfilerDelegate {
     private static Path findJcmdBin() {
         Path javaHome = Paths.get(System.getProperty("java.home"));
         //find jcmd binary
-        Path jcmdBin = javaHome.resolve("bin/jcmd");
+        Path jcmdBin = javaHome.resolve("bin/jcmd.exe");
         if (!Files.isExecutable(jcmdBin)) {
             jcmdBin = javaHome.getParent().resolve("bin/jcmd");
             if (!Files.isExecutable(jcmdBin)) {
diff --git a/agent/src/main/java/io/pyroscope/javaagent/impl/PyroscopeExporter.java b/agent/src/main/java/io/pyroscope/javaagent/impl/PyroscopeExporter.java
index 850ab4d..71f9ef6 100644
--- a/agent/src/main/java/io/pyroscope/javaagent/impl/PyroscopeExporter.java
+++ b/agent/src/main/java/io/pyroscope/javaagent/impl/PyroscopeExporter.java
@@ -9,6 +9,7 @@ import io.pyroscope.javaagent.util.zip.GzipSink;
 import io.pyroscope.labels.Pyroscope;
 import okhttp3.*;

+import java.io.FileOutputStream;
 import java.io.IOException;
 import java.time.Duration;
 import java.time.Instant;
@@ -43,7 +44,16 @@ public class PyroscopeExporter implements Exporter {
         }
     }

+    int counter;
     private void uploadSnapshot(final Snapshot snapshot) throws InterruptedException {
+        String fname = String.format("dump_%d.jfr", counter++);
+        try {
+            FileOutputStream fos = new FileOutputStream(fname);
+            fos.write(snapshot.data);
+            fos.close();
+        } catch (IOException e) {
+            throw new RuntimeException(e);
+        }
         final HttpUrl url = urlForSnapshot(snapshot);
         final ExponentialBackoff exponentialBackoff = new ExponentialBackoff(1_000, 30_000, new Random());
         boolean retry = true;
diff --git a/demo/src/main/java/App.java b/demo/src/main/java/App.java
index cf9480e..2295e34 100644
--- a/demo/src/main/java/App.java
+++ b/demo/src/main/java/App.java
@@ -4,6 +4,7 @@ import io.pyroscope.javaagent.Snapshot;
 import io.pyroscope.javaagent.api.Exporter;
 import io.pyroscope.javaagent.api.Logger;
 import io.pyroscope.javaagent.config.Config;
+import io.pyroscope.javaagent.config.ProfilerType;
 import io.pyroscope.javaagent.impl.DefaultConfigurationProvider;
 import io.pyroscope.labels.Pyroscope;
 import io.pyroscope.labels.LabelsSet;
@@ -20,9 +21,10 @@ public class App {
         PyroscopeAgent.start(
             new PyroscopeAgent.Options.Builder(
                 new Config.Builder()
-                    .setApplicationName("demo.app{qweqwe=asdasd}")
-                    .setServerAddress("http://localhost:4040")
+                    .setApplicationName("windows.app")
+                    .setServerAddress("http://192.168.0.13:4040")
                     .setFormat(Format.JFR)
+                    .setProfilerType(ProfilerType.JFR)
                     .setLogLevel(Logger.Level.DEBUG)
                     .setLabels(mapOf("user", "tolyan"))
                     .build())
@@ -38,7 +40,7 @@ public class App {
         ExecutorService executors = Executors.newFixedThreadPool(N_THREADS);
         for (int i = 0; i < N_THREADS; i++) {
             executors.submit(() -> {
-                Pyroscope.LabelsWrapper.run(new LabelsSet("thread_name", Thread.currentThread().getName()), () -> {
+//                Pyroscope.LabelsWrapper.run(new LabelsSet("thread_name", Thread.currentThread().getName()), () -> {
                         while (true) {
                             try {
                                 fib(32L);
@@ -47,8 +49,8 @@ public class App {
                                 break;
                             }
                         }
-                    }
-                );
+//                    }
+//                );
             });
         }
     }
@@ -68,7 +70,6 @@ public class App {
         if (n == 1L) {
             return 1L;
         }
-        Thread.sleep(100);
         return fib(n - 1) + fib(n - 2);
     }

It started profiling on my VM.

However it failed to ingest

2024-01-01 22:25:01.512 [ERROR] Error uploading snapshot: 422 {"code":"unknown","message":"parsing IngestInput-pprof failed jfr parser ParseEvent error: error reading metadata: unknown string type 4"}

dump_2.jfr.zip

we will need to update jfr-parser I guess

korniltsev commented 5 months ago

just FYI I've made some changes to our jfr-parser https://github.com/grafana/jfr-parser/pull/27 The jfr-parser may now be a little bit closer to be able to parse Flight Recorder's jfr files. Did not test it yet though.

korniltsev commented 5 months ago

please let me know when the PR is ready for second round review. look forward to merge it.

korniltsev commented 1 week ago

Hey, thanks for updating. I'll try to look next week.