From 8583ac21e84e13e92e83c82a1dfb357a171848ff Mon Sep 17 00:00:00 2001 From: Jeff Fenchel Date: Sun, 22 Nov 2015 14:26:53 -0800 Subject: [PATCH 1/3] - Added port incrementing logic when the configured port is already taken - Added exception handling and a logging endpoint for agents - Added a endpoint to track the number of times metrics have been senct from each agent --- .../java/com/etsy/statsd/profiler/Agent.java | 20 +++--- .../com/etsy/statsd/profiler/Profiler.java | 5 ++ .../profiler/server/ProfilerServer.java | 22 +++++-- .../profiler/server/RequestHandler.java | 65 ++++++++++++++++--- .../worker/ProfilerShutdownHookWorker.java | 8 ++- .../profiler/worker/ProfilerWorkerThread.java | 20 +++++- .../ProfilerShutdownHookWorkerTest.java | 6 +- .../worker/ProfilerWorkerThreadTest.java | 3 +- 8 files changed, 122 insertions(+), 27 deletions(-) diff --git a/src/main/java/com/etsy/statsd/profiler/Agent.java b/src/main/java/com/etsy/statsd/profiler/Agent.java index 19ae9a4..2827826 100644 --- a/src/main/java/com/etsy/statsd/profiler/Agent.java +++ b/src/main/java/com/etsy/statsd/profiler/Agent.java @@ -10,14 +10,12 @@ import java.lang.instrument.Instrumentation; import java.lang.reflect.Constructor; import java.lang.reflect.InvocationTargetException; -import java.util.ArrayList; -import java.util.Collection; -import java.util.HashMap; -import java.util.Map; +import java.util.*; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.ScheduledThreadPoolExecutor; +import java.util.concurrent.atomic.AtomicReference; /** * javaagent profiler using StatsD as a backend @@ -27,6 +25,8 @@ public class Agent { public static final int EXECUTOR_DELAY = 0; + static AtomicReference isRunning = new AtomicReference<>(true); + static LinkedList errors = new LinkedList<>(); /** * Start the profiler * @@ -59,12 +59,14 @@ private static void scheduleProfilers(Collection profilers, int httpPo (ScheduledThreadPoolExecutor) Executors.newScheduledThreadPool(profilers.size(), new ProfilerThreadFactory())); Map> runningProfilers = new HashMap<>(profilers.size()); + Map activeProfilers = new HashMap<>(profilers.size()); for (Profiler profiler : profilers) { - ProfilerWorkerThread worker = new ProfilerWorkerThread(profiler); - runningProfilers.put(profiler.getClass().getSimpleName(), - scheduledExecutorService.scheduleAtFixedRate(worker, EXECUTOR_DELAY, profiler.getPeriod(), profiler.getTimeUnit())); + activeProfilers.put(profiler.getClass().getSimpleName(), profiler); + ProfilerWorkerThread worker = new ProfilerWorkerThread(profiler, errors); + ScheduledFuture future = scheduledExecutorService.scheduleAtFixedRate(worker, EXECUTOR_DELAY, profiler.getPeriod(), profiler.getTimeUnit()); + runningProfilers.put(profiler.getClass().getSimpleName(), future); } - ProfilerServer.startServer(runningProfilers, httpPort); + ProfilerServer.startServer(runningProfilers, activeProfilers, httpPort, isRunning, errors); } /** @@ -73,7 +75,7 @@ private static void scheduleProfilers(Collection profilers, int httpPo * @param profilers The profilers to flush at shutdown */ private static void registerShutdownHook(Collection profilers) { - Thread shutdownHook = new Thread(new ProfilerShutdownHookWorker(profilers)); + Thread shutdownHook = new Thread(new ProfilerShutdownHookWorker(profilers, isRunning)); Runtime.getRuntime().addShutdownHook(shutdownHook); } diff --git a/src/main/java/com/etsy/statsd/profiler/Profiler.java b/src/main/java/com/etsy/statsd/profiler/Profiler.java index d76b21d..6825163 100644 --- a/src/main/java/com/etsy/statsd/profiler/Profiler.java +++ b/src/main/java/com/etsy/statsd/profiler/Profiler.java @@ -16,6 +16,7 @@ public abstract class Profiler { private Reporter reporter; + private long recordedStats = 0; public Profiler(Reporter reporter, Arguments arguments) { Preconditions.checkNotNull(reporter); this.reporter = reporter; @@ -71,6 +72,7 @@ protected boolean emitBounds() { * @param value The value of the gauge */ protected void recordGaugeValue(String key, long value) { + recordedStats++; reporter.recordGaugeValue(key, value); } @@ -81,6 +83,9 @@ protected void recordGaugeValue(String key, long value) { * @param gauges A map of gauge names to values */ protected void recordGaugeValues(Map gauges) { + recordedStats++; reporter.recordGaugeValues(gauges); } + + public long getRecordedStats() { return recordedStats; } } diff --git a/src/main/java/com/etsy/statsd/profiler/server/ProfilerServer.java b/src/main/java/com/etsy/statsd/profiler/server/ProfilerServer.java index 7af87fb..ce3be4b 100644 --- a/src/main/java/com/etsy/statsd/profiler/server/ProfilerServer.java +++ b/src/main/java/com/etsy/statsd/profiler/server/ProfilerServer.java @@ -1,11 +1,17 @@ package com.etsy.statsd.profiler.server; +import com.etsy.statsd.profiler.Profiler; +import com.sun.org.apache.xpath.internal.operations.Bool; +import org.vertx.java.core.AsyncResult; +import org.vertx.java.core.Handler; import org.vertx.java.core.Vertx; import org.vertx.java.core.VertxFactory; import org.vertx.java.core.http.HttpServer; +import java.util.LinkedList; import java.util.Map; import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.atomic.AtomicReference; /** * Sets up a simple embedded HTTP server for interacting with the profiler while it runs @@ -21,9 +27,17 @@ public class ProfilerServer { * @param activeProfilers The active profilers * @param port The port on which to bind the server */ - public static void startServer(final Map> activeProfilers, final int port) { - HttpServer server = vertx.createHttpServer(); - server.requestHandler(RequestHandler.getMatcher(activeProfilers)); - server.listen(port); + public static void startServer(final Map> runningProfilers, final Map activeProfilers, final int port, final AtomicReference isRunning, final LinkedList errors) { + final HttpServer server = vertx.createHttpServer(); + server.requestHandler(RequestHandler.getMatcher(runningProfilers, activeProfilers, isRunning, errors)); + server.listen(port, new Handler>() { + @Override + public void handle(AsyncResult event) { + if (event.failed()) { + server.close(); + startServer(runningProfilers, activeProfilers, port + 1, isRunning, errors); + } + } + }); } } \ No newline at end of file diff --git a/src/main/java/com/etsy/statsd/profiler/server/RequestHandler.java b/src/main/java/com/etsy/statsd/profiler/server/RequestHandler.java index e891fca..648c53f 100644 --- a/src/main/java/com/etsy/statsd/profiler/server/RequestHandler.java +++ b/src/main/java/com/etsy/statsd/profiler/server/RequestHandler.java @@ -1,5 +1,6 @@ package com.etsy.statsd.profiler.server; +import com.etsy.statsd.profiler.Profiler; import com.google.common.base.Function; import com.google.common.base.Joiner; import com.google.common.base.Predicate; @@ -9,8 +10,10 @@ import org.vertx.java.core.http.RouteMatcher; import java.util.Collection; +import java.util.LinkedList; import java.util.Map; import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.atomic.AtomicReference; /** * Handler for HTTP requests to the profiler server @@ -18,32 +21,61 @@ * @author Andrew Johnson */ public class RequestHandler { - + /** * Construct a RouteMatcher for the supported routes * * @param activeProfilers The active profilers * @return A RouteMatcher that matches all supported routes */ - public static RouteMatcher getMatcher(final Map> activeProfilers) { + public static RouteMatcher getMatcher(final Map> runningProfilers, Map activeProfilers, AtomicReference isRunning, LinkedList errors) { RouteMatcher matcher = new RouteMatcher(); - matcher.get("/profilers", RequestHandler.handleGetProfilers(activeProfilers)); - matcher.get("/disable/:profiler", RequestHandler.handleDisableProfiler(activeProfilers)); - + matcher.get("/profilers", RequestHandler.handleGetProfilers(runningProfilers)); + matcher.get("/disable/:profiler", RequestHandler.handleDisableProfiler(runningProfilers)); + matcher.get("/status/profiler/:profiler", RequestHandler.handleProfilerStatus(activeProfilers)); + matcher.get("/errors", RequestHandler.handleErrorMessages(errors)); + matcher.get("/isRunning", RequestHandler.isRunning(isRunning)); return matcher; } + /** + * Handle a GET to /isRunning + * + * @return A Handler that returns all running profilers + */ + public static Handler isRunning(final AtomicReference isRunning) { + return new Handler() { + @Override + public void handle(HttpServerRequest httpServerRequest) { + httpServerRequest.response().end(String.format("isRunning: %b", isRunning.get())); + } + }; + } + /** * Handle a GET to /profilers * - * @param activeProfilers The active profilers * @return A Handler that handles a request to the /profilers endpoint */ - public static Handler handleGetProfilers(final Map> activeProfilers) { + public static Handler handleGetProfilers(final Map> runningProfilers) { return new Handler() { @Override public void handle(HttpServerRequest httpServerRequest) { - httpServerRequest.response().end(Joiner.on("\n").join(getEnabledProfilers(activeProfilers))); + httpServerRequest.response().end(Joiner.on("\n").join(getEnabledProfilers(runningProfilers))); + } + }; + } + + /** + * Handle a GET to /errors + * + * @return The last 10 error stacktraces + */ + public static Handler handleErrorMessages(final LinkedList errors) { + return new Handler() { + @Override + public void handle(HttpServerRequest httpServerRequest) { + httpServerRequest.response().end("Errors: " + Joiner.on("\n").join(errors)); } }; } @@ -66,6 +98,23 @@ public void handle(HttpServerRequest httpServerRequest) { }; } + /** + * Handle a GET to /status/profiler/:profiler + * + * @param activeProfilers The active profilers + * @return A Handler that handles a request to the /disable/:profiler endpoint + */ + public static Handler handleProfilerStatus(final Map activeProfilers) { + return new Handler() { + @Override + public void handle(HttpServerRequest httpServerRequest) { + String profilerName = httpServerRequest.params().get("profiler"); + Profiler profiler = activeProfilers.get(profilerName); + httpServerRequest.response().end(String.format("Recorded stats %d\n", profiler.getRecordedStats())); + } + }; + } + /** * Get all enabled profilers * @param activeProfilers The active profilers diff --git a/src/main/java/com/etsy/statsd/profiler/worker/ProfilerShutdownHookWorker.java b/src/main/java/com/etsy/statsd/profiler/worker/ProfilerShutdownHookWorker.java index ebc1ea4..df2fa4a 100644 --- a/src/main/java/com/etsy/statsd/profiler/worker/ProfilerShutdownHookWorker.java +++ b/src/main/java/com/etsy/statsd/profiler/worker/ProfilerShutdownHookWorker.java @@ -3,6 +3,7 @@ import com.etsy.statsd.profiler.Profiler; import java.util.Collection; +import java.util.concurrent.atomic.AtomicReference; /** * Worker thread for profiler shutdown hook @@ -11,9 +12,10 @@ */ public class ProfilerShutdownHookWorker implements Runnable { private Collection profilers; - - public ProfilerShutdownHookWorker(Collection profilers) { + private AtomicReference isRunning; + public ProfilerShutdownHookWorker(Collection profilers, AtomicReference isRunning) { this.profilers = profilers; + this.isRunning = isRunning; } @Override @@ -21,5 +23,7 @@ public void run() { for (Profiler p : profilers) { p.flushData(); } + + isRunning.set(false); } } diff --git a/src/main/java/com/etsy/statsd/profiler/worker/ProfilerWorkerThread.java b/src/main/java/com/etsy/statsd/profiler/worker/ProfilerWorkerThread.java index eda1406..97ba4c5 100644 --- a/src/main/java/com/etsy/statsd/profiler/worker/ProfilerWorkerThread.java +++ b/src/main/java/com/etsy/statsd/profiler/worker/ProfilerWorkerThread.java @@ -2,6 +2,10 @@ import com.etsy.statsd.profiler.Profiler; +import java.io.PrintWriter; +import java.io.StringWriter; +import java.util.LinkedList; + /** * Worker thread for executing a profiler * @@ -9,13 +13,25 @@ */ public class ProfilerWorkerThread implements Runnable { private Profiler profiler; + private LinkedList errors; - public ProfilerWorkerThread(Profiler profiler) { + public ProfilerWorkerThread(Profiler profiler, LinkedList errors) { this.profiler = profiler; + this.errors = errors; } @Override public void run() { - profiler.profile(); + try { + profiler.profile(); + } catch(Exception e) { + StringWriter sw = new StringWriter(); + PrintWriter pw = new PrintWriter(sw); + e.printStackTrace(pw); + errors.add(String.format("Received an error running profiler: %s, error: %s", profiler.getClass().getName(), sw.toString())); + if ( errors.size() > 10) { + errors.pollLast(); + } + } } } diff --git a/src/test/java/com/etsy/statsd/profiler/worker/ProfilerShutdownHookWorkerTest.java b/src/test/java/com/etsy/statsd/profiler/worker/ProfilerShutdownHookWorkerTest.java index 33c6cb6..bd9dff9 100644 --- a/src/test/java/com/etsy/statsd/profiler/worker/ProfilerShutdownHookWorkerTest.java +++ b/src/test/java/com/etsy/statsd/profiler/worker/ProfilerShutdownHookWorkerTest.java @@ -9,6 +9,8 @@ import java.util.Collection; import java.util.HashSet; import java.util.Set; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; import static org.junit.Assert.assertEquals; @@ -19,8 +21,9 @@ public void testRunnable() throws InterruptedException { Profiler mockProfiler1 = new MockProfiler1(output); Profiler mockProfiler2 = new MockProfiler2(output); Collection profilers = Arrays.asList(mockProfiler1, mockProfiler2); + AtomicReference isRunning = new AtomicReference<>(true); - Thread t = new Thread(new ProfilerShutdownHookWorker(profilers)); + Thread t = new Thread(new ProfilerShutdownHookWorker(profilers, isRunning)); t.run(); t.join(); @@ -28,5 +31,6 @@ public void testRunnable() throws InterruptedException { expectedOutput.add(MockProfiler1.class.getSimpleName() + "-flushData"); expectedOutput.add(MockProfiler2.class.getSimpleName() + "-flushData"); assertEquals(expectedOutput, output); + assertEquals(isRunning.get(), false); } } \ No newline at end of file diff --git a/src/test/java/com/etsy/statsd/profiler/worker/ProfilerWorkerThreadTest.java b/src/test/java/com/etsy/statsd/profiler/worker/ProfilerWorkerThreadTest.java index b3cba1d..8bb18ff 100644 --- a/src/test/java/com/etsy/statsd/profiler/worker/ProfilerWorkerThreadTest.java +++ b/src/test/java/com/etsy/statsd/profiler/worker/ProfilerWorkerThreadTest.java @@ -5,6 +5,7 @@ import org.junit.Test; import java.util.HashSet; +import java.util.LinkedList; import java.util.Set; import static org.junit.Assert.*; @@ -15,7 +16,7 @@ public void testRunnable() throws InterruptedException { Set output = new HashSet<>(); Profiler mockProfiler1 = new MockProfiler1(output); - Thread t = new Thread(new ProfilerWorkerThread(mockProfiler1)); + Thread t = new Thread(new ProfilerWorkerThread(mockProfiler1, new LinkedList())); t.run(); t.join(); From a40599879f3990d188080575a6f3c365b364a2e0 Mon Sep 17 00:00:00 2001 From: Jeff Fenchel Date: Thu, 17 Dec 2015 12:08:49 -0800 Subject: [PATCH 2/3] Updated the docs Added jfenc91 to the contributers list --- CONTRIBUTING.md | 3 ++- README.md | 13 ++++++++----- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2f9baee..b20954a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -18,4 +18,5 @@ Every pull request will be built with [Travis CI](https://travis-ci.org/etsy/sta - Andrew Stiegmann [stieg](https://github.com/stieg) - Joe Meissler [stickperson](https://github.com/stickperson) - Ben Darfler [bdarfler](https://github.com/bdarfler) -- Ihor Bobak [ibobak](https://github.com/ibobak) \ No newline at end of file +- Ihor Bobak [ibobak](https://github.com/ibobak) +- Jeff Fenchel [jfenc91](https://github.com/jfenc91) \ No newline at end of file diff --git a/README.md b/README.md index 21b2e04..3b6ae5a 100644 --- a/README.md +++ b/README.md @@ -35,15 +35,18 @@ packageWhitelist | Colon-delimited whitelist for packages to include (optional, packageBlacklist | Colon-delimited whitelist for packages to exclude (optional, defaults to exclude nothing) profilers | Colon-delimited list of profiler class names (optional, defaults to CPUProfiler and MemoryProfiler) reporter | Class name of the reporter to use (optional, defaults to StatsDReporter) -httpPort | The port on which to bind the embedded HTTP server (optional, defaults to 5005) +httpPort | The port on which to bind the embedded HTTP server (optional, defaults to 5005). If this port is already in use, the next free port will be taken. ### Embedded HTTP Server statsd-jvm-profiler embeds an HTTP server to support simple interactions with the profiler while it is in operation. You can configure the port on which this server runs with the `httpPort` option. -Endpoint | Usage ---------------- | ----- -/profilers | List the currently enabled profilers -/disable/:profiler | Disable the profiler specified by `:profiler`. The name must match what is returned by `/profilers`. +Endpoint | Usage +--------------- | ----- +/profilers | List the currently enabled profilers +/isRunning | List the running profilers. This should be the same as /profilers. +/disable/:profiler | Disable the profiler specified by `:profiler`. The name must match what is returned by `/profilers`. +/errors | List the past 10 errors from the running profilers and reporters. +/status/profiler/:profiler | Displays a status message with the number of recorded stats for the requested profiler. ### Reporters statsd-jvm-profiler supports multiple backends. StatsD is the default, but InfluxDB is also supported. You can select the backend to use by passing the `reporter` argument to the profiler; `StatsDReporter` and `InfluxDBReporter` are the supported values. From 87bc2aae010acc6dc344875121ff1c3481f83600 Mon Sep 17 00:00:00 2001 From: Jeff Fenchel Date: Thu, 17 Dec 2015 13:59:40 -0800 Subject: [PATCH 3/3] - Added logging for the port that the profiler server successfully starts up on - Corrected the error recording to drop the oldest errors when at capacity. --- .../java/com/etsy/statsd/profiler/server/ProfilerServer.java | 5 +++++ .../etsy/statsd/profiler/worker/ProfilerWorkerThread.java | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/etsy/statsd/profiler/server/ProfilerServer.java b/src/main/java/com/etsy/statsd/profiler/server/ProfilerServer.java index ce3be4b..0fa77cd 100644 --- a/src/main/java/com/etsy/statsd/profiler/server/ProfilerServer.java +++ b/src/main/java/com/etsy/statsd/profiler/server/ProfilerServer.java @@ -12,6 +12,8 @@ import java.util.Map; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.atomic.AtomicReference; +import java.util.logging.Level; +import java.util.logging.Logger; /** * Sets up a simple embedded HTTP server for interacting with the profiler while it runs @@ -19,6 +21,7 @@ * @author Andrew Johnson */ public class ProfilerServer { + private static final Logger log = Logger.getLogger(ProfilerServer.class.getName()); private static final Vertx vertx = VertxFactory.newVertx(); /** @@ -36,6 +39,8 @@ public void handle(AsyncResult event) { if (event.failed()) { server.close(); startServer(runningProfilers, activeProfilers, port + 1, isRunning, errors); + } else if (event.succeeded()) { + log.info("Profiler server started on port " + port); } } }); diff --git a/src/main/java/com/etsy/statsd/profiler/worker/ProfilerWorkerThread.java b/src/main/java/com/etsy/statsd/profiler/worker/ProfilerWorkerThread.java index 97ba4c5..b9fe3e2 100644 --- a/src/main/java/com/etsy/statsd/profiler/worker/ProfilerWorkerThread.java +++ b/src/main/java/com/etsy/statsd/profiler/worker/ProfilerWorkerThread.java @@ -30,7 +30,7 @@ public void run() { e.printStackTrace(pw); errors.add(String.format("Received an error running profiler: %s, error: %s", profiler.getClass().getName(), sw.toString())); if ( errors.size() > 10) { - errors.pollLast(); + errors.pollFirst(); } } }