From 020448d363efe06990217f88509ac862a72ed6e2 Mon Sep 17 00:00:00 2001 From: Steffen Offermann Date: Tue, 16 Aug 2016 10:22:52 +0200 Subject: [PATCH 1/9] Fixed Javadoc comment --- .../java/com/codahale/metrics/servlets/MetricsServlet.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/metrics-servlets/src/main/java/com/codahale/metrics/servlets/MetricsServlet.java b/metrics-servlets/src/main/java/com/codahale/metrics/servlets/MetricsServlet.java index 547c5ee70a..2bb24422ea 100644 --- a/metrics-servlets/src/main/java/com/codahale/metrics/servlets/MetricsServlet.java +++ b/metrics-servlets/src/main/java/com/codahale/metrics/servlets/MetricsServlet.java @@ -64,6 +64,9 @@ protected String getAllowedOrigin() { /** * Returns the name of the parameter used to specify the jsonp callback, if any. + * + * @return the name of the parameter used to specify the jsonp callback, or null, if + * no such parameter exists. */ protected String getJsonpCallbackParameter() { return null; @@ -72,6 +75,9 @@ protected String getJsonpCallbackParameter() { /** * Returns the {@link MetricFilter} that shall be used to filter metrics, or {@link MetricFilter#ALL} if * the default should be used. + * + * @return the {@link MetricFilter} that shall be used to filter metrics, or {@link MetricFilter#ALL} if + * the default should be used. */ protected MetricFilter getMetricFilter() { // use the default From 90d0dc73ccc46b310b3a8396143daded9833421c Mon Sep 17 00:00:00 2001 From: Steffen Offermann Date: Tue, 16 Aug 2016 10:23:44 +0200 Subject: [PATCH 2/9] Added fix for GitHub issue 998 --- .../codahale/metrics/ScheduledReporter.java | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/metrics-core/src/main/java/com/codahale/metrics/ScheduledReporter.java b/metrics-core/src/main/java/com/codahale/metrics/ScheduledReporter.java index 8ddcd31222..cbfffe4d4f 100644 --- a/metrics-core/src/main/java/com/codahale/metrics/ScheduledReporter.java +++ b/metrics-core/src/main/java/com/codahale/metrics/ScheduledReporter.java @@ -122,6 +122,26 @@ public void run() { }, period, period, unit); } + /** + * Starts the reporter polling at the given period. + * + * @param period the amount of time between polls + * @param initialDelay the time to delay the first execution + * @param unit the unit for {@code period} + */ + public void start(long period, long initialDelay, TimeUnit unit) { + executor.scheduleAtFixedRate(new Runnable() { + @Override + public void run() { + try { + report(); + } catch (RuntimeException ex) { + LOG.error("RuntimeException thrown from {}#report. Exception was suppressed.", ScheduledReporter.this.getClass().getSimpleName(), ex); + } + } + }, initialDelay, period, unit); + } + /** * Stops the reporter and shuts down its thread of execution. * From 9a34950943c9882ece45f5939497aa11d2cf0a63 Mon Sep 17 00:00:00 2001 From: Steffen Offermann Date: Mon, 9 Jan 2017 10:33:10 +0100 Subject: [PATCH 3/9] Fix for GitHub issue #998 --- .../codahale/metrics/ScheduledReporter.java | 30 +++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/metrics-core/src/main/java/com/codahale/metrics/ScheduledReporter.java b/metrics-core/src/main/java/com/codahale/metrics/ScheduledReporter.java index a3f300b21b..52417195a7 100644 --- a/metrics-core/src/main/java/com/codahale/metrics/ScheduledReporter.java +++ b/metrics-core/src/main/java/com/codahale/metrics/ScheduledReporter.java @@ -3,7 +3,13 @@ import java.io.Closeable; import java.util.Locale; import java.util.SortedMap; -import java.util.concurrent.*; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.ThreadFactory; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicInteger; import org.slf4j.Logger; @@ -66,7 +72,7 @@ public Thread newThread(Runnable r) { * reporter will report * @param name the reporter's name * @param filter the filter for which metrics to report - * @param rateUnit a unit of time + * @param rateUnit a unit of time * @param durationUnit a unit of time */ protected ScheduledReporter(MetricRegistry registry, @@ -144,6 +150,26 @@ public void run() { }, period, period, unit); } + /** + * Starts the reporter polling at the given period. + * + * @param period the amount of time between polls + * @param initialDelay the time to delay the first execution + * @param unit the unit for {@code period} + */ + public void start(long period, long initialDelay, TimeUnit unit) { + executor.scheduleAtFixedRate(new Runnable() { + @Override + public void run() { + try { + report(); + } catch (RuntimeException ex) { + LOG.error("RuntimeException thrown from {}#report. Exception was suppressed.", ScheduledReporter.this.getClass().getSimpleName(), ex); + } + } + }, initialDelay, period, unit); + } + /** * Stops the reporter and if shutdownExecutorOnStop is true then shuts down its thread of execution. * From 59b30191cac16dace78dcce4f81cfbcdcd6e2d44 Mon Sep 17 00:00:00 2001 From: Steffen Offermann Date: Mon, 9 Jan 2017 11:53:26 +0100 Subject: [PATCH 4/9] Added test cases for issue #998 --- .../metrics/ScheduledReporterTest.java | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/metrics-core/src/test/java/com/codahale/metrics/ScheduledReporterTest.java b/metrics-core/src/test/java/com/codahale/metrics/ScheduledReporterTest.java index c1affd39ff..97c0d75f44 100644 --- a/metrics-core/src/test/java/com/codahale/metrics/ScheduledReporterTest.java +++ b/metrics-core/src/test/java/com/codahale/metrics/ScheduledReporterTest.java @@ -39,7 +39,7 @@ public void setUp() throws Exception { registry.register("meter", meter); registry.register("timer", timer); - when(executor.scheduleAtFixedRate(any(Runnable.class), eq(200L), eq(200L), eq(TimeUnit.MILLISECONDS))) + when(executor.scheduleAtFixedRate(any(Runnable.class), any(Long.class), any(Long.class), eq(TimeUnit.MILLISECONDS))) .thenReturn(scheduledFuture); } @@ -63,6 +63,28 @@ public void pollsPeriodically() throws Exception { ); } + @Test + public void shouldUsePeriodAsInitialDelayIfNotSpecifiedOtherwise() throws Exception { + reporterWithCustomExecutor.start(200, TimeUnit.MILLISECONDS); + + verify(executor, times(1)).scheduleAtFixedRate( + any(Runnable.class), eq(200L), eq(200L), eq(TimeUnit.MILLISECONDS) + ); + + Thread.sleep(100); + } + + @Test + public void shouldStartWithSpecifiedInitialDelay() throws Exception { + reporterWithCustomExecutor.start(350, 100, TimeUnit.MILLISECONDS); + + verify(executor).scheduleAtFixedRate( + any(Runnable.class), eq(350L), eq(100L), eq(TimeUnit.MILLISECONDS) + ); + + Thread.sleep(100); + } + @Test public void shouldAutoCreateExecutorWhenItNull() throws Exception { reporterWithNullExecutor.start(200, TimeUnit.MILLISECONDS); From 588214f733871e7cc7fbfe2667b713b15ca5c412 Mon Sep 17 00:00:00 2001 From: Steffen Offermann Date: Mon, 9 Jan 2017 11:55:15 +0100 Subject: [PATCH 5/9] Changed order of arguments in newly added start() method --- .../codahale/metrics/ScheduledReporter.java | 46 ++++++++----------- 1 file changed, 19 insertions(+), 27 deletions(-) diff --git a/metrics-core/src/main/java/com/codahale/metrics/ScheduledReporter.java b/metrics-core/src/main/java/com/codahale/metrics/ScheduledReporter.java index 52417195a7..3b6a147872 100644 --- a/metrics-core/src/main/java/com/codahale/metrics/ScheduledReporter.java +++ b/metrics-core/src/main/java/com/codahale/metrics/ScheduledReporter.java @@ -134,41 +134,33 @@ protected ScheduledReporter(MetricRegistry registry, * @param period the amount of time between polls * @param unit the unit for {@code period} */ - synchronized public void start(long period, TimeUnit unit) { - if (this.scheduledFuture != null) { - throw new IllegalArgumentException("Reporter already started"); - } - this.scheduledFuture = executor.scheduleAtFixedRate(new Runnable() { - @Override - public void run() { - try { - report(); - } catch (RuntimeException ex) { - LOG.error("RuntimeException thrown from {}#report. Exception was suppressed.", ScheduledReporter.this.getClass().getSimpleName(), ex); - } - } - }, period, period, unit); + public void start(long period, TimeUnit unit) { + start(period, period, unit); } /** * Starts the reporter polling at the given period. * - * @param period the amount of time between polls * @param initialDelay the time to delay the first execution + * @param period the amount of time between polls * @param unit the unit for {@code period} */ - public void start(long period, long initialDelay, TimeUnit unit) { - executor.scheduleAtFixedRate(new Runnable() { - @Override - public void run() { - try { - report(); - } catch (RuntimeException ex) { - LOG.error("RuntimeException thrown from {}#report. Exception was suppressed.", ScheduledReporter.this.getClass().getSimpleName(), ex); - } - } - }, initialDelay, period, unit); - } + synchronized public void start(long initialDelay, long period, TimeUnit unit) { + if (this.scheduledFuture != null) { + throw new IllegalArgumentException("Reporter already started"); + } + + this.scheduledFuture = executor.scheduleAtFixedRate(new Runnable() { + @Override + public void run() { + try { + report(); + } catch (RuntimeException ex) { + LOG.error("RuntimeException thrown from {}#report. Exception was suppressed.", ScheduledReporter.this.getClass().getSimpleName(), ex); + } + } + }, initialDelay, period, unit); + } /** * Stops the reporter and if shutdownExecutorOnStop is true then shuts down its thread of execution. From 989d8e10f9de91d4168bd736a39b466eadf7ab05 Mon Sep 17 00:00:00 2001 From: Steffen Offermann Date: Mon, 9 Jan 2017 12:05:07 +0100 Subject: [PATCH 6/9] Removed unnecessary Thread.sleep() calls --- .../test/java/com/codahale/metrics/ScheduledReporterTest.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/metrics-core/src/test/java/com/codahale/metrics/ScheduledReporterTest.java b/metrics-core/src/test/java/com/codahale/metrics/ScheduledReporterTest.java index 97c0d75f44..8051ebabab 100644 --- a/metrics-core/src/test/java/com/codahale/metrics/ScheduledReporterTest.java +++ b/metrics-core/src/test/java/com/codahale/metrics/ScheduledReporterTest.java @@ -70,8 +70,6 @@ public void shouldUsePeriodAsInitialDelayIfNotSpecifiedOtherwise() throws Except verify(executor, times(1)).scheduleAtFixedRate( any(Runnable.class), eq(200L), eq(200L), eq(TimeUnit.MILLISECONDS) ); - - Thread.sleep(100); } @Test @@ -81,8 +79,6 @@ public void shouldStartWithSpecifiedInitialDelay() throws Exception { verify(executor).scheduleAtFixedRate( any(Runnable.class), eq(350L), eq(100L), eq(TimeUnit.MILLISECONDS) ); - - Thread.sleep(100); } @Test From 33c91f907197d3b4786c96575e2033a31c3a1043 Mon Sep 17 00:00:00 2001 From: Steffen Offermann Date: Mon, 9 Jan 2017 12:09:30 +0100 Subject: [PATCH 7/9] Removed duplicate method --- .../codahale/metrics/ScheduledReporter.java | 24 ------------------- 1 file changed, 24 deletions(-) diff --git a/metrics-core/src/main/java/com/codahale/metrics/ScheduledReporter.java b/metrics-core/src/main/java/com/codahale/metrics/ScheduledReporter.java index e94be67fc7..05f7045cc9 100644 --- a/metrics-core/src/main/java/com/codahale/metrics/ScheduledReporter.java +++ b/metrics-core/src/main/java/com/codahale/metrics/ScheduledReporter.java @@ -138,30 +138,6 @@ public void start(long period, TimeUnit unit) { start(period, period, unit); } - /** - * Starts the reporter polling at the given period. - * - * @param initialDelay the time to delay the first execution - * @param period the amount of time between polls - * @param unit the unit for {@code period} - */ - synchronized public void start(long initialDelay, long period, TimeUnit unit) { - if (this.scheduledFuture != null) { - throw new IllegalArgumentException("Reporter already started"); - } - - this.scheduledFuture = executor.scheduleAtFixedRate(new Runnable() { - @Override - public void run() { - try { - report(); - } catch (RuntimeException ex) { - LOG.error("RuntimeException thrown from {}#report. Exception was suppressed.", ScheduledReporter.this.getClass().getSimpleName(), ex); - } - } - }, initialDelay, period, unit); - } - /** * Starts the reporter polling at the given period. * From 7acca8af8c28ef9c1c31a8e18200f030b2dd7fe4 Mon Sep 17 00:00:00 2001 From: Steffen Offermann Date: Mon, 9 Jan 2017 12:21:27 +0100 Subject: [PATCH 8/9] Fixed order of arguments (again) --- .../codahale/metrics/ScheduledReporter.java | 30 +++++++++++-------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/metrics-core/src/main/java/com/codahale/metrics/ScheduledReporter.java b/metrics-core/src/main/java/com/codahale/metrics/ScheduledReporter.java index 05f7045cc9..3b6a147872 100644 --- a/metrics-core/src/main/java/com/codahale/metrics/ScheduledReporter.java +++ b/metrics-core/src/main/java/com/codahale/metrics/ScheduledReporter.java @@ -141,22 +141,26 @@ public void start(long period, TimeUnit unit) { /** * Starts the reporter polling at the given period. * - * @param period the amount of time between polls * @param initialDelay the time to delay the first execution + * @param period the amount of time between polls * @param unit the unit for {@code period} */ - public void start(long period, long initialDelay, TimeUnit unit) { - executor.scheduleAtFixedRate(new Runnable() { - @Override - public void run() { - try { - report(); - } catch (RuntimeException ex) { - LOG.error("RuntimeException thrown from {}#report. Exception was suppressed.", ScheduledReporter.this.getClass().getSimpleName(), ex); - } - } - }, initialDelay, period, unit); - } + synchronized public void start(long initialDelay, long period, TimeUnit unit) { + if (this.scheduledFuture != null) { + throw new IllegalArgumentException("Reporter already started"); + } + + this.scheduledFuture = executor.scheduleAtFixedRate(new Runnable() { + @Override + public void run() { + try { + report(); + } catch (RuntimeException ex) { + LOG.error("RuntimeException thrown from {}#report. Exception was suppressed.", ScheduledReporter.this.getClass().getSimpleName(), ex); + } + } + }, initialDelay, period, unit); + } /** * Stops the reporter and if shutdownExecutorOnStop is true then shuts down its thread of execution. From 9e08cb2e81db5d2189b1bb2a64d8c61116e7ae6b Mon Sep 17 00:00:00 2001 From: Steffen Offermann Date: Mon, 9 Jan 2017 12:27:24 +0100 Subject: [PATCH 9/9] Reverted previous change, because it was unrelated to issue 998 --- .../com/codahale/metrics/servlets/MetricsServlet.java | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/metrics-servlets/src/main/java/com/codahale/metrics/servlets/MetricsServlet.java b/metrics-servlets/src/main/java/com/codahale/metrics/servlets/MetricsServlet.java index 2bb24422ea..31232cf157 100644 --- a/metrics-servlets/src/main/java/com/codahale/metrics/servlets/MetricsServlet.java +++ b/metrics-servlets/src/main/java/com/codahale/metrics/servlets/MetricsServlet.java @@ -64,9 +64,6 @@ protected String getAllowedOrigin() { /** * Returns the name of the parameter used to specify the jsonp callback, if any. - * - * @return the name of the parameter used to specify the jsonp callback, or null, if - * no such parameter exists. */ protected String getJsonpCallbackParameter() { return null; @@ -75,9 +72,6 @@ protected String getJsonpCallbackParameter() { /** * Returns the {@link MetricFilter} that shall be used to filter metrics, or {@link MetricFilter#ALL} if * the default should be used. - * - * @return the {@link MetricFilter} that shall be used to filter metrics, or {@link MetricFilter#ALL} if - * the default should be used. */ protected MetricFilter getMetricFilter() { // use the default @@ -173,7 +167,7 @@ protected void doGet(HttpServletRequest req, } resp.setHeader("Cache-Control", "must-revalidate,no-cache,no-store"); resp.setStatus(HttpServletResponse.SC_OK); - + final OutputStream output = resp.getOutputStream(); try { if (jsonpParamName != null && req.getParameter(jsonpParamName) != null) {