You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by bm...@apache.org on 2014/04/23 01:57:40 UTC

[1/2] git commit: Added a Timer Metric type.

Repository: mesos
Updated Branches:
  refs/heads/master 7e0e686d2 -> d6f51c5a2


Added a Timer Metric type.

Review: https://reviews.apache.org/r/20339/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/d6f51c5a
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/d6f51c5a
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/d6f51c5a

Branch: refs/heads/master
Commit: d6f51c5a2bcf4cf9590e6dd8685f91b2fa379cb9
Parents: 9f4fb06
Author: Dominic Hamon <dh...@twopensource.com>
Authored: Tue Apr 22 16:22:28 2014 -0700
Committer: Benjamin Mahler <bm...@twitter.com>
Committed: Tue Apr 22 16:56:25 2014 -0700

----------------------------------------------------------------------
 3rdparty/libprocess/Makefile.am                 |  1 +
 .../include/process/metrics/timer.hpp           | 96 ++++++++++++++++++++
 3rdparty/libprocess/src/tests/metrics_tests.cpp | 44 +++++++++
 3 files changed, 141 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/d6f51c5a/3rdparty/libprocess/Makefile.am
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/Makefile.am b/3rdparty/libprocess/Makefile.am
index 0a8a31b..8990f38 100644
--- a/3rdparty/libprocess/Makefile.am
+++ b/3rdparty/libprocess/Makefile.am
@@ -92,6 +92,7 @@ libprocess_la_SOURCES +=					\
   $(top_srcdir)/include/process/metrics/gauge.hpp		\
   $(top_srcdir)/include/process/metrics/metric.hpp		\
   $(top_srcdir)/include/process/metrics/metrics.hpp		\
+  $(top_srcdir)/include/process/metrics/timer.hpp		\
   $(top_srcdir)/include/process/mime.hpp			\
   $(top_srcdir)/include/process/mutex.hpp			\
   $(top_srcdir)/include/process/once.hpp			\

http://git-wip-us.apache.org/repos/asf/mesos/blob/d6f51c5a/3rdparty/libprocess/include/process/metrics/timer.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/include/process/metrics/timer.hpp b/3rdparty/libprocess/include/process/metrics/timer.hpp
new file mode 100644
index 0000000..c8e1d91
--- /dev/null
+++ b/3rdparty/libprocess/include/process/metrics/timer.hpp
@@ -0,0 +1,96 @@
+#ifndef __PROCESS_METRICS_TIMER_HPP__
+#define __PROCESS_METRICS_TIMER_HPP__
+
+#include <string>
+
+#include <process/future.hpp>
+#include <process/internal.hpp>
+
+#include <process/metrics/metric.hpp>
+
+#include <stout/duration.hpp>
+#include <stout/hashmap.hpp>
+#include <stout/option.hpp>
+#include <stout/stopwatch.hpp>
+#include <stout/try.hpp>
+
+namespace process {
+namespace metrics {
+
+// A Metric that represents a timed event in milliseconds.
+// TODO(dhamon): Allow the user to choose the unit of duration.
+// We could do this by adding methods on Duration subclasses to return
+// the double value and unit string directly.
+// TODO(dhamon): Support timing of concurrent operations. Possibly by
+// exposing a 'timed' method that takes a Future and binds to onAny.
+class Timer : public Metric
+{
+public:
+  // The Timer name will have "_ms" as an implicit unit suffix.
+  Timer(const std::string& name, const Option<Duration>& window = None())
+    : Metric(name + "_ms", window),
+      data(new Data()) {}
+
+  Future<double> value() const
+  {
+    Future<double> value;
+
+    process::internal::acquire(&data->lock);
+    {
+      if (data->lastValue.isSome()) {
+        value = data->lastValue.get();
+      } else {
+        value = Failure("No value");
+      }
+    }
+    process::internal::release(&data->lock);
+
+    return value;
+  }
+
+  // Start the stopwatch.
+  void start()
+  {
+    process::internal::acquire(&data->lock);
+    {
+      data->stopwatch.start();
+    }
+    process::internal::release(&data->lock);
+  }
+
+  // Stop the stopwatch.
+  void stop()
+  {
+    double value;
+
+    process::internal::acquire(&data->lock);
+    {
+      data->stopwatch.stop();
+
+      // Assume milliseconds for now.
+      data->lastValue = data->stopwatch.elapsed().ms();
+
+      value = data->lastValue.get();
+    }
+    process::internal::release(&data->lock);
+
+    push(value);
+  }
+
+
+private:
+  struct Data {
+    Data() : lock(0) {}
+
+    int lock;
+    Stopwatch stopwatch;
+    Option<double> lastValue;
+  };
+
+  memory::shared_ptr<Data> data;
+};
+
+} // namespace metrics
+} // namespace process
+
+#endif // __PROCESS_METRICS_TIMER_HPP__

