SIRONA-57 Bad timestamp unit for Graphite push
git-svn-id: https://svn.apache.org/repos/asf/incubator/sirona/trunk@1720105 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/agent/store/graphite/src/main/java/org/apache/sirona/graphite/GraphiteCounterDataStore.java b/agent/store/graphite/src/main/java/org/apache/sirona/graphite/GraphiteCounterDataStore.java
index cf48650..71dbc2f 100755
--- a/agent/store/graphite/src/main/java/org/apache/sirona/graphite/GraphiteCounterDataStore.java
+++ b/agent/store/graphite/src/main/java/org/apache/sirona/graphite/GraphiteCounterDataStore.java
@@ -40,7 +40,8 @@
try {
graphite.open();
- final long ts = System.currentTimeMillis();
+ // timestamp is the unix epoch time in seconds NOT ms.
+ final long ts = System.currentTimeMillis() / 1000l;
for (final Counter counter : instances) {
final Counter.Key key = counter.getKey();
diff --git a/agent/store/graphite/src/main/java/org/apache/sirona/graphite/GraphiteGaugeDataStore.java b/agent/store/graphite/src/main/java/org/apache/sirona/graphite/GraphiteGaugeDataStore.java
index cd1e5c4..821cad1 100755
--- a/agent/store/graphite/src/main/java/org/apache/sirona/graphite/GraphiteGaugeDataStore.java
+++ b/agent/store/graphite/src/main/java/org/apache/sirona/graphite/GraphiteGaugeDataStore.java
@@ -38,7 +38,8 @@
try {
graphite.open();
- final long ts = System.currentTimeMillis();
+ // timestamp is the unix epoch time in seconds NOT ms.
+ final long ts = System.currentTimeMillis() / 1000l;
for (final Map.Entry<Role, Value> gauge : gauges.entrySet()) {
graphite.push(GAUGE_PREFIX + gauge.getKey().getName(), gauge.getValue().getMean(), ts);
diff --git a/agent/store/graphite/src/test/java/org/apache/sirona/graphite/GraphiteTest.java b/agent/store/graphite/src/test/java/org/apache/sirona/graphite/GraphiteTest.java
index 54ccd5f..bf3e3d2 100644
--- a/agent/store/graphite/src/test/java/org/apache/sirona/graphite/GraphiteTest.java
+++ b/agent/store/graphite/src/test/java/org/apache/sirona/graphite/GraphiteTest.java
@@ -23,8 +23,10 @@
import java.util.ArrayList;
import java.util.Collection;
+import java.util.Iterator;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
-import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
public class GraphiteTest extends GraphiteTestBase {
@@ -42,9 +44,9 @@
}
{ // counters
- final Collection<String> counters = extract(extract(messages(), "counter"), "counter"); // don't keep values
+ final Collection<String> counters = extract(messages(), "(counter-performances-test-[a-zA-Z]+) .+ [0-9]+"); // don't keep values
- assertTrue(counters.size() >= 30);
+ assertTrue(String.valueOf(counters.size()), counters.size() >= 30);
assertTrue(counters.contains("counter-performances-test-Hits"));
assertTrue(counters.contains("counter-performances-test-Max"));
assertTrue(counters.contains("counter-performances-test-Mean"));
@@ -57,7 +59,7 @@
{ // gauges
Thread.sleep(450);
- final Collection<String> gauges = extract(messages(), "gauge");
+ final Collection<String> gauges = extract(messages(), "(gauge-mock [0-9.]+) [0-9]+");
assertTrue(gauges.size() >= 3);
@@ -69,11 +71,59 @@
}
}
- private static Collection<String> extract(final Collection<String> messages, final String prefix) {
+ // The graphite protocol uses Unix Timestamp which is the number or seconds since epoc
+ // In Java System.currentTimeMillis() is the number of milliseconds since epoc.
+ // test case to reproduce the Unix Timestamp issue
+ // It sends 2 events in the same second and verifies they have the same timestamp.
+ @Test
+ public void issueUnixTimestampEpoc() throws InterruptedException {
+ final Counter counter = Repository.INSTANCE.getCounter(new Counter.Key(Role.PERFORMANCES, "UnixTimestamp"));
+ counter.add(2.4);
+ counter.add(2.9);
+
+ // wait a bit so the that we can send more than once the data to the graphite server
+ // wait less than a second obviously
+ Thread.sleep(600);
+
+ {
+ final Collection<String> counters = extract(messages(), "counter-performances-UnixTimestamp-Hits [0-9.]+ ([0-9]+)"); // don't keep values
+ validateTimestamp(counters, 1);
+ }
+
+ {
+ final Collection<String> counters = extract(messages(), "gauge-mock [0-9.]+ ([0-9]+)"); // don't keep values
+ validateTimestamp(counters, 5);
+ }
+ }
+
+ private void validateTimestamp(final Collection<String> counters, final int delta) {
+ assertTrue(counters.size() > 0);
+ final Iterator<String> iterator = counters.iterator();
+ String val = null;
+ while (iterator.hasNext()) {
+ final String next = iterator.next();
+ if (val == null) {
+ val = next;
+ }
+ // allow one second difference cause we might switch to the following second in the middle
+ // with milliseconds 1ms never succeeds
+ assertTrue(Integer.parseInt(next) - Integer.parseInt(val) <= delta);
+ }
+ }
+
+ private static Collection<String> extract(final Collection<String> messages, final String regexp) {
+ final Pattern pattern = Pattern.compile(regexp);
final Collection<String> list = new ArrayList<String>(messages.size());
for (final String msg : messages) {
- if (msg.startsWith(prefix)) {
- list.add(msg.substring(0, msg.lastIndexOf(" ")));
+ final Matcher matcher = pattern.matcher(msg);
+ if (matcher.matches()) {
+ if (matcher.groupCount() != 1) {
+ throw new IllegalArgumentException("Can only capture 1 group. Current count is " + matcher.groupCount());
+ }
+
+ list.add(matcher.group(1));
+// } else {
+// System.out.println(msg + " does not match pattern " + regexp);
}
}
return list;
diff --git a/agent/store/graphite/src/test/java/org/apache/sirona/graphite/GraphiteTestBase.java b/agent/store/graphite/src/test/java/org/apache/sirona/graphite/GraphiteTestBase.java
index 8cbd7f3..d93518f 100644
--- a/agent/store/graphite/src/test/java/org/apache/sirona/graphite/GraphiteTestBase.java
+++ b/agent/store/graphite/src/test/java/org/apache/sirona/graphite/GraphiteTestBase.java
@@ -21,7 +21,9 @@
import org.apache.sirona.graphite.server.GraphiteMockServer;
import org.apache.sirona.repositories.Repository;
import org.junit.After;
+import org.junit.AfterClass;
import org.junit.Before;
+import org.junit.BeforeClass;
import java.io.IOException;
import java.util.Collection;
@@ -31,23 +33,29 @@
private Gauge.LoaderHelper gauges;
@Before
- public void startGraphite() throws IOException {
+ public void startGraphite() throws IOException, InterruptedException {
server = new GraphiteMockServer(Integer.getInteger("collector.server.port", 1234)).start();
if (System.getProperty("org.apache.sirona.graphite.GraphiteBuilder.port", "").isEmpty()) {
System.setProperty("org.apache.sirona.graphite.GraphiteBuilder.port", Integer.toString(server.getPort()));
}
+ Thread.sleep(200); // make sure it gets time to restart between tests
+ server.clear();
Repository.INSTANCE.clearCounters();
gauges = new Gauge.LoaderHelper(false);
}
@After
public void shutdownGraphite() throws IOException {
- IoCs.shutdown();
gauges.destroy();
server.stop();
Repository.INSTANCE.clearCounters();
}
+ @AfterClass
+ public static void shutdownSirona() throws IOException {
+ IoCs.shutdown();
+ }
+
protected Collection<String> messages() {
return server.getMessages();
}
diff --git a/agent/store/graphite/src/test/java/org/apache/sirona/graphite/server/GraphiteMockServer.java b/agent/store/graphite/src/test/java/org/apache/sirona/graphite/server/GraphiteMockServer.java
index c1290a4..d8482ac 100644
--- a/agent/store/graphite/src/test/java/org/apache/sirona/graphite/server/GraphiteMockServer.java
+++ b/agent/store/graphite/src/test/java/org/apache/sirona/graphite/server/GraphiteMockServer.java
@@ -22,7 +22,6 @@
import java.io.InputStreamReader;
import java.net.ServerSocket;
import java.net.Socket;
-import java.util.ArrayList;
import java.util.Collection;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.atomic.AtomicBoolean;
@@ -60,6 +59,10 @@
return messages;
}
+ public void clear() {
+ messages.clear();
+ }
+
private static class GraphiteThread extends Thread {
private final Collection<String> messages;
diff --git a/agent/store/graphite/src/test/resources/sirona.properties b/agent/store/graphite/src/test/resources/sirona.properties
index 744d19d..54838be 100644
--- a/agent/store/graphite/src/test/resources/sirona.properties
+++ b/agent/store/graphite/src/test/resources/sirona.properties
@@ -16,7 +16,7 @@
# under the License.
org.apache.sirona.store.DataStoreFactory = org.apache.sirona.graphite.GraphiteDataStoreFactory
-org.apache.sirona.graphite.aggregated.gauge.period = 220
+org.apache.sirona.graphite.aggregated.gauge.period = 200
org.apache.sirona.graphite.period = 125
org.apache.sirona.graphite.GraphiteBuilder.address = localhost
