Some fixes in StatsdStats, and add some unit tests

+ Add timestamp for when metric data is reported.

Test: statsd_test

Change-Id: Ief5ec5172feed4ec74b7422b77cf69ec8361ef2f

6 files changed, 238 insertions, 24 deletions

diff --git a/cmds/statsd/src/StatsLogProcessor.cpp b/cmds/statsd/src/StatsLogProcessor.cpp
index 2690c7e43d79..7f4d67adbd25 100644
--- a/cmds/statsd/src/StatsLogProcessor.cpp
+++ b/cmds/statsd/src/StatsLogProcessor.cpp
@@ -201,6 +201,7 @@ void StatsLogProcessor::onDumpReport(const ConfigKey& key, vector<uint8_t>* outD
             iter.rp()->move(toRead);
         }
     }
+    StatsdStats::getInstance().noteMetricsReportSent(key);
 }
 
 void StatsLogProcessor::OnConfigRemoved(const ConfigKey& key) {
@@ -236,7 +237,7 @@ void StatsLogProcessor::flushIfNecessary(uint64_t timestampNs,
     } else if (totalBytes > kMaxSerializedBytes) { // Too late. We need to start clearing data.
         // We ignore the return value so we force each metric producer to clear its contents.
         metricsManager->onDumpReport();
-        StatsdStats::getInstance().noteDataDrop(key);
+        StatsdStats::getInstance().noteDataDropped(key);
         VLOG("StatsD had to toss out metrics for %s", key.ToString().c_str());
     }
 }
diff --git a/cmds/statsd/src/StatsService.cpp b/cmds/statsd/src/StatsService.cpp
index 8b64f0db5728..7a127e07c55d 100644
--- a/cmds/statsd/src/StatsService.cpp
+++ b/cmds/statsd/src/StatsService.cpp
@@ -488,7 +488,7 @@ status_t StatsService::cmd_print_stats(FILE* out, const Vector<String8>& args) {
     if (args.size() > 1) {
         reset = strtol(args[1].string(), NULL, 10);
     }
-    vector<int8_t> output;
+    vector<uint8_t> output;
     statsdStats.dumpStats(&output, reset);
     return NO_ERROR;
 }
diff --git a/cmds/statsd/src/guardrail/StatsdStats.cpp b/cmds/statsd/src/guardrail/StatsdStats.cpp
index 815e03f6d3f1..2ab1146208ee 100644
--- a/cmds/statsd/src/guardrail/StatsdStats.cpp
+++ b/cmds/statsd/src/guardrail/StatsdStats.cpp
@@ -61,7 +61,7 @@ const int FIELD_ID_ATOM_STATS_COUNT = 2;
 // TODO: add stats for pulled atoms.
 StatsdStats::StatsdStats() {
     mPushedAtomStats.resize(android::util::kMaxPushedAtomId + 1);
-    mStartTime = time(nullptr);
+    mStartTimeSec = time(nullptr);
 }
 
 StatsdStats& StatsdStats::getInstance() {
@@ -107,6 +107,7 @@ void StatsdStats::noteConfigRemovedInternalLocked(const ConfigKey& key) {
         mMetricsStats.erase(key);
         mConditionStats.erase(key);
         mIceBox.push_back(it->second);
+        mConfigStats.erase(it);
     }
 }
 
@@ -126,7 +127,7 @@ void StatsdStats::noteBroadcastSent(const ConfigKey& key) {
     it->second.add_broadcast_sent_time_sec(time(nullptr));
 }
 
