From 26943095b311ac4d12986c2c56600708ed3550c2 Mon Sep 17 00:00:00 2001 From: Tej Singh Date: Tue, 6 Oct 2020 13:18:56 -0700 Subject: [PATCH] Perform partial update for gauge metric Creates a new metric producer if either the metric definition or any of its dependencies changed. Appropriately updates all indices both within the producer and for MetricsManager if the metric will be preserved. Test: atest statsd_test Bug: 162323123 Change-Id: I414668c81dd71073285cc82b8f567d201344c5cd --- .../src/metrics/GaugeMetricProducer.cpp | 58 +++- cmds/statsd/src/metrics/GaugeMetricProducer.h | 24 +- cmds/statsd/src/metrics/MetricProducer.h | 1 + .../parsing_utils/config_update_utils.cpp | 139 +++++--- .../parsing_utils/metrics_manager_util.cpp | 269 ++++++++------- .../parsing_utils/metrics_manager_util.h | 27 +- .../config_update_utils_test.cpp | 315 ++++++++++++++++-- 7 files changed, 628 insertions(+), 205 deletions(-) diff --git a/cmds/statsd/src/metrics/GaugeMetricProducer.cpp b/cmds/statsd/src/metrics/GaugeMetricProducer.cpp index 9dda248a6d1f1..2a37b587fbce3 100644 --- a/cmds/statsd/src/metrics/GaugeMetricProducer.cpp +++ b/cmds/statsd/src/metrics/GaugeMetricProducer.cpp @@ -17,9 +17,11 @@ #define DEBUG false // STOPSHIP if true #include "Log.h" -#include "../guardrail/StatsdStats.h" #include "GaugeMetricProducer.h" -#include "../stats_log_util.h" + +#include "guardrail/StatsdStats.h" +#include "metrics/parsing_utils/metrics_manager_util.h" +#include "stats_log_util.h" using android::util::FIELD_COUNT_REPEATED; using android::util::FIELD_TYPE_BOOL; @@ -154,6 +156,58 @@ GaugeMetricProducer::~GaugeMetricProducer() { } } +bool GaugeMetricProducer::onConfigUpdatedLocked( + const StatsdConfig& config, const int configIndex, const int metricIndex, + const vector>& allAtomMatchingTrackers, + const unordered_map& oldAtomMatchingTrackerMap, + const unordered_map& newAtomMatchingTrackerMap, + const sp& matcherWizard, + const vector>& allConditionTrackers, + const unordered_map& conditionTrackerMap, const sp& wizard, + const unordered_map& metricToActivationMap, + unordered_map>& trackerToMetricMap, + unordered_map>& conditionToMetricMap, + unordered_map>& activationAtomTrackerToMetricMap, + unordered_map>& deactivationAtomTrackerToMetricMap, + vector& metricsWithActivation) { + if (!MetricProducer::onConfigUpdatedLocked( + config, configIndex, metricIndex, allAtomMatchingTrackers, + oldAtomMatchingTrackerMap, newAtomMatchingTrackerMap, matcherWizard, + allConditionTrackers, conditionTrackerMap, wizard, metricToActivationMap, + trackerToMetricMap, conditionToMetricMap, activationAtomTrackerToMetricMap, + deactivationAtomTrackerToMetricMap, metricsWithActivation)) { + return false; + } + + const GaugeMetric& metric = config.gauge_metric(configIndex); + // Update appropriate indices: mWhatMatcherIndex, mConditionIndex and MetricsManager maps. + if (!handleMetricWithAtomMatchingTrackers(metric.what(), metricIndex, /*enforceOneAtom=*/false, + allAtomMatchingTrackers, newAtomMatchingTrackerMap, + trackerToMetricMap, mWhatMatcherIndex)) { + return false; + } + + // Need to update maps since the index changed, but mTriggerAtomId will not change. + int triggerTrackerIndex; + if (metric.has_trigger_event() && + !handleMetricWithAtomMatchingTrackers(metric.trigger_event(), metricIndex, + /*enforceOneAtom=*/true, allAtomMatchingTrackers, + newAtomMatchingTrackerMap, trackerToMetricMap, + triggerTrackerIndex)) { + return false; + } + + if (metric.has_condition() && + !handleMetricWithConditions(metric.condition(), metricIndex, conditionTrackerMap, + metric.links(), allConditionTrackers, mConditionTrackerIndex, + conditionToMetricMap)) { + return false; + } + sp tmpEventWizard = mEventMatcherWizard; + mEventMatcherWizard = matcherWizard; + return true; +} + void GaugeMetricProducer::dumpStatesLocked(FILE* out, bool verbose) const { if (mCurrentSlicedBucket == nullptr || mCurrentSlicedBucket->size() == 0) { diff --git a/cmds/statsd/src/metrics/GaugeMetricProducer.h b/cmds/statsd/src/metrics/GaugeMetricProducer.h index e933d4b197165..9bdaac96c9ef7 100644 --- a/cmds/statsd/src/metrics/GaugeMetricProducer.h +++ b/cmds/statsd/src/metrics/GaugeMetricProducer.h @@ -53,8 +53,8 @@ typedef std::unordered_map> // This gauge metric producer first register the puller to automatically pull the gauge at the // beginning of each bucket. If the condition is met, insert it to the bucket info. Otherwise // proactively pull the gauge when the condition is changed to be true. Therefore, the gauge metric -// producer always reports the guage at the earliest time of the bucket when the condition is met. -class GaugeMetricProducer : public virtual MetricProducer, public virtual PullDataReceiver { +// producer always reports the gauge at the earliest time of the bucket when the condition is met. +class GaugeMetricProducer : public MetricProducer, public virtual PullDataReceiver { public: GaugeMetricProducer( const ConfigKey& key, const GaugeMetric& gaugeMetric, const int conditionIndex, @@ -142,7 +142,23 @@ private: void pullAndMatchEventsLocked(const int64_t timestampNs); - const int mWhatMatcherIndex; + bool onConfigUpdatedLocked( + const StatsdConfig& config, const int configIndex, const int metricIndex, + const std::vector>& allAtomMatchingTrackers, + const std::unordered_map& oldAtomMatchingTrackerMap, + const std::unordered_map& newAtomMatchingTrackerMap, + const sp& matcherWizard, + const std::vector>& allConditionTrackers, + const std::unordered_map& conditionTrackerMap, + const sp& wizard, + const std::unordered_map& metricToActivationMap, + std::unordered_map>& trackerToMetricMap, + std::unordered_map>& conditionToMetricMap, + std::unordered_map>& activationAtomTrackerToMetricMap, + std::unordered_map>& deactivationAtomTrackerToMetricMap, + std::vector& metricsWithActivation) override; + + int mWhatMatcherIndex; sp mEventMatcherWizard; @@ -209,6 +225,8 @@ private: FRIEND_TEST(GaugeMetricProducerTest_PartialBucket, TestPushedEvents); FRIEND_TEST(GaugeMetricProducerTest_PartialBucket, TestPulled); + + FRIEND_TEST(ConfigUpdateTest, TestUpdateGaugeMetrics); }; } // namespace statsd diff --git a/cmds/statsd/src/metrics/MetricProducer.h b/cmds/statsd/src/metrics/MetricProducer.h index 4360010746ae6..18e62d28ba464 100644 --- a/cmds/statsd/src/metrics/MetricProducer.h +++ b/cmds/statsd/src/metrics/MetricProducer.h @@ -568,6 +568,7 @@ protected: FRIEND_TEST(ConfigUpdateTest, TestUpdateMetricActivations); FRIEND_TEST(ConfigUpdateTest, TestUpdateCountMetrics); FRIEND_TEST(ConfigUpdateTest, TestUpdateEventMetrics); + FRIEND_TEST(ConfigUpdateTest, TestUpdateGaugeMetrics); FRIEND_TEST(ConfigUpdateTest, TestUpdateMetricsMultipleTypes); }; 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 8477dba4abe52..93795d4b25e63 100644 --- a/cmds/statsd/src/metrics/parsing_utils/config_update_utils.cpp +++ b/cmds/statsd/src/metrics/parsing_utils/config_update_utils.cpp @@ -556,6 +556,44 @@ bool determineAllMetricUpdateStatuses(const StatsdConfig& config, return true; } +// Called when a metric is preserved during a config update. Finds the metric in oldMetricProducers +// and calls onConfigUpdated to update all indices. +optional> updateMetric( + const StatsdConfig& config, const int configIndex, const int metricIndex, + const int64_t metricId, const vector>& allAtomMatchingTrackers, + const unordered_map& oldAtomMatchingTrackerMap, + const unordered_map& newAtomMatchingTrackerMap, + const sp& matcherWizard, + const vector>& allConditionTrackers, + const unordered_map& conditionTrackerMap, const sp& wizard, + const unordered_map& oldMetricProducerMap, + const vector>& oldMetricProducers, + const unordered_map& metricToActivationMap, + unordered_map>& trackerToMetricMap, + unordered_map>& conditionToMetricMap, + unordered_map>& activationAtomTrackerToMetricMap, + unordered_map>& deactivationAtomTrackerToMetricMap, + vector& metricsWithActivation) { + const auto& oldMetricProducerIt = oldMetricProducerMap.find(metricId); + if (oldMetricProducerIt == oldMetricProducerMap.end()) { + ALOGE("Could not find Metric %lld in the previous config, but expected it " + "to be there", + (long long)metricId); + return nullopt; + } + const int oldIndex = oldMetricProducerIt->second; + sp producer = oldMetricProducers[oldIndex]; + if (!producer->onConfigUpdated(config, configIndex, metricIndex, allAtomMatchingTrackers, + oldAtomMatchingTrackerMap, newAtomMatchingTrackerMap, + matcherWizard, allConditionTrackers, conditionTrackerMap, wizard, + metricToActivationMap, trackerToMetricMap, conditionToMetricMap, + activationAtomTrackerToMetricMap, + deactivationAtomTrackerToMetricMap, metricsWithActivation)) { + return nullopt; + } + return {producer}; +} + bool updateMetrics(const ConfigKey& key, const StatsdConfig& config, const int64_t timeBaseNs, const int64_t currentTimeNs, const sp& pullerManager, const unordered_map& oldAtomMatchingTrackerMap, @@ -609,41 +647,29 @@ bool updateMetrics(const ConfigKey& key, const StatsdConfig& config, const int64 // Now, perform the update. Must iterate the metric types in the same order int metricIndex = 0; for (int i = 0; i < config.count_metric_size(); i++, metricIndex++) { - newMetricProducerMap[config.count_metric(i).id()] = metricIndex; const CountMetric& metric = config.count_metric(i); + newMetricProducerMap[metric.id()] = metricIndex; + optional> producer; switch (metricsToUpdate[metricIndex]) { case UPDATE_PRESERVE: { - const auto& oldMetricProducerIt = oldMetricProducerMap.find(metric.id()); - if (oldMetricProducerIt == oldMetricProducerMap.end()) { - ALOGE("Could not find Metric %lld in the previous config, but expected it " - "to be there", - (long long)metric.id()); - return false; - } - const int oldIndex = oldMetricProducerIt->second; - sp producer = oldMetricProducers[oldIndex]; - producer->onConfigUpdated( - config, i, metricIndex, allAtomMatchingTrackers, oldAtomMatchingTrackerMap, - newAtomMatchingTrackerMap, matcherWizard, allConditionTrackers, - conditionTrackerMap, wizard, metricToActivationMap, trackerToMetricMap, + producer = updateMetric( + config, i, metricIndex, metric.id(), allAtomMatchingTrackers, + oldAtomMatchingTrackerMap, newAtomMatchingTrackerMap, matcherWizard, + allConditionTrackers, conditionTrackerMap, wizard, oldMetricProducerMap, + oldMetricProducers, metricToActivationMap, trackerToMetricMap, conditionToMetricMap, activationAtomTrackerToMetricMap, deactivationAtomTrackerToMetricMap, metricsWithActivation); - newMetricProducers.push_back(producer); break; } case UPDATE_REPLACE: case UPDATE_NEW: { - sp producer = createCountMetricProducerAndUpdateMetadata( + producer = createCountMetricProducerAndUpdateMetadata( key, config, timeBaseNs, currentTimeNs, metric, metricIndex, allAtomMatchingTrackers, newAtomMatchingTrackerMap, allConditionTrackers, conditionTrackerMap, initialConditionCache, wizard, stateAtomIdMap, allStateGroupMaps, metricToActivationMap, trackerToMetricMap, conditionToMetricMap, activationAtomTrackerToMetricMap, deactivationAtomTrackerToMetricMap, metricsWithActivation); - if (producer == nullptr) { - return false; - } - newMetricProducers.push_back(producer); break; } default: { @@ -652,42 +678,34 @@ bool updateMetrics(const ConfigKey& key, const StatsdConfig& config, const int64 return false; } } + if (!producer) { + return false; + } + newMetricProducers.push_back(producer.value()); } for (int i = 0; i < config.event_metric_size(); i++, metricIndex++) { newMetricProducerMap[config.event_metric(i).id()] = metricIndex; const EventMetric& metric = config.event_metric(i); + optional> producer; switch (metricsToUpdate[metricIndex]) { case UPDATE_PRESERVE: { - const auto& oldMetricProducerIt = oldMetricProducerMap.find(metric.id()); - if (oldMetricProducerIt == oldMetricProducerMap.end()) { - ALOGE("Could not find Metric %lld in the previous config, but expected it " - "to be there", - (long long)metric.id()); - return false; - } - const int oldIndex = oldMetricProducerIt->second; - sp producer = oldMetricProducers[oldIndex]; - producer->onConfigUpdated( - config, i, metricIndex, allAtomMatchingTrackers, oldAtomMatchingTrackerMap, - newAtomMatchingTrackerMap, matcherWizard, allConditionTrackers, - conditionTrackerMap, wizard, metricToActivationMap, trackerToMetricMap, + producer = updateMetric( + config, i, metricIndex, metric.id(), allAtomMatchingTrackers, + oldAtomMatchingTrackerMap, newAtomMatchingTrackerMap, matcherWizard, + allConditionTrackers, conditionTrackerMap, wizard, oldMetricProducerMap, + oldMetricProducers, metricToActivationMap, trackerToMetricMap, conditionToMetricMap, activationAtomTrackerToMetricMap, deactivationAtomTrackerToMetricMap, metricsWithActivation); - newMetricProducers.push_back(producer); break; } case UPDATE_REPLACE: case UPDATE_NEW: { - sp producer = createEventMetricProducerAndUpdateMetadata( + producer = createEventMetricProducerAndUpdateMetadata( key, config, timeBaseNs, metric, metricIndex, allAtomMatchingTrackers, newAtomMatchingTrackerMap, allConditionTrackers, conditionTrackerMap, initialConditionCache, wizard, metricToActivationMap, trackerToMetricMap, conditionToMetricMap, activationAtomTrackerToMetricMap, deactivationAtomTrackerToMetricMap, metricsWithActivation); - if (producer == nullptr) { - return false; - } - newMetricProducers.push_back(producer); break; } default: { @@ -696,8 +714,49 @@ bool updateMetrics(const ConfigKey& key, const StatsdConfig& config, const int64 return false; } } + if (!producer) { + return false; + } + newMetricProducers.push_back(producer.value()); } - // TODO: perform update for count, gauge, value, duration metric. + for (int i = 0; i < config.gauge_metric_size(); i++, metricIndex++) { + const GaugeMetric& metric = config.gauge_metric(i); + newMetricProducerMap[metric.id()] = metricIndex; + optional> producer; + switch (metricsToUpdate[metricIndex]) { + case UPDATE_PRESERVE: { + producer = updateMetric( + config, i, metricIndex, metric.id(), allAtomMatchingTrackers, + oldAtomMatchingTrackerMap, newAtomMatchingTrackerMap, matcherWizard, + allConditionTrackers, conditionTrackerMap, wizard, oldMetricProducerMap, + oldMetricProducers, metricToActivationMap, trackerToMetricMap, + conditionToMetricMap, activationAtomTrackerToMetricMap, + deactivationAtomTrackerToMetricMap, metricsWithActivation); + break; + } + case UPDATE_REPLACE: + case UPDATE_NEW: { + producer = createGaugeMetricProducerAndUpdateMetadata( + key, config, timeBaseNs, currentTimeNs, pullerManager, metric, metricIndex, + allAtomMatchingTrackers, newAtomMatchingTrackerMap, allConditionTrackers, + conditionTrackerMap, initialConditionCache, wizard, matcherWizard, + metricToActivationMap, trackerToMetricMap, conditionToMetricMap, + activationAtomTrackerToMetricMap, deactivationAtomTrackerToMetricMap, + metricsWithActivation); + break; + } + default: { + ALOGE("Metric \"%lld\" update state is unknown. This should never happen", + (long long)metric.id()); + return false; + } + } + if (!producer) { + return false; + } + newMetricProducers.push_back(producer.value()); + } + // TODO: perform update for value, duration metric. const set atomsAllowedFromAnyUid(config.whitelisted_atom_ids().begin(), config.whitelisted_atom_ids().end()); 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 98c6222c43267..34e265c3b2ea9 100644 --- a/cmds/statsd/src/metrics/parsing_utils/metrics_manager_util.cpp +++ b/cmds/statsd/src/metrics/parsing_utils/metrics_manager_util.cpp @@ -348,7 +348,7 @@ bool handleMetricActivationOnConfigUpdate( return true; } -sp createCountMetricProducerAndUpdateMetadata( +optional> createCountMetricProducerAndUpdateMetadata( const ConfigKey& key, const StatsdConfig& config, const int64_t timeBaseNs, const int64_t currentTimeNs, const CountMetric& metric, const int metricIndex, const vector>& allAtomMatchingTrackers, @@ -366,14 +366,14 @@ sp createCountMetricProducerAndUpdateMetadata( vector& metricsWithActivation) { if (!metric.has_id() || !metric.has_what()) { ALOGW("cannot find metric id or \"what\" in CountMetric \"%lld\"", (long long)metric.id()); - return nullptr; + return nullopt; } int trackerIndex; if (!handleMetricWithAtomMatchingTrackers(metric.what(), metricIndex, metric.has_dimensions_in_what(), allAtomMatchingTrackers, atomMatchingTrackerMap, trackerToMetricMap, trackerIndex)) { - return nullptr; + return nullopt; } int conditionIndex = -1; @@ -381,12 +381,12 @@ sp createCountMetricProducerAndUpdateMetadata( if (!handleMetricWithConditions(metric.condition(), metricIndex, conditionTrackerMap, metric.links(), allConditionTrackers, conditionIndex, conditionToMetricMap)) { - return nullptr; + return nullopt; } } else { if (metric.links_size() > 0) { ALOGW("metrics has a MetricConditionLink but doesn't have a condition"); - return nullptr; + return nullopt; } } @@ -395,12 +395,12 @@ sp createCountMetricProducerAndUpdateMetadata( if (metric.slice_by_state_size() > 0) { if (!handleMetricWithStates(config, metric.slice_by_state(), stateAtomIdMap, allStateGroupMaps, slicedStateAtoms, stateGroupMap)) { - return nullptr; + return nullopt; } } else { if (metric.state_link_size() > 0) { ALOGW("CountMetric has a MetricStateLink but doesn't have a slice_by_state"); - return nullptr; + return nullopt; } } @@ -410,20 +410,20 @@ sp createCountMetricProducerAndUpdateMetadata( atomMatchingTrackerMap, activationAtomTrackerToMetricMap, deactivationAtomTrackerToMetricMap, metricsWithActivation, eventActivationMap, eventDeactivationMap)) { - return nullptr; + return nullopt; } uint64_t metricHash; if (!getMetricProtoHash(config, metric, metric.id(), metricToActivationMap, metricHash)) { - return nullptr; + return nullopt; } - return new CountMetricProducer(key, metric, conditionIndex, initialConditionCache, wizard, - metricHash, timeBaseNs, currentTimeNs, eventActivationMap, - eventDeactivationMap, slicedStateAtoms, stateGroupMap); + return {new CountMetricProducer(key, metric, conditionIndex, initialConditionCache, wizard, + metricHash, timeBaseNs, currentTimeNs, eventActivationMap, + eventDeactivationMap, slicedStateAtoms, stateGroupMap)}; } -sp createEventMetricProducerAndUpdateMetadata( +optional> createEventMetricProducerAndUpdateMetadata( const ConfigKey& key, const StatsdConfig& config, const int64_t timeBaseNs, const EventMetric& metric, const int metricIndex, const vector>& allAtomMatchingTrackers, @@ -439,13 +439,13 @@ sp createEventMetricProducerAndUpdateMetadata( vector& metricsWithActivation) { if (!metric.has_id() || !metric.has_what()) { ALOGW("cannot find the metric name or what in config"); - return nullptr; + return nullopt; } int trackerIndex; if (!handleMetricWithAtomMatchingTrackers(metric.what(), metricIndex, false, allAtomMatchingTrackers, atomMatchingTrackerMap, trackerToMetricMap, trackerIndex)) { - return nullptr; + return nullopt; } int conditionIndex = -1; @@ -453,12 +453,12 @@ sp createEventMetricProducerAndUpdateMetadata( if (!handleMetricWithConditions(metric.condition(), metricIndex, conditionTrackerMap, metric.links(), allConditionTrackers, conditionIndex, conditionToMetricMap)) { - return nullptr; + return nullopt; } } else { if (metric.links_size() > 0) { ALOGW("metrics has a MetricConditionLink but doesn't have a condition"); - return nullptr; + return nullopt; } } @@ -472,12 +472,125 @@ sp createEventMetricProducerAndUpdateMetadata( uint64_t metricHash; if (!getMetricProtoHash(config, metric, metric.id(), metricToActivationMap, metricHash)) { - return nullptr; + return nullopt; } - return new EventMetricProducer(key, metric, conditionIndex, initialConditionCache, wizard, - metricHash, timeBaseNs, eventActivationMap, - eventDeactivationMap); + return {new EventMetricProducer(key, metric, conditionIndex, initialConditionCache, wizard, + metricHash, timeBaseNs, eventActivationMap, + eventDeactivationMap)}; +} + +optional> createGaugeMetricProducerAndUpdateMetadata( + const ConfigKey& key, const StatsdConfig& config, const int64_t timeBaseNs, + const int64_t currentTimeNs, const sp& pullerManager, + const GaugeMetric& metric, const int metricIndex, + const vector>& allAtomMatchingTrackers, + const unordered_map& atomMatchingTrackerMap, + vector>& allConditionTrackers, + const unordered_map& conditionTrackerMap, + const vector& initialConditionCache, const sp& wizard, + const sp& matcherWizard, + const unordered_map& metricToActivationMap, + unordered_map>& trackerToMetricMap, + unordered_map>& conditionToMetricMap, + unordered_map>& activationAtomTrackerToMetricMap, + unordered_map>& deactivationAtomTrackerToMetricMap, + vector& metricsWithActivation) { + if (!metric.has_id() || !metric.has_what()) { + ALOGW("cannot find metric id or \"what\" in GaugeMetric \"%lld\"", (long long)metric.id()); + return nullopt; + } + + if ((!metric.gauge_fields_filter().has_include_all() || + (metric.gauge_fields_filter().include_all() == false)) && + !hasLeafNode(metric.gauge_fields_filter().fields())) { + ALOGW("Incorrect field filter setting in GaugeMetric %lld", (long long)metric.id()); + return nullopt; + } + if ((metric.gauge_fields_filter().has_include_all() && + metric.gauge_fields_filter().include_all() == true) && + hasLeafNode(metric.gauge_fields_filter().fields())) { + ALOGW("Incorrect field filter setting in GaugeMetric %lld", (long long)metric.id()); + return nullopt; + } + + int trackerIndex; + if (!handleMetricWithAtomMatchingTrackers(metric.what(), metricIndex, + metric.has_dimensions_in_what(), + allAtomMatchingTrackers, atomMatchingTrackerMap, + trackerToMetricMap, trackerIndex)) { + return nullopt; + } + + sp atomMatcher = allAtomMatchingTrackers.at(trackerIndex); + // For GaugeMetric atom, it should be simple matcher with one tagId. + if (atomMatcher->getAtomIds().size() != 1) { + return nullopt; + } + int atomTagId = *(atomMatcher->getAtomIds().begin()); + int pullTagId = pullerManager->PullerForMatcherExists(atomTagId) ? atomTagId : -1; + + int triggerTrackerIndex; + int triggerAtomId = -1; + if (metric.has_trigger_event()) { + if (pullTagId == -1) { + ALOGW("Pull atom not specified for trigger"); + return nullopt; + } + // trigger_event should be used with FIRST_N_SAMPLES + if (metric.sampling_type() != GaugeMetric::FIRST_N_SAMPLES) { + ALOGW("Gauge Metric with trigger event must have sampling type FIRST_N_SAMPLES"); + return nullopt; + } + if (!handleMetricWithAtomMatchingTrackers(metric.trigger_event(), metricIndex, + /*enforceOneAtom=*/true, allAtomMatchingTrackers, + atomMatchingTrackerMap, trackerToMetricMap, + triggerTrackerIndex)) { + return nullopt; + } + sp triggerAtomMatcher = + allAtomMatchingTrackers.at(triggerTrackerIndex); + triggerAtomId = *(triggerAtomMatcher->getAtomIds().begin()); + } + + if (!metric.has_trigger_event() && pullTagId != -1 && + metric.sampling_type() == GaugeMetric::FIRST_N_SAMPLES) { + ALOGW("FIRST_N_SAMPLES is only for pushed event or pull_on_trigger"); + return nullopt; + } + + int conditionIndex = -1; + if (metric.has_condition()) { + if (!handleMetricWithConditions(metric.condition(), metricIndex, conditionTrackerMap, + metric.links(), allConditionTrackers, conditionIndex, + conditionToMetricMap)) { + return nullopt; + } + } else { + if (metric.links_size() > 0) { + ALOGW("metrics has a MetricConditionLink but doesn't have a condition"); + return nullopt; + } + } + + unordered_map> eventActivationMap; + unordered_map>> eventDeactivationMap; + if (!handleMetricActivation(config, metric.id(), metricIndex, metricToActivationMap, + atomMatchingTrackerMap, activationAtomTrackerToMetricMap, + deactivationAtomTrackerToMetricMap, metricsWithActivation, + eventActivationMap, eventDeactivationMap)) { + return nullopt; + } + + uint64_t metricHash; + if (!getMetricProtoHash(config, metric, metric.id(), metricToActivationMap, metricHash)) { + return nullopt; + } + + return {new GaugeMetricProducer(key, metric, conditionIndex, initialConditionCache, wizard, + metricHash, trackerIndex, matcherWizard, pullTagId, + triggerAtomId, atomTagId, timeBaseNs, currentTimeNs, + pullerManager, eventActivationMap, eventDeactivationMap)}; } bool initAtomMatchingTrackers(const StatsdConfig& config, const sp& uidMap, @@ -628,17 +741,17 @@ bool initMetrics(const ConfigKey& key, const StatsdConfig& config, const int64_t int metricIndex = allMetricProducers.size(); const CountMetric& metric = config.count_metric(i); metricMap.insert({metric.id(), metricIndex}); - sp producer = createCountMetricProducerAndUpdateMetadata( + optional> producer = createCountMetricProducerAndUpdateMetadata( key, config, timeBaseTimeNs, currentTimeNs, metric, metricIndex, allAtomMatchingTrackers, atomMatchingTrackerMap, allConditionTrackers, conditionTrackerMap, initialConditionCache, wizard, stateAtomIdMap, allStateGroupMaps, metricToActivationMap, trackerToMetricMap, conditionToMetricMap, activationAtomTrackerToMetricMap, deactivationAtomTrackerToMetricMap, metricsWithActivation); - if (producer == nullptr) { + if (!producer) { return false; } - allMetricProducers.push_back(producer); + allMetricProducers.push_back(producer.value()); } // build DurationMetricProducer @@ -762,16 +875,16 @@ bool initMetrics(const ConfigKey& key, const StatsdConfig& config, const int64_t int metricIndex = allMetricProducers.size(); const EventMetric& metric = config.event_metric(i); metricMap.insert({metric.id(), metricIndex}); - sp producer = createEventMetricProducerAndUpdateMetadata( + optional> producer = createEventMetricProducerAndUpdateMetadata( key, config, timeBaseTimeNs, metric, metricIndex, allAtomMatchingTrackers, atomMatchingTrackerMap, allConditionTrackers, conditionTrackerMap, initialConditionCache, wizard, metricToActivationMap, trackerToMetricMap, conditionToMetricMap, activationAtomTrackerToMetricMap, deactivationAtomTrackerToMetricMap, metricsWithActivation); - if (producer == nullptr) { + if (!producer) { return false; } - allMetricProducers.push_back(producer); + allMetricProducers.push_back(producer.value()); } // build ValueMetricProducer @@ -871,104 +984,20 @@ bool initMetrics(const ConfigKey& key, const StatsdConfig& config, const int64_t // Gauge metrics. for (int i = 0; i < config.gauge_metric_size(); i++) { - const GaugeMetric& metric = config.gauge_metric(i); - if (!metric.has_what()) { - ALOGW("cannot find \"what\" in GaugeMetric \"%lld\"", (long long)metric.id()); - return false; - } - - if ((!metric.gauge_fields_filter().has_include_all() || - (metric.gauge_fields_filter().include_all() == false)) && - !hasLeafNode(metric.gauge_fields_filter().fields())) { - ALOGW("Incorrect field filter setting in GaugeMetric %lld", (long long)metric.id()); - return false; - } - if ((metric.gauge_fields_filter().has_include_all() && - metric.gauge_fields_filter().include_all() == true) && - hasLeafNode(metric.gauge_fields_filter().fields())) { - ALOGW("Incorrect field filter setting in GaugeMetric %lld", (long long)metric.id()); - return false; - } - int metricIndex = allMetricProducers.size(); + const GaugeMetric& metric = config.gauge_metric(i); metricMap.insert({metric.id(), metricIndex}); - int trackerIndex; - if (!handleMetricWithAtomMatchingTrackers(metric.what(), metricIndex, - metric.has_dimensions_in_what(), - allAtomMatchingTrackers, atomMatchingTrackerMap, - trackerToMetricMap, trackerIndex)) { - return false; - } - - sp atomMatcher = allAtomMatchingTrackers.at(trackerIndex); - // For GaugeMetric atom, it should be simple matcher with one tagId. - if (atomMatcher->getAtomIds().size() != 1) { - return false; - } - int atomTagId = *(atomMatcher->getAtomIds().begin()); - int pullTagId = pullerManager->PullerForMatcherExists(atomTagId) ? atomTagId : -1; - - int triggerTrackerIndex; - int triggerAtomId = -1; - if (metric.has_trigger_event()) { - if (pullTagId == -1) { - ALOGW("Pull atom not specified for trigger"); - return false; - } - // event_trigger should be used with FIRST_N_SAMPLES - if (metric.sampling_type() != GaugeMetric::FIRST_N_SAMPLES) { - return false; - } - if (!handleMetricWithAtomMatchingTrackers( - metric.trigger_event(), metricIndex, /*enforceOneAtom=*/true, - allAtomMatchingTrackers, atomMatchingTrackerMap, trackerToMetricMap, - triggerTrackerIndex)) { - return false; - } - sp triggerAtomMatcher = - allAtomMatchingTrackers.at(triggerTrackerIndex); - triggerAtomId = *(triggerAtomMatcher->getAtomIds().begin()); - } - - if (!metric.has_trigger_event() && pullTagId != -1 && - metric.sampling_type() == GaugeMetric::FIRST_N_SAMPLES) { - ALOGW("FIRST_N_SAMPLES is only for pushed event or pull_on_trigger"); - return false; - } - - int conditionIndex = -1; - if (metric.has_condition()) { - 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 MetricConditionLink but doesn't have a condition"); - return false; - } - } - - unordered_map> eventActivationMap; - unordered_map>> eventDeactivationMap; - bool success = handleMetricActivation( - config, metric.id(), metricIndex, metricToActivationMap, atomMatchingTrackerMap, + optional> producer = createGaugeMetricProducerAndUpdateMetadata( + key, config, timeBaseTimeNs, currentTimeNs, pullerManager, metric, metricIndex, + allAtomMatchingTrackers, atomMatchingTrackerMap, allConditionTrackers, + conditionTrackerMap, initialConditionCache, wizard, matcherWizard, + metricToActivationMap, trackerToMetricMap, conditionToMetricMap, activationAtomTrackerToMetricMap, deactivationAtomTrackerToMetricMap, - metricsWithActivation, eventActivationMap, eventDeactivationMap); - if (!success) return false; - - uint64_t metricHash; - if (!getMetricProtoHash(config, metric, metric.id(), metricToActivationMap, metricHash)) { + metricsWithActivation); + if (!producer) { return false; } - - sp gaugeProducer = new GaugeMetricProducer( - key, metric, conditionIndex, initialConditionCache, wizard, metricHash, - trackerIndex, matcherWizard, pullTagId, triggerAtomId, atomTagId, timeBaseTimeNs, - currentTimeNs, pullerManager, eventActivationMap, eventDeactivationMap); - allMetricProducers.push_back(gaugeProducer); + allMetricProducers.push_back(producer.value()); } for (int i = 0; i < config.no_report_metric_size(); ++i) { const auto no_report_metric = config.no_report_metric(i); 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 f6b2b7d1602cd..f909aff48faf6 100644 --- a/cmds/statsd/src/metrics/parsing_utils/metrics_manager_util.h +++ b/cmds/statsd/src/metrics/parsing_utils/metrics_manager_util.h @@ -94,8 +94,8 @@ bool handleMetricActivationOnConfigUpdate( std::unordered_map>>& newEventDeactivationMap); // Creates a CountMetricProducer and updates the vectors/maps used by MetricsManager with -// the appropriate indices. Returns an sp to the producer, or null if there was an error. -sp createCountMetricProducerAndUpdateMetadata( +// the appropriate indices. Returns an sp to the producer, or nullopt if there was an error. +optional> createCountMetricProducerAndUpdateMetadata( const ConfigKey& key, const StatsdConfig& config, const int64_t timeBaseNs, const int64_t currentTimeNs, const CountMetric& metric, const int metricIndex, const std::vector>& allAtomMatchingTrackers, @@ -113,8 +113,8 @@ sp createCountMetricProducerAndUpdateMetadata( std::vector& metricsWithActivation); // Creates an EventMetricProducer and updates the vectors/maps used by MetricsManager with -// the appropriate indices. Returns an sp to the producer, or null if there was an error. -sp createEventMetricProducerAndUpdateMetadata( +// the appropriate indices. Returns an sp to the producer, or nullopt if there was an error. +optional> createEventMetricProducerAndUpdateMetadata( const ConfigKey& key, const StatsdConfig& config, const int64_t timeBaseNs, const EventMetric& metric, const int metricIndex, const std::vector>& allAtomMatchingTrackers, @@ -129,6 +129,25 @@ sp createEventMetricProducerAndUpdateMetadata( std::unordered_map>& deactivationAtomTrackerToMetricMap, std::vector& metricsWithActivation); +// Creates a GaugeMetricProducer and updates the vectors/maps used by MetricsManager with +// the appropriate indices. Returns an sp to the producer, or nullopt if there was an error. +optional> createGaugeMetricProducerAndUpdateMetadata( + const ConfigKey& key, const StatsdConfig& config, const int64_t timeBaseNs, + const int64_t currentTimeNs, const sp& pullerManager, + const GaugeMetric& metric, const int metricIndex, + const std::vector>& allAtomMatchingTrackers, + const std::unordered_map& atomMatchingTrackerMap, + std::vector>& allConditionTrackers, + const std::unordered_map& conditionTrackerMap, + const std::vector& initialConditionCache, const sp& wizard, + const sp& matcherWizard, + const std::unordered_map& metricToActivationMap, + std::unordered_map>& trackerToMetricMap, + std::unordered_map>& conditionToMetricMap, + std::unordered_map>& activationAtomTrackerToMetricMap, + std::unordered_map>& deactivationAtomTrackerToMetricMap, + std::vector& metricsWithActivation); + // 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 70e4dea77fe48..60705cf5ffe76 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 @@ -27,6 +27,7 @@ #include "src/condition/CombinationConditionTracker.h" #include "src/condition/SimpleConditionTracker.h" #include "src/matchers/CombinationAtomMatchingTracker.h" +#include "src/metrics/GaugeMetricProducer.h" #include "src/metrics/parsing_utils/metrics_manager_util.h" #include "tests/statsd_test_util.h" @@ -137,6 +138,23 @@ CountMetric createCountMetric(string name, int64_t what, optional condi return metric; } +GaugeMetric createGaugeMetric(string name, int64_t what, GaugeMetric::SamplingType samplingType, + optional condition, optional triggerEvent) { + GaugeMetric metric; + metric.set_id(StringToId(name)); + metric.set_what(what); + metric.set_bucket(TEN_MINUTES); + metric.set_sampling_type(samplingType); + if (condition) { + metric.set_condition(condition.value()); + } + if (triggerEvent) { + metric.set_trigger_event(triggerEvent.value()); + } + metric.mutable_gauge_fields_filter()->set_include_all(true); + return metric; +} + } // anonymous namespace TEST_F(ConfigUpdateTest, TestSimpleMatcherPreserve) { @@ -1278,11 +1296,8 @@ TEST_F(ConfigUpdateTest, TestGaugeMetricPreserve) { Predicate predicate = CreateScreenIsOnPredicate(); *config.add_predicate() = predicate; - GaugeMetric* metric = config.add_gauge_metric(); - metric->set_id(12345); - metric->set_what(whatMatcher.id()); - metric->set_condition(predicate.id()); - metric->mutable_gauge_fields_filter()->set_include_all(true); + *config.add_gauge_metric() = createGaugeMetric( + "GAUGE1", whatMatcher.id(), GaugeMetric::RANDOM_ONE_SAMPLE, predicate.id(), nullopt); EXPECT_TRUE(initConfig(config)); @@ -1300,15 +1315,13 @@ TEST_F(ConfigUpdateTest, TestGaugeMetricDefinitionChange) { AtomMatcher whatMatcher = CreateScreenBrightnessChangedAtomMatcher(); *config.add_atom_matcher() = whatMatcher; - GaugeMetric* metric = config.add_gauge_metric(); - metric->set_id(12345); - metric->set_what(whatMatcher.id()); - metric->mutable_gauge_fields_filter()->set_include_all(true); + *config.add_gauge_metric() = createGaugeMetric( + "GAUGE1", whatMatcher.id(), GaugeMetric::RANDOM_ONE_SAMPLE, nullopt, nullopt); EXPECT_TRUE(initConfig(config)); // Change split bucket on app upgrade, which should change the proto, causing replacement. - metric->set_split_bucket_for_app_upgrade(false); + config.mutable_gauge_metric(0)->set_split_bucket_for_app_upgrade(false); unordered_map metricToActivationMap; vector metricsToUpdate(1, UPDATE_UNKNOWN); @@ -1324,10 +1337,8 @@ TEST_F(ConfigUpdateTest, TestGaugeMetricWhatChanged) { AtomMatcher whatMatcher = CreateScreenBrightnessChangedAtomMatcher(); *config.add_atom_matcher() = whatMatcher; - GaugeMetric* metric = config.add_gauge_metric(); - metric->set_id(12345); - metric->set_what(whatMatcher.id()); - metric->mutable_gauge_fields_filter()->set_include_all(true); + *config.add_gauge_metric() = createGaugeMetric( + "GAUGE1", whatMatcher.id(), GaugeMetric::RANDOM_ONE_SAMPLE, nullopt, nullopt); EXPECT_TRUE(initConfig(config)); @@ -1352,11 +1363,8 @@ TEST_F(ConfigUpdateTest, TestGaugeMetricConditionChanged) { Predicate predicate = CreateScreenIsOnPredicate(); *config.add_predicate() = predicate; - GaugeMetric* metric = config.add_gauge_metric(); - metric->set_id(12345); - metric->set_what(whatMatcher.id()); - metric->set_condition(predicate.id()); - metric->mutable_gauge_fields_filter()->set_include_all(true); + *config.add_gauge_metric() = createGaugeMetric( + "GAUGE1", whatMatcher.id(), GaugeMetric::RANDOM_ONE_SAMPLE, predicate.id(), nullopt); EXPECT_TRUE(initConfig(config)); @@ -1376,14 +1384,9 @@ TEST_F(ConfigUpdateTest, TestGaugeMetricTriggerEventChanged) { AtomMatcher whatMatcher = CreateTemperatureAtomMatcher(); *config.add_atom_matcher() = whatMatcher; - GaugeMetric* metric = config.add_gauge_metric(); - metric->set_id(12345); - metric->set_what(whatMatcher.id()); - metric->set_trigger_event(triggerEvent.id()); - metric->mutable_gauge_fields_filter()->set_include_all(true); - metric->set_sampling_type(GaugeMetric::FIRST_N_SAMPLES); + *config.add_gauge_metric() = createGaugeMetric( + "GAUGE1", whatMatcher.id(), GaugeMetric::FIRST_N_SAMPLES, nullopt, triggerEvent.id()); - // Create an initial config. EXPECT_TRUE(initConfig(config)); unordered_map metricToActivationMap; @@ -1856,6 +1859,221 @@ TEST_F(ConfigUpdateTest, TestUpdateCountMetrics) { EXPECT_EQ(screenState.mValue.int_value, android::view::DisplayStateEnum::DISPLAY_STATE_ON); } +TEST_F(ConfigUpdateTest, TestUpdateGaugeMetrics) { + StatsdConfig config; + + // Add atom matchers/predicates/states. These are mostly needed for initStatsdConfig. + AtomMatcher matcher1 = CreateScreenTurnedOnAtomMatcher(); + int64_t matcher1Id = matcher1.id(); + *config.add_atom_matcher() = matcher1; + + AtomMatcher matcher2 = CreateScreenTurnedOffAtomMatcher(); + int64_t matcher2Id = matcher2.id(); + *config.add_atom_matcher() = matcher2; + + AtomMatcher matcher3 = CreateStartScheduledJobAtomMatcher(); + int64_t matcher3Id = matcher3.id(); + *config.add_atom_matcher() = matcher3; + + AtomMatcher matcher4 = CreateTemperatureAtomMatcher(); + int64_t matcher4Id = matcher4.id(); + *config.add_atom_matcher() = matcher4; + + AtomMatcher matcher5 = CreateSimpleAtomMatcher("SubsystemSleep", util::SUBSYSTEM_SLEEP_STATE); + int64_t matcher5Id = matcher5.id(); + *config.add_atom_matcher() = matcher5; + + Predicate predicate1 = CreateScreenIsOnPredicate(); + int64_t predicate1Id = predicate1.id(); + *config.add_predicate() = predicate1; + + // Add a few gauge metrics. + // Will be preserved. + GaugeMetric gauge1 = createGaugeMetric("GAUGE1", matcher4Id, GaugeMetric::FIRST_N_SAMPLES, + predicate1Id, matcher1Id); + int64_t gauge1Id = gauge1.id(); + *config.add_gauge_metric() = gauge1; + + // Will be replaced. + GaugeMetric gauge2 = + createGaugeMetric("GAUGE2", matcher1Id, GaugeMetric::FIRST_N_SAMPLES, nullopt, nullopt); + int64_t gauge2Id = gauge2.id(); + *config.add_gauge_metric() = gauge2; + + // Will be replaced. + GaugeMetric gauge3 = createGaugeMetric("GAUGE3", matcher5Id, GaugeMetric::FIRST_N_SAMPLES, + nullopt, matcher3Id); + int64_t gauge3Id = gauge3.id(); + *config.add_gauge_metric() = gauge3; + + // Will be replaced. + GaugeMetric gauge4 = createGaugeMetric("GAUGE4", matcher3Id, GaugeMetric::RANDOM_ONE_SAMPLE, + predicate1Id, nullopt); + int64_t gauge4Id = gauge4.id(); + *config.add_gauge_metric() = gauge4; + + // Will be deleted. + GaugeMetric gauge5 = + createGaugeMetric("GAUGE5", matcher2Id, GaugeMetric::RANDOM_ONE_SAMPLE, nullopt, {}); + int64_t gauge5Id = gauge5.id(); + *config.add_gauge_metric() = gauge5; + + EXPECT_TRUE(initConfig(config)); + + // Used later to ensure the condition wizard is replaced. Get it before doing the update. + sp oldMatcherWizard = + static_cast(oldMetricProducers[0].get())->mEventMatcherWizard; + EXPECT_EQ(oldMatcherWizard->getStrongCount(), 6); + + // Change gauge2, causing it to be replaced. + gauge2.set_max_num_gauge_atoms_per_bucket(50); + + // Mark matcher 3 as replaced. Causes gauge3 and gauge4 to be replaced. + set replacedMatchers = {matcher3Id}; + + // New gauge metric. + GaugeMetric gauge6 = createGaugeMetric("GAUGE6", matcher5Id, GaugeMetric::FIRST_N_SAMPLES, + predicate1Id, matcher3Id); + int64_t gauge6Id = gauge6.id(); + + // Map the matchers and predicates in reverse order to force the indices to change. + std::unordered_map newAtomMatchingTrackerMap; + const int matcher5Index = 0; + newAtomMatchingTrackerMap[matcher5Id] = 0; + const int matcher4Index = 1; + newAtomMatchingTrackerMap[matcher4Id] = 1; + const int matcher3Index = 2; + newAtomMatchingTrackerMap[matcher3Id] = 2; + const int matcher2Index = 3; + newAtomMatchingTrackerMap[matcher2Id] = 3; + const int matcher1Index = 4; + newAtomMatchingTrackerMap[matcher1Id] = 4; + // Use the existing matchers. A bit hacky, but saves code and we don't rely on them. + vector> newAtomMatchingTrackers(5); + std::reverse_copy(oldAtomMatchingTrackers.begin(), oldAtomMatchingTrackers.end(), + newAtomMatchingTrackers.begin()); + + std::unordered_map newConditionTrackerMap; + const int predicate1Index = 0; + newConditionTrackerMap[predicate1Id] = 0; + // Use the existing conditionTrackers. A bit hacky, but saves code and we don't rely on them. + vector> newConditionTrackers(1); + std::reverse_copy(oldConditionTrackers.begin(), oldConditionTrackers.end(), + newConditionTrackers.begin()); + // Say that predicate1 is unknown since the initial condition never changed. + vector conditionCache = {ConditionState::kUnknown}; + + StatsdConfig newConfig; + *newConfig.add_gauge_metric() = gauge6; + const int gauge6Index = 0; + *newConfig.add_gauge_metric() = gauge3; + const int gauge3Index = 1; + *newConfig.add_gauge_metric() = gauge1; + const int gauge1Index = 2; + *newConfig.add_gauge_metric() = gauge4; + const int gauge4Index = 3; + *newConfig.add_gauge_metric() = gauge2; + const int gauge2Index = 4; + + // Output data structures to validate. + unordered_map newMetricProducerMap; + vector> newMetricProducers; + unordered_map> conditionToMetricMap; + unordered_map> trackerToMetricMap; + set noReportMetricIds; + unordered_map> activationAtomTrackerToMetricMap; + unordered_map> deactivationAtomTrackerToMetricMap; + vector metricsWithActivation; + EXPECT_TRUE(updateMetrics( + key, newConfig, /*timeBaseNs=*/123, /*currentTimeNs=*/12345, new StatsPullerManager(), + oldAtomMatchingTrackerMap, newAtomMatchingTrackerMap, replacedMatchers, + newAtomMatchingTrackers, newConditionTrackerMap, /*replacedConditions=*/{}, + newConditionTrackers, conditionCache, /*stateAtomIdMap=*/{}, /*allStateGroupMaps=*/{}, + /*replacedStates=*/{}, oldMetricProducerMap, oldMetricProducers, newMetricProducerMap, + newMetricProducers, conditionToMetricMap, trackerToMetricMap, noReportMetricIds, + activationAtomTrackerToMetricMap, deactivationAtomTrackerToMetricMap, + metricsWithActivation)); + + unordered_map expectedMetricProducerMap = { + {gauge1Id, gauge1Index}, {gauge2Id, gauge2Index}, {gauge3Id, gauge3Index}, + {gauge4Id, gauge4Index}, {gauge6Id, gauge6Index}, + }; + EXPECT_THAT(newMetricProducerMap, ContainerEq(expectedMetricProducerMap)); + + // Make sure preserved metrics are the same. + ASSERT_EQ(newMetricProducers.size(), 5); + EXPECT_EQ(oldMetricProducers[oldMetricProducerMap.at(gauge1Id)], + newMetricProducers[newMetricProducerMap.at(gauge1Id)]); + + // Make sure replaced metrics are different. + EXPECT_NE(oldMetricProducers[oldMetricProducerMap.at(gauge2Id)], + newMetricProducers[newMetricProducerMap.at(gauge2Id)]); + EXPECT_NE(oldMetricProducers[oldMetricProducerMap.at(gauge3Id)], + newMetricProducers[newMetricProducerMap.at(gauge3Id)]); + EXPECT_NE(oldMetricProducers[oldMetricProducerMap.at(gauge4Id)], + newMetricProducers[newMetricProducerMap.at(gauge4Id)]); + + // Verify the conditionToMetricMap. + ASSERT_EQ(conditionToMetricMap.size(), 1); + const vector& condition1Metrics = conditionToMetricMap[predicate1Index]; + EXPECT_THAT(condition1Metrics, UnorderedElementsAre(gauge1Index, gauge4Index, gauge6Index)); + + // Verify the trackerToMetricMap. + ASSERT_EQ(trackerToMetricMap.size(), 4); + const vector& matcher1Metrics = trackerToMetricMap[matcher1Index]; + EXPECT_THAT(matcher1Metrics, UnorderedElementsAre(gauge1Index, gauge2Index)); + const vector& matcher3Metrics = trackerToMetricMap[matcher3Index]; + EXPECT_THAT(matcher3Metrics, UnorderedElementsAre(gauge3Index, gauge4Index, gauge6Index)); + const vector& matcher4Metrics = trackerToMetricMap[matcher4Index]; + EXPECT_THAT(matcher4Metrics, UnorderedElementsAre(gauge1Index)); + const vector& matcher5Metrics = trackerToMetricMap[matcher5Index]; + EXPECT_THAT(matcher5Metrics, UnorderedElementsAre(gauge3Index, gauge6Index)); + + // Verify event activation/deactivation maps. + ASSERT_EQ(activationAtomTrackerToMetricMap.size(), 0); + ASSERT_EQ(deactivationAtomTrackerToMetricMap.size(), 0); + ASSERT_EQ(metricsWithActivation.size(), 0); + + // Verify tracker indices/ids/conditions/states are correct. + GaugeMetricProducer* gaugeProducer1 = + static_cast(newMetricProducers[gauge1Index].get()); + EXPECT_EQ(gaugeProducer1->getMetricId(), gauge1Id); + EXPECT_EQ(gaugeProducer1->mConditionTrackerIndex, predicate1Index); + EXPECT_EQ(gaugeProducer1->mCondition, ConditionState::kUnknown); + EXPECT_EQ(gaugeProducer1->mWhatMatcherIndex, matcher4Index); + GaugeMetricProducer* gaugeProducer2 = + static_cast(newMetricProducers[gauge2Index].get()); + EXPECT_EQ(gaugeProducer2->getMetricId(), gauge2Id); + EXPECT_EQ(gaugeProducer2->mConditionTrackerIndex, -1); + EXPECT_EQ(gaugeProducer2->mCondition, ConditionState::kTrue); + EXPECT_EQ(gaugeProducer2->mWhatMatcherIndex, matcher1Index); + GaugeMetricProducer* gaugeProducer3 = + static_cast(newMetricProducers[gauge3Index].get()); + EXPECT_EQ(gaugeProducer3->getMetricId(), gauge3Id); + EXPECT_EQ(gaugeProducer3->mConditionTrackerIndex, -1); + EXPECT_EQ(gaugeProducer3->mCondition, ConditionState::kTrue); + EXPECT_EQ(gaugeProducer3->mWhatMatcherIndex, matcher5Index); + GaugeMetricProducer* gaugeProducer4 = + static_cast(newMetricProducers[gauge4Index].get()); + EXPECT_EQ(gaugeProducer4->getMetricId(), gauge4Id); + EXPECT_EQ(gaugeProducer4->mConditionTrackerIndex, predicate1Index); + EXPECT_EQ(gaugeProducer4->mCondition, ConditionState::kUnknown); + EXPECT_EQ(gaugeProducer4->mWhatMatcherIndex, matcher3Index); + GaugeMetricProducer* gaugeProducer6 = + static_cast(newMetricProducers[gauge6Index].get()); + EXPECT_EQ(gaugeProducer6->getMetricId(), gauge6Id); + EXPECT_EQ(gaugeProducer6->mConditionTrackerIndex, predicate1Index); + EXPECT_EQ(gaugeProducer6->mCondition, ConditionState::kUnknown); + EXPECT_EQ(gaugeProducer6->mWhatMatcherIndex, matcher5Index); + + sp newMatcherWizard = gaugeProducer1->mEventMatcherWizard; + EXPECT_NE(newMatcherWizard, oldMatcherWizard); + EXPECT_EQ(newMatcherWizard->getStrongCount(), 6); + oldMetricProducers.clear(); + // Only reference to the old wizard should be the one in the test. + EXPECT_EQ(oldMatcherWizard->getStrongCount(), 1); +} + TEST_F(ConfigUpdateTest, TestUpdateMetricActivations) { StatsdConfig config; // Add atom matchers @@ -1995,6 +2213,10 @@ TEST_F(ConfigUpdateTest, TestUpdateMetricsMultipleTypes) { int64_t matcher2Id = matcher2.id(); *config.add_atom_matcher() = matcher2; + AtomMatcher matcher3 = CreateTemperatureAtomMatcher(); + int64_t matcher3Id = matcher3.id(); + *config.add_atom_matcher() = matcher3; + Predicate predicate1 = CreateScreenIsOnPredicate(); int64_t predicate1Id = predicate1.id(); *config.add_predicate() = predicate1; @@ -2010,24 +2232,35 @@ TEST_F(ConfigUpdateTest, TestUpdateMetricsMultipleTypes) { int64_t eventMetricId = eventMetric.id(); *config.add_event_metric() = eventMetric; + // Will be replaced because the definition changes - a predicate is added. + GaugeMetric gaugeMetric = createGaugeMetric("GAUGE1", matcher3Id, + GaugeMetric::RANDOM_ONE_SAMPLE, nullopt, nullopt); + int64_t gaugeMetricId = gaugeMetric.id(); + *config.add_gauge_metric() = gaugeMetric; + EXPECT_TRUE(initConfig(config)); // Used later to ensure the condition wizard is replaced. Get it before doing the update. sp oldConditionWizard = oldMetricProducers[0]->mWizard; - EXPECT_EQ(oldConditionWizard->getStrongCount(), 3); + EXPECT_EQ(oldConditionWizard->getStrongCount(), 4); // Mark matcher 2 as replaced. Causes eventMetric to be replaced. set replacedMatchers; replacedMatchers.insert(matcher2Id); + // Add predicate1 as a predicate on gaugeMetric, causing it to be replaced. + gaugeMetric.set_condition(predicate1Id); + // Map the matchers and predicates in reverse order to force the indices to change. std::unordered_map newAtomMatchingTrackerMap; - const int matcher2Index = 0; - newAtomMatchingTrackerMap[matcher2Id] = 0; - const int matcher1Index = 1; - newAtomMatchingTrackerMap[matcher1Id] = 1; + const int matcher3Index = 0; + newAtomMatchingTrackerMap[matcher3Id] = 0; + const int matcher2Index = 1; + newAtomMatchingTrackerMap[matcher2Id] = 1; + const int matcher1Index = 2; + newAtomMatchingTrackerMap[matcher1Id] = 2; // Use the existing matchers. A bit hacky, but saves code and we don't rely on them. - vector> newAtomMatchingTrackers(2); + vector> newAtomMatchingTrackers(3); std::reverse_copy(oldAtomMatchingTrackers.begin(), oldAtomMatchingTrackers.end(), newAtomMatchingTrackers.begin()); @@ -2045,6 +2278,8 @@ TEST_F(ConfigUpdateTest, TestUpdateMetricsMultipleTypes) { const int countMetricIndex = 0; *newConfig.add_event_metric() = eventMetric; const int eventMetricIndex = 1; + *newConfig.add_gauge_metric() = gaugeMetric; + const int gaugeMetricIndex = 2; // Output data structures to validate. unordered_map newMetricProducerMap; @@ -2068,29 +2303,34 @@ TEST_F(ConfigUpdateTest, TestUpdateMetricsMultipleTypes) { unordered_map expectedMetricProducerMap = { {countMetricId, countMetricIndex}, {eventMetricId, eventMetricIndex}, + {gaugeMetricId, gaugeMetricIndex}, }; EXPECT_THAT(newMetricProducerMap, ContainerEq(expectedMetricProducerMap)); // Make sure preserved metrics are the same. - ASSERT_EQ(newMetricProducers.size(), 2); + ASSERT_EQ(newMetricProducers.size(), 3); EXPECT_EQ(oldMetricProducers[oldMetricProducerMap.at(countMetricId)], newMetricProducers[newMetricProducerMap.at(countMetricId)]); // Make sure replaced metrics are different. EXPECT_NE(oldMetricProducers[oldMetricProducerMap.at(eventMetricId)], newMetricProducers[newMetricProducerMap.at(eventMetricId)]); + EXPECT_NE(oldMetricProducers[oldMetricProducerMap.at(gaugeMetricId)], + newMetricProducers[newMetricProducerMap.at(gaugeMetricId)]); // Verify the conditionToMetricMap. ASSERT_EQ(conditionToMetricMap.size(), 1); const vector& condition1Metrics = conditionToMetricMap[predicate1Index]; - EXPECT_THAT(condition1Metrics, UnorderedElementsAre(countMetricIndex)); + EXPECT_THAT(condition1Metrics, UnorderedElementsAre(countMetricIndex, gaugeMetricIndex)); // Verify the trackerToMetricMap. - ASSERT_EQ(trackerToMetricMap.size(), 2); + ASSERT_EQ(trackerToMetricMap.size(), 3); const vector& matcher1Metrics = trackerToMetricMap[matcher1Index]; EXPECT_THAT(matcher1Metrics, UnorderedElementsAre(countMetricIndex)); const vector& matcher2Metrics = trackerToMetricMap[matcher2Index]; EXPECT_THAT(matcher2Metrics, UnorderedElementsAre(eventMetricIndex)); + const vector& matcher3Metrics = trackerToMetricMap[matcher3Index]; + EXPECT_THAT(matcher3Metrics, UnorderedElementsAre(gaugeMetricIndex)); // Verify event activation/deactivation maps. ASSERT_EQ(activationAtomTrackerToMetricMap.size(), 0); @@ -2104,10 +2344,13 @@ TEST_F(ConfigUpdateTest, TestUpdateMetricsMultipleTypes) { EXPECT_EQ(newMetricProducers[eventMetricIndex]->getMetricId(), eventMetricId); EXPECT_EQ(newMetricProducers[eventMetricIndex]->mConditionTrackerIndex, -1); EXPECT_EQ(newMetricProducers[eventMetricIndex]->mCondition, ConditionState::kTrue); + EXPECT_EQ(newMetricProducers[gaugeMetricIndex]->getMetricId(), gaugeMetricId); + EXPECT_EQ(newMetricProducers[gaugeMetricIndex]->mConditionTrackerIndex, predicate1Index); + EXPECT_EQ(newMetricProducers[gaugeMetricIndex]->mCondition, ConditionState::kUnknown); sp newConditionWizard = newMetricProducers[0]->mWizard; EXPECT_NE(newConditionWizard, oldConditionWizard); - EXPECT_EQ(newConditionWizard->getStrongCount(), 3); + EXPECT_EQ(newConditionWizard->getStrongCount(), 4); oldMetricProducers.clear(); // Only reference to the old wizard should be the one in the test. EXPECT_EQ(oldConditionWizard->getStrongCount(), 1);