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 tags = Collections.emptyList(); + Set metrics = new HashSet(); + 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();