Perform update for value metric

Like other metrics, preserved metrics have indices and maps updated.
New/replaced metrics get recreated as if it was a brand new metric.

Test: atest statsd_test
Bug: 162321957
Change-Id: Ia3896d47d861f7193bf81fc7c7bb10db3103fc40

6 files changed, 495 insertions, 101 deletions

diff --git a/cmds/statsd/src/metrics/ValueMetricProducer.cpp b/cmds/statsd/src/metrics/ValueMetricProducer.cpp
index 3d57cfe318c5..22fdf1604435 100644
--- a/cmds/statsd/src/metrics/ValueMetricProducer.cpp
+++ b/cmds/statsd/src/metrics/ValueMetricProducer.cpp
@@ -18,12 +18,14 @@
 #include "Log.h"
 
 #include "ValueMetricProducer.h"
-#include "../guardrail/StatsdStats.h"
-#include "../stats_log_util.h"
 
 #include <limits.h>
 #include <stdlib.h>
 
+#include "../guardrail/StatsdStats.h"
+#include "../stats_log_util.h"
+#include "metrics/parsing_utils/metrics_manager_util.h"
+
 using android::util::FIELD_COUNT_REPEATED;
 using android::util::FIELD_TYPE_BOOL;
 using android::util::FIELD_TYPE_DOUBLE;
@@ -184,6 +186,48 @@ ValueMetricProducer::~ValueMetricProducer() {
     }
 }
 
