diff --git a/modules/metrics/src/main/java/org/springside/modules/metrics/Execution.java b/modules/metrics/src/main/java/org/springside/modules/metrics/Execution.java index 252a8772..8cf3a8b7 100644 --- a/modules/metrics/src/main/java/org/springside/modules/metrics/Execution.java +++ b/modules/metrics/src/main/java/org/springside/modules/metrics/Execution.java @@ -17,11 +17,12 @@ public class Execution { } public ExecutionTimer start() { - return new ExecutionTimer(this, clock); + return new ExecutionTimer(this, clock.getCurrentTime()); } - private void update(long duration) { - histogram.update(duration); + private void update(long startTime) { + final long elapsed = clock.getCurrentTime() - startTime; + histogram.update(elapsed); counter.inc(); } @@ -32,25 +33,22 @@ public class Execution { return metric; } - public static class ExecutionTimer { - private final Execution execution; - private final Clock clock; - private final long startTime; - - private ExecutionTimer(Execution execution, Clock clock) { - this.execution = execution; - this.clock = clock; - this.startTime = clock.getCurrentTime(); - } - - public void stop() { - final long elapsed = clock.getCurrentTime() - startTime; - execution.update(elapsed); - } - } - @Override public String toString() { return "Execution [counter=" + counter + ", histogram=" + histogram + "]"; } + + public static class ExecutionTimer { + private final Execution execution; + private final long startTime; + + private ExecutionTimer(Execution execution, long startTime) { + this.execution = execution; + this.startTime = startTime; + } + + public void stop() { + execution.update(startTime); + } + } } diff --git a/modules/metrics/src/main/java/org/springside/modules/metrics/Histogram.java b/modules/metrics/src/main/java/org/springside/modules/metrics/Histogram.java index b0d77922..551762db 100644 --- a/modules/metrics/src/main/java/org/springside/modules/metrics/Histogram.java +++ b/modules/metrics/src/main/java/org/springside/modules/metrics/Histogram.java @@ -9,6 +9,7 @@ public class Histogram { private LinkedList measurements = new LinkedList(); private Double[] pcts; + private Object lock = new Object(); public Histogram(Double[] pcts) { if (pcts == null) { @@ -18,15 +19,18 @@ public class Histogram { } public void update(long value) { - synchronized (measurements) { + synchronized (lock) { measurements.add(value); } } public HistogramMetric getMetric() { + LinkedList snapshotList = null; - LinkedList snapshotList = measurements; - measurements = new LinkedList(); + synchronized (lock) { + snapshotList = measurements; + measurements = new LinkedList(); + } if (snapshotList.isEmpty()) { return createEmptyMetric(); @@ -53,7 +57,7 @@ public class Histogram { return metric; } - public Long getPercent(List snapshotList, int count, double pct) { + private Long getPercent(List snapshotList, int count, double pct) { final double pos = (pct * (count + 1)) / 100; diff --git a/modules/metrics/src/main/java/org/springside/modules/metrics/report/GraphiteReporter.java b/modules/metrics/src/main/java/org/springside/modules/metrics/report/GraphiteReporter.java index 87dd467f..05a53977 100644 --- a/modules/metrics/src/main/java/org/springside/modules/metrics/report/GraphiteReporter.java +++ b/modules/metrics/src/main/java/org/springside/modules/metrics/report/GraphiteReporter.java @@ -23,31 +23,26 @@ import org.springside.modules.metrics.MetricRegistry; public class GraphiteReporter implements Reporter { - private static final Logger logger = LoggerFactory.getLogger(GraphiteReporter.class); + private static final Pattern WHITESPACE = Pattern.compile("[\\s]+"); + private static final Charset UTF_8 = Charset.forName("UTF-8"); + + private static Logger logger = LoggerFactory.getLogger(GraphiteReporter.class); private String prefix; - private static final Pattern WHITESPACE = Pattern.compile("[\\s]+"); - // this may be optimistic about Carbon/Graphite - private static final Charset UTF_8 = Charset.forName("UTF-8"); - private InetSocketAddress address; private SocketFactory socketFactory; - private Charset charset; - private Socket socket; private Writer writer; - private int failures; public GraphiteReporter(InetSocketAddress address) { - this("metrics", address); + this(address, "metrics"); } - public GraphiteReporter(String prefix, InetSocketAddress address) { + public GraphiteReporter(InetSocketAddress address, String prefix) { this.prefix = prefix; this.address = address; this.socketFactory = SocketFactory.getDefault(); - this.charset = UTF_8; } @Override @@ -69,6 +64,8 @@ public class GraphiteReporter implements Reporter { reportExecution(entry.getKey(), entry.getValue(), timestamp); } + flush(); + } catch (IOException e) { logger.warn("Unable to report to Graphite", e); } finally { @@ -78,7 +75,6 @@ public class GraphiteReporter implements Reporter { logger.warn("Error disconnecting from Graphite", e); } } - } private void reportCounter(String name, CounterMetric counter, long timestamp) throws IOException { @@ -132,13 +128,13 @@ public class GraphiteReporter implements Reporter { return String.format(Locale.US, "%2.2f", v); } - public void connect() throws IllegalStateException, IOException { + private void connect() throws IllegalStateException, IOException { if (socket != null) { throw new IllegalStateException("Already connected"); } this.socket = socketFactory.createSocket(address.getAddress(), address.getPort()); - this.writer = new BufferedWriter(new OutputStreamWriter(socket.getOutputStream(), charset)); + this.writer = new BufferedWriter(new OutputStreamWriter(socket.getOutputStream(), UTF_8)); } /** @@ -149,7 +145,7 @@ public class GraphiteReporter implements Reporter { * @param timestamp the timestamp of the metric * @throws IOException if there was an error sending the metric */ - public void send(String name, String value, long timestamp) throws IOException { + private void send(String name, String value, long timestamp) throws IOException { try { writer.write(sanitize(name)); writer.write(' '); @@ -157,27 +153,24 @@ public class GraphiteReporter implements Reporter { writer.write(' '); writer.write(Long.toString(timestamp)); writer.write('\n'); - writer.flush(); - this.failures = 0; } catch (IOException e) { - failures++; throw e; } } - public int getFailures() { - return failures; + private String sanitize(String s) { + return WHITESPACE.matcher(s).replaceAll("-"); } - public void close() throws IOException { + private void flush() throws IOException { + writer.flush(); + } + + private void close() throws IOException { if (socket != null) { socket.close(); } this.socket = null; this.writer = null; } - - protected String sanitize(String s) { - return WHITESPACE.matcher(s).replaceAll("-"); - } } diff --git a/modules/metrics/src/test/java/org/springside/modules/metrics/ReporterTest.java b/modules/metrics/src/test/java/org/springside/modules/metrics/ReporterTest.java index d27c841e..7c183841 100644 --- a/modules/metrics/src/test/java/org/springside/modules/metrics/ReporterTest.java +++ b/modules/metrics/src/test/java/org/springside/modules/metrics/ReporterTest.java @@ -7,7 +7,6 @@ import org.junit.Ignore; import org.junit.Test; import org.springside.modules.metrics.Execution.ExecutionTimer; import org.springside.modules.metrics.report.ConsoleReporter; -import org.springside.modules.metrics.report.Graphite; import org.springside.modules.metrics.report.GraphiteReporter; import org.springside.modules.metrics.report.ReportScheduler; import org.springside.modules.metrics.report.Reporter; @@ -29,8 +28,7 @@ public class ReporterTest { @Test @Ignore("manual test") public void graphiteReporter() { - Graphite graphite = new Graphite(new InetSocketAddress("localhost", 2003)); - runReport(new GraphiteReporter(graphite)); + runReport(new GraphiteReporter(new InetSocketAddress("localhost", 2003))); } @Test