Merge "Change condition init to return current condition"

6 files changed, 35 insertions, 18 deletions

diff --git a/cmds/statsd/src/condition/CombinationConditionTracker.cpp b/cmds/statsd/src/condition/CombinationConditionTracker.cpp
index 7a1ece94fd3e..3b65f8225ee9 100644
--- a/cmds/statsd/src/condition/CombinationConditionTracker.cpp
+++ b/cmds/statsd/src/condition/CombinationConditionTracker.cpp
@@ -38,9 +38,23 @@ bool CombinationConditionTracker::init(const vector<Predicate>& allConditionConf
                                        const vector<sp<ConditionTracker>>& allConditionTrackers,
                                        const unordered_map<int64_t, int>& conditionIdIndexMap,
                                        vector<bool>& stack,
-                                       vector<ConditionState>& initialConditionCache) {
+                                       vector<ConditionState>& conditionCache) {
     VLOG("Combination predicate init() %lld", (long long)mConditionId);
     if (mInitialized) {
+        // All the children are guaranteed to be initialized, but the recursion is needed to
+        // fill the conditionCache properly, since another combination condition or metric
+        // might rely on this. The recursion is needed to compute the current condition.
+
+        // Init is called instead of isConditionMet so that the ConditionKey can be filled with the
+        // default key for sliced conditions, since we do not know all indirect descendants here.
+        for (const int childIndex : mChildren) {
+            if (conditionCache[childIndex] == ConditionState::kNotEvaluated) {
+                allConditionTrackers[childIndex]->init(allConditionConfig, allConditionTrackers,
+                                                       conditionIdIndexMap, stack, conditionCache);
+            }
+        }
+        conditionCache[mIndex] =
+                evaluateCombinationCondition(mChildren, mLogicalOperation, conditionCache);
         return true;
     }
 
@@ -74,9 +88,8 @@ bool CombinationConditionTracker::init(const vector<Predicate>& allConditionConf
             return false;
         }
 
-        bool initChildSucceeded =
-                childTracker->init(allConditionConfig, allConditionTrackers, conditionIdIndexMap,
-                                   stack, initialConditionCache);
+        bool initChildSucceeded = childTracker->init(allConditionConfig, allConditionTrackers,
+                                                     conditionIdIndexMap, stack, conditionCache);
 
         if (!initChildSucceeded) {
             ALOGW("Child initialization failed %lld ", (long long)child);
@@ -96,10 +109,10 @@ bool CombinationConditionTracker::init(const vector<Predicate>& allConditionConf
                              childTracker->getAtomMatchingTrackerIndex().end());
     }
 
-    mUnSlicedPartCondition = evaluateCombinationCondition(mUnSlicedChildren, mLogicalOperation,
-                                                          initialConditionCache);
-    initialConditionCache[mIndex] =
-            evaluateCombinationCondition(mChildren, mLogicalOperation, initialConditionCache);
+    mUnSlicedPartCondition =
+            evaluateCombinationCondition(mUnSlicedChildren, mLogicalOperation, conditionCache);
+    conditionCache[mIndex] =
+            evaluateCombinationCondition(mChildren, mLogicalOperation, conditionCache);
 
     // unmark this node in the recursion stack.
     stack[mIndex] = false;
diff --git a/cmds/statsd/src/condition/CombinationConditionTracker.h b/cmds/statsd/src/condition/CombinationConditionTracker.h
index 39ff0ab03266..a7fac3deaabe 100644
--- a/cmds/statsd/src/condition/CombinationConditionTracker.h
+++ b/cmds/statsd/src/condition/CombinationConditionTracker.h
@@ -33,7 +33,7 @@ public:
     bool init(const std::vector<Predicate>& allConditionConfig,
               const std::vector<sp<ConditionTracker>>& allConditionTrackers,
               const std::unordered_map<int64_t, int>& conditionIdIndexMap, std::vector<bool>& stack,
-              std::vector<ConditionState>& initialConditionCache) override;
+              std::vector<ConditionState>& conditionCache) override;
 
     void evaluateCondition(const LogEvent& event,
                            const std::vector<MatchingState>& eventMatcherValues,
diff --git a/cmds/statsd/src/condition/ConditionTracker.h b/cmds/statsd/src/condition/ConditionTracker.h
index 9da1af427e5f..4e1253506be7 100644
--- a/cmds/statsd/src/condition/ConditionTracker.h
+++ b/cmds/statsd/src/condition/ConditionTracker.h
@@ -46,17 +46,19 @@ public:
     // Initialize this ConditionTracker. This initialization is done recursively (DFS). It can also
     // be done in the constructor, but we do it separately because (1) easy to return a bool to
     // indicate whether the initialization is successful. (2) makes unit test easier.
+    // This function can also be called on config updates, in which case it does nothing other than
+    // fill the condition cache with the current condition.
     // allConditionConfig: the list of all Predicate config from statsd_config.
     // allConditionTrackers: the list of all ConditionTrackers (this is needed because we may also
     //                       need to call init() on children conditions)
     // conditionIdIndexMap: the mapping from condition id to its index.
     // stack: a bit map to keep track which nodes have been visited on the stack in the recursion.
-    // initialConditionCache: tracks initial conditions of all ConditionTrackers.
+    // conditionCache: tracks initial conditions of all ConditionTrackers. returns the
+    //                        current condition if called on a config update.
     virtual bool init(const std::vector<Predicate>& allConditionConfig,
                       const std::vector<sp<ConditionTracker>>& allConditionTrackers,
                       const std::unordered_map<int64_t, int>& conditionIdIndexMap,
-                      std::vector<bool>& stack,
-                      std::vector<ConditionState>& initialConditionCache) = 0;
+                      std::vector<bool>& stack, std::vector<ConditionState>& conditionCache) = 0;
 
     // evaluate current condition given the new event.
     // event: the new log event
