Fix typo, add more comments and tests

include/ppx/application.h

@@ -524,18 +524,18 @@ class Application
     // See StartMetricsRun for why this wrapper is necessary.
     virtual bool HasActiveMetricsRun() const;
 
-    // Allocate a metric id to be used for a combind live/recorded metric.
+    // Allocate a metric id to be used for a combined live/recorded metric.
     metrics::MetricID AllocateMetricID();
 
     // Adds a metric to the current run. If no run is active, returns metrics::kInvalidMetricID.
     // See StartMetricsRun for why this wrapper is necessary.
     metrics::MetricID AddMetric(const metrics::MetricMetadata& metadata);
 
-    // Bind a metric to the current run, Return false if no run is active.
+    // Bind a metric to the current run, return false if no run is active.
     bool BindMetric(metrics::MetricID metricID, const metrics::MetricMetadata& metadata);
 
-    // Add a live metric, the returned MetricID can also be used for recorded metric.
-    bool BindLiveMetric(metrics::MetricID metricID = metrics::kInvalidMetricID);
+    // Bind a live metric, return true on success.
+    bool BindLiveMetric(metrics::MetricID metricID);
 
     // Clear history of live metric, usually after knob changed.
     void ClearLiveMetricsHistory();

include/ppx/metrics.h

@@ -233,7 +233,7 @@ class MetricCounter final : public Metric
 
 ////////////////////////////////////////////////////////////////////////////////
 
-// Live statistics are computerd on the fly, for recently reported metric.
+// Live statistics are computed on the fly, for recently reported metric.
 // The weight assigned to each entry is assigned as follows
 //   w_i = exp((t_i-t_now)/halfLife), default halfLife = 0.5s
 // Min/Max are not affected by the weight.
@@ -396,8 +396,13 @@ class Manager final
         MetricID metricID = AllocateID();
         return BindMetric(metricID, metadata) ? metricID : kInvalidMetricID;
     }
-
+    // Binds a metric to the current run. A run must be started to bind a metric.
+    // Failure to add a metric returns false.
     bool BindMetric(MetricID metricID, const MetricMetadata& metadata);
+
+    // Binds a live metric to the metricID.
+    // A live metric is independent of runs, the same metricID can be used to bind a metric
+    // to a run, in which case RecordMetricData records to both live metric and the run.
     bool BindLiveMetric(MetricID metricID, double halfLife = LiveMetric::kDefaultHalfLife);
 
     // Records data for the given metric ID. Metrics for completed runs will be discarded.

src/test/metrics_test.cpp

@@ -107,6 +107,14 @@ TEST(MetricsTest, ManagerStartDuplicateRunFails)
 }
 #endif
 
+TEST(MetricsTest, ManagerBindLiveMetricWithoutRun)
+{
+    metrics::Manager manager;
+    EXPECT_FALSE(manager.HasActiveRun());
+    auto metricId = manager.AllocateID();
+    EXPECT_TRUE(manager.BindLiveMetric(metricId));
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // Run Tests
 ////////////////////////////////////////////////////////////////////////////////
@@ -546,7 +554,7 @@ TEST_F(MetricsTestFixture, MetricsGaugeIgnoresSameSeconds)
     EXPECT_EQ(gauge["time_series"][1][1], 11.0);
 }
 
-TEST_F(MetricsTestFixture, MetricsLiveStatistics)
+TEST_F(MetricsTestFixture, MetricsLiveMetricStatistics)
 {
     metrics::MetricMetadata metadata;
     metadata.type = metrics::MetricType::GAUGE;
@@ -596,4 +604,78 @@ TEST_F(MetricsTestFixture, MetricsLiveStatistics)
     EXPECT_EQ(reportStats["standard_deviation"], 1.0);
 }
 
+TEST_F(MetricsTestFixture, MetricsLiveMetricAfterEndRun)
+{
+    metrics::MetricMetadata metadata;
+    metadata.type = metrics::MetricType::GAUGE;
+    metadata.name = "gauge";
+
+    auto metricId = pManager->AllocateID();
+    ASSERT_TRUE(pManager->BindLiveMetric(metricId, 1.0));
+    ASSERT_TRUE(pManager->BindMetric(metricId, metadata));
+
+    metrics::MetricData data = {metrics::MetricType::GAUGE};
+    data.gauge.seconds       = 1.0;
+    data.gauge.value         = 10.0;
+    pManager->RecordMetricData(metricId, data);
+    pManager->EndRun();
+    data.gauge.seconds = 2.0;
+    data.gauge.value   = 12.0;
+    pManager->RecordMetricData(metricId, data);
+
+    auto liveStats = pManager->GetLiveStatistics(metricId);
+
+    EXPECT_NEAR(liveStats.mean, 11.3333, 0.0010);
+    EXPECT_NEAR(liveStats.variance, 8.0 / 9.0, 0.0010);
+}
+
+TEST_F(MetricsTestFixture, MetricsLiveMetricClearHistory)
+{
+    metrics::MetricMetadata metadata;
+    metadata.type = metrics::MetricType::GAUGE;
+    metadata.name = "gauge";
+
+    auto metricId = pManager->AllocateID();
+    ASSERT_TRUE(pManager->BindLiveMetric(metricId, 1.0));
+    ASSERT_TRUE(pManager->BindMetric(metricId, metadata));
+
+    metrics::MetricData data = {metrics::MetricType::GAUGE};
+    data.gauge.seconds       = 1.0;
+    data.gauge.value         = 10.0;
+    pManager->RecordMetricData(metricId, data);
+    pManager->ClearLiveMetricsHistory();
+    data.gauge.seconds = 2.0;
+    data.gauge.value   = 12.0;
+    pManager->RecordMetricData(metricId, data);
+
+    auto result = pManager->CreateReport("report").GetContentString();
+    auto parsed = nlohmann::json::parse(result);
+    auto gauge  = parsed["runs"][0]["gauges"][0];
+    EXPECT_EQ(gauge["time_series"].size(), 2);
+    EXPECT_EQ(gauge["time_series"][0][0], 1.0000);
+    EXPECT_EQ(gauge["time_series"][0][1], 10.0);
+    EXPECT_EQ(gauge["time_series"][1][0], 2.0);
+    EXPECT_EQ(gauge["time_series"][1][1], 12.0);
+
+    auto liveStats = pManager->GetLiveStatistics(metricId);
+
+    // 10.0 with weight 0.5, and 12.0 with weight 1.0
+    constexpr double kExpectedMean = (10.0 * 0.5 + 12.0 * 1.0) / 1.5;
+    constexpr double kExpectedVariance =
+        ((10.0 - kExpectedMean) * (10.0 - kExpectedMean) * 0.5 + (12.0 - kExpectedMean) * (12.0 - kExpectedMean) * 1.0) / 1.5;
+    EXPECT_EQ(liveStats.latest, 12.0);
+    EXPECT_EQ(liveStats.seconds, 2.0);
+    EXPECT_EQ(liveStats.min, 12.0);
+    EXPECT_EQ(liveStats.max, 12.0);
+    EXPECT_EQ(liveStats.weight, 1.0);
+    EXPECT_NEAR(liveStats.mean, 12.0, 0.0010);
+    EXPECT_NEAR(liveStats.variance, 0.0, 0.0010);
+
+    auto reportStats = gauge["statistics"];
+    EXPECT_EQ(reportStats["min"], 10.0);
+    EXPECT_EQ(reportStats["max"], 12.0);
+    EXPECT_EQ(reportStats["average"], 11.0);
+    EXPECT_EQ(reportStats["standard_deviation"], 1.0);
+}
+
 } // namespace ppx