+bool ValueMetricProducer::onConfigUpdatedLocked(
+        const StatsdConfig& config, const int configIndex, const int metricIndex,
+        const vector<sp<AtomMatchingTracker>>& allAtomMatchingTrackers,
+        const unordered_map<int64_t, int>& oldAtomMatchingTrackerMap,
+        const unordered_map<int64_t, int>& newAtomMatchingTrackerMap,
+        const sp<EventMatcherWizard>& matcherWizard,
+        const vector<sp<ConditionTracker>>& allConditionTrackers,
+        const unordered_map<int64_t, int>& conditionTrackerMap, const sp<ConditionWizard>& wizard,
+        const unordered_map<int64_t, int>& metricToActivationMap,
+        unordered_map<int, vector<int>>& trackerToMetricMap,
+        unordered_map<int, vector<int>>& conditionToMetricMap,
+        unordered_map<int, vector<int>>& activationAtomTrackerToMetricMap,
+        unordered_map<int, vector<int>>& deactivationAtomTrackerToMetricMap,
+        vector<int>& metricsWithActivation) {
+    if (!MetricProducer::onConfigUpdatedLocked(
+                config, configIndex, metricIndex, allAtomMatchingTrackers,
+                oldAtomMatchingTrackerMap, newAtomMatchingTrackerMap, matcherWizard,
+                allConditionTrackers, conditionTrackerMap, wizard, metricToActivationMap,
+                trackerToMetricMap, conditionToMetricMap, activationAtomTrackerToMetricMap,
+                deactivationAtomTrackerToMetricMap, metricsWithActivation)) {
+        return false;
+    }
+
+    const ValueMetric& metric = config.value_metric(configIndex);
+    // Update appropriate indices: mWhatMatcherIndex, mConditionIndex and MetricsManager maps.
+    if (!handleMetricWithAtomMatchingTrackers(metric.what(), metricIndex, /*enforceOneAtom=*/false,
+                                              allAtomMatchingTrackers, newAtomMatchingTrackerMap,
+                                              trackerToMetricMap, mWhatMatcherIndex)) {
+        return false;
+    }
+
+    if (metric.has_condition() &&
+        !handleMetricWithConditions(metric.condition(), metricIndex, conditionTrackerMap,
+                                    metric.links(), allConditionTrackers, mConditionTrackerIndex,
+                                    conditionToMetricMap)) {
+        return false;
+    }
+    sp<EventMatcherWizard> tmpEventWizard = mEventMatcherWizard;
+    mEventMatcherWizard = matcherWizard;
+    return true;
+}
+
 void ValueMetricProducer::onStateChanged(int64_t eventTimeNs, int32_t atomId,
                                          const HashableDimensionKey& primaryKey,
                                          const FieldValue& oldState, const FieldValue& newState) {
diff --git a/cmds/statsd/src/metrics/ValueMetricProducer.h b/cmds/statsd/src/metrics/ValueMetricProducer.h
index 67de214e655c..ebd8fecd55d0 100644
--- a/cmds/statsd/src/metrics/ValueMetricProducer.h
+++ b/cmds/statsd/src/metrics/ValueMetricProducer.h
@@ -47,7 +47,7 @@ struct PastValueBucket {
 // - a condition change
 // - an app upgrade
 // - an alarm set to the end of the bucket
-class ValueMetricProducer : public virtual MetricProducer, public virtual PullDataReceiver {
+class ValueMetricProducer : public MetricProducer, public virtual PullDataReceiver {
 public:
     ValueMetricProducer(
             const ConfigKey& key, const ValueMetric& valueMetric, const int conditionIndex,
@@ -155,7 +155,23 @@ private:
     // causes the bucket to be invalidated will not notify StatsdStats.
     void skipCurrentBucket(const int64_t dropTimeNs, const BucketDropReason reason);
 
-    const int mWhatMatcherIndex;
+    bool onConfigUpdatedLocked(
+            const StatsdConfig& config, const int configIndex, const int metricIndex,
+            const std::vector<sp<AtomMatchingTracker>>& allAtomMatchingTrackers,
+            const std::unordered_map<int64_t, int>& oldAtomMatchingTrackerMap,
+            const std::unordered_map<int64_t, int>& newAtomMatchingTrackerMap,
+            const sp<EventMatcherWizard>& matcherWizard,
+            const std::vector<sp<ConditionTracker>>& allConditionTrackers,
+            const std::unordered_map<int64_t, int>& conditionTrackerMap,
+            const sp<ConditionWizard>& wizard,
+            const std::unordered_map<int64_t, int>& metricToActivationMap,
+            std::unordered_map<int, std::vector<int>>& trackerToMetricMap,
+            std::unordered_map<int, std::vector<int>>& conditionToMetricMap,
+            std::unordered_map<int, std::vector<int>>& activationAtomTrackerToMetricMap,
+            std::unordered_map<int, std::vector<int>>& deactivationAtomTrackerToMetricMap,
+            std::vector<int>& metricsWithActivation) override;
+
+    int mWhatMatcherIndex;
 
     sp<EventMatcherWizard> mEventMatcherWizard;
 
@@ -370,6 +386,8 @@ private:
     FRIEND_TEST(ValueMetricProducerTest_PartialBucket, TestPulledValue);
     FRIEND_TEST(ValueMetricProducerTest_PartialBucket, TestPulledValueWhileConditionFalse);
 
+    FRIEND_TEST(ConfigUpdateTest, TestUpdateValueMetrics);
+
     friend class ValueMetricProducerTestHelper;
 };
 
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 b6e5d8846fd9..21527214b9d8 100644
--- a/cmds/statsd/src/metrics/parsing_utils/config_update_utils.cpp
+++ b/cmds/statsd/src/metrics/parsing_utils/config_update_utils.cpp
@@ -580,7 +580,6 @@ bool determineAllMetricUpdateStatuses(const StatsdConfig& config,
             return false;
         }
     }
-    // TODO: determine update status for value metrics.
     return true;
 }
 
@@ -749,8 +748,8 @@ bool updateMetrics(const ConfigKey& key, const StatsdConfig& config, const int64
         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);
+        newMetricProducerMap[metric.id()] = metricIndex;
         optional<sp<MetricProducer>> producer;
         switch (metricsToUpdate[metricIndex]) {
             case UPDATE_PRESERVE: {
@@ -784,6 +783,45 @@ bool updateMetrics(const ConfigKey& key, const StatsdConfig& config, const int64
         }
         newMetricProducers.push_back(producer.value());
     }
+
+    for (int i = 0; i < config.value_metric_size(); i++, metricIndex++) {
+        const ValueMetric& metric = config.value_metric(i);
+        newMetricProducerMap[metric.id()] = metricIndex;
+        optional<sp<MetricProducer>> 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 = createValueMetricProducerAndUpdateMetadata(
+                        key, config, timeBaseNs, currentTimeNs, pullerManager, metric, metricIndex,
+                        allAtomMatchingTrackers, newAtomMatchingTrackerMap, allConditionTrackers,
+                        conditionTrackerMap, initialConditionCache, wizard, matcherWizard,
+                        stateAtomIdMap, allStateGroupMaps, 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());
+    }
+
     for (int i = 0; i < config.gauge_metric_size(); i++, metricIndex++) {
         const GaugeMetric& metric = config.gauge_metric(i);
         newMetricProducerMap[metric.id()] = metricIndex;
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 b7dc2c7fd0de..8fc039a7d6b3 100644
--- a/cmds/statsd/src/metrics/parsing_utils/metrics_manager_util.cpp
+++ b/cmds/statsd/src/metrics/parsing_utils/metrics_manager_util.cpp
@@ -599,6 +599,108 @@ optional<sp<MetricProducer>> createEventMetricProducerAndUpdateMetadata(
                                     eventDeactivationMap)};
 }
 
+optional<sp<MetricProducer>> createValueMetricProducerAndUpdateMetadata(
+        const ConfigKey& key, const StatsdConfig& config, const int64_t timeBaseNs,
+        const int64_t currentTimeNs, const sp<StatsPullerManager>& pullerManager,
+        const ValueMetric& metric, const int metricIndex,
+        const vector<sp<AtomMatchingTracker>>& allAtomMatchingTrackers,
+        const unordered_map<int64_t, int>& atomMatchingTrackerMap,
+        vector<sp<ConditionTracker>>& allConditionTrackers,
+        const unordered_map<int64_t, int>& conditionTrackerMap,
+        const vector<ConditionState>& initialConditionCache, const sp<ConditionWizard>& wizard,
+        const sp<EventMatcherWizard>& matcherWizard,
+        const unordered_map<int64_t, int>& stateAtomIdMap,
+        const unordered_map<int64_t, unordered_map<int, int64_t>>& allStateGroupMaps,
+        const unordered_map<int64_t, int>& metricToActivationMap,
+        unordered_map<int, vector<int>>& trackerToMetricMap,
+        unordered_map<int, vector<int>>& conditionToMetricMap,
+        unordered_map<int, vector<int>>& activationAtomTrackerToMetricMap,
+        unordered_map<int, vector<int>>& deactivationAtomTrackerToMetricMap,
+        vector<int>& metricsWithActivation) {
+    if (!metric.has_id() || !metric.has_what()) {
+        ALOGE("cannot find metric id or \"what\" in ValueMetric \"%lld\"", (long long)metric.id());
+        return nullopt;
+    }
+    if (!metric.has_value_field()) {
+        ALOGE("cannot find \"value_field\" in ValueMetric \"%lld\"", (long long)metric.id());
+        return nullopt;
+    }
+    std::vector<Matcher> fieldMatchers;
+    translateFieldMatcher(metric.value_field(), &fieldMatchers);
+    if (fieldMatchers.size() < 1) {
+        ALOGE("incorrect \"value_field\" in ValueMetric \"%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<AtomMatchingTracker> atomMatcher = allAtomMatchingTrackers.at(trackerIndex);
+    // If it is pulled 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 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) {
+        ALOGE("metrics has a MetricConditionLink but doesn't have a condition");
+        return nullopt;
+    }
+
+    std::vector<int> slicedStateAtoms;
+    unordered_map<int, unordered_map<int, int64_t>> stateGroupMap;
+    if (metric.slice_by_state_size() > 0) {
+        if (!handleMetricWithStates(config, metric.slice_by_state(), stateAtomIdMap,
+                                    allStateGroupMaps, slicedStateAtoms, stateGroupMap)) {
+            return nullopt;
+        }
+    } else if (metric.state_link_size() > 0) {
+        ALOGE("ValueMetric has a MetricStateLink but doesn't have a sliced state");
+        return nullopt;
+    }
+
+    // Check that all metric state links are a subset of dimensions_in_what fields.
+    std::vector<Matcher> dimensionsInWhat;
+    translateFieldMatcher(metric.dimensions_in_what(), &dimensionsInWhat);
+    for (const auto& stateLink : metric.state_link()) {
+        if (!handleMetricWithStateLink(stateLink.fields_in_what(), dimensionsInWhat)) {
+            return nullopt;
+        }
+    }
+
+    unordered_map<int, shared_ptr<Activation>> eventActivationMap;
+    unordered_map<int, vector<shared_ptr<Activation>>> 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 ValueMetricProducer(key, metric, conditionIndex, initialConditionCache, wizard,
+                                    metricHash, trackerIndex, matcherWizard, pullTagId, timeBaseNs,
+                                    currentTimeNs, pullerManager, eventActivationMap,
+                                    eventDeactivationMap, slicedStateAtoms, stateGroupMap)};
+}
+
 optional<sp<MetricProducer>> createGaugeMetricProducerAndUpdateMetadata(
         const ConfigKey& key, const StatsdConfig& config, const int64_t timeBaseNs,
         const int64_t currentTimeNs, const sp<StatsPullerManager>& pullerManager,
@@ -911,97 +1013,20 @@ bool initMetrics(const ConfigKey& key, const StatsdConfig& config, const int64_t
 
     // build ValueMetricProducer
     for (int i = 0; i < config.value_metric_size(); i++) {
-        const ValueMetric& metric = config.value_metric(i);
-        if (!metric.has_what()) {
-            ALOGW("cannot find \"what\" in ValueMetric \"%lld\"", (long long)metric.id());
-            return false;
-        }
-        if (!metric.has_value_field()) {
-            ALOGW("cannot find \"value_field\" in ValueMetric \"%lld\"", (long long)metric.id());
-            return false;
-        }
-        std::vector<Matcher> fieldMatchers;
-        translateFieldMatcher(metric.value_field(), &fieldMatchers);
-        if (fieldMatchers.size() < 1) {
-            ALOGW("incorrect \"value_field\" in ValueMetric \"%lld\"", (long long)metric.id());
-            return false;
-        }
-
         int metricIndex = allMetricProducers.size();
+        const ValueMetric& metric = config.value_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<AtomMatchingTracker> atomMatcher = allAtomMatchingTrackers.at(trackerIndex);
-        // If it is pulled 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 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;
-            }
-        }
-
-        std::vector<int> slicedStateAtoms;
-        unordered_map<int, unordered_map<int, int64_t>> stateGroupMap;
-        if (metric.slice_by_state_size() > 0) {
-            if (!handleMetricWithStates(config, metric.slice_by_state(), stateAtomIdMap,
-                                        allStateGroupMaps, slicedStateAtoms, stateGroupMap)) {
-                return false;
-            }
-        } else {
-            if (metric.state_link_size() > 0) {
-                ALOGW("ValueMetric has a MetricStateLink but doesn't have a sliced state");
-                return false;
-            }
-        }
-
-        // Check that all metric state links are a subset of dimensions_in_what fields.
-        std::vector<Matcher> dimensionsInWhat;
-        translateFieldMatcher(metric.dimensions_in_what(), &dimensionsInWhat);
-        for (const auto& stateLink : metric.state_link()) {
-            if (!handleMetricWithStateLink(stateLink.fields_in_what(), dimensionsInWhat)) {
-                return false;
-            }
-        }
-
-        unordered_map<int, shared_ptr<Activation>> eventActivationMap;
-        unordered_map<int, vector<shared_ptr<Activation>>> eventDeactivationMap;
-        bool success = handleMetricActivation(
-                config, metric.id(), metricIndex, metricToActivationMap, atomMatchingTrackerMap,
+        optional<sp<MetricProducer>> producer = createValueMetricProducerAndUpdateMetadata(
+                key, config, timeBaseTimeNs, currentTimeNs, pullerManager, metric, metricIndex,
+                allAtomMatchingTrackers, atomMatchingTrackerMap, allConditionTrackers,
+                conditionTrackerMap, initialConditionCache, wizard, matcherWizard, stateAtomIdMap,
+                allStateGroupMaps, 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<MetricProducer> valueProducer = new ValueMetricProducer(
-                key, metric, conditionIndex, initialConditionCache, wizard, metricHash,
-                trackerIndex, matcherWizard, pullTagId, timeBaseTimeNs, currentTimeNs,
-                pullerManager, eventActivationMap, eventDeactivationMap, slicedStateAtoms,
-                stateGroupMap);
-        allMetricProducers.push_back(valueProducer);
+        allMetricProducers.push_back(producer.value());
     }
 
     // Gauge metrics.
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 6d1e6dde7e89..e4585cd578f8 100644
--- a/cmds/statsd/src/metrics/parsing_utils/metrics_manager_util.h
+++ b/cmds/statsd/src/metrics/parsing_utils/metrics_manager_util.h
@@ -148,6 +148,27 @@ optional<sp<MetricProducer>> createEventMetricProducerAndUpdateMetadata(
         std::unordered_map<int, std::vector<int>>& deactivationAtomTrackerToMetricMap,
         std::vector<int>& metricsWithActivation);
 
+// Creates a CountMetricProducer 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<sp<MetricProducer>> createValueMetricProducerAndUpdateMetadata(
+        const ConfigKey& key, const StatsdConfig& config, const int64_t timeBaseNs,
+        const int64_t currentTimeNs, const sp<StatsPullerManager>& pullerManager,
+        const ValueMetric& metric, const int metricIndex,
+        const std::vector<sp<AtomMatchingTracker>>& allAtomMatchingTrackers,
+        const std::unordered_map<int64_t, int>& atomMatchingTrackerMap,
+        std::vector<sp<ConditionTracker>>& allConditionTrackers,
+        const std::unordered_map<int64_t, int>& conditionTrackerMap,
+        const std::vector<ConditionState>& initialConditionCache, const sp<ConditionWizard>& wizard,
+        const sp<EventMatcherWizard>& matcherWizard,
+        const std::unordered_map<int64_t, int>& stateAtomIdMap,
+        const std::unordered_map<int64_t, std::unordered_map<int, int64_t>>& allStateGroupMaps,
+        const std::unordered_map<int64_t, int>& metricToActivationMap,
+        std::unordered_map<int, std::vector<int>>& trackerToMetricMap,
+        std::unordered_map<int, std::vector<int>>& conditionToMetricMap,
+        std::unordered_map<int, std::vector<int>>& activationAtomTrackerToMetricMap,
+        std::unordered_map<int, std::vector<int>>& deactivationAtomTrackerToMetricMap,
+        std::vector<int>& 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<sp<MetricProducer>> createGaugeMetricProducerAndUpdateMetadata(
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 0066030ade54..687b80ad7741 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
@@ -29,6 +29,7 @@
 #include "src/matchers/CombinationAtomMatchingTracker.h"
 #include "src/metrics/DurationMetricProducer.h"
 #include "src/metrics/GaugeMetricProducer.h"
+#include "src/metrics/ValueMetricProducer.h"
 #include "src/metrics/parsing_utils/metrics_manager_util.h"
 #include "tests/statsd_test_util.h"
 
@@ -2685,6 +2686,243 @@ TEST_F(ConfigUpdateTest, TestUpdateDurationMetrics) {
     EXPECT_EQ(oldConditionWizard->getStrongCount(), 1);
 }
 
+TEST_F(ConfigUpdateTest, TestUpdateValueMetrics) {
+    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;
+
+    Predicate predicate2 = CreateScreenIsOffPredicate();
+    int64_t predicate2Id = predicate2.id();
+    *config.add_predicate() = predicate2;
+
+    State state1 = CreateScreenStateWithOnOffMap(0x123, 0x321);
+    int64_t state1Id = state1.id();
+    *config.add_state() = state1;
+
+    State state2 = CreateScreenState();
+    int64_t state2Id = state2.id();
+    *config.add_state() = state2;
+
+    // Add a few value metrics.
+    // Note that these will not work as "real" metrics since the value field is always 2.
+    // Will be preserved.
+    ValueMetric value1 = createValueMetric("VALUE1", matcher4, predicate1Id, {state1Id});
+    int64_t value1Id = value1.id();
+    *config.add_value_metric() = value1;
+
+    // Will be replaced - definition change.
+    ValueMetric value2 = createValueMetric("VALUE2", matcher1, nullopt, {});
+    int64_t value2Id = value2.id();
+    *config.add_value_metric() = value2;
+
+    // Will be replaced - condition change.
+    ValueMetric value3 = createValueMetric("VALUE3", matcher5, predicate2Id, {});
+    int64_t value3Id = value3.id();
+    *config.add_value_metric() = value3;
+
+    // Will be replaced - state change.
+    ValueMetric value4 = createValueMetric("VALUE4", matcher3, nullopt, {state2Id});
+    int64_t value4Id = value4.id();
+    *config.add_value_metric() = value4;
+
+    // Will be deleted.
+    ValueMetric value5 = createValueMetric("VALUE5", matcher2, nullopt, {});
+    int64_t value5Id = value5.id();
+    *config.add_value_metric() = value5;
+
+    EXPECT_TRUE(initConfig(config));
+
+    // Used later to ensure the condition wizard is replaced. Get it before doing the update.
+    sp<EventMatcherWizard> oldMatcherWizard =
+            static_cast<ValueMetricProducer*>(oldMetricProducers[0].get())->mEventMatcherWizard;
+    EXPECT_EQ(oldMatcherWizard->getStrongCount(), 6);
+
+    // Change value2, causing it to be replaced.
+    value2.set_aggregation_type(ValueMetric::AVG);
+
+    // Mark predicate 2 as replaced. Causes value3 to be replaced.
+    set<int64_t> replacedConditions = {predicate2Id};
+
+    // Mark state 2 as replaced. Causes value4 to be replaced.
+    set<int64_t> replacedStates = {state2Id};
+
+    // New value metric.
+    ValueMetric value6 = createValueMetric("VALUE6", matcher5, predicate1Id, {state1Id});
+    int64_t value6Id = value6.id();
+
+    // Map the matchers and predicates in reverse order to force the indices to change.
+    std::unordered_map<int64_t, int> 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<sp<AtomMatchingTracker>> newAtomMatchingTrackers(5);
+    std::reverse_copy(oldAtomMatchingTrackers.begin(), oldAtomMatchingTrackers.end(),
+                      newAtomMatchingTrackers.begin());
+
+    std::unordered_map<int64_t, int> newConditionTrackerMap;
+    const int predicate2Index = 0;
+    newConditionTrackerMap[predicate2Id] = 0;
+    const int predicate1Index = 1;
+    newConditionTrackerMap[predicate1Id] = 1;
+    // Use the existing conditionTrackers. A bit hacky, but saves code and we don't rely on them.
+    vector<sp<ConditionTracker>> newConditionTrackers(2);
+    std::reverse_copy(oldConditionTrackers.begin(), oldConditionTrackers.end(),
+                      newConditionTrackers.begin());
+    // Say that predicate1 & predicate2 is unknown since the initial condition never changed.
+    vector<ConditionState> conditionCache = {ConditionState::kUnknown, ConditionState::kUnknown};
+
+    StatsdConfig newConfig;
+    *newConfig.add_value_metric() = value6;
+    const int value6Index = 0;
+    *newConfig.add_value_metric() = value3;
+    const int value3Index = 1;
+    *newConfig.add_value_metric() = value1;
+    const int value1Index = 2;
+    *newConfig.add_value_metric() = value4;
+    const int value4Index = 3;
+    *newConfig.add_value_metric() = value2;
+    const int value2Index = 4;
+
+    *newConfig.add_state() = state1;
+    *newConfig.add_state() = state2;
+
+    unordered_map<int64_t, int> stateAtomIdMap;
+    unordered_map<int64_t, unordered_map<int, int64_t>> allStateGroupMaps;
+    map<int64_t, uint64_t> stateProtoHashes;
+    EXPECT_TRUE(initStates(newConfig, stateAtomIdMap, allStateGroupMaps, stateProtoHashes));
+
+    // Output data structures to validate.
+    unordered_map<int64_t, int> newMetricProducerMap;
+    vector<sp<MetricProducer>> newMetricProducers;
+    unordered_map<int, vector<int>> conditionToMetricMap;
+    unordered_map<int, vector<int>> trackerToMetricMap;
+    set<int64_t> noReportMetricIds;
+    unordered_map<int, vector<int>> activationAtomTrackerToMetricMap;
+    unordered_map<int, vector<int>> deactivationAtomTrackerToMetricMap;
+    vector<int> 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<int64_t, int> expectedMetricProducerMap = {
+            {value1Id, value1Index}, {value2Id, value2Index}, {value3Id, value3Index},
+            {value4Id, value4Index}, {value6Id, value6Index},
+    };
+    EXPECT_THAT(newMetricProducerMap, ContainerEq(expectedMetricProducerMap));
+
+    // Make sure preserved metrics are the same.
+    ASSERT_EQ(newMetricProducers.size(), 5);
+    EXPECT_EQ(oldMetricProducers[oldMetricProducerMap.at(value1Id)],
+              newMetricProducers[newMetricProducerMap.at(value1Id)]);
+
+    // Make sure replaced metrics are different.
+    EXPECT_NE(oldMetricProducers[oldMetricProducerMap.at(value2Id)],
+              newMetricProducers[newMetricProducerMap.at(value2Id)]);
+    EXPECT_NE(oldMetricProducers[oldMetricProducerMap.at(value3Id)],
+              newMetricProducers[newMetricProducerMap.at(value3Id)]);
+    EXPECT_NE(oldMetricProducers[oldMetricProducerMap.at(value4Id)],
+              newMetricProducers[newMetricProducerMap.at(value4Id)]);
+
+    // Verify the conditionToMetricMap.
+    ASSERT_EQ(conditionToMetricMap.size(), 2);
+    const vector<int>& condition1Metrics = conditionToMetricMap[predicate1Index];
+    EXPECT_THAT(condition1Metrics, UnorderedElementsAre(value1Index, value6Index));
+    const vector<int>& condition2Metrics = conditionToMetricMap[predicate2Index];
+    EXPECT_THAT(condition2Metrics, UnorderedElementsAre(value3Index));
+
+    // Verify the trackerToMetricMap.
+    ASSERT_EQ(trackerToMetricMap.size(), 4);
+    const vector<int>& matcher1Metrics = trackerToMetricMap[matcher1Index];
+    EXPECT_THAT(matcher1Metrics, UnorderedElementsAre(value2Index));
+    const vector<int>& matcher3Metrics = trackerToMetricMap[matcher3Index];
+    EXPECT_THAT(matcher3Metrics, UnorderedElementsAre(value4Index));
+    const vector<int>& matcher4Metrics = trackerToMetricMap[matcher4Index];
+    EXPECT_THAT(matcher4Metrics, UnorderedElementsAre(value1Index));
+    const vector<int>& matcher5Metrics = trackerToMetricMap[matcher5Index];
+    EXPECT_THAT(matcher5Metrics, UnorderedElementsAre(value3Index, value6Index));
+
+    // 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.
+    ValueMetricProducer* valueProducer1 =
+            static_cast<ValueMetricProducer*>(newMetricProducers[value1Index].get());
+    EXPECT_EQ(valueProducer1->getMetricId(), value1Id);
+    EXPECT_EQ(valueProducer1->mConditionTrackerIndex, predicate1Index);
+    EXPECT_EQ(valueProducer1->mCondition, ConditionState::kUnknown);
+    EXPECT_EQ(valueProducer1->mWhatMatcherIndex, matcher4Index);
+    ValueMetricProducer* valueProducer2 =
+            static_cast<ValueMetricProducer*>(newMetricProducers[value2Index].get());
+    EXPECT_EQ(valueProducer2->getMetricId(), value2Id);
+    EXPECT_EQ(valueProducer2->mConditionTrackerIndex, -1);
+    EXPECT_EQ(valueProducer2->mCondition, ConditionState::kTrue);
+    EXPECT_EQ(valueProducer2->mWhatMatcherIndex, matcher1Index);
+    ValueMetricProducer* valueProducer3 =
+            static_cast<ValueMetricProducer*>(newMetricProducers[value3Index].get());
+    EXPECT_EQ(valueProducer3->getMetricId(), value3Id);
+    EXPECT_EQ(valueProducer3->mConditionTrackerIndex, predicate2Index);
+    EXPECT_EQ(valueProducer3->mCondition, ConditionState::kUnknown);
+    EXPECT_EQ(valueProducer3->mWhatMatcherIndex, matcher5Index);
+    ValueMetricProducer* valueProducer4 =
+            static_cast<ValueMetricProducer*>(newMetricProducers[value4Index].get());
+    EXPECT_EQ(valueProducer4->getMetricId(), value4Id);
+    EXPECT_EQ(valueProducer4->mConditionTrackerIndex, -1);
+    EXPECT_EQ(valueProducer4->mCondition, ConditionState::kTrue);
+    EXPECT_EQ(valueProducer4->mWhatMatcherIndex, matcher3Index);
+    ValueMetricProducer* valueProducer6 =
+            static_cast<ValueMetricProducer*>(newMetricProducers[value6Index].get());
+    EXPECT_EQ(valueProducer6->getMetricId(), value6Id);
+    EXPECT_EQ(valueProducer6->mConditionTrackerIndex, predicate1Index);
+    EXPECT_EQ(valueProducer6->mCondition, ConditionState::kUnknown);
+    EXPECT_EQ(valueProducer6->mWhatMatcherIndex, matcher5Index);
+
+    sp<EventMatcherWizard> newMatcherWizard = valueProducer1->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
@@ -2850,6 +3088,11 @@ TEST_F(ConfigUpdateTest, TestUpdateMetricsMultipleTypes) {
     *config.add_gauge_metric() = gaugeMetric;
 
     // Preserved.
+    ValueMetric valueMetric = createValueMetric("VALUE1", matcher3, predicate1Id, {});
+    int64_t valueMetricId = valueMetric.id();
+    *config.add_value_metric() = valueMetric;
+
+    // Preserved.
     DurationMetric durationMetric = createDurationMetric("DURATION1", predicate1Id, nullopt, {});
     int64_t durationMetricId = durationMetric.id();
     *config.add_duration_metric() = durationMetric;
@@ -2858,7 +3101,7 @@ TEST_F(ConfigUpdateTest, TestUpdateMetricsMultipleTypes) {
 
     // Used later to ensure the condition wizard is replaced. Get it before doing the update.
     sp<ConditionWizard> oldConditionWizard = oldMetricProducers[0]->mWizard;
-    EXPECT_EQ(oldConditionWizard->getStrongCount(), 5);
+    EXPECT_EQ(oldConditionWizard->getStrongCount(), 6);
 
     // Mark matcher 2 as replaced. Causes eventMetric to be replaced.
     set<int64_t> replacedMatchers;
@@ -2889,6 +3132,7 @@ TEST_F(ConfigUpdateTest, TestUpdateMetricsMultipleTypes) {
                       newConditionTrackers.begin());
     vector<ConditionState> conditionCache = {ConditionState::kUnknown};
 
+    // The order matters. we parse in the order of: count, duration, event, value, gauge.
     StatsdConfig newConfig;
     *newConfig.add_count_metric() = countMetric;
     const int countMetricIndex = 0;
@@ -2896,8 +3140,10 @@ TEST_F(ConfigUpdateTest, TestUpdateMetricsMultipleTypes) {
     const int durationMetricIndex = 1;
     *newConfig.add_event_metric() = eventMetric;
     const int eventMetricIndex = 2;
+    *newConfig.add_value_metric() = valueMetric;
+    const int valueMetricIndex = 3;
     *newConfig.add_gauge_metric() = gaugeMetric;
-    const int gaugeMetricIndex = 3;
+    const int gaugeMetricIndex = 4;
 
     // Add the predicate since duration metric needs it.
     *newConfig.add_predicate() = predicate1;
@@ -2922,19 +3168,20 @@ TEST_F(ConfigUpdateTest, TestUpdateMetricsMultipleTypes) {
             metricsWithActivation));
 
     unordered_map<int64_t, int> expectedMetricProducerMap = {
-            {countMetricId, countMetricIndex},
-            {durationMetricId, durationMetricIndex},
-            {eventMetricId, eventMetricIndex},
+            {countMetricId, countMetricIndex}, {durationMetricId, durationMetricIndex},
+            {eventMetricId, eventMetricIndex}, {valueMetricId, valueMetricIndex},
             {gaugeMetricId, gaugeMetricIndex},
     };
     EXPECT_THAT(newMetricProducerMap, ContainerEq(expectedMetricProducerMap));
 
     // Make sure preserved metrics are the same.
-    ASSERT_EQ(newMetricProducers.size(), 4);
+    ASSERT_EQ(newMetricProducers.size(), 5);
     EXPECT_EQ(oldMetricProducers[oldMetricProducerMap.at(countMetricId)],
               newMetricProducers[newMetricProducerMap.at(countMetricId)]);
     EXPECT_EQ(oldMetricProducers[oldMetricProducerMap.at(durationMetricId)],
               newMetricProducers[newMetricProducerMap.at(durationMetricId)]);
+    EXPECT_EQ(oldMetricProducers[oldMetricProducerMap.at(valueMetricId)],
+              newMetricProducers[newMetricProducerMap.at(valueMetricId)]);
 
     // Make sure replaced metrics are different.
     EXPECT_NE(oldMetricProducers[oldMetricProducerMap.at(eventMetricId)],
@@ -2945,7 +3192,8 @@ TEST_F(ConfigUpdateTest, TestUpdateMetricsMultipleTypes) {
     // Verify the conditionToMetricMap.
     ASSERT_EQ(conditionToMetricMap.size(), 1);
     const vector<int>& condition1Metrics = conditionToMetricMap[predicate1Index];
-    EXPECT_THAT(condition1Metrics, UnorderedElementsAre(countMetricIndex, gaugeMetricIndex));
+    EXPECT_THAT(condition1Metrics,
+                UnorderedElementsAre(countMetricIndex, gaugeMetricIndex, valueMetricIndex));
 
     // Verify the trackerToMetricMap.
     ASSERT_EQ(trackerToMetricMap.size(), 3);
@@ -2954,7 +3202,7 @@ TEST_F(ConfigUpdateTest, TestUpdateMetricsMultipleTypes) {
     const vector<int>& matcher2Metrics = trackerToMetricMap[matcher2Index];
     EXPECT_THAT(matcher2Metrics, UnorderedElementsAre(eventMetricIndex, durationMetricIndex));
     const vector<int>& matcher3Metrics = trackerToMetricMap[matcher3Index];
-    EXPECT_THAT(matcher3Metrics, UnorderedElementsAre(gaugeMetricIndex));
+    EXPECT_THAT(matcher3Metrics, UnorderedElementsAre(gaugeMetricIndex, valueMetricIndex));
 
     // Verify event activation/deactivation maps.
     ASSERT_EQ(activationAtomTrackerToMetricMap.size(), 0);
@@ -2977,7 +3225,7 @@ TEST_F(ConfigUpdateTest, TestUpdateMetricsMultipleTypes) {
 
     sp<ConditionWizard> newConditionWizard = newMetricProducers[0]->mWizard;
     EXPECT_NE(newConditionWizard, oldConditionWizard);
-    EXPECT_EQ(newConditionWizard->getStrongCount(), 5);
+    EXPECT_EQ(newConditionWizard->getStrongCount(), 6);
     oldMetricProducers.clear();
     // Only reference to the old wizard should be the one in the test.
     EXPECT_EQ(oldConditionWizard->getStrongCount(), 1);