http://git-wip-us.apache.org/repos/asf/mesos/blob/d6f51c5a/3rdparty/libprocess/src/tests/metrics_tests.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/tests/metrics_tests.cpp b/3rdparty/libprocess/src/tests/metrics_tests.cpp
index 2db459d..aef9ff4 100644
--- a/3rdparty/libprocess/src/tests/metrics_tests.cpp
+++ b/3rdparty/libprocess/src/tests/metrics_tests.cpp
@@ -14,6 +14,7 @@
 #include <process/metrics/counter.hpp>
 #include <process/metrics/gauge.hpp>
 #include <process/metrics/metrics.hpp>
+#include <process/metrics/timer.hpp>
 
 using namespace process;
 
@@ -22,6 +23,7 @@ using process::http::Response;
 
 using process::metrics::Counter;
 using process::metrics::Gauge;
+using process::metrics::Timer;
 
 
 class GaugeProcess : public Process<GaugeProcess>
@@ -210,3 +212,45 @@ TEST(Metrics, SnapshotStatistics)
 
   AWAIT_READY(metrics::remove(counter));
 }
+
+
+TEST(MetricsTest, Timer)
+{
+  metrics::Timer timer("test/timer");
+
+  AWAIT_READY(metrics::add(timer));
+
+  // It is not an error to stop a timer that hasn't been started.
+  timer.stop();
+
+  // Time a no-op.
+  Time started = Clock::now();
+
+  timer.start();
+  timer.stop();
+
+  Time stopped = Clock::now();
+
+  Future<double> value = timer.value();
+  AWAIT_READY(value);
+  EXPECT_LE(value.get(), (stopped - started).ms());
+
+  // Make sure that re-starting a timer records the correct value.
+  timer.start();
+
+  started = Clock::now();
+
+  timer.start();
+  timer.stop();
+
+  stopped = Clock::now();
+
+  value = timer.value();
+  AWAIT_READY(value);
+  EXPECT_LE(value.get(), (stopped - started).ms());
+
+  // It is not an error to stop a timer that has already been stopped.
+  timer.stop();
+
+  AWAIT_READY(metrics::remove(timer));
+}


[2/2] git commit: Added Statistics support for Metrics.

Posted by bm...@apache.org.
Added Statistics support for Metrics.

Review: https://reviews.apache.org/r/20018


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/9f4fb06f
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/9f4fb06f
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/9f4fb06f

Branch: refs/heads/master
Commit: 9f4fb06f0cac3768b4f031a6c0c1813743348055
Parents: 7e0e686
Author: Dominic Hamon <dh...@twopensource.com>
Authored: Tue Apr 22 00:26:13 2014 -0700
Committer: Benjamin Mahler <bm...@twitter.com>
Committed: Tue Apr 22 16:56:25 2014 -0700

----------------------------------------------------------------------
 .../include/process/metrics/metric.hpp          |  21 +-
 .../include/process/metrics/metrics.hpp         |   4 +-
 3rdparty/libprocess/src/metrics/metrics.cpp     |  24 +-
 3rdparty/libprocess/src/tests/metrics_tests.cpp | 230 +++++++++++++------
 .../libprocess/src/tests/statistics_tests.cpp   |  15 +-
 5 files changed, 205 insertions(+), 89 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/9f4fb06f/3rdparty/libprocess/include/process/metrics/metric.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/include/process/metrics/metric.hpp b/3rdparty/libprocess/include/process/metrics/metric.hpp
