Refactor determine metric update status

Refactor and create a generic determineMetricUpdateStatus() function
that works for all metrics. Will simplify the process of determining
if a metric needs an update for all future metric types.

Test: atest statsd_test
Bug: 168863134
Change-Id: If89e1625cf7536406fbd168aca63193d5e9a908f
This commit is contained in:
Tej Singh
2020-09-16 15:31:33 -07:00
parent 0f7f7b4882
commit 13264fbbe7
3 changed files with 124 additions and 88 deletions

View File

@@ -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<int64_t, int>& oldMetricProducerMap,
const vector<sp<MetricProducer>>& oldMetricProducers,
const unordered_map<int64_t, int>& metricToActivationMap,
const set<int64_t>& replacedMatchers,
const set<int64_t>& 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<int64_t>& matcherDependencies,
const set<int64_t>& conditionDependencies,
const ::google::protobuf::RepeatedField<int64_t>& stateDependencies,
const ::google::protobuf::RepeatedPtrField<MetricConditionLink>& conditionLinks,
const unordered_map<int64_t, int>& oldMetricProducerMap,
const vector<sp<MetricProducer>>& oldMetricProducers,
const unordered_map<int64_t, int>& metricToActivationMap,
const set<int64_t>& replacedMatchers, const set<int64_t>& replacedConditions,
const set<int64_t>& 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<MetricProducer> 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<int64_t> 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<int64_t, int>& oldMetricProducerMap,
const vector<sp<MetricProducer>>& oldMetricProducers,
const unordered_map<int64_t, int>& metricToActivationMap,
const set<int64_t>& replacedMatchers,
const set<int64_t>& replacedConditions,
const set<int64_t>& replacedStates,
vector<UpdateStatus>& metricsToUpdate) {
int metricIndex = 0;
for (int i = 0; i < config.event_metric_size(); i++, metricIndex++) {
const EventMetric& metric = config.event_metric(i);
set<int64_t> conditionDependencies;
if (metric.has_condition()) {
conditionDependencies.insert(metric.condition());
}
if (!determineMetricUpdateStatus(
config, metric, metric.id(), METRIC_TYPE_EVENT, {metric.what()},
conditionDependencies, ::google::protobuf::RepeatedField<int64_t>(),
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<StatsPullerManager>& pullerManager,
const unordered_map<int64_t, int>& oldAtomMatchingTrackerMap,
@@ -518,22 +564,16 @@ bool updateMetrics(const ConfigKey& key, const StatsdConfig& config, const int64
}
vector<UpdateStatus> 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: {

View File

@@ -125,23 +125,25 @@ bool updateConditions(const ConfigKey& key, const StatsdConfig& config,
std::vector<ConditionState>& conditionCache,
std::set<int64_t>& 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<int64_t, int>& oldMetricProducerMap,
const vector<sp<MetricProducer>>& oldMetricProducers,
const unordered_map<int64_t, int>& metricToActivationMap,
const set<int64_t>& replacedMatchers,
const set<int64_t>& replacedConditions,
UpdateStatus& updateStatus);
const set<int64_t>& replacedStates,
vector<UpdateStatus>& metricsToUpdate);
// Update MetricProducers.
// input:

View File

@@ -920,14 +920,13 @@ TEST_F(ConfigUpdateTest, TestEventMetricPreserve) {
// Create an initial config.
EXPECT_TRUE(initConfig(config));
set<int64_t> replacedMatchers;
set<int64_t> replacedConditions;
unordered_map<int64_t, int> metricToActivationMap;
UpdateStatus status = UPDATE_UNKNOWN;
EXPECT_TRUE(determineEventMetricUpdateStatus(config, *metric, oldMetricProducerMap,
oldMetricProducers, metricToActivationMap,
replacedMatchers, replacedConditions, status));
EXPECT_EQ(status, UPDATE_PRESERVE);
vector<UpdateStatus> 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<int64_t> replacedMatchers;
set<int64_t> replacedConditions;
unordered_map<int64_t, int> metricToActivationMap = {{12345, 0}};
UpdateStatus status = UPDATE_UNKNOWN;
EXPECT_TRUE(determineEventMetricUpdateStatus(config, *metric, oldMetricProducerMap,
oldMetricProducers, metricToActivationMap,
replacedMatchers, replacedConditions, status));
EXPECT_EQ(status, UPDATE_REPLACE);
vector<UpdateStatus> 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<int64_t> replacedMatchers = {whatMatcher.id()};
set<int64_t> replacedConditions;
unordered_map<int64_t, int> metricToActivationMap;
UpdateStatus status = UPDATE_UNKNOWN;
EXPECT_TRUE(determineEventMetricUpdateStatus(config, *metric, oldMetricProducerMap,
oldMetricProducers, metricToActivationMap,
replacedMatchers, replacedConditions, status));
EXPECT_EQ(status, UPDATE_REPLACE);
vector<UpdateStatus> 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<int64_t> replacedMatchers;
set<int64_t> replacedConditions = {predicate.id()};
unordered_map<int64_t, int> metricToActivationMap;
UpdateStatus status = UPDATE_UNKNOWN;
EXPECT_TRUE(determineEventMetricUpdateStatus(config, *metric, oldMetricProducerMap,
oldMetricProducers, metricToActivationMap,
replacedMatchers, replacedConditions, status));
EXPECT_EQ(status, UPDATE_REPLACE);
vector<UpdateStatus> 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<int64_t> replacedMatchers;
set<int64_t> replacedConditions = {linkPredicate.id()};
unordered_map<int64_t, int> metricToActivationMap;
UpdateStatus status = UPDATE_UNKNOWN;
EXPECT_TRUE(determineEventMetricUpdateStatus(config, *metric, oldMetricProducerMap,
oldMetricProducers, metricToActivationMap,
replacedMatchers, replacedConditions, status));
EXPECT_EQ(status, UPDATE_REPLACE);
vector<UpdateStatus> 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<int64_t> replacedMatchers = {startMatcher.id()}; // The activation matcher is replaced.
set<int64_t> replacedConditions;
unordered_map<int64_t, int> metricToActivationMap = {{12345, 0}};
UpdateStatus status = UPDATE_UNKNOWN;
EXPECT_TRUE(determineEventMetricUpdateStatus(config, *metric, oldMetricProducerMap,
oldMetricProducers, metricToActivationMap,
replacedMatchers, replacedConditions, status));
EXPECT_EQ(status, UPDATE_REPLACE);
vector<UpdateStatus> 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) {