diff --git a/cmds/statsd/Android.mk b/cmds/statsd/Android.mk index 87cde032ba795..337aeaaf37abd 100644 --- a/cmds/statsd/Android.mk +++ b/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 \ diff --git a/cmds/statsd/src/StatsLogProcessor.cpp b/cmds/statsd/src/StatsLogProcessor.cpp index c29164781e479..2df0c90e9baf8 100644 --- a/cmds/statsd/src/StatsLogProcessor.cpp +++ b/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) { +void StatsLogProcessor::flushIfNecessary(uint64_t timestampNs, const ConfigKey& key, + MetricsManager& metricsManager) { std::lock_guard 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 e9ac01536b423..a4df23a16c0f9 100644 --- a/cmds/statsd/src/StatsLogProcessor.h +++ b/cmds/statsd/src/StatsLogProcessor.h @@ -60,29 +60,25 @@ private: std::unordered_map mLastBroadcastTimes; + // Tracks when we last checked the bytes consumed for each config key. + std::unordered_map mLastByteSizeTimes; + sp mUidMap; // Reference to the UidMap to lookup app name and version for each uid. sp 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); + 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 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 3940563037224..4cf168eb40bf9 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 +#include #include #include #include @@ -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. * diff --git a/cmds/statsd/src/metrics/MetricsManager.h b/cmds/statsd/src/metrics/MetricsManager.h index 4e6a0ce5439b2..86c47333d7e6f 100644 --- a/cmds/statsd/src/metrics/MetricsManager.h +++ b/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); // Config source owner can call onDumpReport() to get all the metrics collected. - std::vector>> onDumpReport(); + virtual std::vector>> 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 0000000000000..ff04d955ecd0f --- /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 +#include + +#include + +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>>()); +}; + +TEST(StatsLogProcessorTest, TestRateLimitByteSize) { + sp m = new UidMap(); + sp 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 m = new UidMap(); + sp 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 m = new UidMap(); + sp 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