diff --git a/cmds/statsd/src/condition/SimpleConditionTracker.cpp b/cmds/statsd/src/condition/SimpleConditionTracker.cpp
index efb4d4989425..f45759b6a77e 100644
--- a/cmds/statsd/src/condition/SimpleConditionTracker.cpp
+++ b/cmds/statsd/src/condition/SimpleConditionTracker.cpp
@@ -95,11 +95,14 @@ SimpleConditionTracker::~SimpleConditionTracker() {
 bool SimpleConditionTracker::init(const vector<Predicate>& allConditionConfig,
                                   const vector<sp<ConditionTracker>>& allConditionTrackers,
                                   const unordered_map<int64_t, int>& conditionIdIndexMap,
-                                  vector<bool>& stack,
-                                  vector<ConditionState>& initialConditionCache) {
+                                  vector<bool>& stack, vector<ConditionState>& conditionCache) {
     // SimpleConditionTracker does not have dependency on other conditions, thus we just return
     // if the initialization was successful.
-    initialConditionCache[mIndex] = mInitialValue;
+    ConditionKey conditionKey;
+    if (mSliced) {
+        conditionKey[mConditionId] = DEFAULT_DIMENSION_KEY;
+    }
+    isConditionMet(conditionKey, allConditionTrackers, mSliced, conditionCache);
     return mInitialized;
 }
 
diff --git a/cmds/statsd/src/condition/SimpleConditionTracker.h b/cmds/statsd/src/condition/SimpleConditionTracker.h
index ea7f87bde2b8..1a9e35e38207 100644
--- a/cmds/statsd/src/condition/SimpleConditionTracker.h
+++ b/cmds/statsd/src/condition/SimpleConditionTracker.h
@@ -38,7 +38,7 @@ public:
     bool init(const std::vector<Predicate>& allConditionConfig,
               const std::vector<sp<ConditionTracker>>& allConditionTrackers,
               const std::unordered_map<int64_t, int>& conditionIdIndexMap, std::vector<bool>& stack,
-              std::vector<ConditionState>& initialConditionCache) override;
+              std::vector<ConditionState>& conditionCache) override;
 
     void evaluateCondition(const LogEvent& event,
                            const std::vector<MatchingState>& eventMatcherValues,
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 daf67e93557c..e40fbdb250f1 100644
--- a/cmds/statsd/src/metrics/parsing_utils/metrics_manager_util.cpp
+++ b/cmds/statsd/src/metrics/parsing_utils/metrics_manager_util.cpp
@@ -303,8 +303,7 @@ bool initConditions(const ConfigKey& key, const StatsdConfig& config,
     const int conditionTrackerCount = config.predicate_size();
     conditionConfigs.reserve(conditionTrackerCount);
     allConditionTrackers.reserve(conditionTrackerCount);
-    initialConditionCache.reserve(conditionTrackerCount);
-    std::fill(initialConditionCache.begin(), initialConditionCache.end(), ConditionState::kUnknown);
+    initialConditionCache.assign(conditionTrackerCount, ConditionState::kNotEvaluated);
 
     for (int i = 0; i < conditionTrackerCount; i++) {
         const Predicate& condition = config.predicate(i);