index 6a384de..00ace49 100644
--- a/3rdparty/libprocess/include/process/metrics/metric.hpp
+++ b/3rdparty/libprocess/include/process/metrics/metric.hpp
@@ -6,6 +6,7 @@
 #include <process/future.hpp>
 #include <process/internal.hpp>
 #include <process/owned.hpp>
+#include <process/statistics.hpp>
 #include <process/timeseries.hpp>
 
 #include <stout/duration.hpp>
@@ -22,7 +23,25 @@ public:
 
   virtual Future<double> value() const = 0;
 
-  const std::string& name() const { return data->name; }
+  const std::string& name() const
+  {
+    return data->name;
+  }
+
+  Option<Statistics<double> > statistics() const
+  {
+    Option<Statistics<double> > statistics = None();
+
+    if (data->history.isSome()) {
+      internal::acquire(&data->lock);
+      {
+        statistics = Statistics<double>::from(*data->history.get());
+      }
+      internal::release(&data->lock);
+    }
+
+    return statistics;
+  }
 
 protected:
   // Only derived classes can construct.

http://git-wip-us.apache.org/repos/asf/mesos/blob/9f4fb06f/3rdparty/libprocess/include/process/metrics/metrics.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/include/process/metrics/metrics.hpp b/3rdparty/libprocess/include/process/metrics/metrics.hpp
index c20bb63..ba12727 100644
--- a/3rdparty/libprocess/include/process/metrics/metrics.hpp
+++ b/3rdparty/libprocess/include/process/metrics/metrics.hpp
@@ -40,9 +40,11 @@ private:
   MetricsProcess& operator = (const MetricsProcess&);
 
   Future<http::Response> snapshot(const http::Request& request);
+
   static Future<http::Response> _snapshot(
       const http::Request& request,
-      const hashmap<std::string, Future<double> >& metrics);
+      const hashmap<std::string, Future<double> >& metrics,
+      const hashmap<std::string, Option<Statistics<double> > >& statistics);
 
   // The Owned<Metric> is an explicit copy of the Metric passed to 'add'.
   hashmap<std::string, Owned<Metric> > metrics;

http://git-wip-us.apache.org/repos/asf/mesos/blob/9f4fb06f/3rdparty/libprocess/src/metrics/metrics.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/metrics/metrics.cpp b/3rdparty/libprocess/src/metrics/metrics.cpp
index 391295a..aa35605 100644
--- a/3rdparty/libprocess/src/metrics/metrics.cpp
+++ b/3rdparty/libprocess/src/metrics/metrics.cpp
@@ -78,31 +78,49 @@ Future<Nothing> MetricsProcess::remove(const std::string& name)
 }
 
 
