From 466f08792f11c2f95bf293ac9b6facd7018a5bb8 Mon Sep 17 00:00:00 2001
From: Ravi Prakash <raviprak@apache.org>
Date: Wed, 15 Oct 2014 15:49:46 -0700
Subject: [PATCH] HADOOP-11181. GraphiteSink emits wrong timestamps (Sascha
 Coenen via raviprak)

---
 .../hadoop-common/CHANGES.txt                 |  2 +
 .../hadoop/metrics2/sink/GraphiteSink.java    |  4 +-
 .../metrics2/impl/TestGraphiteMetrics.java    | 44 ++++++++++++++++++-
 3 files changed, 46 insertions(+), 4 deletions(-)

diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt
index 9f43937b0d..f6171370e4 100644
--- a/hadoop-common-project/hadoop-common/CHANGES.txt
+++ b/hadoop-common-project/hadoop-common/CHANGES.txt
@@ -661,6 +661,8 @@ Release 2.6.0 - UNRELEASED
 
   BUG FIXES
 
+    HADOOP-11181. GraphiteSink emits wrong timestamps (Sascha Coenen via raviprak)
+
     HADOOP-10781. Unportable getgrouplist() usage breaks FreeBSD (Dmitry
     Sivachenko via Colin Patrick McCabe)
 
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/sink/GraphiteSink.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/sink/GraphiteSink.java
index f474d82a98..9bc3f15d97 100644
--- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/sink/GraphiteSink.java
+++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/sink/GraphiteSink.java
@@ -89,8 +89,8 @@ public void putMetrics(MetricsRecord record) {
             }
         }
 
-        // Round the timestamp to second as Graphite accepts it in such format.
-        int timestamp = Math.round(record.timestamp() / 1000.0f);
+        // The record timestamp is in milliseconds while Graphite expects an epoc time in seconds.
+        long timestamp = record.timestamp() / 1000L;
 
         // Collect datapoints.
         for (AbstractMetric metric : record.metrics()) {
diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/metrics2/impl/TestGraphiteMetrics.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/metrics2/impl/TestGraphiteMetrics.java
index f54c27dc6d..09f0081276 100644
--- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/metrics2/impl/TestGraphiteMetrics.java
+++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/metrics2/impl/TestGraphiteMetrics.java
@@ -19,11 +19,16 @@
 package org.apache.hadoop.metrics2.impl;
 
 import static org.junit.Assert.assertEquals;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
 
+import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.OutputStreamWriter;
 import java.io.Writer;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
@@ -34,8 +39,6 @@
 import org.apache.hadoop.metrics2.MetricsTag;
 import org.apache.hadoop.metrics2.sink.GraphiteSink;
 import org.junit.Test;
-
-import static org.mockito.Mockito.*;
 import org.mockito.ArgumentCaptor;
 import org.mockito.internal.util.reflection.Whitebox;
 
@@ -108,6 +111,43 @@ public void testPutMetrics2() {
             result.equals("null.all.Context.Context=all.foo2 2 10\n" + 
             "null.all.Context.Context=all.foo1 1 10\n"));
     }
+
+    /**
+     * Assert that timestamps are converted correctly, ticket HADOOP-11182
+     */
+    @Test
+    public void testPutMetrics3() {
+
+      // setup GraphiteSink
+      GraphiteSink sink = new GraphiteSink();
+      ByteArrayOutputStream out = new ByteArrayOutputStream();
+      Whitebox.setInternalState(sink, "writer", new OutputStreamWriter(out));
+
+      // given two metrics records with timestamps 1000 milliseconds apart.
+      List<MetricsTag> tags = Collections.emptyList();
+      Set<AbstractMetric> metrics = new HashSet<AbstractMetric>();
+      metrics.add(makeMetric("foo1", 1));
+      MetricsRecord record1 = new MetricsRecordImpl(MsInfo.Context, 1000000000000L, tags, metrics);
+      MetricsRecord record2 = new MetricsRecordImpl(MsInfo.Context, 1000000001000L, tags, metrics);
+
+      sink.putMetrics(record1);
+      sink.putMetrics(record2);
+
+      sink.flush();
+      try {
+        sink.close();
+      } catch(IOException e) {
+        e.printStackTrace();
+      }
+
+      // then the timestamps in the graphite stream should differ by one second.
+      String expectedOutput
+        = "null.default.Context.foo1 1 1000000000\n"
+        + "null.default.Context.foo1 1 1000000001\n";
+      assertEquals(expectedOutput, out.toString());
+    }
+
+
     @Test(expected=MetricsException.class)
     public void testCloseAndWrite() throws IOException {
       GraphiteSink sink = new GraphiteSink();