-void StatsdStats::noteDataDrop(const ConfigKey& key) {
+void StatsdStats::noteDataDropped(const ConfigKey& key) {
     lock_guard<std::mutex> lock(mLock);
     auto it = mConfigStats.find(key);
     if (it == mConfigStats.end()) {
@@ -137,6 +138,17 @@ void StatsdStats::noteDataDrop(const ConfigKey& key) {
     it->second.add_data_drop_time_sec(time(nullptr));
 }
 
+void StatsdStats::noteMetricsReportSent(const ConfigKey& key) {
+    lock_guard<std::mutex> lock(mLock);
+    auto it = mConfigStats.find(key);
+    if (it == mConfigStats.end()) {
+        ALOGE("Config key %s not found!", key.ToString().c_str());
+        return;
+    }
+
+    it->second.add_dump_report_time_sec(time(nullptr));
+}
+
 void StatsdStats::noteConditionDimensionSize(const ConfigKey& key, const string& name, int size) {
     lock_guard<std::mutex> lock(mLock);
     // if name doesn't exist before, it will create the key with count 0.
@@ -164,7 +176,7 @@ void StatsdStats::noteMatcherMatched(const ConfigKey& key, const string& name) {
 void StatsdStats::noteAtomLogged(int atomId, int32_t timeSec) {
     lock_guard<std::mutex> lock(mLock);
 
-    if (timeSec < mStartTime) {
+    if (timeSec < mStartTimeSec) {
         return;
     }
 
@@ -183,7 +195,7 @@ void StatsdStats::reset() {
 
 void StatsdStats::resetInternalLocked() {
     // Reset the historical data, but keep the active ConfigStats
-    mStartTime = time(nullptr);
+    mStartTimeSec = time(nullptr);
     mIceBox.clear();
     mConditionStats.clear();
     mMetricsStats.clear();
@@ -225,11 +237,11 @@ void StatsdStats::addSubStatsToConfig(const ConfigKey& key,
     }
 }
 
-void StatsdStats::dumpStats(std::vector<int8_t>* output, bool reset) {
+void StatsdStats::dumpStats(std::vector<uint8_t>* output, bool reset) {
     lock_guard<std::mutex> lock(mLock);
 
     if (DEBUG) {
-        time_t t = time(nullptr);
+        time_t t = mStartTimeSec;
         struct tm* tm = localtime(&t);
         char timeBuffer[80];
         strftime(timeBuffer, sizeof(timeBuffer), "%Y-%m-%d %I:%M%p", tm);
@@ -237,7 +249,7 @@ void StatsdStats::dumpStats(std::vector<int8_t>* output, bool reset) {
         VLOG("Stats collection start second: %s", timeBuffer);
     }
     ProtoOutputStream proto;
-    proto.write(FIELD_TYPE_INT32 | FIELD_ID_BEGIN_TIME, mStartTime);
+    proto.write(FIELD_TYPE_INT32 | FIELD_ID_BEGIN_TIME, mStartTimeSec);
     proto.write(FIELD_TYPE_INT32 | FIELD_ID_END_TIME, (int32_t)time(nullptr));
 
     VLOG("%lu Config in icebox: ", (unsigned long)mIceBox.size());
@@ -286,6 +298,10 @@ void StatsdStats::dumpStats(std::vector<int8_t>* output, bool reset) {
             for (const auto& dataDropTime : configStats.data_drop_time_sec()) {
                 VLOG("\tdata drop time: %d", dataDropTime);
             }
+
+            for (const auto& dumpTime : configStats.dump_report_time_sec()) {
+                VLOG("\tdump report time: %d", dumpTime);
+            }
         }
 
         addSubStatsToConfig(pair.first, configStats);
diff --git a/cmds/statsd/src/guardrail/StatsdStats.h b/cmds/statsd/src/guardrail/StatsdStats.h
index 73ce2798cd6c..6fd9e4b6cd33 100644
--- a/cmds/statsd/src/guardrail/StatsdStats.h
+++ b/cmds/statsd/src/guardrail/StatsdStats.h
@@ -18,6 +18,7 @@
 #include "config/ConfigKey.h"
 #include "frameworks/base/cmds/statsd/src/stats_log.pb.h"
 
+#include <gtest/gtest_prod.h>
 #include <mutex>
 #include <string>
 #include <vector>
@@ -63,7 +64,14 @@ public:
     /**
      * Report a config's metrics data has been dropped.
      */
-    void noteDataDrop(const ConfigKey& key);
+    void noteDataDropped(const ConfigKey& key);
+
+    /**
+     * Report metrics data report has been sent.
+     *
+     * The report may be requested via StatsManager API, or through adb cmd.
+     */
+    void noteMetricsReportSent(const ConfigKey& key);
 
     /**
      * Report the size of output tuple of a condition.
@@ -114,14 +122,14 @@ public:
      *
      * [reset]: whether to clear the historical stats after the call.
      */
-    void dumpStats(std::vector<int8_t>* buffer, bool reset);
+    void dumpStats(std::vector<uint8_t>* buffer, bool reset);
 
 private:
     StatsdStats();
 
     mutable std::mutex mLock;
 
-    int32_t mStartTime;
+    int32_t mStartTimeSec;
 
     // The stats about the configs that are still in use.
     std::map<const ConfigKey, StatsdStatsReport_ConfigStats> mConfigStats;
@@ -153,6 +161,12 @@ private:
     void resetInternalLocked();
 
     void addSubStatsToConfig(const ConfigKey& key, StatsdStatsReport_ConfigStats& configStats);
+
+    FRIEND_TEST(StatsdStatsTest, TestValidConfigAdd);
+    FRIEND_TEST(StatsdStatsTest, TestInvalidConfigAdd);
+    FRIEND_TEST(StatsdStatsTest, TestConfigRemove);
+    FRIEND_TEST(StatsdStatsTest, TestSubStats);
+    FRIEND_TEST(StatsdStatsTest, TestAtomLog);
 };
 
 }  // namespace statsd
diff --git a/cmds/statsd/src/stats_log.proto b/cmds/statsd/src/stats_log.proto
index 81f8eb619d35..e7e1d43974af 100644
--- a/cmds/statsd/src/stats_log.proto
+++ b/cmds/statsd/src/stats_log.proto
@@ -206,9 +206,10 @@ message StatsdStatsReport {
 
         repeated int32 broadcast_sent_time_sec = 10;
         repeated int32 data_drop_time_sec = 11;
-        repeated MatcherStats matcher_stats = 12;
-        repeated ConditionStats condition_stats = 13;
-        repeated MetricStats metric_stats = 14;
+        repeated int32 dump_report_time_sec = 12;
+        repeated MatcherStats matcher_stats = 13;
+        repeated ConditionStats condition_stats = 14;
+        repeated MetricStats metric_stats = 15;
     }
 
     repeated ConfigStats config_stats = 3;
diff --git a/cmds/statsd/tests/guardrail/StatsdStats_test.cpp b/cmds/statsd/tests/guardrail/StatsdStats_test.cpp
index 286f6bd75442..b14b52cf595c 100644
--- a/cmds/statsd/tests/guardrail/StatsdStats_test.cpp
+++ b/cmds/statsd/tests/guardrail/StatsdStats_test.cpp
@@ -13,8 +13,10 @@
 // limitations under the License.
 
 #include "src/guardrail/StatsdStats.h"
+#include "statslog.h"
 
 #include <gtest/gtest.h>
+#include <vector>
 
 #ifdef __ANDROID__
 
@@ -22,24 +24,204 @@ namespace android {
 namespace os {
 namespace statsd {
 
-TEST(StatsdStatsTest, TestConfigAdd) {
-    // TODO: implement
+using std::vector;
+
+TEST(StatsdStatsTest, TestValidConfigAdd) {
+    StatsdStats stats;
+    string name = "StatsdTest";
+    ConfigKey key(0, name);
+    const int metricsCount = 10;
+    const int conditionsCount = 20;
+    const int matchersCount = 30;
+    const int alertsCount = 10;
+    stats.noteConfigReceived(key, metricsCount, conditionsCount, matchersCount, alertsCount,
+                             true /*valid config*/);
+    vector<uint8_t> output;
+    stats.dumpStats(&output, false /*reset stats*/);
+
+    StatsdStatsReport report;
+    bool good = report.ParseFromArray(&output[0], output.size());
+    EXPECT_TRUE(good);
+    EXPECT_EQ(1, report.config_stats_size());
+    const auto& configReport = report.config_stats(0);
+    EXPECT_EQ(0, configReport.uid());
+    EXPECT_EQ(name, configReport.name());
+    EXPECT_EQ(metricsCount, configReport.metric_count());
+    EXPECT_EQ(conditionsCount, configReport.condition_count());
+    EXPECT_EQ(matchersCount, configReport.matcher_count());
+    EXPECT_EQ(alertsCount, configReport.alert_count());
+    EXPECT_EQ(true, configReport.is_valid());
+    EXPECT_FALSE(configReport.has_deletion_time_sec());
 }
 
-TEST(StatsdStatsTest, TestConfigRemove) {
-    // TODO: implement
+TEST(StatsdStatsTest, TestInvalidConfigAdd) {
+    StatsdStats stats;
+    string name = "StatsdTest";
+    ConfigKey key(0, name);
+    const int metricsCount = 10;
+    const int conditionsCount = 20;
+    const int matchersCount = 30;
+    const int alertsCount = 10;
+    stats.noteConfigReceived(key, metricsCount, conditionsCount, matchersCount, alertsCount,
+                             false /*bad config*/);
+    vector<uint8_t> output;
+    stats.dumpStats(&output, false);
+
+    StatsdStatsReport report;
+    bool good = report.ParseFromArray(&output[0], output.size());
+    EXPECT_TRUE(good);
+    EXPECT_EQ(1, report.config_stats_size());
+    const auto& configReport = report.config_stats(0);
+    // The invalid config should be put into icebox with a deletion time.
+    EXPECT_TRUE(configReport.has_deletion_time_sec());
 }
 
-TEST(StatsdStatsTest, TestMatcherReport) {
-    // TODO: implement
+TEST(StatsdStatsTest, TestConfigRemove) {
+    StatsdStats stats;
+    string name = "StatsdTest";
+    ConfigKey key(0, name);
+    const int metricsCount = 10;
+    const int conditionsCount = 20;
+    const int matchersCount = 30;
+    const int alertsCount = 10;
+    stats.noteConfigReceived(key, metricsCount, conditionsCount, matchersCount, alertsCount, true);
+    vector<uint8_t> output;
+    stats.dumpStats(&output, false);
+    StatsdStatsReport report;
+    bool good = report.ParseFromArray(&output[0], output.size());
+    EXPECT_TRUE(good);
+    EXPECT_EQ(1, report.config_stats_size());
+    const auto& configReport = report.config_stats(0);
+    EXPECT_FALSE(configReport.has_deletion_time_sec());
+
+    stats.noteConfigRemoved(key);
+    stats.dumpStats(&output, false);
+    good = report.ParseFromArray(&output[0], output.size());
+    EXPECT_TRUE(good);
+    EXPECT_EQ(1, report.config_stats_size());
+    const auto& configReport2 = report.config_stats(0);
+    EXPECT_TRUE(configReport2.has_deletion_time_sec());
 }
 
-TEST(StatsdStatsTest, TestConditionReport) {
-    // TODO: implement
+TEST(StatsdStatsTest, TestSubStats) {
+    StatsdStats stats;
+    ConfigKey key(0, "test");
+    stats.noteConfigReceived(key, 2, 3, 4, 5, true);
+
+    stats.noteMatcherMatched(key, "matcher1");
+    stats.noteMatcherMatched(key, "matcher1");
+    stats.noteMatcherMatched(key, "matcher2");
+
+    stats.noteConditionDimensionSize(key, "condition1", 250);
+    stats.noteConditionDimensionSize(key, "condition1", 240);
+
+    stats.noteMetricDimensionSize(key, "metric1", 201);
+    stats.noteMetricDimensionSize(key, "metric1", 202);
+
+    // broadcast-> 2
+    stats.noteBroadcastSent(key);
+    stats.noteBroadcastSent(key);
+
+    // data drop -> 1
+    stats.noteDataDropped(key);
+
+    // dump report -> 3
+    stats.noteMetricsReportSent(key);
+    stats.noteMetricsReportSent(key);
+    stats.noteMetricsReportSent(key);
+
+    vector<uint8_t> output;
+    stats.dumpStats(&output, true);  // Dump and reset stats
+    StatsdStatsReport report;
+    bool good = report.ParseFromArray(&output[0], output.size());
+    EXPECT_TRUE(good);
+    EXPECT_EQ(1, report.config_stats_size());
+    const auto& configReport = report.config_stats(0);
+    EXPECT_EQ(2, configReport.broadcast_sent_time_sec_size());
+    EXPECT_EQ(1, configReport.data_drop_time_sec_size());
+    EXPECT_EQ(3, configReport.dump_report_time_sec_size());
+
+    EXPECT_EQ(2, configReport.matcher_stats_size());
+
+    // matcher1 is the first in the list
+    if (!configReport.matcher_stats(0).name().compare("matcher1")) {
+        EXPECT_EQ(2, configReport.matcher_stats(0).matched_times());
+        EXPECT_EQ(1, configReport.matcher_stats(1).matched_times());
+        EXPECT_EQ("matcher2", configReport.matcher_stats(1).name());
+    } else {
+        // matcher1 is the second in the list.
+        EXPECT_EQ(1, configReport.matcher_stats(0).matched_times());
+        EXPECT_EQ("matcher2", configReport.matcher_stats(0).name());
+
+        EXPECT_EQ(2, configReport.matcher_stats(1).matched_times());
+        EXPECT_EQ("matcher1", configReport.matcher_stats(1).name());
+    }
+
+    EXPECT_EQ(1, configReport.condition_stats_size());
+    EXPECT_EQ("condition1", configReport.condition_stats(0).name());
+    EXPECT_EQ(250, configReport.condition_stats(0).max_tuple_counts());
+
+    EXPECT_EQ(1, configReport.metric_stats_size());
+    EXPECT_EQ("metric1", configReport.metric_stats(0).name());
+    EXPECT_EQ(202, configReport.metric_stats(0).max_tuple_counts());
+
+    // after resetting the stats, some new events come
+    stats.noteMatcherMatched(key, "matcher99");
+    stats.noteConditionDimensionSize(key, "condition99", 300);
+    stats.noteMetricDimensionSize(key, "metric99", 270);
+
+    // now the config stats should only contain the stats about the new event.
+    stats.dumpStats(&output, false);
+    good = report.ParseFromArray(&output[0], output.size());
+    EXPECT_TRUE(good);
+    EXPECT_EQ(1, report.config_stats_size());
+    const auto& configReport2 = report.config_stats(0);
+    EXPECT_EQ(1, configReport2.matcher_stats_size());
+    EXPECT_EQ("matcher99", configReport2.matcher_stats(0).name());
+    EXPECT_EQ(1, configReport2.matcher_stats(0).matched_times());
+
+    EXPECT_EQ(1, configReport2.condition_stats_size());
+    EXPECT_EQ("condition99", configReport2.condition_stats(0).name());
+    EXPECT_EQ(300, configReport2.condition_stats(0).max_tuple_counts());
+
+    EXPECT_EQ(1, configReport2.metric_stats_size());
+    EXPECT_EQ("metric99", configReport2.metric_stats(0).name());
+    EXPECT_EQ(270, configReport2.metric_stats(0).max_tuple_counts());
 }
 
 TEST(StatsdStatsTest, TestAtomLog) {
-    // TODO: implement
+    StatsdStats stats;
+    time_t now = time(nullptr);
+    // old event, we get it from the stats buffer. should be ignored.
+    stats.noteAtomLogged(android::util::SENSOR_STATE_CHANGED, 1000);
+
+    stats.noteAtomLogged(android::util::SENSOR_STATE_CHANGED, now + 1);
+    stats.noteAtomLogged(android::util::SENSOR_STATE_CHANGED, now + 2);
+    stats.noteAtomLogged(android::util::DROPBOX_ERROR_CHANGED, now + 3);
+    // pulled event, should ignore
+    stats.noteAtomLogged(android::util::WIFI_BYTES_TRANSFERRED, now + 4);
+
+    vector<uint8_t> output;
+    stats.dumpStats(&output, false);
+    StatsdStatsReport report;
+    bool good = report.ParseFromArray(&output[0], output.size());
+    EXPECT_TRUE(good);
+
+    EXPECT_EQ(2, report.atom_stats_size());
+    bool sensorAtomGood = false;
+    bool dropboxAtomGood = false;
+
+    for (const auto& atomStats : report.atom_stats()) {
+        if (atomStats.tag() == android::util::SENSOR_STATE_CHANGED && atomStats.count() == 2) {
+            sensorAtomGood = true;
+        }
+        if (atomStats.tag() == android::util::DROPBOX_ERROR_CHANGED && atomStats.count() == 1) {
+            dropboxAtomGood = true;
+        }
+    }
+
+    EXPECT_TRUE(dropboxAtomGood);
+    EXPECT_TRUE(sensorAtomGood);
 }
 
 }  // namespace statsd