-// TODO(dhamon): Allow querying by context and context/name.
 Future<http::Response> MetricsProcess::snapshot(const http::Request& request)
 {
   hashmap<string, Future<double> > futures;
+  hashmap<string, Option<Statistics<double> > > statistics;
 
   foreachkey (const string& metric, metrics) {
     CHECK_NOTNULL(metrics[metric].get());
     futures[metric] = metrics[metric]->value();
+    // TODO(dhamon): It would be nice to get these in parallel.
+    statistics[metric] = metrics[metric]->statistics();
   }
 
   return await(futures.values())
-    .then(lambda::bind(_snapshot, request, futures));
+    .then(lambda::bind(_snapshot, request, futures, statistics));
 }
 
 
 Future<http::Response> MetricsProcess::_snapshot(
     const http::Request& request,
-    const hashmap<string, Future<double> >& metrics)
+    const hashmap<string, Future<double> >& metrics,
+    const hashmap<string, Option<Statistics<double> > >& statistics)
 {
   JSON::Object object;
 
   foreachpair (const string& key, const Future<double>& value, metrics) {
+    // Value.
     if (value.isReady()) {
       object.values[key] = value.get();
     }
+
+    // Statistics.
+    Option<Statistics<double> > statistics_ = statistics.get(key).get();
+
+    if (statistics_.isSome()) {
+      object.values[key + "/min"] = statistics_.get().min;
+      object.values[key + "/max"] = statistics_.get().max;
+      object.values[key + "/p50"] = statistics_.get().p50;
+      object.values[key + "/p90"] = statistics_.get().p90;
+      object.values[key + "/p95"] = statistics_.get().p95;
+      object.values[key + "/p99"] = statistics_.get().p99;
+      object.values[key + "/p999"] = statistics_.get().p999;
+      object.values[key + "/p9999"] = statistics_.get().p9999;
+    }
   }
 
   return http::OK(object, request.query.get("jsonp"));

http://git-wip-us.apache.org/repos/asf/mesos/blob/9f4fb06f/3rdparty/libprocess/src/tests/metrics_tests.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/tests/metrics_tests.cpp b/3rdparty/libprocess/src/tests/metrics_tests.cpp
index abe1588..2db459d 100644
--- a/3rdparty/libprocess/src/tests/metrics_tests.cpp
+++ b/3rdparty/libprocess/src/tests/metrics_tests.cpp
@@ -1,28 +1,25 @@
 #include <gtest/gtest.h>
 
+#include <stout/duration.hpp>
 #include <stout/gtest.hpp>
 
 #include <process/clock.hpp>
 #include <process/future.hpp>
 #include <process/gtest.hpp>
+#include <process/http.hpp>
 #include <process/process.hpp>
+#include <process/statistics.hpp>
 #include <process/time.hpp>
 
 #include <process/metrics/counter.hpp>
 #include <process/metrics/gauge.hpp>
 #include <process/metrics/metrics.hpp>
 
+using namespace process;
 
-using process::Clock;
-using process::Deferred;
-using process::Failure;
-using process::Future;
-using process::PID;
-using process::Process;
-using process::Time;
+using process::http::OK;
+using process::http::Response;
 
-using process::metrics::add;
-using process::metrics::remove;
 using process::metrics::Counter;
 using process::metrics::Gauge;
 
@@ -30,105 +27,186 @@ using process::metrics::Gauge;
 class GaugeProcess : public Process<GaugeProcess>
 {
 public:
-  double get() { return 42.0; }
-  Future<double> fail() { return Failure("failure"); }
+  double get()
+  {
+    return 42.0;
+  }
+
+  Future<double> fail()
+  {
+    return Failure("failure");
+  }
 };
 
-// TODO(dhamon): Add test for JSON equality.
-// TODO(dhamon): Add tests for JSON access with and without removal.
 
-TEST(MetricsTest, Counter)
+TEST(Metrics, Counter)
 {
-  Counter c("test/counter");
-  AWAIT_READY(add(c));
+  Counter counter("test/counter");
 
-  EXPECT_FLOAT_EQ(0.0, c.value().get());
-  ++c;
-  EXPECT_FLOAT_EQ(1.0, c.value().get());
-  c++;
-  EXPECT_FLOAT_EQ(2.0, c.value().get());
+  AWAIT_READY(metrics::add(counter));
 
-  c.reset();
-  EXPECT_FLOAT_EQ(0.0, c.value().get());
+  AWAIT_EXPECT_EQ(0.0, counter.value());
 
-  c += 42;
-  EXPECT_FLOAT_EQ(42.0, c.value().get());
+  ++counter;
+  AWAIT_EXPECT_EQ(1.0, counter.value());
 
-  AWAIT_READY(remove(c));
+  counter++;
+  AWAIT_EXPECT_EQ(2.0, counter.value());
+
+  counter.reset();
+  AWAIT_EXPECT_EQ(0.0, counter.value());
+
+  counter += 42;
+  AWAIT_EXPECT_EQ(42.0, counter.value());
+
+  EXPECT_NONE(counter.statistics());
+
+  AWAIT_READY(metrics::remove(counter));
 }
 
 
-TEST(MetricsTest, CounterHistory)
+TEST(Metrics, Gauge)
 {
-  Clock::pause();
-  Time t0 = Clock::now();
+  ASSERT_TRUE(GTEST_IS_THREADSAFE);
+
+  GaugeProcess process;
+  PID<GaugeProcess> pid = spawn(&process);
+  ASSERT_TRUE(pid);
+
+  // Gauge with a value.
+  Gauge gauge("test/gauge", defer(pid, &GaugeProcess::get));
+
+  AWAIT_READY(metrics::add(gauge));
+
+  AWAIT_EXPECT_EQ(42.0, gauge.value());
+
+  AWAIT_READY(metrics::remove(gauge));
+
+  // Failing gauge.
+  gauge = Gauge("test/failedgauge", defer(pid, &GaugeProcess::fail));
+
+  AWAIT_READY(metrics::add(gauge));
+
+  AWAIT_EXPECT_FAILED(gauge.value());
+
+  AWAIT_READY(metrics::remove(gauge));
+
+  terminate(process);
+  wait(process);
+}
+
+
+TEST(Metrics, Statistics)
+{
+  Counter counter("test/counter", process::TIME_SERIES_WINDOW);
+
+  AWAIT_READY(metrics::add(counter));
 
-  Counter c("test/counter", process::TIME_SERIES_WINDOW);
-  AWAIT_READY(add(c));
+  for (size_t i = 0; i < 10; ++i) {
+    ++counter;
+  }
 
-  Clock::advance(Seconds(1));
-  Time t1 = Clock::now();
-  ++c;
+  Option<Statistics<double> > statistics = counter.statistics();
+  EXPECT_SOME(statistics);
 
-  Clock::advance(Seconds(1));
-  Time t2 = Clock::now();
-  ++c;
+  EXPECT_FLOAT_EQ(0.0, statistics.get().min);
+  EXPECT_FLOAT_EQ(10.0, statistics.get().max);
 
-  // TODO(dhamon): get json/history from metrics process and check
-  // the history.
+  EXPECT_FLOAT_EQ(5.0, statistics.get().p50);
+  EXPECT_FLOAT_EQ(9.0, statistics.get().p90);
+  EXPECT_FLOAT_EQ(9.5, statistics.get().p95);
+  EXPECT_FLOAT_EQ(9.9, statistics.get().p99);
+  EXPECT_FLOAT_EQ(9.99, statistics.get().p999);
+  EXPECT_FLOAT_EQ(9.999, statistics.get().p9999);
 
-  AWAIT_READY(remove(c));
+  AWAIT_READY(metrics::remove(counter));
 }
 
 
-// TODO(dhamon): Expand benchmarks and enable them.
-// TEST(MetricsTest, CounterBM)
-// {
-//   for (int i = 0; i < 10; ++i) {
-//     Counter c("test/counter", Seconds(1));
-//
-//     // Fill the history
-//     Time t0 = Clock::now();
-//     while (Clock::now() - t0 < Seconds(1)) {
-//       c++;
-//     }
-//
-//     // Run the benchmark
-//     t0 = Clock::now();
-//     int numInc = 0;
-//     while (Clock::now() - t0 < Seconds(1)) {
-//       c++;
-//       ++numInc;
-//     }
-//     std::cout << numInc << "\n";
-//   }
-//
-//   EXPECT_TRUE(true);
-// }
-
-
-TEST(MetricsTest, Gauge)
+TEST(Metrics, Snapshot)
 {
   ASSERT_TRUE(GTEST_IS_THREADSAFE);
 
+  UPID upid("metrics", process::ip(), process::port());
+
+  // Before adding any metrics, the response should be empty.
+  Future<Response> response = http::get(upid, "snapshot");
+
+  AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response);
+  AWAIT_EXPECT_RESPONSE_BODY_EQ(stringify(JSON::Object()), response);
+
+  // Add a gauge and a counter.
   GaugeProcess process;
   PID<GaugeProcess> pid = spawn(&process);
   ASSERT_TRUE(pid);
 
-  Gauge g("test/gauge", defer(pid, &GaugeProcess::get));
-  Gauge g2("test/failedgauge", defer(pid, &GaugeProcess::fail));
+  Gauge gauge("test/gauge", defer(pid, &GaugeProcess::get));
+  Counter counter("test/counter");
+
+  AWAIT_READY(metrics::add(gauge));
+  AWAIT_READY(metrics::add(counter));
 
-  AWAIT_READY(add(g));
-  AWAIT_READY(add(g2));
+  // Get the snapshot.
+  response = http::get(upid, "snapshot");
+  AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response);
 
