Merge "Adds rate limit to checking byte size."
diff --git a/cmds/statsd/Android.mk b/cmds/statsd/Android.mk
index 87cde03..337aeaa 100644
--- a/cmds/statsd/Android.mk
+++ b/cmds/statsd/Android.mk
@@ -161,6 +161,7 @@
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 \
diff --git a/cmds/statsd/src/StatsLogProcessor.cpp b/cmds/statsd/src/StatsLogProcessor.cpp
index c291647..2df0c90 100644
--- a/cmds/statsd/src/StatsLogProcessor.cpp
+++ b/cmds/statsd/src/StatsLogProcessor.cpp
@@ -86,9 +86,7 @@
// 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 @@
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;
}
}
diff --git a/cmds/statsd/src/StatsLogProcessor.h b/cmds/statsd/src/StatsLogProcessor.h
index e9ac015..a4df23a 100644
--- a/cmds/statsd/src/StatsLogProcessor.h
+++ b/cmds/statsd/src/StatsLogProcessor.h
@@ -60,29 +60,25 @@
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
diff --git a/cmds/statsd/src/guardrail/StatsdStats.h b/cmds/statsd/src/guardrail/StatsdStats.h
index 3940563..4cf168e 100644
--- a/cmds/statsd/src/guardrail/StatsdStats.h
+++ b/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 @@
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.
*
diff --git a/cmds/statsd/src/metrics/MetricsManager.h b/cmds/statsd/src/metrics/MetricsManager.h
index 4e6a0ce..86c4733 100644
--- a/cmds/statsd/src/metrics/MetricsManager.h
+++ b/cmds/statsd/src/metrics/MetricsManager.h
@@ -36,7 +36,7 @@
public:
MetricsManager(const ConfigKey& configKey, const StatsdConfig& config);
- ~MetricsManager();
+ virtual ~MetricsManager();
// Return whether the configuration is valid.
bool isConfigValid() const;
@@ -52,11 +52,11 @@
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;
diff --git a/cmds/statsd/tests/StatsLogProcessor_test.cpp b/cmds/statsd/tests/StatsLogProcessor_test.cpp
new file mode 100644
index 0000000..ff04d95
--- /dev/null
+++ b/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
\ No newline at end of file