Merge "Adds rate limit to checking byte size."

cmds/statsd/Android.mk

@@ -161,6 +161,7 @@ LOCAL_SRC_FILES := \
     tests/LogEntryMatcher_test.cpp \
     tests/LogReader_test.cpp \
     tests/MetricsManager_test.cpp \
+    tests/StatsLogProcessor_test.cpp \
     tests/UidMap_test.cpp \
     tests/condition/CombinationConditionTracker_test.cpp \
     tests/condition/SimpleConditionTracker_test.cpp \

cmds/statsd/src/StatsLogProcessor.cpp

@@ -86,9 +86,7 @@ void StatsLogProcessor::OnLogEvent(const LogEvent& msg) {
     // pass the event to metrics managers.
     for (auto& pair : mMetricsManagers) {
         pair.second->onLogEvent(msg);
-        // TODO: THIS CHECK FAILS BECAUSE ONCE UIDMAP SIZE EXCEEDS LIMIT, DROPPING METRICS DATA
-        // DOESN'T HELP. FIX THIS.
-        // flushIfNecessary(msg.GetTimestampNs(), pair.first, pair.second);
+        flushIfNecessary(msg.GetTimestampNs(), pair.first, *(pair.second));
     }
 
     // Hard-coded logic to update the isolated uid's in the uid-map.
@@ -217,23 +215,32 @@ void StatsLogProcessor::OnConfigRemoved(const ConfigKey& key) {
     mLastBroadcastTimes.erase(key);
 }
 
-void StatsLogProcessor::flushIfNecessary(uint64_t timestampNs,
-                                         const ConfigKey& key,
-                                         const unique_ptr<MetricsManager>& metricsManager) {
+void StatsLogProcessor::flushIfNecessary(uint64_t timestampNs, const ConfigKey& key,
+                                         MetricsManager& metricsManager) {
     std::lock_guard<std::mutex> lock(mBroadcastTimesMutex);
 
-    size_t totalBytes = metricsManager->byteSize() + mUidMap->getBytesUsed();
-    // TODO: Find a way to test that the dropping and broadcasts are sent when memory is exceeded.
-    if (totalBytes > kMaxSerializedBytes) {  // Too late. We need to start clearing data.
+    auto lastCheckTime = mLastByteSizeTimes.find(key);
+    if (lastCheckTime != mLastByteSizeTimes.end()) {
+        if (timestampNs - lastCheckTime->second < StatsdStats::kMinByteSizeCheckPeriodNs) {
+            return;
+        }
+    }
+
+    // We suspect that the byteSize() computation is expensive, so we set a rate limit.
+    size_t totalBytes = metricsManager.byteSize();
+    mLastByteSizeTimes[key] = timestampNs;
+    if (totalBytes >
+        StatsdStats::kMaxMetricsBytesPerConfig) {  // 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();
+        metricsManager.onDumpReport();
         StatsdStats::getInstance().noteDataDropped(key);
         VLOG("StatsD had to toss out metrics for %s", key.ToString().c_str());
-    } else if (totalBytes >
-               .9 * kMaxSerializedBytes) {  // Send broadcast so that receivers can pull data.
-        auto lastFlushNs = mLastBroadcastTimes.find(key);
-        if (lastFlushNs != mLastBroadcastTimes.end()) {
-            if (timestampNs - lastFlushNs->second < kMinBroadcastPeriod) {
+    } else if (totalBytes > .9 * StatsdStats::kMaxMetricsBytesPerConfig) {
+        // Send broadcast so that receivers can pull data.
+        auto lastBroadcastTime = mLastBroadcastTimes.find(key);
+        if (lastBroadcastTime != mLastBroadcastTimes.end()) {
+            if (timestampNs - lastBroadcastTime->second < StatsdStats::kMinBroadcastPeriodNs) {
+                VLOG("StatsD would've sent a broadcast but the rate limit stopped us.");
                 return;
             }
         }

cmds/statsd/src/StatsLogProcessor.h

@@ -60,29 +60,25 @@ private:
 
     std::unordered_map<ConfigKey, long> mLastBroadcastTimes;
 
+    // Tracks when we last checked the bytes consumed for each config key.
+    std::unordered_map<ConfigKey, long> mLastByteSizeTimes;
+
     sp<UidMap> mUidMap;  // Reference to the UidMap to lookup app name and version for each uid.
 
     sp<AnomalyMonitor> mAnomalyMonitor;
 
-    /* Max *serialized* size of the logs kept in memory before flushing through binder call.
-       Proto lite does not implement the SpaceUsed() function which gives the in memory byte size.
-       So we cap memory usage by limiting the serialized size. Note that protobuf's in memory size
-       is higher than its serialized size.
-     */
-    static const size_t kMaxSerializedBytes = 16 * 1024;
-
     /* Check if we should send a broadcast if approaching memory limits and if we're over, we
      * actually delete the data. */
-    void flushIfNecessary(uint64_t timestampNs,
-                          const ConfigKey& key,
-                          const unique_ptr<MetricsManager>& metricsManager);
+    void flushIfNecessary(uint64_t timestampNs, const ConfigKey& key,
+                          MetricsManager& metricsManager);
 
     // Function used to send a broadcast so that receiver for the config key can call getData
     // to retrieve the stored data.
     std::function<void(const ConfigKey& key)> mSendBroadcast;
 
-    /* Minimum period between two broadcasts in nanoseconds. Currently set to 60 seconds. */
-    static const unsigned long long kMinBroadcastPeriod = 60 * NS_PER_SEC;
+    FRIEND_TEST(StatsLogProcessorTest, TestRateLimitByteSize);
+    FRIEND_TEST(StatsLogProcessorTest, TestRateLimitBroadcast);
+    FRIEND_TEST(StatsLogProcessorTest, TestDropWhenByteSizeTooLarge);
 };
 
 }  // namespace statsd

cmds/statsd/src/guardrail/StatsdStats.h

@@ -19,6 +19,7 @@
 #include "frameworks/base/cmds/statsd/src/stats_log.pb.h"
 
 #include <gtest/gtest_prod.h>
+#include <log/log_time.h>
 #include <mutex>
 #include <string>
 #include <vector>
@@ -45,10 +46,20 @@ public:
 
     const static int kMaxTimestampCount = 20;
 
+    // Max memory allowed for storing metrics per configuration. When this limit is approached,
+    // statsd will send a broadcast so that the client can fetch the data and clear this memory.
+    static const size_t kMaxMetricsBytesPerConfig = 128 * 1024;
+
     // Cap the UID map's memory usage to this. This should be fairly high since the UID information
     // is critical for understanding the metrics.
     const static size_t kMaxBytesUsedUidMap = 50 * 1024;
 
+    /* Minimum period between two broadcasts in nanoseconds. */
+    static const unsigned long long kMinBroadcastPeriodNs = 60 * NS_PER_SEC;
+
+    /* Min period between two checks of byte size per config key in nanoseconds. */
+    static const unsigned long long kMinByteSizeCheckPeriodNs = 10 * NS_PER_SEC;
+
     /**
      * Report a new config has been received and report the static stats about the config.
      *

cmds/statsd/src/metrics/MetricsManager.h

@@ -36,7 +36,7 @@ class MetricsManager {
 public:
     MetricsManager(const ConfigKey& configKey, const StatsdConfig& config);
 
-    ~MetricsManager();
+    virtual ~MetricsManager();
 
     // Return whether the configuration is valid.
     bool isConfigValid() const;
@@ -52,11 +52,11 @@ public:
     void setAnomalyMonitor(const sp<AnomalyMonitor>& anomalyMonitor);
 
     // Config source owner can call onDumpReport() to get all the metrics collected.
-    std::vector<std::unique_ptr<std::vector<uint8_t>>> onDumpReport();
+    virtual std::vector<std::unique_ptr<std::vector<uint8_t>>> onDumpReport();
 
     // Computes the total byte size of all metrics managed by a single config source.
     // Does not change the state.
-    size_t byteSize();
+    virtual size_t byteSize();
 
 private:
     const ConfigKey mConfigKey;

cmds/statsd/tests/StatsLogProcessor_test.cpp

@@ -0,0 +1,117 @@
+// Copyright (C) 2017 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "StatsLogProcessor.h"
+#include "config/ConfigKey.h"
+#include "frameworks/base/cmds/statsd/src/statsd_config.pb.h"
+#include "guardrail/StatsdStats.h"
+#include "logd/LogEvent.h"
+#include "packages/UidMap.h"
+#include "statslog.h"
+
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+#include <stdio.h>
+
+using namespace android;
+using namespace testing;
+
+namespace android {
+namespace os {
+namespace statsd {
+
+#ifdef __ANDROID__
+
+/**
+ * Mock MetricsManager (ByteSize() is called).
+ */
+class MockMetricsManager : public MetricsManager {
+public:
+    MockMetricsManager() : MetricsManager(ConfigKey(1, "key"), StatsdConfig()) {
+    }
+
+    MOCK_METHOD0(byteSize, size_t());
+    MOCK_METHOD0(onDumpReport, std::vector<std::unique_ptr<std::vector<uint8_t>>>());
+};
+
+TEST(StatsLogProcessorTest, TestRateLimitByteSize) {
+    sp<UidMap> m = new UidMap();
+    sp<AnomalyMonitor> anomalyMonitor;
+    // Construct the processor with a dummy sendBroadcast function that does nothing.
+    StatsLogProcessor p(m, anomalyMonitor, [](const ConfigKey& key) {});
+
+    MockMetricsManager mockMetricsManager;
+
+    ConfigKey key(100, "key");
+    // Expect only the first flush to trigger a check for byte size since the last two are
+    // rate-limited.
+    EXPECT_CALL(mockMetricsManager, byteSize()).Times(1);
+    p.flushIfNecessary(99, key, mockMetricsManager);
+    p.flushIfNecessary(100, key, mockMetricsManager);
+    p.flushIfNecessary(101, key, mockMetricsManager);
+}
+
+TEST(StatsLogProcessorTest, TestRateLimitBroadcast) {
+    sp<UidMap> m = new UidMap();
+    sp<AnomalyMonitor> anomalyMonitor;
+    int broadcastCount = 0;
+    StatsLogProcessor p(m, anomalyMonitor,
+                        [&broadcastCount](const ConfigKey& key) { broadcastCount++; });
+
+    MockMetricsManager mockMetricsManager;
+
+    ConfigKey key(100, "key");
+    EXPECT_CALL(mockMetricsManager, byteSize())
+            .Times(2)
+            .WillRepeatedly(Return(int(StatsdStats::kMaxMetricsBytesPerConfig * .95)));
+
+    // Expect only one broadcast despite always returning a size that should trigger broadcast.
+    p.flushIfNecessary(1, key, mockMetricsManager);
+    EXPECT_EQ(1, broadcastCount);
+
+    // This next call to flush should not trigger a broadcast.
+    p.mLastByteSizeTimes.clear();  // Force another check for byte size.
+    p.flushIfNecessary(2, key, mockMetricsManager);
+    EXPECT_EQ(1, broadcastCount);
+}
+
+TEST(StatsLogProcessorTest, TestDropWhenByteSizeTooLarge) {
+    sp<UidMap> m = new UidMap();
+    sp<AnomalyMonitor> anomalyMonitor;
+    int broadcastCount = 0;
+    StatsLogProcessor p(m, anomalyMonitor,
+                        [&broadcastCount](const ConfigKey& key) { broadcastCount++; });
+
+    MockMetricsManager mockMetricsManager;
+
+    ConfigKey key(100, "key");
+    EXPECT_CALL(mockMetricsManager, byteSize())
+            .Times(1)
+            .WillRepeatedly(Return(int(StatsdStats::kMaxMetricsBytesPerConfig * 1.2)));
+
+    EXPECT_CALL(mockMetricsManager, onDumpReport()).Times(1);
+
+    // Expect to call the onDumpReport and skip the broadcast.
+    p.flushIfNecessary(1, key, mockMetricsManager);
+    EXPECT_EQ(0, broadcastCount);
+}
+
+#else
+GTEST_LOG_(INFO) << "This test does nothing.\n";
+#endif
+
+}  // namespace statsd
+}  // namespace os
+}  // namespace android