feat: support for other event types (#2)

dmos62 · web-flow · commit 451681ebe2e3 · 2021-11-08T20:53:15.000-08:00
* Add .editorconfig

* Add TODO

* Refactor to support multiple event types

* Include Java specific .gitignore entries

* Narrow indent spec to subset of files

* Use enums for units and agg. type

* Move HTTP API enums to own namespace

* Specify units and agg. type per event type

* Fix some bugs

* Set units for ALLOC to BYTES

* Fix NPE; introduce timeseries name

* Use Units.OBJECTS for EventType.ALLOC
diff --git a/.editorconfig b/.editorconfig
@@ -0,0 +1,8 @@
+root = true
+
+[*]
+end_of_line = lf
+indent_style = space
+
+[{*.java,build.gradle,settings.gradle}]
+indent_size = 4
diff --git a/.gitignore b/.gitignore
@@ -5,3 +5,31 @@
 
 # Ignore Gradle build output directory
 build
+
+# Compiled class file
+*.class
+
+# Log file
+*.log
+
+# BlueJ files
+*.ctxt
+
+# Mobile Tools for Java (J2ME)
+.mtj.tmp/
+
+# Package Files #
+*.jar
+*.war
+*.nar
+*.ear
+*.zip
+*.tar.gz
+*.rar
+
+# virtual machine crash logs, see http://www.java.com/en/download/help/error_hotspot.xml
+hs_err_pid*
+
+.settings
+.project
+.classpath
diff --git a/TODO b/TODO
@@ -0,0 +1,15 @@
+TODO support multiple simultaneous profiling modes (profiling event types)
+  - async-profiler dump format
+    - ATM, the collapsed dump format (what we're using) is not supported for multiple-event profiling
+      - can it be supported?
+      - see
+        - https://github.com/jvm-profiling-tools/async-profiler/issues/150
+        - https://github.com/jvm-profiling-tools/async-profiler/issues/357
+  - data flow changes
+    - ATM, sample snapshots are uploaded specifying event-type-dependent parameters per snapshot
+      - so multiple-event profiling needs a dedicated queue+snapshot+upload pipeline for each event type used simultaneously
+
+TODO support per-thread profiling
+  - use AsnycProfiler::execute
+    - see
+      - https://github.com/jvm-profiling-tools/async-profiler/issues/473
diff --git a/agent/src/main/java/io/pyroscope/http/AggregationType.java b/agent/src/main/java/io/pyroscope/http/AggregationType.java
@@ -0,0 +1,15 @@
+package io.pyroscope.http;
+
+public enum AggregationType {
+    SUM ("sum"),
+    AVERAGE ("average");
+
+    /**
+    * Pyroscope aggregation type id, as expected by Pyroscope's HTTP API.
+    */
+    public final String id;
+
+    AggregationType(String id) {
+        this.id = id;
+    }
+}
diff --git a/agent/src/main/java/io/pyroscope/http/Units.java b/agent/src/main/java/io/pyroscope/http/Units.java
@@ -0,0 +1,16 @@
+package io.pyroscope.http;
+
+public enum Units {
+    SAMPLES ("samples"),
+    OBJECTS ("objects"),
+    BYTES ("bytes");
+
+    /**
+    * Pyroscope units id, as expected by Pyroscope's HTTP API.
+    */
+    public final String id;
+
+    Units(String id) {
+        this.id = id;
+    }
+}
diff --git a/agent/src/main/java/io/pyroscope/javaagent/EventType.java b/agent/src/main/java/io/pyroscope/javaagent/EventType.java
@@ -0,0 +1,46 @@
+package io.pyroscope.javaagent;
+
+import java.util.EnumSet;
+import java.util.Optional;
+
+import one.profiler.Events;
+import io.pyroscope.http.Units;
+import io.pyroscope.http.AggregationType;
+
+public enum EventType {
+    CPU (Events.CPU, Units.SAMPLES, AggregationType.SUM),
+    ALLOC (Events.ALLOC, Units.OBJECTS, AggregationType.SUM),
+    LOCK (Events.LOCK, Units.SAMPLES, AggregationType.SUM),
+    WALL (Events.WALL, Units.SAMPLES, AggregationType.SUM),
+    ITIMER (Events.ITIMER, Units.SAMPLES, AggregationType.SUM);
+
+    /**
+    * Event type id, as defined in one.profiler.Events.
+    */
+    public final String id;
+
+    /**
+    * Unit option, as expected by Pyroscope's HTTP API.
+    */
+    public final Units units;
+
+    /**
+    * Aggregation type option, as expected by Pyroscope's HTTP API.
+    */
+    public final AggregationType aggregationType;
+
+    EventType(String id, Units units, AggregationType aggregationType) {
+        this.id = id;
+        this.units = units;
+        this.aggregationType = aggregationType;
+    }
+
+    public static EventType fromId(String id) throws IllegalArgumentException {
+        Optional<EventType> maybeEventType =
+            EnumSet.allOf(EventType.class)
+            .stream()
+            .filter(eventType -> eventType.id.equals(id))
+            .findAny();
+        return maybeEventType.orElseThrow(IllegalArgumentException::new);
+    }
+}
diff --git a/agent/src/main/java/io/pyroscope/javaagent/Profiler.java b/agent/src/main/java/io/pyroscope/javaagent/Profiler.java
@@ -14,7 +14,7 @@
 
 class Profiler {
     private final Logger logger;
-    private final String event;
+    private final EventType eventType;
     private final Duration interval;
 
     private static String libraryPath;
@@ -123,16 +123,18 @@ public static void init() {
 
     private final AsyncProfiler instance = AsyncProfiler.getInstance(libraryPath);
 
+    // TODO this is actually start of snapshot, not profiling as a whole
     private Instant profilingStarted = null;
 
-    Profiler(final Logger logger, final String event, final Duration interval) {
+    Profiler(final Logger logger, final EventType eventType, final Duration interval) {
         this.logger = logger;
-        this.event = event;
+        this.eventType = eventType;
         this.interval = interval;
     }
 
+    // TODO new method for starting new snapshot/batch
     final synchronized void start() {
-        instance.start(event, interval.toNanos());
+        instance.start(eventType.id, interval.toNanos());
         profilingStarted = Instant.now();
         logger.info("Profiling started");
     }
@@ -142,10 +144,16 @@ final synchronized Snapshot dump() {
             throw new IllegalStateException("Profiling is not started");
         }
 
-        final Snapshot result = new Snapshot(profilingStarted, Instant.now(), instance.dumpCollapsed(Counter.SAMPLES));
+        final Snapshot result = new Snapshot(
+            eventType,
+            profilingStarted,
+            Instant.now(),
+            instance.dumpCollapsed(Counter.SAMPLES)
+        );
 
+        // TODO use `this.start()` or analogue
         profilingStarted = Instant.now();
-        instance.start(event, interval.toNanos());
+        instance.start(eventType.id, interval.toNanos());
         return result;
     }
 }
diff --git a/agent/src/main/java/io/pyroscope/javaagent/PyroscopeAgent.java b/agent/src/main/java/io/pyroscope/javaagent/PyroscopeAgent.java
@@ -40,7 +40,7 @@ public static void premain(final String agentArgs,
         }
 
         try {
-            final Profiler profiler = new Profiler(logger, Events.ITIMER, config.profilingInterval);
+            final Profiler profiler = new Profiler(logger, config.profilingEvent, config.profilingInterval);
 
             final ScheduledExecutorService executor = Executors.newSingleThreadScheduledExecutor();
             profiler.start();
diff --git a/agent/src/main/java/io/pyroscope/javaagent/Snapshot.java b/agent/src/main/java/io/pyroscope/javaagent/Snapshot.java
@@ -3,11 +3,13 @@
 import java.time.Instant;
 
 final class Snapshot {
+    public final EventType eventType;
     public final Instant started;
     public final Instant finished;
     public final String data;
 
-    Snapshot(final Instant started, final Instant finished, final String data) {
+    Snapshot(final EventType eventType, final Instant started, final Instant finished, final String data) {
+        this.eventType = eventType;
         this.started = started;
         this.finished = finished;
         this.data = data;
diff --git a/agent/src/main/java/io/pyroscope/javaagent/Uploader.java b/agent/src/main/java/io/pyroscope/javaagent/Uploader.java
@@ -68,7 +68,10 @@ private void uploadSnapshot(final Snapshot snapshot) throws InterruptedException
     private URL urlForSnapshot(final Snapshot snapshot) {
         try {
             final URI baseUri = URI.create(config.serverAddress);
-            final String query = urlParam("name", config.applicationName)
+            final String query = urlParam("name", config.timeseriesName)
+                    + "&" + urlParam("units", snapshot.eventType.units.id)
+                    + "&" + urlParam("aggregationType", snapshot.eventType.aggregationType.id)
+                    + "&" + urlParam("sampleRate", Long.toString(config.profilingIntervalInHertz()))
                     + "&" + urlParam("from", Long.toString(snapshot.started.getEpochSecond()))
                     + "&" + urlParam("until", Long.toString(snapshot.finished.getEpochSecond()))
                     + "&" + urlParam("spyName", config.spyName);
diff --git a/agent/src/main/java/io/pyroscope/javaagent/config/Config.java b/agent/src/main/java/io/pyroscope/javaagent/config/Config.java
@@ -1,7 +1,7 @@
 package io.pyroscope.javaagent.config;
 
+import io.pyroscope.javaagent.EventType;
 import io.pyroscope.javaagent.PreConfigLogger;
-import one.profiler.Events;
 import org.apache.logging.log4j.Level;
 
 import java.nio.ByteBuffer;
@@ -21,38 +21,57 @@ public final class Config {
 
     private static final String DEFAULT_SPY_NAME = "javaspy";
     private static final Duration DEFAULT_PROFILING_INTERVAL = Duration.ofMillis(10);
-    private static final String DEFAULT_PROFILER_EVENT = Events.ITIMER;
+    private static final EventType DEFAULT_PROFILER_EVENT = EventType.ITIMER;
     private static final Duration DEFAULT_UPLOAD_INTERVAL = Duration.ofSeconds(10);
     private static final String DEFAULT_SERVER_ADDRESS = "http://localhost:4040";
 
     public final String spyName = DEFAULT_SPY_NAME;
     public final String applicationName;
     public final Duration profilingInterval;
-    public final String profilingEvent;
+    public final EventType profilingEvent;
     public final Duration uploadInterval;
     public final Level logLevel;
     public final String serverAddress;
     public final String authToken;
+    public final String timeseriesName;
 
     Config(final String applicationName,
            final Duration profilingInterval,
-           final String profilingEvent,
+           final EventType profilingEvent,
            final Duration uploadInterval,
            final Level logLevel,
            final String serverAddress,
-           final String authToken) {
+           final String authToken
+        ) {
         this.applicationName = applicationName;
         this.profilingInterval = profilingInterval;
         this.profilingEvent = profilingEvent;
         this.uploadInterval = uploadInterval;
         this.logLevel = logLevel;
         this.serverAddress = serverAddress;
         this.authToken = authToken;
+        this.timeseriesName = timeseriesName(applicationName, profilingEvent);
+    }
+
+    public long profilingIntervalInHertz() {
+        return durationToHertz(this.profilingInterval);
+    }
+
+    private static long durationToHertz(Duration duration) {
+        Duration oneSecond = Duration.ofSeconds(1);
+        return oneSecond.toNanos() / duration.toNanos();
     }
 
     public static Config build() {
-        return new Config(applicationName(), profilingInterval(),
-                profilingEvent(), uploadInterval(), logLevel(), serverAddress(), authToken());
+        return new Config(
+            applicationName(),
+            profilingInterval(),
+            profilingEvent(),
+            uploadInterval(),
+            logLevel(),
+            serverAddress(),
+            authToken()
+        );
     }
 
     private static String applicationName() {
@@ -88,22 +107,24 @@ private static Duration profilingInterval() {
         }
     }
 
-    private static String profilingEvent() {
-        final String profilingIntervalStr = System.getenv(PYROSCOPE_PROFILER_EVENT_CONFIG);
-        if (profilingIntervalStr == null || profilingIntervalStr.isEmpty()) {
+    private String timeseriesName(String applicationName, EventType eventType) {
+        return applicationName + "." + eventType.id;
+    }
+
+    private static EventType profilingEvent() {
+        final String profilingEventStr =
+            System.getenv(PYROSCOPE_PROFILER_EVENT_CONFIG);
+        if (profilingEventStr == null || profilingEventStr.isEmpty()) {
             return DEFAULT_PROFILER_EVENT;
         }
 
-        final String profilingIntervalStrLC = profilingIntervalStr.toLowerCase(Locale.ROOT);
-        if (profilingIntervalStrLC.equals(Events.ITIMER)
-                || profilingIntervalStrLC.equals(Events.CPU)
-                || profilingIntervalStrLC.equals(Events.ALLOC)
-                || profilingIntervalStrLC.equals(Events.LOCK)
-                || profilingIntervalStrLC.equals(Events.WALL)) {
-            return profilingIntervalStr;
-        } else {
+        final String lowerCaseTrimmed = profilingEventStr.trim().toLowerCase();
+
+        try {
+            return EventType.fromId(lowerCaseTrimmed);
+        } catch (IllegalArgumentException e) {
             PreConfigLogger.LOGGER.warn("Invalid {} value {}, using {}",
-                    PYROSCOPE_PROFILER_EVENT_CONFIG, profilingIntervalStrLC, DEFAULT_PROFILER_EVENT);
+                    PYROSCOPE_PROFILER_EVENT_CONFIG, profilingEventStr, DEFAULT_PROFILER_EVENT.id);
             return DEFAULT_PROFILER_EVENT;
         }
     }

Original file line number	Diff line number	Diff line change
`@@ -40,7 +40,7 @@ public static void premain(final String agentArgs,`
`40`	`40`	`}`
`41`	`41`
`42`	`42`	`try {`
`43`		`- final Profiler profiler = new Profiler(logger, Events.ITIMER, config.profilingInterval);`
	`43`	`+ final Profiler profiler = new Profiler(logger, config.profilingEvent, config.profilingInterval);`
`44`	`44`
`45`	`45`	`final ScheduledExecutorService executor = Executors.newSingleThreadScheduledExecutor();`
`46`	`46`	`profiler.start();`