From d07d0ff02f62627f1831acd05f3e153b607858f7 Mon Sep 17 00:00:00 2001 From: Tej Singh Date: Tue, 27 Oct 2020 20:56:11 -0700 Subject: [PATCH 1/2] Partial config update for alerts/subscriptions Preserves alerts if the definition didn't change and the underlying metric didnt change. All alerts need to be readded to the metric b/c metrics will always clear the alerts on config updates. In the existing code, the metric producer created the AnomalyTracker (since duration metrics need DurationAnomalyTrackers). Preserved alerts already have an AnomalyTracker. Therefore, we need a second overload of addAnomalyTracker within MetricProducer to add existing ones. Note that we don't need to worry about the type of AnomlayTracker, since if the alert is preserved, both the AnomalyTracker and metric must not have changed, so they will both be the correct type. Subscriptions are also redone - alerts will clear their subscriptions, and all subscriptions will be readded. For duration metric, we now need to propagate adding anomaly trackers to the DurationTrackers since it's now possible to add anomaly trackers after the durationTrackers have been created. Test: atest statsd_test Bug: 162323547 Change-Id: Id3afa04b252f3a3523114407a389be25b9171a97 --- cmds/statsd/src/anomaly/AnomalyTracker.cpp | 21 +- cmds/statsd/src/anomaly/AnomalyTracker.h | 31 ++- .../src/anomaly/DurationAnomalyTracker.h | 6 +- .../src/metrics/DurationMetricProducer.cpp | 22 +- .../src/metrics/DurationMetricProducer.h | 8 +- cmds/statsd/src/metrics/MetricProducer.cpp | 1 + cmds/statsd/src/metrics/MetricProducer.h | 17 +- cmds/statsd/src/metrics/MetricsManager.cpp | 11 +- .../metrics/duration_helper/DurationTracker.h | 12 +- .../duration_helper/MaxDurationTracker.cpp | 4 +- .../duration_helper/MaxDurationTracker.h | 12 +- .../duration_helper/OringDurationTracker.cpp | 7 +- .../duration_helper/OringDurationTracker.h | 4 +- .../parsing_utils/config_update_utils.cpp | 119 ++++++++- .../parsing_utils/config_update_utils.h | 43 ++++ .../parsing_utils/metrics_manager_util.cpp | 79 +++--- .../parsing_utils/metrics_manager_util.h | 35 +++ .../config_update_utils_test.cpp | 238 +++++++++++++++++- .../metrics_manager_util_test.cpp | 88 +++++++ 19 files changed, 662 insertions(+), 96 deletions(-) diff --git a/cmds/statsd/src/anomaly/AnomalyTracker.cpp b/cmds/statsd/src/anomaly/AnomalyTracker.cpp index 619752c7c44af..6aa410b180b81 100644 --- a/cmds/statsd/src/anomaly/AnomalyTracker.cpp +++ b/cmds/statsd/src/anomaly/AnomalyTracker.cpp @@ -37,14 +37,6 @@ namespace statsd { AnomalyTracker::AnomalyTracker(const Alert& alert, const ConfigKey& configKey) : mAlert(alert), mConfigKey(configKey), mNumOfPastBuckets(mAlert.num_buckets() - 1) { VLOG("AnomalyTracker() called"); - if (mAlert.num_buckets() <= 0) { - ALOGE("Cannot create AnomalyTracker with %lld buckets", (long long)mAlert.num_buckets()); - return; - } - if (!mAlert.has_trigger_if_sum_gt()) { - ALOGE("Cannot create AnomalyTracker without threshold"); - return; - } resetStorage(); // initialization } @@ -52,6 +44,10 @@ AnomalyTracker::~AnomalyTracker() { VLOG("~AnomalyTracker() called"); } +void AnomalyTracker::onConfigUpdated() { + mSubscriptions.clear(); +} + void AnomalyTracker::resetStorage() { VLOG("resetStorage() called."); mPastBuckets.clear(); @@ -259,6 +255,15 @@ bool AnomalyTracker::isInRefractoryPeriod(const int64_t& timestampNs, return false; } +std::pair AnomalyTracker::getProtoHash() const { + string serializedAlert; + if (!mAlert.SerializeToString(&serializedAlert)) { + ALOGW("Unable to serialize alert %lld", (long long)mAlert.id()); + return {false, 0}; + } + return {true, Hash64(serializedAlert)}; +} + void AnomalyTracker::informSubscribers(const MetricDimensionKey& key, int64_t metric_id, int64_t metricValue) { triggerSubscribers(mAlert.id(), metric_id, key, metricValue, mConfigKey, mSubscriptions); diff --git a/cmds/statsd/src/anomaly/AnomalyTracker.h b/cmds/statsd/src/anomaly/AnomalyTracker.h index bf36a3bc89901..9a578ee0696dd 100644 --- a/cmds/statsd/src/anomaly/AnomalyTracker.h +++ b/cmds/statsd/src/anomaly/AnomalyTracker.h @@ -16,15 +16,15 @@ #pragma once -#include - #include +#include #include #include "AlarmMonitor.h" #include "config/ConfigKey.h" -#include "frameworks/base/cmds/statsd/src/statsd_config.pb.h" // Alert +#include "frameworks/base/cmds/statsd/src/statsd_config.pb.h" // Alert #include "frameworks/base/cmds/statsd/src/statsd_metadata.pb.h" // AlertMetadata +#include "hash.h" #include "stats_util.h" // HashableDimensionKey and DimToValMap namespace android { @@ -41,6 +41,9 @@ public: virtual ~AnomalyTracker(); + // Reset appropriate state on a config update. Clear subscriptions so they can be reset. + void onConfigUpdated(); + // Add subscriptions that depend on this alert. void addSubscription(const Subscription& subscription) { mSubscriptions.push_back(subscription); @@ -106,6 +109,26 @@ public: return mNumOfPastBuckets; } + std::pair getProtoHash() const; + + // Sets an alarm for the given timestamp. + // Replaces previous alarm if one already exists. + virtual void startAlarm(const MetricDimensionKey& dimensionKey, const int64_t& eventTime) { + return; // The base AnomalyTracker class doesn't have alarms. + } + + // Stops the alarm. + // If it should have already fired, but hasn't yet (e.g. because the AlarmManager is delayed), + // declare the anomaly now. + virtual void stopAlarm(const MetricDimensionKey& dimensionKey, const int64_t& timestampNs) { + return; // The base AnomalyTracker class doesn't have alarms. + } + + // Stop all the alarms owned by this tracker. Does not declare any anomalies. + virtual void cancelAllAlarms() { + return; // The base AnomalyTracker class doesn't have alarms. + } + // Declares an anomaly for each alarm in firedAlarms that belongs to this AnomalyTracker, // and removes it from firedAlarms. Does NOT remove the alarm from the AlarmMonitor. virtual void informAlarmsFired(const int64_t& timestampNs, @@ -197,6 +220,8 @@ protected: FRIEND_TEST(AnomalyDetectionE2eTest, TestDurationMetric_SUM_single_bucket); FRIEND_TEST(AnomalyDetectionE2eTest, TestDurationMetric_SUM_multiple_buckets); FRIEND_TEST(AnomalyDetectionE2eTest, TestDurationMetric_SUM_long_refractory_period); + + FRIEND_TEST(ConfigUpdateTest, TestUpdateAlerts); }; } // namespace statsd diff --git a/cmds/statsd/src/anomaly/DurationAnomalyTracker.h b/cmds/statsd/src/anomaly/DurationAnomalyTracker.h index 686d8f95c7f63..46419149580bb 100644 --- a/cmds/statsd/src/anomaly/DurationAnomalyTracker.h +++ b/cmds/statsd/src/anomaly/DurationAnomalyTracker.h @@ -34,15 +34,15 @@ public: // Sets an alarm for the given timestamp. // Replaces previous alarm if one already exists. - void startAlarm(const MetricDimensionKey& dimensionKey, const int64_t& eventTime); + void startAlarm(const MetricDimensionKey& dimensionKey, const int64_t& eventTime) override; // Stops the alarm. // If it should have already fired, but hasn't yet (e.g. because the AlarmManager is delayed), // declare the anomaly now. - void stopAlarm(const MetricDimensionKey& dimensionKey, const int64_t& timestampNs); + void stopAlarm(const MetricDimensionKey& dimensionKey, const int64_t& timestampNs) override; // Stop all the alarms owned by this tracker. Does not declare any anomalies. - void cancelAllAlarms(); + void cancelAllAlarms() override; // Declares an anomaly for each alarm in firedAlarms that belongs to this DurationAnomalyTracker // and removes it from firedAlarms. The AlarmMonitor is not informed. diff --git a/cmds/statsd/src/metrics/DurationMetricProducer.cpp b/cmds/statsd/src/metrics/DurationMetricProducer.cpp index b2c0b32bf5ef1..8869241ab8aa3 100644 --- a/cmds/statsd/src/metrics/DurationMetricProducer.cpp +++ b/cmds/statsd/src/metrics/DurationMetricProducer.cpp @@ -234,14 +234,26 @@ sp DurationMetricProducer::addAnomalyTracker( return nullptr; } } - sp anomalyTracker = - new DurationAnomalyTracker(alert, mConfigKey, anomalyAlarmMonitor); - if (anomalyTracker != nullptr) { - mAnomalyTrackers.push_back(anomalyTracker); - } + sp anomalyTracker = + new DurationAnomalyTracker(alert, mConfigKey, anomalyAlarmMonitor); + addAnomalyTrackerLocked(anomalyTracker); return anomalyTracker; } +// Adds an AnomalyTracker that has already been created. +// Note: this gets called on config updates, and will only get called if the metric and the +// associated alert are preserved, which means the AnomalyTracker must be a DurationAnomalyTracker. +void DurationMetricProducer::addAnomalyTracker(sp& anomalyTracker) { + std::lock_guard lock(mMutex); + addAnomalyTrackerLocked(anomalyTracker); +} + +void DurationMetricProducer::addAnomalyTrackerLocked(sp& anomalyTracker) { + mAnomalyTrackers.push_back(anomalyTracker); + for (const auto& [_, durationTracker] : mCurrentSlicedDurationTrackerMap) { + durationTracker->addAnomalyTracker(anomalyTracker); + } +} void DurationMetricProducer::onStateChanged(const int64_t eventTimeNs, const int32_t atomId, const HashableDimensionKey& primaryKey, const FieldValue& oldState, diff --git a/cmds/statsd/src/metrics/DurationMetricProducer.h b/cmds/statsd/src/metrics/DurationMetricProducer.h index 01198a9271d3f..5feb09fc1c98a 100644 --- a/cmds/statsd/src/metrics/DurationMetricProducer.h +++ b/cmds/statsd/src/metrics/DurationMetricProducer.h @@ -55,6 +55,8 @@ public: sp addAnomalyTracker(const Alert &alert, const sp& anomalyAlarmMonitor) override; + void addAnomalyTracker(sp& anomalyTracker) override; + void onStateChanged(const int64_t eventTimeNs, const int32_t atomId, const HashableDimensionKey& primaryKey, const FieldValue& oldState, const FieldValue& newState) override; @@ -128,6 +130,8 @@ private: std::unordered_map>& deactivationAtomTrackerToMetricMap, std::vector& metricsWithActivation) override; + void addAnomalyTrackerLocked(sp& anomalyTracker); + const DurationMetric_AggregationType mAggregationType; // Index of the SimpleAtomMatcher which defines the start. @@ -164,9 +168,6 @@ private: std::unique_ptr createDurationTracker( const MetricDimensionKey& eventKey) const; - // This hides the base class's std::vector> mAnomalyTrackers - std::vector> mAnomalyTrackers; - // Util function to check whether the specified dimension hits the guardrail. bool hitGuardRailLocked(const MetricDimensionKey& newKey); @@ -185,6 +186,7 @@ private: FRIEND_TEST(DurationMetricProducerTest_PartialBucket, TestMaxDurationWithSplitInNextBucket); FRIEND_TEST(ConfigUpdateTest, TestUpdateDurationMetrics); + FRIEND_TEST(ConfigUpdateTest, TestUpdateAlerts); }; } // namespace statsd diff --git a/cmds/statsd/src/metrics/MetricProducer.cpp b/cmds/statsd/src/metrics/MetricProducer.cpp index 95a7d40ea9a93..5b321a0e3d603 100644 --- a/cmds/statsd/src/metrics/MetricProducer.cpp +++ b/cmds/statsd/src/metrics/MetricProducer.cpp @@ -101,6 +101,7 @@ bool MetricProducer::onConfigUpdatedLocked( } mEventActivationMap = newEventActivationMap; mEventDeactivationMap = newEventDeactivationMap; + mAnomalyTrackers.clear(); return true; } diff --git a/cmds/statsd/src/metrics/MetricProducer.h b/cmds/statsd/src/metrics/MetricProducer.h index 92c1a6e626408..0dc8edae80562 100644 --- a/cmds/statsd/src/metrics/MetricProducer.h +++ b/cmds/statsd/src/metrics/MetricProducer.h @@ -155,6 +155,7 @@ public: // Update appropriate state on config updates. Primarily, all indices need to be updated. // This metric and all of its dependencies are guaranteed to be preserved across the update. // This function also updates several maps used by metricsManager. + // This function clears all anomaly trackers. All anomaly trackers need to be added again. bool onConfigUpdated( const StatsdConfig& config, const int configIndex, const int metricIndex, const std::vector>& allAtomMatchingTrackers, @@ -237,9 +238,6 @@ public: dumpLatency, str_set, protoOutput); } - // Update appropriate state on config updates. Primarily, all indices need to be updated. - // This metric and all of its dependencies are guaranteed to be preserved across the update. - // This function also updates several maps used by metricsManager. virtual bool onConfigUpdatedLocked( const StatsdConfig& config, const int configIndex, const int metricIndex, const std::vector>& allAtomMatchingTrackers, @@ -338,16 +336,20 @@ public: return mSlicedStateAtoms; } - /* If alert is valid, adds an AnomalyTracker and returns it. If invalid, returns nullptr. */ + /* Adds an AnomalyTracker and returns it. */ virtual sp addAnomalyTracker(const Alert &alert, const sp& anomalyAlarmMonitor) { std::lock_guard lock(mMutex); sp anomalyTracker = new AnomalyTracker(alert, mConfigKey); - if (anomalyTracker != nullptr) { - mAnomalyTrackers.push_back(anomalyTracker); - } + mAnomalyTrackers.push_back(anomalyTracker); return anomalyTracker; } + + /* Adds an AnomalyTracker that has already been created */ + virtual void addAnomalyTracker(sp& anomalyTracker) { + std::lock_guard lock(mMutex); + mAnomalyTrackers.push_back(anomalyTracker); + } // End: getters/setters protected: /** @@ -571,6 +573,7 @@ protected: FRIEND_TEST(ConfigUpdateTest, TestUpdateGaugeMetrics); FRIEND_TEST(ConfigUpdateTest, TestUpdateDurationMetrics); FRIEND_TEST(ConfigUpdateTest, TestUpdateMetricsMultipleTypes); + FRIEND_TEST(ConfigUpdateTest, TestUpdateAlerts); }; } // namespace statsd diff --git a/cmds/statsd/src/metrics/MetricsManager.cpp b/cmds/statsd/src/metrics/MetricsManager.cpp index ab0d286d6b293..b1d4397037097 100644 --- a/cmds/statsd/src/metrics/MetricsManager.cpp +++ b/cmds/statsd/src/metrics/MetricsManager.cpp @@ -209,6 +209,8 @@ bool MetricsManager::updateConfig(const StatsdConfig& config, const int64_t time map newStateProtoHashes; vector> newMetricProducers; unordered_map newMetricProducerMap; + vector> newAnomalyTrackers; + unordered_map newAlertTrackerMap; mTagIds.clear(); mConditionToMetricMap.clear(); mTrackerToMetricMap.clear(); @@ -221,9 +223,10 @@ bool MetricsManager::updateConfig(const StatsdConfig& config, const int64_t time mConfigKey, config, mUidMap, mPullerManager, anomalyAlarmMonitor, periodicAlarmMonitor, timeBaseNs, currentTimeNs, mAllAtomMatchingTrackers, mAtomMatchingTrackerMap, mAllConditionTrackers, mConditionTrackerMap, mAllMetricProducers, mMetricProducerMap, - mStateProtoHashes, mTagIds, newAtomMatchingTrackers, newAtomMatchingTrackerMap, - newConditionTrackers, newConditionTrackerMap, newMetricProducers, newMetricProducerMap, - mConditionToMetricMap, mTrackerToMetricMap, mTrackerToConditionMap, + mAllAnomalyTrackers, mAlertTrackerMap, mStateProtoHashes, mTagIds, + newAtomMatchingTrackers, newAtomMatchingTrackerMap, newConditionTrackers, + newConditionTrackerMap, newMetricProducers, newMetricProducerMap, newAnomalyTrackers, + newAlertTrackerMap, mConditionToMetricMap, mTrackerToMetricMap, mTrackerToConditionMap, mActivationAtomTrackerToMetricMap, mDeactivationAtomTrackerToMetricMap, mMetricIndexesWithActivation, newStateProtoHashes, mNoReportMetricIds); mAllAtomMatchingTrackers = newAtomMatchingTrackers; @@ -233,6 +236,8 @@ bool MetricsManager::updateConfig(const StatsdConfig& config, const int64_t time mAllMetricProducers = newMetricProducers; mMetricProducerMap = newMetricProducerMap; mStateProtoHashes = newStateProtoHashes; + mAllAnomalyTrackers = newAnomalyTrackers; + mAlertTrackerMap = newAlertTrackerMap; return mConfigValid; } diff --git a/cmds/statsd/src/metrics/duration_helper/DurationTracker.h b/cmds/statsd/src/metrics/duration_helper/DurationTracker.h index 657b2e4c3ddf2..cf1f437c41683 100644 --- a/cmds/statsd/src/metrics/duration_helper/DurationTracker.h +++ b/cmds/statsd/src/metrics/duration_helper/DurationTracker.h @@ -71,7 +71,7 @@ public: sp wizard, int conditionIndex, bool nesting, int64_t currentBucketStartNs, int64_t currentBucketNum, int64_t startTimeNs, int64_t bucketSizeNs, bool conditionSliced, bool fullLink, - const std::vector>& anomalyTrackers) + const std::vector>& anomalyTrackers) : mConfigKey(key), mTrackerId(id), mEventKey(eventKey), @@ -93,6 +93,7 @@ public: sp tmpWizard = mWizard; mWizard = wizard; mConditionTrackerIndex = conditionTrackerIndex; + mAnomalyTrackers.clear(); }; virtual void noteStart(const HashableDimensionKey& key, bool condition, const int64_t eventTime, @@ -120,7 +121,7 @@ public: std::unordered_map>* output) = 0; // Predict the anomaly timestamp given the current status. - virtual int64_t predictAnomalyTimestampNs(const DurationAnomalyTracker& anomalyTracker, + virtual int64_t predictAnomalyTimestampNs(const AnomalyTracker& anomalyTracker, const int64_t currentTimestamp) const = 0; // Dump internal states for debugging virtual void dumpStates(FILE* out, bool verbose) const = 0; @@ -132,6 +133,10 @@ public: // Replace old value with new value for the given state atom. virtual void updateCurrentStateKey(const int32_t atomId, const FieldValue& newState) = 0; + void addAnomalyTracker(sp& anomalyTracker) { + mAnomalyTrackers.push_back(anomalyTracker); + } + protected: int64_t getCurrentBucketEndTimeNs() const { return mStartTimeNs + (mCurrentBucketNum + 1) * mBucketSizeNs; @@ -218,13 +223,14 @@ protected: bool mHasLinksToAllConditionDimensionsInTracker; - std::vector> mAnomalyTrackers; + std::vector> mAnomalyTrackers; FRIEND_TEST(OringDurationTrackerTest, TestPredictAnomalyTimestamp); FRIEND_TEST(OringDurationTrackerTest, TestAnomalyDetectionExpiredAlarm); FRIEND_TEST(OringDurationTrackerTest, TestAnomalyDetectionFiredAlarm); FRIEND_TEST(ConfigUpdateTest, TestUpdateDurationMetrics); + FRIEND_TEST(ConfigUpdateTest, TestUpdateAlerts); }; } // namespace statsd diff --git a/cmds/statsd/src/metrics/duration_helper/MaxDurationTracker.cpp b/cmds/statsd/src/metrics/duration_helper/MaxDurationTracker.cpp index ee4e1672411f6..62f49824b8741 100644 --- a/cmds/statsd/src/metrics/duration_helper/MaxDurationTracker.cpp +++ b/cmds/statsd/src/metrics/duration_helper/MaxDurationTracker.cpp @@ -30,7 +30,7 @@ MaxDurationTracker::MaxDurationTracker(const ConfigKey& key, const int64_t& id, int64_t currentBucketStartNs, int64_t currentBucketNum, int64_t startTimeNs, int64_t bucketSizeNs, bool conditionSliced, bool fullLink, - const vector>& anomalyTrackers) + const vector>& anomalyTrackers) : DurationTracker(key, id, eventKey, wizard, conditionIndex, nesting, currentBucketStartNs, currentBucketNum, startTimeNs, bucketSizeNs, conditionSliced, fullLink, anomalyTrackers) { @@ -288,7 +288,7 @@ void MaxDurationTracker::noteConditionChanged(const HashableDimensionKey& key, b // Note that we don't update mDuration here since it's only updated during noteStop. } -int64_t MaxDurationTracker::predictAnomalyTimestampNs(const DurationAnomalyTracker& anomalyTracker, +int64_t MaxDurationTracker::predictAnomalyTimestampNs(const AnomalyTracker& anomalyTracker, const int64_t currentTimestamp) const { // The allowed time we can continue in the current state is the // (anomaly threshold) - max(elapsed time of the started mInfos). diff --git a/cmds/statsd/src/metrics/duration_helper/MaxDurationTracker.h b/cmds/statsd/src/metrics/duration_helper/MaxDurationTracker.h index 2891c6e1138af..be2707c60c1bb 100644 --- a/cmds/statsd/src/metrics/duration_helper/MaxDurationTracker.h +++ b/cmds/statsd/src/metrics/duration_helper/MaxDurationTracker.h @@ -29,12 +29,10 @@ namespace statsd { class MaxDurationTracker : public DurationTracker { public: MaxDurationTracker(const ConfigKey& key, const int64_t& id, const MetricDimensionKey& eventKey, - sp wizard, int conditionIndex, - bool nesting, - int64_t currentBucketStartNs, int64_t currentBucketNum, - int64_t startTimeNs, int64_t bucketSizeNs, bool conditionSliced, - bool fullLink, - const std::vector>& anomalyTrackers); + sp wizard, int conditionIndex, bool nesting, + int64_t currentBucketStartNs, int64_t currentBucketNum, int64_t startTimeNs, + int64_t bucketSizeNs, bool conditionSliced, bool fullLink, + const std::vector>& anomalyTrackers); MaxDurationTracker(const MaxDurationTracker& tracker) = default; @@ -57,7 +55,7 @@ public: void onStateChanged(const int64_t timestamp, const int32_t atomId, const FieldValue& newState) override; - int64_t predictAnomalyTimestampNs(const DurationAnomalyTracker& anomalyTracker, + int64_t predictAnomalyTimestampNs(const AnomalyTracker& anomalyTracker, const int64_t currentTimestamp) const override; void dumpStates(FILE* out, bool verbose) const override; diff --git a/cmds/statsd/src/metrics/duration_helper/OringDurationTracker.cpp b/cmds/statsd/src/metrics/duration_helper/OringDurationTracker.cpp index 0d49bbc269a35..247e2e01c9923 100644 --- a/cmds/statsd/src/metrics/duration_helper/OringDurationTracker.cpp +++ b/cmds/statsd/src/metrics/duration_helper/OringDurationTracker.cpp @@ -28,7 +28,7 @@ OringDurationTracker::OringDurationTracker( const ConfigKey& key, const int64_t& id, const MetricDimensionKey& eventKey, sp wizard, int conditionIndex, bool nesting, int64_t currentBucketStartNs, int64_t currentBucketNum, int64_t startTimeNs, int64_t bucketSizeNs, bool conditionSliced, - bool fullLink, const vector>& anomalyTrackers) + bool fullLink, const vector>& anomalyTrackers) : DurationTracker(key, id, eventKey, wizard, conditionIndex, nesting, currentBucketStartNs, currentBucketNum, startTimeNs, bucketSizeNs, conditionSliced, fullLink, anomalyTrackers), @@ -344,9 +344,8 @@ void OringDurationTracker::onStateChanged(const int64_t timestamp, const int32_t updateCurrentStateKey(atomId, newState); } -int64_t OringDurationTracker::predictAnomalyTimestampNs( - const DurationAnomalyTracker& anomalyTracker, const int64_t eventTimestampNs) const { - +int64_t OringDurationTracker::predictAnomalyTimestampNs(const AnomalyTracker& anomalyTracker, + const int64_t eventTimestampNs) const { // The anomaly threshold. const int64_t thresholdNs = anomalyTracker.getAnomalyThreshold(); diff --git a/cmds/statsd/src/metrics/duration_helper/OringDurationTracker.h b/cmds/statsd/src/metrics/duration_helper/OringDurationTracker.h index bd8017a7decdf..6eddee7da252c 100644 --- a/cmds/statsd/src/metrics/duration_helper/OringDurationTracker.h +++ b/cmds/statsd/src/metrics/duration_helper/OringDurationTracker.h @@ -31,7 +31,7 @@ public: int conditionIndex, bool nesting, int64_t currentBucketStartNs, int64_t currentBucketNum, int64_t startTimeNs, int64_t bucketSizeNs, bool conditionSliced, bool fullLink, - const std::vector>& anomalyTrackers); + const std::vector>& anomalyTrackers); OringDurationTracker(const OringDurationTracker& tracker) = default; @@ -54,7 +54,7 @@ public: int64_t timestampNs, std::unordered_map>* output) override; - int64_t predictAnomalyTimestampNs(const DurationAnomalyTracker& anomalyTracker, + int64_t predictAnomalyTimestampNs(const AnomalyTracker& anomalyTracker, const int64_t currentTimestamp) const override; void dumpStates(FILE* out, bool verbose) const override; diff --git a/cmds/statsd/src/metrics/parsing_utils/config_update_utils.cpp b/cmds/statsd/src/metrics/parsing_utils/config_update_utils.cpp index 335f7753e5e3b..0c4dc4476f480 100644 --- a/cmds/statsd/src/metrics/parsing_utils/config_update_utils.cpp +++ b/cmds/statsd/src/metrics/parsing_utils/config_update_utils.cpp @@ -115,7 +115,6 @@ bool updateAtomMatchingTrackers(const StatsdConfig& config, const sp& ui vector>& newAtomMatchingTrackers, set& replacedMatchers) { const int atomMatcherCount = config.atom_matcher_size(); - vector matcherProtos; matcherProtos.reserve(atomMatcherCount); newAtomMatchingTrackers.reserve(atomMatcherCount); @@ -891,6 +890,111 @@ bool updateMetrics(const ConfigKey& key, const StatsdConfig& config, const int64 return true; } +bool determineAlertUpdateStatus(const Alert& alert, + const unordered_map& oldAlertTrackerMap, + const vector>& oldAnomalyTrackers, + const set& replacedMetrics, UpdateStatus& updateStatus) { + // Check if new alert. + const auto& oldAnomalyTrackerIt = oldAlertTrackerMap.find(alert.id()); + if (oldAnomalyTrackerIt == oldAlertTrackerMap.end()) { + updateStatus = UPDATE_NEW; + return true; + } + + // This is an existing alert, check if it has changed. + string serializedAlert; + if (!alert.SerializeToString(&serializedAlert)) { + ALOGW("Unable to serialize alert %lld", (long long)alert.id()); + return false; + } + uint64_t newProtoHash = Hash64(serializedAlert); + const auto [success, oldProtoHash] = + oldAnomalyTrackers[oldAnomalyTrackerIt->second]->getProtoHash(); + if (!success) { + return false; + } + if (newProtoHash != oldProtoHash) { + updateStatus = UPDATE_REPLACE; + return true; + } + + // Check if the metric this alert relies on has changed. + if (replacedMetrics.find(alert.metric_id()) != replacedMetrics.end()) { + updateStatus = UPDATE_REPLACE; + return true; + } + + updateStatus = UPDATE_PRESERVE; + return true; +} + +bool updateAlerts(const StatsdConfig& config, const unordered_map& metricProducerMap, + const set& replacedMetrics, + const unordered_map& oldAlertTrackerMap, + const vector>& oldAnomalyTrackers, + const sp& anomalyAlarmMonitor, + vector>& allMetricProducers, + unordered_map& newAlertTrackerMap, + vector>& newAnomalyTrackers) { + int alertCount = config.alert_size(); + vector alertUpdateStatuses(alertCount); + for (int i = 0; i < alertCount; i++) { + if (!determineAlertUpdateStatus(config.alert(i), oldAlertTrackerMap, oldAnomalyTrackers, + replacedMetrics, alertUpdateStatuses[i])) { + return false; + } + } + + for (int i = 0; i < alertCount; i++) { + const Alert& alert = config.alert(i); + newAlertTrackerMap[alert.id()] = newAnomalyTrackers.size(); + switch (alertUpdateStatuses[i]) { + case UPDATE_PRESERVE: { + // Find the alert and update it. + const auto& oldAnomalyTrackerIt = oldAlertTrackerMap.find(alert.id()); + if (oldAnomalyTrackerIt == oldAlertTrackerMap.end()) { + ALOGW("Could not find AnomalyTracker %lld in the previous config, but " + "expected it to be there", + (long long)alert.id()); + return false; + } + sp anomalyTracker = oldAnomalyTrackers[oldAnomalyTrackerIt->second]; + anomalyTracker->onConfigUpdated(); + // Add the alert to the relevant metric. + const auto& metricProducerIt = metricProducerMap.find(alert.metric_id()); + if (metricProducerIt == metricProducerMap.end()) { + ALOGW("alert \"%lld\" has unknown metric id: \"%lld\"", (long long)alert.id(), + (long long)alert.metric_id()); + return false; + } + allMetricProducers[metricProducerIt->second]->addAnomalyTracker(anomalyTracker); + newAnomalyTrackers.push_back(anomalyTracker); + break; + } + case UPDATE_REPLACE: + case UPDATE_NEW: { + optional> anomalyTracker = createAnomalyTracker( + alert, anomalyAlarmMonitor, metricProducerMap, allMetricProducers); + if (!anomalyTracker) { + return false; + } + newAnomalyTrackers.push_back(anomalyTracker.value()); + break; + } + default: { + ALOGE("Alert \"%lld\" update state is unknown. This should never happen", + (long long)alert.id()); + return false; + } + } + } + if (!initSubscribersForSubscriptionType(config, Subscription::ALERT, newAlertTrackerMap, + newAnomalyTrackers)) { + return false; + } + return true; +} + bool updateStatsdConfig(const ConfigKey& key, const StatsdConfig& config, const sp& uidMap, const sp& pullerManager, const sp& anomalyAlarmMonitor, @@ -902,6 +1006,8 @@ bool updateStatsdConfig(const ConfigKey& key, const StatsdConfig& config, const const unordered_map& oldConditionTrackerMap, const vector>& oldMetricProducers, const unordered_map& oldMetricProducerMap, + const vector>& oldAnomalyTrackers, + const unordered_map& oldAlertTrackerMap, const map& oldStateProtoHashes, set& allTagIds, vector>& newAtomMatchingTrackers, unordered_map& newAtomMatchingTrackerMap, @@ -909,6 +1015,8 @@ bool updateStatsdConfig(const ConfigKey& key, const StatsdConfig& config, const unordered_map& newConditionTrackerMap, vector>& newMetricProducers, unordered_map& newMetricProducerMap, + vector>& newAnomalyTrackers, + unordered_map& newAlertTrackerMap, unordered_map>& conditionToMetricMap, unordered_map>& trackerToMetricMap, unordered_map>& trackerToConditionMap, @@ -962,7 +1070,14 @@ bool updateStatsdConfig(const ConfigKey& key, const StatsdConfig& config, const newMetricProducerMap, newMetricProducers, conditionToMetricMap, trackerToMetricMap, noReportMetricIds, activationTrackerToMetricMap, deactivationTrackerToMetricMap, metricsWithActivation, replacedMetrics)) { - ALOGE("initMetricProducers failed"); + ALOGE("updateMetrics failed"); + return false; + } + + if (!updateAlerts(config, newMetricProducerMap, replacedMetrics, oldAlertTrackerMap, + oldAnomalyTrackers, anomalyAlarmMonitor, newMetricProducers, + newAlertTrackerMap, newAnomalyTrackers)) { + ALOGE("updateAlerts failed"); return false; } diff --git a/cmds/statsd/src/metrics/parsing_utils/config_update_utils.h b/cmds/statsd/src/metrics/parsing_utils/config_update_utils.h index 3f1c5326b5694..b714f58f6c067 100644 --- a/cmds/statsd/src/metrics/parsing_utils/config_update_utils.h +++ b/cmds/statsd/src/metrics/parsing_utils/config_update_utils.h @@ -189,6 +189,45 @@ bool updateMetrics( std::unordered_map>& deactivationAtomTrackerToMetricMap, std::vector& metricsWithActivation, std::set& replacedMetrics); +// Function to determine the update status (preserve/replace/new) of an alert. +// [alert]: the input Alert +// [oldAlertTrackerMap]: alert id to index mapping in the existing MetricsManager +// [oldAnomalyTrackers]: stores the existing AnomalyTrackers +// [replacedMetrics]: set of replaced metric ids. alerts using these metrics must be replaced +// output: +// [updateStatus]: update status of the alert. Will be changed from UPDATE_UNKNOWN +// Returns whether the function was successful or not. +bool determineAlertUpdateStatus(const Alert& alert, + const std::unordered_map& oldAlertTrackerMap, + const std::vector>& oldAnomalyTrackers, + const std::set& replacedMetrics, + UpdateStatus& updateStatus); + +// Update MetricProducers. +// input: +// [config]: the input config +// [metricProducerMap]: metric id to index mapping in the new config +// [replacedMetrics]: set of metric ids that have changed and were replaced +// [oldAlertTrackerMap]: alert id to index mapping in the existing MetricsManager. +// [oldAnomalyTrackers]: stores the existing AnomalyTrackers +// [anomalyAlarmMonitor]: AlarmMonitor used for duration metric anomaly detection +// [allMetricProducers]: stores the sp of the metric producers, AnomalyTrackers need to be added. +// [stateAtomIdMap]: contains the mapping from state ids to atom ids +// [allStateGroupMaps]: contains the mapping from atom ids and state values to +// state group ids for all states +// output: +// [newAlertTrackerMap]: mapping of alert id to index in the new config +// [newAnomalyTrackers]: contains the list of sp to the AnomalyTrackers created. +bool updateAlerts(const StatsdConfig& config, + const std::unordered_map& metricProducerMap, + const std::set& replacedMetrics, + const std::unordered_map& oldAlertTrackerMap, + const std::vector>& oldAnomalyTrackers, + const sp& anomalyAlarmMonitor, + std::vector>& allMetricProducers, + std::unordered_map& newAlertTrackerMap, + std::vector>& newAnomalyTrackers); + // Updates the existing MetricsManager from a new StatsdConfig. // Parameters are the members of MetricsManager. See MetricsManager for declaration. bool updateStatsdConfig(const ConfigKey& key, const StatsdConfig& config, const sp& uidMap, @@ -202,6 +241,8 @@ bool updateStatsdConfig(const ConfigKey& key, const StatsdConfig& config, const const std::unordered_map& oldConditionTrackerMap, const std::vector>& oldMetricProducers, const std::unordered_map& oldMetricProducerMap, + const std::vector>& oldAnomalyTrackers, + const std::unordered_map& oldAlertTrackerMap, const std::map& oldStateProtoHashes, std::set& allTagIds, std::vector>& newAtomMatchingTrackers, @@ -210,6 +251,8 @@ bool updateStatsdConfig(const ConfigKey& key, const StatsdConfig& config, const std::unordered_map& newConditionTrackerMap, std::vector>& newMetricProducers, std::unordered_map& newMetricProducerMap, + std::vector>& newAlertTrackers, + std::unordered_map& newAlertTrackerMap, std::unordered_map>& conditionToMetricMap, std::unordered_map>& trackerToMetricMap, std::unordered_map>& trackerToConditionMap, diff --git a/cmds/statsd/src/metrics/parsing_utils/metrics_manager_util.cpp b/cmds/statsd/src/metrics/parsing_utils/metrics_manager_util.cpp index 8fc039a7d6b37..f15c8b0f33462 100644 --- a/cmds/statsd/src/metrics/parsing_utils/metrics_manager_util.cpp +++ b/cmds/statsd/src/metrics/parsing_utils/metrics_manager_util.cpp @@ -814,6 +814,35 @@ optional> createGaugeMetricProducerAndUpdateMetadata( pullerManager, eventActivationMap, eventDeactivationMap)}; } +optional> createAnomalyTracker( + const Alert& alert, const sp& anomalyAlarmMonitor, + const unordered_map& metricProducerMap, + vector>& allMetricProducers) { + const auto& itr = metricProducerMap.find(alert.metric_id()); + if (itr == metricProducerMap.end()) { + ALOGW("alert \"%lld\" has unknown metric id: \"%lld\"", (long long)alert.id(), + (long long)alert.metric_id()); + return nullopt; + } + if (!alert.has_trigger_if_sum_gt()) { + ALOGW("invalid alert: missing threshold"); + return nullopt; + } + if (alert.trigger_if_sum_gt() < 0 || alert.num_buckets() <= 0) { + ALOGW("invalid alert: threshold=%f num_buckets= %d", alert.trigger_if_sum_gt(), + alert.num_buckets()); + return nullopt; + } + const int metricIndex = itr->second; + sp metric = allMetricProducers[metricIndex]; + sp anomalyTracker = metric->addAnomalyTracker(alert, anomalyAlarmMonitor); + if (anomalyTracker == nullptr) { + // The ALOGW for this invalid alert was already displayed in addAnomalyTracker(). + return nullopt; + } + return {anomalyTracker}; +} + bool initAtomMatchingTrackers(const StatsdConfig& config, const sp& uidMap, unordered_map& atomMatchingTrackerMap, vector>& allAtomMatchingTrackers, @@ -1079,49 +1108,17 @@ bool initAlerts(const StatsdConfig& config, const unordered_map& m vector>& allAnomalyTrackers) { for (int i = 0; i < config.alert_size(); i++) { const Alert& alert = config.alert(i); - const auto& itr = metricProducerMap.find(alert.metric_id()); - if (itr == metricProducerMap.end()) { - ALOGW("alert \"%lld\" has unknown metric id: \"%lld\"", (long long)alert.id(), - (long long)alert.metric_id()); - return false; - } - if (!alert.has_trigger_if_sum_gt()) { - ALOGW("invalid alert: missing threshold"); - return false; - } - if (alert.trigger_if_sum_gt() < 0 || alert.num_buckets() <= 0) { - ALOGW("invalid alert: threshold=%f num_buckets= %d", alert.trigger_if_sum_gt(), - alert.num_buckets()); - return false; - } - const int metricIndex = itr->second; - sp metric = allMetricProducers[metricIndex]; - sp anomalyTracker = metric->addAnomalyTracker(alert, anomalyAlarmMonitor); - if (anomalyTracker == nullptr) { - // The ALOGW for this invalid alert was already displayed in addAnomalyTracker(). - return false; - } alertTrackerMap.insert(std::make_pair(alert.id(), allAnomalyTrackers.size())); - allAnomalyTrackers.push_back(anomalyTracker); + optional> anomalyTracker = createAnomalyTracker( + alert, anomalyAlarmMonitor, metricProducerMap, allMetricProducers); + if (!anomalyTracker) { + return false; + } + allAnomalyTrackers.push_back(anomalyTracker.value()); } - for (int i = 0; i < config.subscription_size(); ++i) { - const Subscription& subscription = config.subscription(i); - if (subscription.rule_type() != Subscription::ALERT) { - continue; - } - if (subscription.subscriber_information_case() == - Subscription::SubscriberInformationCase::SUBSCRIBER_INFORMATION_NOT_SET) { - ALOGW("subscription \"%lld\" has no subscriber info.\"", (long long)subscription.id()); - return false; - } - const auto& itr = alertTrackerMap.find(subscription.rule_id()); - if (itr == alertTrackerMap.end()) { - ALOGW("subscription \"%lld\" has unknown rule id: \"%lld\"", - (long long)subscription.id(), (long long)subscription.rule_id()); - return false; - } - const int anomalyTrackerIndex = itr->second; - allAnomalyTrackers[anomalyTrackerIndex]->addSubscription(subscription); + if (!initSubscribersForSubscriptionType(config, Subscription::ALERT, alertTrackerMap, + allAnomalyTrackers)) { + return false; } return true; } diff --git a/cmds/statsd/src/metrics/parsing_utils/metrics_manager_util.h b/cmds/statsd/src/metrics/parsing_utils/metrics_manager_util.h index e4585cd578f84..781a4ef16149a 100644 --- a/cmds/statsd/src/metrics/parsing_utils/metrics_manager_util.h +++ b/cmds/statsd/src/metrics/parsing_utils/metrics_manager_util.h @@ -188,6 +188,41 @@ optional> createGaugeMetricProducerAndUpdateMetadata( std::unordered_map>& deactivationAtomTrackerToMetricMap, std::vector& metricsWithActivation); +// Creates an AnomalyTracker and adds it to the appropriate metric. +// Returns an sp to the AnomalyTracker, or nullopt if there was an error. +optional> createAnomalyTracker( + const Alert& alert, const sp& anomalyAlarmMonitor, + const std::unordered_map& metricProducerMap, + std::vector>& allMetricProducers); + +// Templated function for adding subscriptions to alarms or alerts. Returns true if successful. +template +bool initSubscribersForSubscriptionType(const StatsdConfig& config, + const Subscription_RuleType ruleType, + const std::unordered_map& ruleMap, + std::vector& allRules) { + for (int i = 0; i < config.subscription_size(); ++i) { + const Subscription& subscription = config.subscription(i); + if (subscription.rule_type() != ruleType) { + continue; + } + if (subscription.subscriber_information_case() == + Subscription::SubscriberInformationCase::SUBSCRIBER_INFORMATION_NOT_SET) { + ALOGW("subscription \"%lld\" has no subscriber info.\"", (long long)subscription.id()); + return false; + } + const auto& itr = ruleMap.find(subscription.rule_id()); + if (itr == ruleMap.end()) { + ALOGW("subscription \"%lld\" has unknown rule id: \"%lld\"", + (long long)subscription.id(), (long long)subscription.rule_id()); + return false; + } + const int ruleIndex = itr->second; + allRules[ruleIndex]->addSubscription(subscription); + } + return true; +} + // Helper functions for MetricsManager to initialize from StatsdConfig. // *Note*: only initStatsdConfig() should be called from outside. // All other functions are intermediate diff --git a/cmds/statsd/tests/metrics/parsing_utils/config_update_utils_test.cpp b/cmds/statsd/tests/metrics/parsing_utils/config_update_utils_test.cpp index 4fa9bf6ffc010..8a1b74ba0ff0b 100644 --- a/cmds/statsd/tests/metrics/parsing_utils/config_update_utils_test.cpp +++ b/cmds/statsd/tests/metrics/parsing_utils/config_update_utils_test.cpp @@ -65,13 +65,13 @@ unordered_map oldConditionTrackerMap; vector> oldMetricProducers; unordered_map oldMetricProducerMap; std::vector> oldAnomalyTrackers; +unordered_map oldAlertTrackerMap; std::vector> oldAlarmTrackers; unordered_map> tmpConditionToMetricMap; unordered_map> tmpTrackerToMetricMap; unordered_map> tmpTrackerToConditionMap; unordered_map> tmpActivationAtomTrackerToMetricMap; unordered_map> tmpDeactivationAtomTrackerToMetricMap; -unordered_map alertTrackerMap; vector metricsWithActivation; map oldStateHashes; std::set noReportMetricIds; @@ -96,7 +96,7 @@ public: tmpTrackerToConditionMap.clear(); tmpActivationAtomTrackerToMetricMap.clear(); tmpDeactivationAtomTrackerToMetricMap.clear(); - alertTrackerMap.clear(); + oldAlertTrackerMap.clear(); metricsWithActivation.clear(); oldStateHashes.clear(); noReportMetricIds.clear(); @@ -111,7 +111,7 @@ bool initConfig(const StatsdConfig& config) { oldConditionTrackers, oldConditionTrackerMap, oldMetricProducers, oldMetricProducerMap, oldAnomalyTrackers, oldAlarmTrackers, tmpConditionToMetricMap, tmpTrackerToMetricMap, tmpTrackerToConditionMap, tmpActivationAtomTrackerToMetricMap, - tmpDeactivationAtomTrackerToMetricMap, alertTrackerMap, metricsWithActivation, + tmpDeactivationAtomTrackerToMetricMap, oldAlertTrackerMap, metricsWithActivation, oldStateHashes, noReportMetricIds); } @@ -188,6 +188,24 @@ ValueMetric createValueMetric(string name, const AtomMatcher& what, optionalgetStrongCount(), 1); } + +TEST_F(ConfigUpdateTest, TestAlertPreserve) { + StatsdConfig config; + AtomMatcher whatMatcher = CreateScreenBrightnessChangedAtomMatcher(); + *config.add_atom_matcher() = whatMatcher; + + *config.add_count_metric() = createCountMetric("VALUE1", whatMatcher.id(), nullopt, {}); + + Alert alert = createAlert("Alert1", config.count_metric(0).id(), 1, 1); + *config.add_alert() = alert; + EXPECT_TRUE(initConfig(config)); + + UpdateStatus updateStatus = UPDATE_UNKNOWN; + EXPECT_TRUE(determineAlertUpdateStatus(alert, oldAlertTrackerMap, oldAnomalyTrackers, + /*replacedMetrics*/ {}, updateStatus)); + EXPECT_EQ(updateStatus, UPDATE_PRESERVE); +} + +TEST_F(ConfigUpdateTest, TestAlertMetricChanged) { + StatsdConfig config; + AtomMatcher whatMatcher = CreateScreenBrightnessChangedAtomMatcher(); + *config.add_atom_matcher() = whatMatcher; + + CountMetric metric = createCountMetric("VALUE1", whatMatcher.id(), nullopt, {}); + *config.add_count_metric() = metric; + + Alert alert = createAlert("Alert1", config.count_metric(0).id(), 1, 1); + *config.add_alert() = alert; + EXPECT_TRUE(initConfig(config)); + + UpdateStatus updateStatus = UPDATE_UNKNOWN; + EXPECT_TRUE(determineAlertUpdateStatus(alert, oldAlertTrackerMap, oldAnomalyTrackers, + /*replacedMetrics*/ {metric.id()}, updateStatus)); + EXPECT_EQ(updateStatus, UPDATE_REPLACE); +} + +TEST_F(ConfigUpdateTest, TestAlertDefinitionChanged) { + StatsdConfig config; + AtomMatcher whatMatcher = CreateScreenBrightnessChangedAtomMatcher(); + *config.add_atom_matcher() = whatMatcher; + + *config.add_count_metric() = createCountMetric("VALUE1", whatMatcher.id(), nullopt, {}); + + Alert alert = createAlert("Alert1", config.count_metric(0).id(), 1, 1); + *config.add_alert() = alert; + EXPECT_TRUE(initConfig(config)); + + alert.set_num_buckets(2); + + UpdateStatus updateStatus = UPDATE_UNKNOWN; + EXPECT_TRUE(determineAlertUpdateStatus(alert, oldAlertTrackerMap, oldAnomalyTrackers, + /*replacedMetrics*/ {}, updateStatus)); + EXPECT_EQ(updateStatus, UPDATE_REPLACE); +} + +TEST_F(ConfigUpdateTest, TestUpdateAlerts) { + StatsdConfig config; + // Add atom matchers/predicates/metrics. These are mostly needed for initStatsdConfig + *config.add_atom_matcher() = CreateScreenTurnedOnAtomMatcher(); + *config.add_atom_matcher() = CreateScreenTurnedOffAtomMatcher(); + *config.add_predicate() = CreateScreenIsOnPredicate(); + + CountMetric countMetric = createCountMetric("COUNT1", config.atom_matcher(0).id(), nullopt, {}); + int64_t countMetricId = countMetric.id(); + *config.add_count_metric() = countMetric; + + DurationMetric durationMetric = + createDurationMetric("DURATION1", config.predicate(0).id(), nullopt, {}); + int64_t durationMetricId = durationMetric.id(); + *config.add_duration_metric() = durationMetric; + + // Add alerts. + // Preserved. + Alert alert1 = createAlert("Alert1", durationMetricId, /*buckets*/ 1, /*triggerSum*/ 5000); + int64_t alert1Id = alert1.id(); + *config.add_alert() = alert1; + + // Replaced. + Alert alert2 = createAlert("Alert2", countMetricId, /*buckets*/ 1, /*triggerSum*/ 2); + int64_t alert2Id = alert2.id(); + *config.add_alert() = alert2; + + // Replaced. + Alert alert3 = createAlert("Alert3", durationMetricId, /*buckets*/ 3, /*triggerSum*/ 5000); + int64_t alert3Id = alert3.id(); + *config.add_alert() = alert3; + + // Add Subscriptions. + Subscription subscription1 = createSubscription("S1", Subscription::ALERT, alert1Id); + *config.add_subscription() = subscription1; + Subscription subscription2 = createSubscription("S2", Subscription::ALERT, alert1Id); + *config.add_subscription() = subscription2; + Subscription subscription3 = createSubscription("S3", Subscription::ALERT, alert2Id); + *config.add_subscription() = subscription3; + + EXPECT_TRUE(initConfig(config)); + + // Add a duration tracker to the duration metric to ensure durationTrackers are updated + // with the proper anomalyTrackers. + unique_ptr event = CreateScreenStateChangedEvent( + timeBaseNs + 1, android::view::DisplayStateEnum::DISPLAY_STATE_ON); + oldMetricProducers[1]->onMatchedLogEvent(0, *event.get()); + + // Change the count metric. Causes alert2 to be replaced. + config.mutable_count_metric(0)->set_bucket(ONE_DAY); + // Change num buckets on alert3, causing replacement. + alert3.set_num_buckets(5); + + // New alert. + Alert alert4 = createAlert("Alert4", durationMetricId, /*buckets*/ 3, /*triggerSum*/ 10000); + int64_t alert4Id = alert4.id(); + + // Move subscription2 to be on alert2 and make a new subscription. + subscription2.set_rule_id(alert2Id); + Subscription subscription4 = createSubscription("S4", Subscription::ALERT, alert2Id); + + // Create the new config. Modify the old one to avoid adding the matchers/predicates. + // Add alerts in different order so the map is changed. + config.clear_alert(); + *config.add_alert() = alert4; + const int alert4Index = 0; + *config.add_alert() = alert3; + const int alert3Index = 1; + *config.add_alert() = alert1; + const int alert1Index = 2; + *config.add_alert() = alert2; + const int alert2Index = 3; + + // Subscription3 is removed. + config.clear_subscription(); + *config.add_subscription() = subscription4; + *config.add_subscription() = subscription2; + *config.add_subscription() = subscription1; + + // Output data structures from update metrics. Don't care about the outputs besides + // replacedMetrics, but need to do this so that the metrics clear their anomaly trackers. + unordered_map newMetricProducerMap; + vector> newMetricProducers; + unordered_map> conditionToMetricMap; + unordered_map> trackerToMetricMap; + set noReportMetricIds; + unordered_map> activationAtomTrackerToMetricMap; + unordered_map> deactivationAtomTrackerToMetricMap; + vector metricsWithActivation; + set replacedMetrics; + EXPECT_TRUE(updateMetrics( + key, config, /*timeBaseNs=*/123, /*currentTimeNs=*/12345, new StatsPullerManager(), + oldAtomMatchingTrackerMap, oldAtomMatchingTrackerMap, /*replacedMatchers*/ {}, + oldAtomMatchingTrackers, oldConditionTrackerMap, /*replacedConditions=*/{}, + oldConditionTrackers, {ConditionState::kUnknown}, /*stateAtomIdMap*/ {}, + /*allStateGroupMaps=*/{}, + /*replacedStates=*/{}, oldMetricProducerMap, oldMetricProducers, newMetricProducerMap, + newMetricProducers, conditionToMetricMap, trackerToMetricMap, noReportMetricIds, + activationAtomTrackerToMetricMap, deactivationAtomTrackerToMetricMap, + metricsWithActivation, replacedMetrics)); + + EXPECT_EQ(replacedMetrics, set({countMetricId})); + + unordered_map newAlertTrackerMap; + vector> newAnomalyTrackers; + EXPECT_TRUE(updateAlerts(config, newMetricProducerMap, replacedMetrics, oldAlertTrackerMap, + oldAnomalyTrackers, anomalyAlarmMonitor, newMetricProducers, + newAlertTrackerMap, newAnomalyTrackers)); + + unordered_map expectedAlertMap = { + {alert1Id, alert1Index}, + {alert2Id, alert2Index}, + {alert3Id, alert3Index}, + {alert4Id, alert4Index}, + }; + EXPECT_THAT(newAlertTrackerMap, ContainerEq(expectedAlertMap)); + + // Make sure preserved alerts are the same. + ASSERT_EQ(newAnomalyTrackers.size(), 4); + EXPECT_EQ(oldAnomalyTrackers[oldAlertTrackerMap.at(alert1Id)], + newAnomalyTrackers[newAlertTrackerMap.at(alert1Id)]); + + // Make sure replaced alerts are different. + EXPECT_NE(oldAnomalyTrackers[oldAlertTrackerMap.at(alert2Id)], + newAnomalyTrackers[newAlertTrackerMap.at(alert2Id)]); + EXPECT_NE(oldAnomalyTrackers[oldAlertTrackerMap.at(alert3Id)], + newAnomalyTrackers[newAlertTrackerMap.at(alert3Id)]); + + // Verify the alerts have the correct anomaly trackers. + ASSERT_EQ(newMetricProducers.size(), 2); + EXPECT_THAT(newMetricProducers[0]->mAnomalyTrackers, + UnorderedElementsAre(newAnomalyTrackers[alert2Index])); + // For durationMetric, make sure the duration trackers get the updated anomalyTrackers. + DurationMetricProducer* durationProducer = + static_cast(newMetricProducers[1].get()); + EXPECT_THAT( + durationProducer->mAnomalyTrackers, + UnorderedElementsAre(newAnomalyTrackers[alert1Index], newAnomalyTrackers[alert3Index], + newAnomalyTrackers[alert4Index])); + ASSERT_EQ(durationProducer->mCurrentSlicedDurationTrackerMap.size(), 1); + for (const auto& durationTrackerIt : durationProducer->mCurrentSlicedDurationTrackerMap) { + EXPECT_EQ(durationTrackerIt.second->mAnomalyTrackers, durationProducer->mAnomalyTrackers); + } + + // Verify alerts have the correct subscriptions. Use subscription id as proxy for equivalency. + vector alert1Subscriptions; + for (const Subscription& subscription : newAnomalyTrackers[alert1Index]->mSubscriptions) { + alert1Subscriptions.push_back(subscription.id()); + } + EXPECT_THAT(alert1Subscriptions, UnorderedElementsAre(subscription1.id())); + vector alert2Subscriptions; + for (const Subscription& subscription : newAnomalyTrackers[alert2Index]->mSubscriptions) { + alert2Subscriptions.push_back(subscription.id()); + } + EXPECT_THAT(alert2Subscriptions, UnorderedElementsAre(subscription2.id(), subscription4.id())); + EXPECT_THAT(newAnomalyTrackers[alert3Index]->mSubscriptions, IsEmpty()); + EXPECT_THAT(newAnomalyTrackers[alert4Index]->mSubscriptions, IsEmpty()); +} + } // namespace statsd } // namespace os } // namespace android diff --git a/cmds/statsd/tests/metrics/parsing_utils/metrics_manager_util_test.cpp b/cmds/statsd/tests/metrics/parsing_utils/metrics_manager_util_test.cpp index 0d0a8960043e8..9e2350b330182 100644 --- a/cmds/statsd/tests/metrics/parsing_utils/metrics_manager_util_test.cpp +++ b/cmds/statsd/tests/metrics/parsing_utils/metrics_manager_util_test.cpp @@ -27,6 +27,7 @@ #include "src/condition/ConditionTracker.h" #include "src/matchers/AtomMatchingTracker.h" #include "src/metrics/CountMetricProducer.h" +#include "src/metrics/DurationMetricProducer.h" #include "src/metrics/GaugeMetricProducer.h" #include "src/metrics/MetricProducer.h" #include "src/metrics/ValueMetricProducer.h" @@ -793,6 +794,93 @@ TEST(MetricsManagerTest, TestCreateConditionTrackerCombination) { EXPECT_FALSE(tracker->IsSimpleCondition()); } +TEST(MetricsManagerTest, TestCreateAnomalyTrackerInvalidMetric) { + Alert alert; + alert.set_id(123); + alert.set_metric_id(1); + alert.set_trigger_if_sum_gt(1); + alert.set_num_buckets(1); + + sp anomalyAlarmMonitor; + vector> metricProducers; + // Pass in empty metric producers, causing an error. + EXPECT_EQ(createAnomalyTracker(alert, anomalyAlarmMonitor, {}, metricProducers), nullopt); +} + +TEST(MetricsManagerTest, TestCreateAnomalyTrackerNoThreshold) { + int64_t metricId = 1; + Alert alert; + alert.set_id(123); + alert.set_metric_id(metricId); + alert.set_num_buckets(1); + + CountMetric metric; + metric.set_id(metricId); + metric.set_bucket(ONE_MINUTE); + sp wizard = new NaggyMock(); + vector> metricProducers({new CountMetricProducer( + kConfigKey, metric, 0, {ConditionState::kUnknown}, wizard, 0x0123456789, 0, 0)}); + sp anomalyAlarmMonitor; + EXPECT_EQ(createAnomalyTracker(alert, anomalyAlarmMonitor, {{1, 0}}, metricProducers), nullopt); +} + +TEST(MetricsManagerTest, TestCreateAnomalyTrackerMissingBuckets) { + int64_t metricId = 1; + Alert alert; + alert.set_id(123); + alert.set_metric_id(metricId); + alert.set_trigger_if_sum_gt(1); + + CountMetric metric; + metric.set_id(metricId); + metric.set_bucket(ONE_MINUTE); + sp wizard = new NaggyMock(); + vector> metricProducers({new CountMetricProducer( + kConfigKey, metric, 0, {ConditionState::kUnknown}, wizard, 0x0123456789, 0, 0)}); + sp anomalyAlarmMonitor; + EXPECT_EQ(createAnomalyTracker(alert, anomalyAlarmMonitor, {{1, 0}}, metricProducers), nullopt); +} + +TEST(MetricsManagerTest, TestCreateAnomalyTrackerGood) { + int64_t metricId = 1; + Alert alert; + alert.set_id(123); + alert.set_metric_id(metricId); + alert.set_trigger_if_sum_gt(1); + alert.set_num_buckets(1); + + CountMetric metric; + metric.set_id(metricId); + metric.set_bucket(ONE_MINUTE); + sp wizard = new NaggyMock(); + vector> metricProducers({new CountMetricProducer( + kConfigKey, metric, 0, {ConditionState::kUnknown}, wizard, 0x0123456789, 0, 0)}); + sp anomalyAlarmMonitor; + EXPECT_NE(createAnomalyTracker(alert, anomalyAlarmMonitor, {{1, 0}}, metricProducers), nullopt); +} + +TEST(MetricsManagerTest, TestCreateAnomalyTrackerDurationTooLong) { + int64_t metricId = 1; + Alert alert; + alert.set_id(123); + alert.set_metric_id(metricId); + // Impossible for alert to fire since the time is bigger than bucketSize * numBuckets + alert.set_trigger_if_sum_gt(MillisToNano(TimeUnitToBucketSizeInMillis(ONE_MINUTE)) + 1); + alert.set_num_buckets(1); + + DurationMetric metric; + metric.set_id(metricId); + metric.set_bucket(ONE_MINUTE); + metric.set_aggregation_type(DurationMetric_AggregationType_SUM); + FieldMatcher dimensions; + sp wizard = new NaggyMock(); + vector> metricProducers({new DurationMetricProducer( + kConfigKey, metric, -1 /*no condition*/, {}, 1 /* start index */, 2 /* stop index */, + 3 /* stop_all index */, false /*nesting*/, wizard, 0x0123456789, dimensions, 0, 0)}); + sp anomalyAlarmMonitor; + EXPECT_EQ(createAnomalyTracker(alert, anomalyAlarmMonitor, {{1, 0}}, metricProducers), nullopt); +} + } // namespace statsd } // namespace os } // namespace android From 8b46bc11b43926989d129cf17dbad3648ee79045 Mon Sep 17 00:00:00 2001 From: Tej Singh Date: Thu, 29 Oct 2020 22:54:41 -0700 Subject: [PATCH 2/2] Update Alarms/Subscriptions This is the same as initializing because alarms have no state to carry over. We can just replace them all without losing anything. Test: atest statsd_test Bug: 162323547 Change-Id: Ia2fb33e9ac79476babce57e2e0fb1ca49091e260 --- cmds/statsd/src/anomaly/AlarmTracker.h | 1 + cmds/statsd/src/metrics/MetricsManager.cpp | 9 +- .../parsing_utils/config_update_utils.cpp | 7 ++ .../parsing_utils/config_update_utils.h | 2 + .../parsing_utils/metrics_manager_util.cpp | 21 +--- .../parsing_utils/metrics_manager_util.h | 6 ++ .../config_update_utils_test.cpp | 101 +++++++++++++++++- 7 files changed, 124 insertions(+), 23 deletions(-) diff --git a/cmds/statsd/src/anomaly/AlarmTracker.h b/cmds/statsd/src/anomaly/AlarmTracker.h index 2da4a18682ae8..406086da557bf 100644 --- a/cmds/statsd/src/anomaly/AlarmTracker.h +++ b/cmds/statsd/src/anomaly/AlarmTracker.h @@ -73,6 +73,7 @@ protected: FRIEND_TEST(AlarmTrackerTest, TestTriggerTimestamp); FRIEND_TEST(AlarmE2eTest, TestMultipleAlarms); + FRIEND_TEST(ConfigUpdateTest, TestUpdateAlarms); }; } // namespace statsd diff --git a/cmds/statsd/src/metrics/MetricsManager.cpp b/cmds/statsd/src/metrics/MetricsManager.cpp index b1d4397037097..d80f9dbb42562 100644 --- a/cmds/statsd/src/metrics/MetricsManager.cpp +++ b/cmds/statsd/src/metrics/MetricsManager.cpp @@ -211,6 +211,7 @@ bool MetricsManager::updateConfig(const StatsdConfig& config, const int64_t time unordered_map newMetricProducerMap; vector> newAnomalyTrackers; unordered_map newAlertTrackerMap; + vector> newPeriodicAlarmTrackers; mTagIds.clear(); mConditionToMetricMap.clear(); mTrackerToMetricMap.clear(); @@ -226,9 +227,10 @@ bool MetricsManager::updateConfig(const StatsdConfig& config, const int64_t time mAllAnomalyTrackers, mAlertTrackerMap, mStateProtoHashes, mTagIds, newAtomMatchingTrackers, newAtomMatchingTrackerMap, newConditionTrackers, newConditionTrackerMap, newMetricProducers, newMetricProducerMap, newAnomalyTrackers, - newAlertTrackerMap, mConditionToMetricMap, mTrackerToMetricMap, mTrackerToConditionMap, - mActivationAtomTrackerToMetricMap, mDeactivationAtomTrackerToMetricMap, - mMetricIndexesWithActivation, newStateProtoHashes, mNoReportMetricIds); + newAlertTrackerMap, newPeriodicAlarmTrackers, mConditionToMetricMap, + mTrackerToMetricMap, mTrackerToConditionMap, mActivationAtomTrackerToMetricMap, + mDeactivationAtomTrackerToMetricMap, mMetricIndexesWithActivation, newStateProtoHashes, + mNoReportMetricIds); mAllAtomMatchingTrackers = newAtomMatchingTrackers; mAtomMatchingTrackerMap = newAtomMatchingTrackerMap; mAllConditionTrackers = newConditionTrackers; @@ -238,6 +240,7 @@ bool MetricsManager::updateConfig(const StatsdConfig& config, const int64_t time mStateProtoHashes = newStateProtoHashes; mAllAnomalyTrackers = newAnomalyTrackers; mAlertTrackerMap = newAlertTrackerMap; + mAllPeriodicAlarmTrackers = newPeriodicAlarmTrackers; return mConfigValid; } diff --git a/cmds/statsd/src/metrics/parsing_utils/config_update_utils.cpp b/cmds/statsd/src/metrics/parsing_utils/config_update_utils.cpp index 0c4dc4476f480..637236145bf5a 100644 --- a/cmds/statsd/src/metrics/parsing_utils/config_update_utils.cpp +++ b/cmds/statsd/src/metrics/parsing_utils/config_update_utils.cpp @@ -1017,6 +1017,7 @@ bool updateStatsdConfig(const ConfigKey& key, const StatsdConfig& config, const unordered_map& newMetricProducerMap, vector>& newAnomalyTrackers, unordered_map& newAlertTrackerMap, + vector>& newPeriodicAlarmTrackers, unordered_map>& conditionToMetricMap, unordered_map>& trackerToMetricMap, unordered_map>& trackerToConditionMap, @@ -1081,6 +1082,12 @@ bool updateStatsdConfig(const ConfigKey& key, const StatsdConfig& config, const return false; } + // Alarms do not have any state, so we can reuse the initialization logic. + if (!initAlarms(config, key, periodicAlarmMonitor, timeBaseNs, currentTimeNs, + newPeriodicAlarmTrackers)) { + ALOGE("initAlarms failed"); + return false; + } return true; } diff --git a/cmds/statsd/src/metrics/parsing_utils/config_update_utils.h b/cmds/statsd/src/metrics/parsing_utils/config_update_utils.h index b714f58f6c067..178a9d220b4dd 100644 --- a/cmds/statsd/src/metrics/parsing_utils/config_update_utils.h +++ b/cmds/statsd/src/metrics/parsing_utils/config_update_utils.h @@ -19,6 +19,7 @@ #include #include "anomaly/AlarmMonitor.h" +#include "anomaly/AlarmTracker.h" #include "condition/ConditionTracker.h" #include "external/StatsPullerManager.h" #include "matchers/AtomMatchingTracker.h" @@ -253,6 +254,7 @@ bool updateStatsdConfig(const ConfigKey& key, const StatsdConfig& config, const std::unordered_map& newMetricProducerMap, std::vector>& newAlertTrackers, std::unordered_map& newAlertTrackerMap, + std::vector>& newPeriodicAlarmTrackers, std::unordered_map>& conditionToMetricMap, std::unordered_map>& trackerToMetricMap, std::unordered_map>& trackerToConditionMap, diff --git a/cmds/statsd/src/metrics/parsing_utils/metrics_manager_util.cpp b/cmds/statsd/src/metrics/parsing_utils/metrics_manager_util.cpp index f15c8b0f33462..4474df4346cf3 100644 --- a/cmds/statsd/src/metrics/parsing_utils/metrics_manager_util.cpp +++ b/cmds/statsd/src/metrics/parsing_utils/metrics_manager_util.cpp @@ -1143,24 +1143,9 @@ bool initAlarms(const StatsdConfig& config, const ConfigKey& key, allAlarmTrackers.push_back( new AlarmTracker(startMillis, currentTimeMillis, alarm, key, periodicAlarmMonitor)); } - for (int i = 0; i < config.subscription_size(); ++i) { - const Subscription& subscription = config.subscription(i); - if (subscription.rule_type() != Subscription::ALARM) { - continue; - } - if (subscription.subscriber_information_case() == - Subscription::SubscriberInformationCase::SUBSCRIBER_INFORMATION_NOT_SET) { - ALOGW("subscription \"%lld\" has no subscriber info.\"", (long long)subscription.id()); - return false; - } - const auto& itr = alarmTrackerMap.find(subscription.rule_id()); - if (itr == alarmTrackerMap.end()) { - ALOGW("subscription \"%lld\" has unknown rule id: \"%lld\"", - (long long)subscription.id(), (long long)subscription.rule_id()); - return false; - } - const int trackerIndex = itr->second; - allAlarmTrackers[trackerIndex]->addSubscription(subscription); + if (!initSubscribersForSubscriptionType(config, Subscription::ALARM, alarmTrackerMap, + allAlarmTrackers)) { + return false; } return true; } diff --git a/cmds/statsd/src/metrics/parsing_utils/metrics_manager_util.h b/cmds/statsd/src/metrics/parsing_utils/metrics_manager_util.h index 781a4ef16149a..84e1e4e043398 100644 --- a/cmds/statsd/src/metrics/parsing_utils/metrics_manager_util.h +++ b/cmds/statsd/src/metrics/parsing_utils/metrics_manager_util.h @@ -306,6 +306,12 @@ bool initMetrics( std::unordered_map>& deactivationAtomTrackerToMetricMap, std::vector& metricsWithActivation); +// Initialize alarms +// Is called both on initialize new configs and config updates since alarms do not have any state. +bool initAlarms(const StatsdConfig& config, const ConfigKey& key, + const sp& periodicAlarmMonitor, const int64_t timeBaseNs, + const int64_t currentTimeNs, std::vector>& allAlarmTrackers); + // Initialize MetricsManager from StatsdConfig. // Parameters are the members of MetricsManager. See MetricsManager for declaration. bool initStatsdConfig(const ConfigKey& key, const StatsdConfig& config, const sp& uidMap, diff --git a/cmds/statsd/tests/metrics/parsing_utils/config_update_utils_test.cpp b/cmds/statsd/tests/metrics/parsing_utils/config_update_utils_test.cpp index 8a1b74ba0ff0b..66bab4ea70daf 100644 --- a/cmds/statsd/tests/metrics/parsing_utils/config_update_utils_test.cpp +++ b/cmds/statsd/tests/metrics/parsing_utils/config_update_utils_test.cpp @@ -52,11 +52,15 @@ namespace statsd { namespace { ConfigKey key(123, 456); -const int64_t timeBaseNs = 1000; +const int64_t timeBaseNs = 1000 * NS_PER_SEC; + sp uidMap = new UidMap(); sp pullerManager = new StatsPullerManager(); sp anomalyAlarmMonitor; -sp periodicAlarmMonitor; +sp periodicAlarmMonitor = new AlarmMonitor( + /*minDiffToUpdateRegisteredAlarmTimeSec=*/0, + [](const shared_ptr&, int64_t) {}, + [](const shared_ptr&) {}); set allTagIds; vector> oldAtomMatchingTrackers; unordered_map oldAtomMatchingTrackerMap; @@ -206,6 +210,14 @@ Subscription createSubscription(string name, Subscription_RuleType type, int64_t subscription.mutable_broadcast_subscriber_details(); return subscription; } + +Alarm createAlarm(string name, int64_t offsetMillis, int64_t periodMillis) { + Alarm alarm; + alarm.set_id(StringToId(name)); + alarm.set_offset_millis(offsetMillis); + alarm.set_period_millis(periodMillis); + return alarm; +} } // anonymous namespace TEST_F(ConfigUpdateTest, TestSimpleMatcherPreserve) { @@ -3475,6 +3487,91 @@ TEST_F(ConfigUpdateTest, TestUpdateAlerts) { EXPECT_THAT(newAnomalyTrackers[alert4Index]->mSubscriptions, IsEmpty()); } +TEST_F(ConfigUpdateTest, TestUpdateAlarms) { + StatsdConfig config; + // Add alarms. + Alarm alarm1 = createAlarm("Alarm1", /*offset*/ 1 * MS_PER_SEC, /*period*/ 50 * MS_PER_SEC); + int64_t alarm1Id = alarm1.id(); + *config.add_alarm() = alarm1; + + Alarm alarm2 = createAlarm("Alarm2", /*offset*/ 1 * MS_PER_SEC, /*period*/ 2000 * MS_PER_SEC); + int64_t alarm2Id = alarm2.id(); + *config.add_alarm() = alarm2; + + Alarm alarm3 = createAlarm("Alarm3", /*offset*/ 10 * MS_PER_SEC, /*period*/ 5000 * MS_PER_SEC); + int64_t alarm3Id = alarm3.id(); + *config.add_alarm() = alarm3; + + // Add Subscriptions. + Subscription subscription1 = createSubscription("S1", Subscription::ALARM, alarm1Id); + *config.add_subscription() = subscription1; + Subscription subscription2 = createSubscription("S2", Subscription::ALARM, alarm1Id); + *config.add_subscription() = subscription2; + Subscription subscription3 = createSubscription("S3", Subscription::ALARM, alarm2Id); + *config.add_subscription() = subscription3; + + EXPECT_TRUE(initConfig(config)); + + ASSERT_EQ(oldAlarmTrackers.size(), 3); + // Config is created at statsd start time, so just add the offsets. + EXPECT_EQ(oldAlarmTrackers[0]->getAlarmTimestampSec(), timeBaseNs / NS_PER_SEC + 1); + EXPECT_EQ(oldAlarmTrackers[1]->getAlarmTimestampSec(), timeBaseNs / NS_PER_SEC + 1); + EXPECT_EQ(oldAlarmTrackers[2]->getAlarmTimestampSec(), timeBaseNs / NS_PER_SEC + 10); + + // Change alarm2/alarm3. + config.mutable_alarm(1)->set_offset_millis(5 * MS_PER_SEC); + config.mutable_alarm(2)->set_period_millis(10000 * MS_PER_SEC); + + // Move subscription2 to be on alarm2 and make a new subscription. + config.mutable_subscription(1)->set_rule_id(alarm2Id); + Subscription subscription4 = createSubscription("S4", Subscription::ALARM, alarm1Id); + *config.add_subscription() = subscription4; + + // Update time is 2 seconds after the base time. + int64_t currentTimeNs = timeBaseNs + 2 * NS_PER_SEC; + vector> newAlarmTrackers; + EXPECT_TRUE(initAlarms(config, key, periodicAlarmMonitor, timeBaseNs, currentTimeNs, + newAlarmTrackers)); + + ASSERT_EQ(newAlarmTrackers.size(), 3); + // Config is updated 2 seconds after statsd start + // The offset has passed for alarm1, but not for alarms 2/3. + EXPECT_EQ(newAlarmTrackers[0]->getAlarmTimestampSec(), timeBaseNs / NS_PER_SEC + 1 + 50); + EXPECT_EQ(newAlarmTrackers[1]->getAlarmTimestampSec(), timeBaseNs / NS_PER_SEC + 5); + EXPECT_EQ(newAlarmTrackers[2]->getAlarmTimestampSec(), timeBaseNs / NS_PER_SEC + 10); + + // Verify alarms have the correct subscriptions. Use subscription id as proxy for equivalency. + vector alarm1Subscriptions; + for (const Subscription& subscription : newAlarmTrackers[0]->mSubscriptions) { + alarm1Subscriptions.push_back(subscription.id()); + } + EXPECT_THAT(alarm1Subscriptions, UnorderedElementsAre(subscription1.id(), subscription4.id())); + vector alarm2Subscriptions; + for (const Subscription& subscription : newAlarmTrackers[1]->mSubscriptions) { + alarm2Subscriptions.push_back(subscription.id()); + } + EXPECT_THAT(alarm2Subscriptions, UnorderedElementsAre(subscription2.id(), subscription3.id())); + EXPECT_THAT(newAlarmTrackers[2]->mSubscriptions, IsEmpty()); + + // Verify the alarm monitor is updated accordingly once the old alarms are removed. + // Alarm2 fires the earliest. + oldAlarmTrackers.clear(); + EXPECT_EQ(periodicAlarmMonitor->getRegisteredAlarmTimeSec(), timeBaseNs / NS_PER_SEC + 5); + + // Do another update 60 seconds after config creation time, after the offsets of each alarm. + currentTimeNs = timeBaseNs + 60 * NS_PER_SEC; + newAlarmTrackers.clear(); + EXPECT_TRUE(initAlarms(config, key, periodicAlarmMonitor, timeBaseNs, currentTimeNs, + newAlarmTrackers)); + + ASSERT_EQ(newAlarmTrackers.size(), 3); + // Config is updated one minute after statsd start. + // Two periods have passed for alarm 1, one has passed for alarms2/3. + EXPECT_EQ(newAlarmTrackers[0]->getAlarmTimestampSec(), timeBaseNs / NS_PER_SEC + 1 + 2 * 50); + EXPECT_EQ(newAlarmTrackers[1]->getAlarmTimestampSec(), timeBaseNs / NS_PER_SEC + 5 + 2000); + EXPECT_EQ(newAlarmTrackers[2]->getAlarmTimestampSec(), timeBaseNs / NS_PER_SEC + 10 + 10000); +} + } // namespace statsd } // namespace os } // namespace android