-  AWAIT_READY(g.value());
-  EXPECT_EQ(42.0, g.value().get());
+  JSON::Object expected;
+  expected.values["test/counter"] = 0.0;
+  expected.values["test/gauge"] = 42.0;
+  AWAIT_EXPECT_RESPONSE_BODY_EQ(stringify(expected), response);
 
-  AWAIT_FAILED(g2.value());
+  // Remove the metrics and ensure they are no longer in the snapshot.
+  AWAIT_READY(metrics::remove(gauge));
+  AWAIT_READY(metrics::remove(counter));
 
-  AWAIT_READY(remove(g2));
-  AWAIT_READY(remove(g));
+  response = http::get(upid, "snapshot");
+  AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response);
+  AWAIT_EXPECT_RESPONSE_BODY_EQ(stringify(JSON::Object()), response);
 
   terminate(process);
   wait(process);
 }
+
+
+// Ensures that the aggregate statistics are correct in the snapshot.
+TEST(Metrics, SnapshotStatistics)
+{
+  UPID upid("metrics", process::ip(), process::port());
+
+  Future<Response> response = http::get(upid, "snapshot");
+
+  AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response);
+  AWAIT_EXPECT_RESPONSE_BODY_EQ(stringify(JSON::Object()), response);
+
+  Clock::pause();
+
+  Counter counter("test/counter", process::TIME_SERIES_WINDOW);
+
+  AWAIT_READY(metrics::add(counter));
+
+  for (size_t i = 0; i < 10; ++i) {
+    Clock::advance(Seconds(1));
+    ++counter;
+  }
+
+  JSON::Object expected;
+
+  expected.values["test/counter"] = 10.0;
+
+  expected.values["test/counter/min"] = 0.0;
+  expected.values["test/counter/max"] = 10.0;
+
+  expected.values["test/counter/p50"] = 5.0;
+  expected.values["test/counter/p90"] = 9.0;
+  expected.values["test/counter/p95"] = 9.5;
+  expected.values["test/counter/p99"] = 9.9;
+  expected.values["test/counter/p999"] = 9.99;
+  expected.values["test/counter/p9999"] = 9.999;
+
+  response = http::get(upid, "snapshot");
+
+  AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response);
+  AWAIT_EXPECT_RESPONSE_BODY_EQ(stringify(expected), response);
+
+  AWAIT_READY(metrics::remove(counter));
+}

