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 5dbf16deb5527..2ae57638791e6 100644 --- a/cmds/statsd/src/metrics/parsing_utils/config_update_utils.cpp +++ b/cmds/statsd/src/metrics/parsing_utils/config_update_utils.cpp @@ -24,6 +24,8 @@ #include "matchers/EventMatcherWizard.h" #include "metrics_manager_util.h" +using google::protobuf::MessageLite; + namespace android { namespace os { namespace statsd { @@ -419,16 +421,19 @@ bool metricActivationDepsChange(const StatsdConfig& config, return false; } -bool determineEventMetricUpdateStatus(const StatsdConfig& config, const EventMetric& metric, - const unordered_map& oldMetricProducerMap, - const vector>& oldMetricProducers, - const unordered_map& metricToActivationMap, - const set& replacedMatchers, - const set& replacedConditions, - UpdateStatus& updateStatus) { - int64_t id = metric.id(); +bool determineMetricUpdateStatus( + const StatsdConfig& config, const MessageLite& metric, const int64_t metricId, + const MetricType metricType, const set& matcherDependencies, + const set& conditionDependencies, + const ::google::protobuf::RepeatedField& stateDependencies, + const ::google::protobuf::RepeatedPtrField& conditionLinks, + const unordered_map& oldMetricProducerMap, + const vector>& oldMetricProducers, + const unordered_map& metricToActivationMap, + const set& replacedMatchers, const set& replacedConditions, + const set& replacedStates, UpdateStatus& updateStatus) { // Check if new metric - const auto& oldMetricProducerIt = oldMetricProducerMap.find(id); + const auto& oldMetricProducerIt = oldMetricProducerMap.find(metricId); if (oldMetricProducerIt == oldMetricProducerMap.end()) { updateStatus = UPDATE_NEW; return true; @@ -436,44 +441,85 @@ bool determineEventMetricUpdateStatus(const StatsdConfig& config, const EventMet // This is an existing metric, check if it has changed. uint64_t metricHash; - if (!getMetricProtoHash(config, metric, id, metricToActivationMap, metricHash)) { + if (!getMetricProtoHash(config, metric, metricId, metricToActivationMap, metricHash)) { return false; } const sp oldMetricProducer = oldMetricProducers[oldMetricProducerIt->second]; - if (oldMetricProducer->getMetricType() != METRIC_TYPE_EVENT || + if (oldMetricProducer->getMetricType() != metricType || oldMetricProducer->getProtoHash() != metricHash) { updateStatus = UPDATE_REPLACE; return true; } - // Metric type and definition are the same. Need to check dependencies to see if they changed. - if (replacedMatchers.find(metric.what()) != replacedMatchers.end()) { + // Take intersections of the matchers/predicates/states that the metric + // depends on with those that have been replaced. If a metric depends on any + // replaced component, it too must be replaced. + set intersection; + set_intersection(matcherDependencies.begin(), matcherDependencies.end(), + replacedMatchers.begin(), replacedMatchers.end(), + inserter(intersection, intersection.begin())); + if (intersection.size() > 0) { + updateStatus = UPDATE_REPLACE; + return true; + } + set_intersection(conditionDependencies.begin(), conditionDependencies.end(), + replacedConditions.begin(), replacedConditions.end(), + inserter(intersection, intersection.begin())); + if (intersection.size() > 0) { + updateStatus = UPDATE_REPLACE; + return true; + } + set_intersection(stateDependencies.begin(), stateDependencies.end(), replacedStates.begin(), + replacedStates.end(), inserter(intersection, intersection.begin())); + if (intersection.size() > 0) { updateStatus = UPDATE_REPLACE; return true; } - if (metric.has_condition()) { - if (replacedConditions.find(metric.condition()) != replacedConditions.end()) { - updateStatus = UPDATE_REPLACE; - return true; - } - } - - if (metricActivationDepsChange(config, metricToActivationMap, id, replacedMatchers)) { - updateStatus = UPDATE_REPLACE; - return true; - } - - for (const auto& metricConditionLink : metric.links()) { + for (const auto& metricConditionLink : conditionLinks) { if (replacedConditions.find(metricConditionLink.condition()) != replacedConditions.end()) { updateStatus = UPDATE_REPLACE; return true; } } + + if (metricActivationDepsChange(config, metricToActivationMap, metricId, replacedMatchers)) { + updateStatus = UPDATE_REPLACE; + return true; + } + updateStatus = UPDATE_PRESERVE; return true; } +bool determineAllMetricUpdateStatuses(const StatsdConfig& config, + const unordered_map& oldMetricProducerMap, + const vector>& oldMetricProducers, + const unordered_map& metricToActivationMap, + const set& replacedMatchers, + const set& replacedConditions, + const set& replacedStates, + vector& metricsToUpdate) { + int metricIndex = 0; + for (int i = 0; i < config.event_metric_size(); i++, metricIndex++) { + const EventMetric& metric = config.event_metric(i); + set conditionDependencies; + if (metric.has_condition()) { + conditionDependencies.insert(metric.condition()); + } + if (!determineMetricUpdateStatus( + config, metric, metric.id(), METRIC_TYPE_EVENT, {metric.what()}, + conditionDependencies, ::google::protobuf::RepeatedField(), + metric.links(), oldMetricProducerMap, oldMetricProducers, metricToActivationMap, + replacedMatchers, replacedConditions, replacedStates, + metricsToUpdate[metricIndex])) { + return false; + } + } + // TODO: determine update status for count, gauge, value, duration metrics. + return true; +} + bool updateMetrics(const ConfigKey& key, const StatsdConfig& config, const int64_t timeBaseNs, const int64_t currentTimeNs, const sp& pullerManager, const unordered_map& oldAtomMatchingTrackerMap, @@ -518,22 +564,16 @@ bool updateMetrics(const ConfigKey& key, const StatsdConfig& config, const int64 } vector metricsToUpdate(allMetricsCount, UPDATE_UNKNOWN); + if (!determineAllMetricUpdateStatuses(config, oldMetricProducerMap, oldMetricProducers, + metricToActivationMap, replacedMatchers, + replacedConditions, replacedStates, metricsToUpdate)) { + return false; + } + + // Now, perform the update. Must iterate the metric types in the same order int metricIndex = 0; for (int i = 0; i < config.event_metric_size(); i++, metricIndex++) { newMetricProducerMap[config.event_metric(i).id()] = metricIndex; - if (!determineEventMetricUpdateStatus(config, config.event_metric(i), oldMetricProducerMap, - oldMetricProducers, metricToActivationMap, - replacedMatchers, replacedConditions, - metricsToUpdate[metricIndex])) { - return false; - } - } - - // TODO: determine update status for count, gauge, value, duration metrics. - - // Now, perform the update. Must iterate the metric types in the same order - metricIndex = 0; - for (int i = 0; i < config.event_metric_size(); i++, metricIndex++) { const EventMetric& metric = config.event_metric(i); switch (metricsToUpdate[metricIndex]) { case UPDATE_PRESERVE: { 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 1cd0ce524b312..34d7e9c7de9e3 100644 --- a/cmds/statsd/src/metrics/parsing_utils/config_update_utils.h +++ b/cmds/statsd/src/metrics/parsing_utils/config_update_utils.h @@ -125,23 +125,25 @@ bool updateConditions(const ConfigKey& key, const StatsdConfig& config, std::vector& conditionCache, std::set& replacedConditions); -// Function to determine if an event metric needs to be updated. Populates updateStatus. +// Function to determine the update status (preserve/replace/new) of all metrics in the config. // [config]: the input StatsdConfig -// [metric]: the current metric to be updated // [oldMetricProducerMap]: metric id to index mapping in the existing MetricsManager // [oldMetricProducers]: stores the existing MetricProducers -// [metricToActivationMap]: map from metric id to metric activation index. -// [replacedMatchers]: set of replaced matcher ids. conditions using these matchers must be replaced +// [metricToActivationMap]: map from metric id to metric activation index +// [replacedMatchers]: set of replaced matcher ids. metrics using these matchers must be replaced +// [replacedConditions]: set of replaced conditions. metrics using these conditions must be replaced +// [replacedStates]: set of replaced state ids. metrics using these states must be replaced // output: -// [updateStatus]: update status for the metric. Will be changed from UPDATE_UNKNOWN after this call +// [metricsToUpdate]: update status of each metric. Will be changed from UPDATE_UNKNOWN // Returns whether the function was successful or not. -bool determineEventMetricUpdateStatus(const StatsdConfig& config, const EventMetric& metric, +bool determineAllMetricUpdateStatuses(const StatsdConfig& config, const unordered_map& oldMetricProducerMap, const vector>& oldMetricProducers, const unordered_map& metricToActivationMap, const set& replacedMatchers, const set& replacedConditions, - UpdateStatus& updateStatus); + const set& replacedStates, + vector& metricsToUpdate); // Update MetricProducers. // input: 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 076000fab80f1..65e5875e39bf2 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 @@ -920,14 +920,13 @@ TEST_F(ConfigUpdateTest, TestEventMetricPreserve) { // Create an initial config. EXPECT_TRUE(initConfig(config)); - set replacedMatchers; - set replacedConditions; unordered_map metricToActivationMap; - UpdateStatus status = UPDATE_UNKNOWN; - EXPECT_TRUE(determineEventMetricUpdateStatus(config, *metric, oldMetricProducerMap, - oldMetricProducers, metricToActivationMap, - replacedMatchers, replacedConditions, status)); - EXPECT_EQ(status, UPDATE_PRESERVE); + vector metricsToUpdate(1, UPDATE_UNKNOWN); + EXPECT_TRUE(determineAllMetricUpdateStatuses(config, oldMetricProducerMap, oldMetricProducers, + metricToActivationMap, + /*replacedMatchers*/ {}, /*replacedConditions=*/{}, + /*replacedStates=*/{}, metricsToUpdate)); + EXPECT_EQ(metricsToUpdate[0], UPDATE_PRESERVE); } TEST_F(ConfigUpdateTest, TestEventMetricActivationAdded) { @@ -957,14 +956,13 @@ TEST_F(ConfigUpdateTest, TestEventMetricActivationAdded) { eventActivation->set_atom_matcher_id(startMatcher.id()); eventActivation->set_ttl_seconds(5); - set replacedMatchers; - set replacedConditions; unordered_map metricToActivationMap = {{12345, 0}}; - UpdateStatus status = UPDATE_UNKNOWN; - EXPECT_TRUE(determineEventMetricUpdateStatus(config, *metric, oldMetricProducerMap, - oldMetricProducers, metricToActivationMap, - replacedMatchers, replacedConditions, status)); - EXPECT_EQ(status, UPDATE_REPLACE); + vector metricsToUpdate(1, UPDATE_UNKNOWN); + EXPECT_TRUE(determineAllMetricUpdateStatuses(config, oldMetricProducerMap, oldMetricProducers, + metricToActivationMap, + /*replacedMatchers*/ {}, /*replacedConditions=*/{}, + /*replacedStates=*/{}, metricsToUpdate)); + EXPECT_EQ(metricsToUpdate[0], UPDATE_REPLACE); } TEST_F(ConfigUpdateTest, TestEventMetricWhatChanged) { @@ -987,14 +985,13 @@ TEST_F(ConfigUpdateTest, TestEventMetricWhatChanged) { // Create an initial config. EXPECT_TRUE(initConfig(config)); - set replacedMatchers = {whatMatcher.id()}; - set replacedConditions; unordered_map metricToActivationMap; - UpdateStatus status = UPDATE_UNKNOWN; - EXPECT_TRUE(determineEventMetricUpdateStatus(config, *metric, oldMetricProducerMap, - oldMetricProducers, metricToActivationMap, - replacedMatchers, replacedConditions, status)); - EXPECT_EQ(status, UPDATE_REPLACE); + vector metricsToUpdate(1, UPDATE_UNKNOWN); + EXPECT_TRUE(determineAllMetricUpdateStatuses( + config, oldMetricProducerMap, oldMetricProducers, metricToActivationMap, + /*replacedMatchers*/ {whatMatcher.id()}, /*replacedConditions=*/{}, + /*replacedStates=*/{}, metricsToUpdate)); + EXPECT_EQ(metricsToUpdate[0], UPDATE_REPLACE); } TEST_F(ConfigUpdateTest, TestEventMetricConditionChanged) { @@ -1017,14 +1014,13 @@ TEST_F(ConfigUpdateTest, TestEventMetricConditionChanged) { // Create an initial config. EXPECT_TRUE(initConfig(config)); - set replacedMatchers; - set replacedConditions = {predicate.id()}; unordered_map metricToActivationMap; - UpdateStatus status = UPDATE_UNKNOWN; - EXPECT_TRUE(determineEventMetricUpdateStatus(config, *metric, oldMetricProducerMap, - oldMetricProducers, metricToActivationMap, - replacedMatchers, replacedConditions, status)); - EXPECT_EQ(status, UPDATE_REPLACE); + vector metricsToUpdate(1, UPDATE_UNKNOWN); + EXPECT_TRUE(determineAllMetricUpdateStatuses( + config, oldMetricProducerMap, oldMetricProducers, metricToActivationMap, + /*replacedMatchers*/ {}, /*replacedConditions=*/{predicate.id()}, + /*replacedStates=*/{}, metricsToUpdate)); + EXPECT_EQ(metricsToUpdate[0], UPDATE_REPLACE); } TEST_F(ConfigUpdateTest, TestMetricConditionLinkDepsChanged) { @@ -1054,14 +1050,13 @@ TEST_F(ConfigUpdateTest, TestMetricConditionLinkDepsChanged) { // Create an initial config. EXPECT_TRUE(initConfig(config)); - set replacedMatchers; - set replacedConditions = {linkPredicate.id()}; unordered_map metricToActivationMap; - UpdateStatus status = UPDATE_UNKNOWN; - EXPECT_TRUE(determineEventMetricUpdateStatus(config, *metric, oldMetricProducerMap, - oldMetricProducers, metricToActivationMap, - replacedMatchers, replacedConditions, status)); - EXPECT_EQ(status, UPDATE_REPLACE); + vector metricsToUpdate(1, UPDATE_UNKNOWN); + EXPECT_TRUE(determineAllMetricUpdateStatuses( + config, oldMetricProducerMap, oldMetricProducers, metricToActivationMap, + /*replacedMatchers*/ {}, /*replacedConditions=*/{linkPredicate.id()}, + /*replacedStates=*/{}, metricsToUpdate)); + EXPECT_EQ(metricsToUpdate[0], UPDATE_REPLACE); } TEST_F(ConfigUpdateTest, TestEventMetricActivationDepsChange) { @@ -1090,14 +1085,13 @@ TEST_F(ConfigUpdateTest, TestEventMetricActivationDepsChange) { // Create an initial config. EXPECT_TRUE(initConfig(config)); - set replacedMatchers = {startMatcher.id()}; // The activation matcher is replaced. - set replacedConditions; unordered_map metricToActivationMap = {{12345, 0}}; - UpdateStatus status = UPDATE_UNKNOWN; - EXPECT_TRUE(determineEventMetricUpdateStatus(config, *metric, oldMetricProducerMap, - oldMetricProducers, metricToActivationMap, - replacedMatchers, replacedConditions, status)); - EXPECT_EQ(status, UPDATE_REPLACE); + vector metricsToUpdate(1, UPDATE_UNKNOWN); + EXPECT_TRUE(determineAllMetricUpdateStatuses( + config, oldMetricProducerMap, oldMetricProducers, metricToActivationMap, + /*replacedMatchers*/ {startMatcher.id()}, /*replacedConditions=*/{}, + /*replacedStates=*/{}, metricsToUpdate)); + EXPECT_EQ(metricsToUpdate[0], UPDATE_REPLACE); } TEST_F(ConfigUpdateTest, TestUpdateEventMetrics) {