From 0654e95b7e05f482a2a325a9baf4916079e734e8 Mon Sep 17 00:00:00 2001 From: Tolya Korniltsev Date: Mon, 4 Sep 2023 09:13:28 +0700 Subject: [PATCH] =?UTF-8?q?fix:=20remove=20og=20pyroscope=20bucket=20align?= =?UTF-8?q?ment=20as=20it=20is=20not=20needed=20afte=20gr=E2=80=A6=20(#112?= =?UTF-8?q?)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: remove og pyroscope bucket alignment as it is not needed afte grafana merge * fix tests * fix * fix: remove og pyroscope bucket alignment as it is not needed afte grafana merge --- .../io/pyroscope/javaagent/DateUtils.java | 23 --------- .../java/io/pyroscope/javaagent/Profiler.java | 25 +++------ .../java/io/pyroscope/javaagent/Snapshot.java | 4 +- .../io/pyroscope/javaagent/config/Config.java | 5 +- .../impl/ContinuousProfilingScheduler.java | 51 +++++++------------ .../javaagent/impl/PyroscopeExporter.java | 5 +- .../impl/SamplingProfilingScheduler.java | 24 +++++---- .../javaagent/ProfilingSchedulerTest.java | 26 ---------- 8 files changed, 49 insertions(+), 114 deletions(-) delete mode 100644 agent/src/main/java/io/pyroscope/javaagent/DateUtils.java delete mode 100644 agent/src/test/java/io/pyroscope/javaagent/ProfilingSchedulerTest.java diff --git a/agent/src/main/java/io/pyroscope/javaagent/DateUtils.java b/agent/src/main/java/io/pyroscope/javaagent/DateUtils.java deleted file mode 100644 index a9f9f6a..0000000 --- a/agent/src/main/java/io/pyroscope/javaagent/DateUtils.java +++ /dev/null @@ -1,23 +0,0 @@ -package io.pyroscope.javaagent; - -import java.time.Duration; -import java.time.Instant; -import java.time.temporal.ChronoField; - -public class DateUtils { - static final long NANOS_PER_SECOND = 1_000_000_000L; - static final long SECONDS_PER_DAY = 86400; - - /** - * copy-paste from java.time.Instant#truncatedTo(java.time.temporal.TemporalUnit) - * to support Duration instead of TemporalUnit - */ - public static Instant truncate(Instant it, Duration unitDur) { - long dur = unitDur.toNanos(); - long seconds = it.getLong(ChronoField.INSTANT_SECONDS); - long nanos = it.getLong(ChronoField.NANO_OF_SECOND); - long nod = (seconds % SECONDS_PER_DAY) * NANOS_PER_SECOND + nanos; - long result = (nod / dur) * dur; - return it.plusNanos(result - nod); - } -} diff --git a/agent/src/main/java/io/pyroscope/javaagent/Profiler.java b/agent/src/main/java/io/pyroscope/javaagent/Profiler.java index cb1d449..a6cbf00 100644 --- a/agent/src/main/java/io/pyroscope/javaagent/Profiler.java +++ b/agent/src/main/java/io/pyroscope/javaagent/Profiler.java @@ -75,27 +75,15 @@ public synchronized void stop() { /** * - * @param profilingIntervalStartTime - time when profiling has been started + * @param started - time when profiling has been started + * @param ended - time when profiling has ended * @return Profiling data and dynamic labels as {@link Snapshot} */ - public synchronized Snapshot dumpProfile(Instant profilingIntervalStartTime) { - return dumpImpl(profilingIntervalStartTime); + public synchronized Snapshot dumpProfile(Instant started, Instant ended) { + return dumpImpl(started, ended); } - /** - * Stop profiling, dump profiling data, start again - * Deprecated, use start, stop, dumpProfile methods instead - */ - @Deprecated - public synchronized Snapshot dump(Instant profilingIntervalStartTime) { - instance.stop(); - Snapshot result = dumpImpl(profilingIntervalStartTime); - - start(); - - return result; - } private String createJFRCommand() { StringBuilder sb = new StringBuilder(); @@ -121,7 +109,7 @@ private String createJFRCommand() { return sb.toString(); } - private Snapshot dumpImpl(Instant profilingIntervalStartTime) { + private Snapshot dumpImpl(Instant started, Instant ended) { if (config.gcBeforeDump) { System.gc(); } @@ -134,7 +122,8 @@ private Snapshot dumpImpl(Instant profilingIntervalStartTime) { return new Snapshot( format, eventType, - profilingIntervalStartTime, + started, + ended, data, Pyroscope.LabelsWrapper.dump() ); diff --git a/agent/src/main/java/io/pyroscope/javaagent/Snapshot.java b/agent/src/main/java/io/pyroscope/javaagent/Snapshot.java index 5ae82c7..b7e0c91 100644 --- a/agent/src/main/java/io/pyroscope/javaagent/Snapshot.java +++ b/agent/src/main/java/io/pyroscope/javaagent/Snapshot.java @@ -9,13 +9,15 @@ public final class Snapshot { public final Format format; public final EventType eventType; public final Instant started; + public final Instant ended; public final byte[] data; public final JfrLabels.Snapshot labels; - Snapshot(Format format, final EventType eventType, final Instant started, final byte[] data, JfrLabels.Snapshot labels) { + Snapshot(Format format, final EventType eventType, final Instant started, final Instant ended,final byte[] data, JfrLabels.Snapshot labels) { this.format = format; this.eventType = eventType; this.started = started; + this.ended = ended; this.data = data; this.labels = labels; } diff --git a/agent/src/main/java/io/pyroscope/javaagent/config/Config.java b/agent/src/main/java/io/pyroscope/javaagent/config/Config.java index 4e9c9eb..e54510e 100644 --- a/agent/src/main/java/io/pyroscope/javaagent/config/Config.java +++ b/agent/src/main/java/io/pyroscope/javaagent/config/Config.java @@ -74,7 +74,7 @@ public final class Config { private static final List DEFAULT_SAMPLING_EVENT_ORDER = null; private static final int DEFAULT_JAVA_STACK_DEPTH_MAX = 2048; private static final String DEFAULT_SERVER_ADDRESS = "http://localhost:4040"; - private static final Format DEFAULT_FORMAT = Format.COLLAPSED; + private static final Format DEFAULT_FORMAT = Format.JFR; // The number of snapshots simultaneously stored in memory is limited by this. // The number is fairly arbitrary. If an average snapshot is 5KB, it's about 160 KB. private static final int DEFAULT_PUSH_QUEUE_CAPACITY = 8; @@ -188,6 +188,9 @@ public final class Config { DefaultLogger.PRECONFIG_LOGGER.log(Logger.Level.WARN, "Setting PYROSCOPE_PROFILER_LOCK to 0 registers every lock event, causing significant overhead and results in large profiles, making it not ideal for production. We recommend a starting value of 10ms, adjusting as needed."); } + if (format == Format.COLLAPSED) { + DefaultLogger.PRECONFIG_LOGGER.log(Logger.Level.WARN, "COLLAPSED format is deprecated"); + } } public long profilingIntervalInHertz() { diff --git a/agent/src/main/java/io/pyroscope/javaagent/impl/ContinuousProfilingScheduler.java b/agent/src/main/java/io/pyroscope/javaagent/impl/ContinuousProfilingScheduler.java index 116ac8b..1ac3505 100644 --- a/agent/src/main/java/io/pyroscope/javaagent/impl/ContinuousProfilingScheduler.java +++ b/agent/src/main/java/io/pyroscope/javaagent/impl/ContinuousProfilingScheduler.java @@ -6,6 +6,7 @@ import io.pyroscope.javaagent.api.Logger; import io.pyroscope.javaagent.api.ProfilingScheduler; import io.pyroscope.javaagent.config.Config; +import kotlin.random.Random; import java.time.Duration; import java.time.Instant; @@ -14,7 +15,6 @@ import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; -import static io.pyroscope.javaagent.DateUtils.truncate; public class ContinuousProfilingScheduler implements ProfilingScheduler { final Config config; @@ -47,18 +47,18 @@ public void start(Profiler profiler) { } final Runnable dumpProfile = () -> { Snapshot snapshot; + Instant now; try { profiler.stop(); - snapshot = profiler.dumpProfile( - alignProfilingIntervalStartTime(this.profilingIntervalStartTime, config.uploadInterval) - ); + now = Instant.now(); + snapshot = profiler.dumpProfile(this.profilingIntervalStartTime, now); profiler.start(); } catch (Throwable throwable) { logger.log(Logger.Level.ERROR, "Error dumping profiler %s", throwable); stop(); return; } - profilingIntervalStartTime = Instant.now(); + profilingIntervalStartTime = now; exporter.export(snapshot); }; @@ -76,41 +76,26 @@ private void stop() { /** * Starts the first profiling interval. - * profilingIntervalStartTime is set to a current time aligned to upload interval - * Duration of the first profiling interval will be smaller than uploadInterval for alignment. - * ... + * profilingIntervalStartTime is set to now + * Duration of the first profiling interval is a random fraction of uploadInterval not smaller than 2000ms. * * @return Duration of the first profiling interval */ private Duration startFirst(Profiler profiler) { Instant now = Instant.now(); - Instant prevUploadInterval = truncate(now, config.uploadInterval); - Instant nextUploadInterval = prevUploadInterval.plus(config.uploadInterval); - Duration firstProfilingDuration = Duration.between(now, nextUploadInterval); + + long uploadIntervalMillis = config.uploadInterval.toMillis(); + float randomOffset = Random.Default.nextFloat(); + uploadIntervalMillis = (long)((float)uploadIntervalMillis * randomOffset); + if (uploadIntervalMillis < 2000) { + uploadIntervalMillis = 2000; + } + Duration firstProfilingDuration = Duration.ofMillis(uploadIntervalMillis); + profiler.start(); - profilingIntervalStartTime = prevUploadInterval; + profilingIntervalStartTime = now; return firstProfilingDuration; } - /** - * Aligns profilingIntervalStartTime to the closest aligned upload time either forward or backward - * For example if upload interval is 10s and profilingIntervalStartTime is 00:00.01 it will return 00:00 - * and if profilingIntervalStartTime is 00:09.239 it will return 00:10 - * ... - * - * @param profilingIntervalStartTime the time to align - * @param uploadInterval - * @return the aligned - */ - public static Instant alignProfilingIntervalStartTime(Instant profilingIntervalStartTime, Duration uploadInterval) { - Instant prev = truncate(profilingIntervalStartTime, uploadInterval); - Instant next = prev.plus(uploadInterval); - Duration d1 = Duration.between(prev, profilingIntervalStartTime); - Duration d2 = Duration.between(profilingIntervalStartTime, next); - if (d1.compareTo(d2) < 0) { - return prev; - } else { - return next; - } - } + } 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 7740c98..171a35e 100644 --- a/agent/src/main/java/io/pyroscope/javaagent/impl/PyroscopeExporter.java +++ b/agent/src/main/java/io/pyroscope/javaagent/impl/PyroscopeExporter.java @@ -53,7 +53,8 @@ private void uploadSnapshot(final Snapshot snapshot) throws InterruptedException final RequestBody requestBody; if (config.format == Format.JFR) { byte[] labels = snapshot.labels.toByteArray(); - logger.log(Logger.Level.DEBUG, "Upload attempt %d. JFR: %s, labels: %s", tries, snapshot.data.length, labels.length); + logger.log(Logger.Level.DEBUG, "Upload attempt %d. %s %s JFR: %s, labels: %s", tries, + snapshot.started.toString(), snapshot.ended.toString(), snapshot.data.length, labels.length); MultipartBody.Builder bodyBuilder = new MultipartBody.Builder() .setType(MultipartBody.FORM); RequestBody jfrBody = RequestBody.create(snapshot.data); @@ -134,7 +135,7 @@ private static void addAuthHeader(Request.Builder request, HttpUrl url, Config c private HttpUrl urlForSnapshot(final Snapshot snapshot) { Instant started = snapshot.started; - Instant finished = started.plus(config.uploadInterval); + Instant finished = snapshot.ended; HttpUrl.Builder builder = HttpUrl.parse(config.serverAddress) .newBuilder() .addPathSegment("ingest") diff --git a/agent/src/main/java/io/pyroscope/javaagent/impl/SamplingProfilingScheduler.java b/agent/src/main/java/io/pyroscope/javaagent/impl/SamplingProfilingScheduler.java index d59acd3..b8f9830 100644 --- a/agent/src/main/java/io/pyroscope/javaagent/impl/SamplingProfilingScheduler.java +++ b/agent/src/main/java/io/pyroscope/javaagent/impl/SamplingProfilingScheduler.java @@ -1,6 +1,5 @@ package io.pyroscope.javaagent.impl; -import static io.pyroscope.javaagent.DateUtils.truncate; import io.pyroscope.javaagent.EventType; import io.pyroscope.javaagent.Profiler; @@ -9,6 +8,8 @@ import io.pyroscope.javaagent.api.Logger; import io.pyroscope.javaagent.api.ProfilingScheduler; import io.pyroscope.javaagent.config.Config; +import kotlin.random.Random; + import io.pyroscope.javaagent.config.Config.Builder; import java.time.Duration; @@ -47,8 +48,8 @@ public SamplingProfilingScheduler(Config config, Exporter exporter, Logger logge public void start(Profiler profiler) { final long samplingDurationMillis = config.samplingDuration.toMillis(); final Duration uploadInterval = config.uploadInterval; - - final Runnable task = (null != config.samplingEventOrder) ? + + final Runnable task = (null != config.samplingEventOrder) ? () -> { for (int i = 0; i < config.samplingEventOrder.size(); i++) { final EventType t = config.samplingEventOrder.get(i); @@ -57,7 +58,7 @@ public void start(Profiler profiler) { profiler.setConfig(tmp); dumpProfile(profiler, samplingDurationMillis, uploadInterval); } - } : + } : () -> dumpProfile(profiler, samplingDurationMillis, uploadInterval); Duration initialDelay = getInitialDelay(); @@ -85,7 +86,7 @@ private void dumpProfile(final Profiler profiler, final long samplingDurationMil } profiler.stop(); - Snapshot snapshot = profiler.dumpProfile(truncate(profilingStartTime, uploadInterval)); + Snapshot snapshot = profiler.dumpProfile(profilingStartTime, Instant.now()); exporter.export(snapshot); } @@ -97,11 +98,14 @@ private void stop() { } private Duration getInitialDelay() { - Instant now = Instant.now(); - Instant prevUploadInterval = truncate(now, config.uploadInterval); - Instant nextUploadInterval = prevUploadInterval.plus(config.uploadInterval); - Duration initialDelay = Duration.between(now, nextUploadInterval); - return initialDelay; + long uploadIntervalMillis = config.uploadInterval.toMillis(); + float randomOffset = Random.Default.nextFloat(); + uploadIntervalMillis = (long)((float)uploadIntervalMillis * randomOffset); + if (uploadIntervalMillis < 2000) { + uploadIntervalMillis = 2000; + } + Duration firstProfilingDuration = Duration.ofMillis(uploadIntervalMillis); + return firstProfilingDuration; } private Config isolate(final EventType type, final Config config) { diff --git a/agent/src/test/java/io/pyroscope/javaagent/ProfilingSchedulerTest.java b/agent/src/test/java/io/pyroscope/javaagent/ProfilingSchedulerTest.java deleted file mode 100644 index 32ba374..0000000 --- a/agent/src/test/java/io/pyroscope/javaagent/ProfilingSchedulerTest.java +++ /dev/null @@ -1,26 +0,0 @@ -package io.pyroscope.javaagent; - -import io.pyroscope.javaagent.impl.ContinuousProfilingScheduler; -import org.junit.jupiter.api.Test; - -import java.time.Duration; -import java.time.Instant; - -import static org.junit.jupiter.api.Assertions.*; - -class ProfilingSchedulerTest { - @Test - void testAlignProfilingTimeBackward() { - Instant t = Instant.parse("2022-06-11T16:10:32.239239Z"); - Instant expected = Instant.parse("2022-06-11T16:10:30Z"); - Instant res = ContinuousProfilingScheduler.alignProfilingIntervalStartTime(t, Duration.ofSeconds(10)); - assertEquals(expected, res); - } - @Test - void testAlignProfilingTimeForward() { - Instant t = Instant.parse("2022-06-11T16:10:39.239239Z"); - Instant expected = Instant.parse("2022-06-11T16:10:40Z"); - Instant res = ContinuousProfilingScheduler.alignProfilingIntervalStartTime(t, Duration.ofSeconds(10)); - assertEquals(expected, res); - } -}