From c9fa2f6d4ea5c3f6730bee67646c0423f5693640 Mon Sep 17 00:00:00 2001 From: Yao Chen Date: Mon, 27 Nov 2017 11:31:55 -0800 Subject: [PATCH] Reject the config if condition config has errors. And add log tag. Test: added unit test. Change-Id: I5a9d6de2492b94bc5f1c88524f743607e60226c1 --- .../src/metrics/metrics_manager_util.cpp | 48 +++++++++++++------ cmds/statsd/tests/MetricsManager_test.cpp | 34 +++++++++++++ 2 files changed, 67 insertions(+), 15 deletions(-) diff --git a/cmds/statsd/src/metrics/metrics_manager_util.cpp b/cmds/statsd/src/metrics/metrics_manager_util.cpp index 466026350dddb..829140a31b530 100644 --- a/cmds/statsd/src/metrics/metrics_manager_util.cpp +++ b/cmds/statsd/src/metrics/metrics_manager_util.cpp @@ -14,6 +14,9 @@ * limitations under the License. */ +#define DEBUG true // STOPSHIP if true +#include "Log.h" + #include "../condition/CombinationConditionTracker.h" #include "../condition/SimpleConditionTracker.h" #include "../external/StatsPullerManager.h" @@ -219,9 +222,12 @@ bool initMetrics(const StatsdConfig& config, const unordered_map& l int conditionIndex = -1; if (metric.has_condition()) { - handleMetricWithConditions(metric.condition(), metricIndex, conditionTrackerMap, - metric.links(), allConditionTrackers, conditionIndex, - conditionToMetricMap); + bool good = handleMetricWithConditions( + metric.condition(), metricIndex, conditionTrackerMap, metric.links(), + allConditionTrackers, conditionIndex, conditionToMetricMap); + if (!good) { + return false; + } } else { if (metric.links_size() > 0) { ALOGW("metrics has a EventConditionLink but doesn't have a condition"); @@ -287,9 +293,12 @@ bool initMetrics(const StatsdConfig& config, const unordered_map& l int conditionIndex = -1; if (metric.has_condition()) { - handleMetricWithConditions(metric.condition(), metricIndex, conditionTrackerMap, - metric.links(), allConditionTrackers, conditionIndex, - conditionToMetricMap); + bool good = handleMetricWithConditions( + metric.condition(), metricIndex, conditionTrackerMap, metric.links(), + allConditionTrackers, conditionIndex, conditionToMetricMap); + if (!good) { + return false; + } } else { if (metric.links_size() > 0) { ALOGW("metrics has a EventConditionLink but doesn't have a condition"); @@ -321,9 +330,12 @@ bool initMetrics(const StatsdConfig& config, const unordered_map& l int conditionIndex = -1; if (metric.has_condition()) { - handleMetricWithConditions(metric.condition(), metricIndex, conditionTrackerMap, - metric.links(), allConditionTrackers, conditionIndex, - conditionToMetricMap); + bool good = handleMetricWithConditions( + metric.condition(), metricIndex, conditionTrackerMap, metric.links(), + allConditionTrackers, conditionIndex, conditionToMetricMap); + if (!good) { + return false; + } } else { if (metric.links_size() > 0) { ALOGW("metrics has a EventConditionLink but doesn't have a condition"); @@ -368,9 +380,12 @@ bool initMetrics(const StatsdConfig& config, const unordered_map& l int conditionIndex = -1; if (metric.has_condition()) { - handleMetricWithConditions(metric.condition(), metricIndex, conditionTrackerMap, - metric.links(), allConditionTrackers, conditionIndex, - conditionToMetricMap); + bool good = handleMetricWithConditions( + metric.condition(), metricIndex, conditionTrackerMap, metric.links(), + allConditionTrackers, conditionIndex, conditionToMetricMap); + if (!good) { + return false; + } } else { if (metric.links_size() > 0) { ALOGW("metrics has a EventConditionLink but doesn't have a condition"); @@ -414,9 +429,12 @@ bool initMetrics(const StatsdConfig& config, const unordered_map& l int conditionIndex = -1; if (metric.has_condition()) { - handleMetricWithConditions(metric.condition(), metricIndex, conditionTrackerMap, - metric.links(), allConditionTrackers, conditionIndex, - conditionToMetricMap); + bool good = handleMetricWithConditions( + metric.condition(), metricIndex, conditionTrackerMap, metric.links(), + allConditionTrackers, conditionIndex, conditionToMetricMap); + if (!good) { + return false; + } } else { if (metric.links_size() > 0) { ALOGW("metrics has a EventConditionLink but doesn't have a condition"); diff --git a/cmds/statsd/tests/MetricsManager_test.cpp b/cmds/statsd/tests/MetricsManager_test.cpp index 3dd4e70ff49c7..e118129a42d0d 100644 --- a/cmds/statsd/tests/MetricsManager_test.cpp +++ b/cmds/statsd/tests/MetricsManager_test.cpp @@ -163,6 +163,25 @@ StatsdConfig buildMissingMatchers() { return config; } +StatsdConfig buildMissingCondition() { + StatsdConfig config; + config.set_name("12345"); + + CountMetric* metric = config.add_count_metric(); + metric->set_name("3"); + metric->set_what("SCREEN_EVENT"); + metric->mutable_bucket()->set_bucket_size_millis(30 * 1000L); + metric->set_condition("SOME_CONDITION"); + + LogEntryMatcher* eventMatcher = config.add_log_entry_matcher(); + eventMatcher->set_name("SCREEN_EVENT"); + + SimpleLogEntryMatcher* simpleLogEntryMatcher = eventMatcher->mutable_simple_log_entry_matcher(); + simpleLogEntryMatcher->set_tag(2); + + return config; +} + StatsdConfig buildDimensionMetricsWithMultiTags() { StatsdConfig config; config.set_name("12345"); @@ -308,6 +327,21 @@ TEST(MetricsManagerTest, TestMissingMatchers) { trackerToMetricMap, trackerToConditionMap)); } +TEST(MetricsManagerTest, TestMissingCondition) { + StatsdConfig config = buildMissingCondition(); + set allTagIds; + vector> allLogEntryMatchers; + vector> allConditionTrackers; + vector> allMetricProducers; + std::vector> allAnomalyTrackers; + unordered_map> conditionToMetricMap; + unordered_map> trackerToMetricMap; + unordered_map> trackerToConditionMap; + EXPECT_FALSE(initStatsdConfig(config, allTagIds, allLogEntryMatchers, allConditionTrackers, + allMetricProducers, allAnomalyTrackers, conditionToMetricMap, + trackerToMetricMap, trackerToConditionMap)); +} + TEST(MetricsManagerTest, TestCircleConditionDependency) { StatsdConfig config = buildCircleConditions(); set allTagIds;