http://git-wip-us.apache.org/repos/asf/mesos/blob/9f4fb06f/3rdparty/libprocess/src/tests/statistics_tests.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/tests/statistics_tests.cpp b/3rdparty/libprocess/src/tests/statistics_tests.cpp
index 478453f..84cfb52 100644
--- a/3rdparty/libprocess/src/tests/statistics_tests.cpp
+++ b/3rdparty/libprocess/src/tests/statistics_tests.cpp
@@ -23,9 +23,8 @@ TEST(Statistics, statistics)
   TimeSeries<double> timeseries;
 
   Time now = Clock::now();
-  timeseries.set(0.0, now);
 
-  for (int i = -5; i < 5; ++i) {
+  for (int i = -5; i <= 5; ++i) {
     now += Seconds(1);
     timeseries.set(i, now);
   }
@@ -35,12 +34,12 @@ TEST(Statistics, statistics)
   EXPECT_SOME(statistics);
 
   EXPECT_FLOAT_EQ(-5.0, statistics.get().min);
-  EXPECT_FLOAT_EQ(4.0, statistics.get().max);
+  EXPECT_FLOAT_EQ(5.0, statistics.get().max);
 
   EXPECT_FLOAT_EQ(0.0, statistics.get().p50);
-  EXPECT_FLOAT_EQ(3.0, statistics.get().p90);
-  EXPECT_FLOAT_EQ(3.5, statistics.get().p95);
-  EXPECT_FLOAT_EQ(3.9, statistics.get().p99);
-  EXPECT_FLOAT_EQ(3.99, statistics.get().p999);
-  EXPECT_FLOAT_EQ(3.999, statistics.get().p9999);
+  EXPECT_FLOAT_EQ(4.0, statistics.get().p90);
+  EXPECT_FLOAT_EQ(4.5, statistics.get().p95);
+  EXPECT_FLOAT_EQ(4.9, statistics.get().p99);
+  EXPECT_FLOAT_EQ(4.99, statistics.get().p999);
+  EXPECT_FLOAT_EQ(4.999, statistics.get().p9999);
 }