Merge "Enable multiple aggregation types in Value Metric" into main

14 files changed, 611 insertions, 13 deletions

diff --git a/statsd/src/FieldValue.cpp b/statsd/src/FieldValue.cpp
index aa129a44..84fad927 100644
--- a/statsd/src/FieldValue.cpp
+++ b/statsd/src/FieldValue.cpp
@@ -60,6 +60,17 @@ bool Field::matches(const Matcher& matcher) const {
     return false;
 }
 
+std::vector<Matcher> dedupFieldMatchers(const std::vector<Matcher>& fieldMatchers) {
+    std::vector<Matcher> dedupedFieldMatchers;
+    for (size_t i = 0; i < fieldMatchers.size(); i++) {
+        if (std::find(dedupedFieldMatchers.begin(), dedupedFieldMatchers.end(), fieldMatchers[i]) ==
+            dedupedFieldMatchers.end()) {
+            dedupedFieldMatchers.push_back(fieldMatchers[i]);
+        }
+    }
+    return dedupedFieldMatchers;
+}
+
 void translateFieldMatcher(int tag, const FieldMatcher& matcher, int depth, int* pos, int* mask,
                            std::vector<Matcher>* output) {
     if (depth > kMaxLogDepth) {
diff --git a/statsd/src/FieldValue.h b/statsd/src/FieldValue.h
index 2ac71cb8..66b54755 100644
--- a/statsd/src/FieldValue.h
+++ b/statsd/src/FieldValue.h
@@ -463,6 +463,8 @@ bool isAttributionUidField(const FieldValue& value);
 /* returns uid if the field is uid field, or -1 if the field is not a uid field */
 int getUidIfExists(const FieldValue& value);
 
+std::vector<Matcher> dedupFieldMatchers(const std::vector<Matcher>& fieldMatchers);
+
 void translateFieldMatcher(const FieldMatcher& matcher, std::vector<Matcher>* output);
 
 bool isAttributionUidField(const Field& field, const Value& value);
diff --git a/statsd/src/StatsLogProcessor.h b/statsd/src/StatsLogProcessor.h
index c1c7dd63..b6d66af9 100644
--- a/statsd/src/StatsLogProcessor.h
+++ b/statsd/src/StatsLogProcessor.h
@@ -485,6 +485,8 @@ private:
     FRIEND_TEST(ValueMetricE2eTest, TestInitWithSlicedState_WithDimensions);
     FRIEND_TEST(ValueMetricE2eTest, TestInitWithSlicedState_WithIncorrectDimensions);
     FRIEND_TEST(ValueMetricE2eTest, TestInitWithValueFieldPositionALL);
+    FRIEND_TEST(ValueMetricE2eTest, TestInitWithMultipleAggTypes);
+    FRIEND_TEST(ValueMetricE2eTest, TestInitWithDefaultAggType);
 
     FRIEND_TEST(KllMetricE2eTest, TestInitWithKllFieldPositionALL);
 
diff --git a/statsd/src/guardrail/stats_log_enums.proto b/statsd/src/guardrail/stats_log_enums.proto
index 7b4c79a4..c468d52e 100644
--- a/statsd/src/guardrail/stats_log_enums.proto
+++ b/statsd/src/guardrail/stats_log_enums.proto
@@ -151,6 +151,8 @@ enum InvalidConfigReasonEnum {
     INVALID_CONFIG_REASON_METRIC_INCORRECT_PULL_PROBABILITY = 93;
     INVALID_CONFIG_REASON_GAUGE_METRIC_PUSHED_WITH_PULL_PROBABILITY = 94;
     INVALID_CONFIG_REASON_GAUGE_METRIC_RANDOM_ONE_SAMPLE_WITH_PULL_PROBABILITY = 95;
+    INVALID_CONFIG_REASON_VALUE_METRIC_DEFINES_SINGLE_AND_MULTIPLE_AGG_TYPES = 96;
+    INVALID_CONFIG_REASON_VALUE_METRIC_AGG_TYPES_DNE_VALUE_FIELDS_SIZE = 97;
 };
 
 enum InvalidQueryReason {
diff --git a/statsd/src/metrics/MetricsManager.h b/statsd/src/metrics/MetricsManager.h
index 2072d61b..4ccee562 100644
--- a/statsd/src/metrics/MetricsManager.h
+++ b/statsd/src/metrics/MetricsManager.h
@@ -437,6 +437,8 @@ private:
     FRIEND_TEST(ValueMetricE2eTest, TestInitWithSlicedState);
     FRIEND_TEST(ValueMetricE2eTest, TestInitWithSlicedState_WithDimensions);
     FRIEND_TEST(ValueMetricE2eTest, TestInitWithSlicedState_WithIncorrectDimensions);
+    FRIEND_TEST(ValueMetricE2eTest, TestInitWithMultipleAggTypes);
+    FRIEND_TEST(ValueMetricE2eTest, TestInitWithDefaultAggType);
 };
 
 }  // namespace statsd
diff --git a/statsd/src/metrics/NumericValueMetricProducer.cpp b/statsd/src/metrics/NumericValueMetricProducer.cpp
index 4024ccef..88c4d16e 100644
--- a/statsd/src/metrics/NumericValueMetricProducer.cpp
+++ b/statsd/src/metrics/NumericValueMetricProducer.cpp
@@ -22,6 +22,7 @@
 #include <limits.h>
 #include <stdlib.h>
 
+#include "FieldValue.h"
 #include "guardrail/StatsdStats.h"
 #include "metrics/parsing_utils/metrics_manager_util.h"
 #include "stats_log_util.h"
@@ -70,7 +71,7 @@ NumericValueMetricProducer::NumericValueMetricProducer(
     : ValueMetricProducer(metric.id(), key, protoHash, pullOptions, bucketOptions, whatOptions,
                           conditionOptions, stateOptions, activationOptions, guardrailOptions),
       mUseAbsoluteValueOnReset(metric.use_absolute_value_on_reset()),
-      mAggregationType(metric.aggregation_type()),
+      mAggregationTypes(whatOptions.aggregationTypes),
       mIncludeSampleSize(metric.has_include_sample_size()
                                  ? metric.include_sample_size()
                                  : metric.aggregation_type() == ValueMetric_AggregationType_AVG),
@@ -80,7 +81,8 @@ NumericValueMetricProducer::NumericValueMetricProducer(
       mUseZeroDefaultBase(metric.use_zero_default_base()),
       mHasGlobalBase(false),
       mMaxPullDelayNs(metric.has_max_pull_delay_sec() ? metric.max_pull_delay_sec() * NS_PER_SEC
-                                                      : StatsdStats::kPullMaxDelayNs) {
+                                                      : StatsdStats::kPullMaxDelayNs),
+      mDedupedFieldMatchers(dedupFieldMatchers(whatOptions.fieldMatchers)) {
     // TODO(b/186677791): Use initializer list to initialize mUploadThreshold.
     if (metric.has_threshold()) {
         mUploadThreshold = metric.threshold();
@@ -274,9 +276,9 @@ void NumericValueMetricProducer::accumulateEvents(const vector<shared_ptr<LogEve
 
             // Get dimensions_in_what key and value indices.
             HashableDimensionKey dimensionsInWhat;
-            vector<int> valueIndices(mFieldMatchers.size(), -1);
+            vector<int> valueIndices(mDedupedFieldMatchers.size(), -1);
             const LogEvent& eventRef = transformedEvent == nullptr ? *data : *transformedEvent;
-            if (!filterValues(mDimensionsInWhat, mFieldMatchers, eventRef.getValues(),
+            if (!filterValues(mDimensionsInWhat, mDedupedFieldMatchers, eventRef.getValues(),
                               dimensionsInWhat, valueIndices)) {
                 StatsdStats::getInstance().noteBadValueType(mMetricId);
             }
@@ -478,7 +480,7 @@ bool NumericValueMetricProducer::aggregateFields(const int64_t eventTimeNs,
         }
 
         if (interval.hasValue()) {
-            switch (mAggregationType) {
+            switch (getAggregationTypeLocked(i)) {
                 case ValueMetric::SUM:
                     // for AVG, we add up and take average when flushing the bucket
                 case ValueMetric::AVG:
@@ -664,7 +666,7 @@ bool NumericValueMetricProducer::valuePassesThreshold(const Interval& interval)
 }
 
 Value NumericValueMetricProducer::getFinalValue(const Interval& interval) const {
-    if (mAggregationType != ValueMetric::AVG) {
+    if (getAggregationTypeLocked(interval.aggIndex) != ValueMetric::AVG) {
         return interval.aggregate;
     } else {
         double sum = interval.aggregate.type == LONG ? (double)interval.aggregate.long_value
diff --git a/statsd/src/metrics/NumericValueMetricProducer.h b/statsd/src/metrics/NumericValueMetricProducer.h
index 775fc1af..9c123f64 100644
--- a/statsd/src/metrics/NumericValueMetricProducer.h
+++ b/statsd/src/metrics/NumericValueMetricProducer.h
@@ -142,12 +142,16 @@ private:
     // Internal function to calculate the current used bytes.
     size_t byteSizeLocked() const override;
 
-    void combineValueFields(pair<LogEvent, vector<int>>& eventValues, const LogEvent& newEvent,
-                            const vector<int>& newValueIndices) const;
+    void combineValueFields(pair<LogEvent, std::vector<int>>& eventValues, const LogEvent& newEvent,
+                            const std::vector<int>& newValueIndices) const;
+
+    ValueMetric::AggregationType getAggregationTypeLocked(int index) const {
+        return mAggregationTypes.size() == 1 ? mAggregationTypes[0] : mAggregationTypes[index];
+    }
 
     const bool mUseAbsoluteValueOnReset;
 
-    const ValueMetric::AggregationType mAggregationType;
+    const std::vector<ValueMetric::AggregationType> mAggregationTypes;
 
     const bool mIncludeSampleSize;
 
@@ -171,6 +175,9 @@ private:
 
     const int64_t mMaxPullDelayNs;
 
+    // Deduped value fields for matching.
+    const std::vector<Matcher> mDedupedFieldMatchers;
+
     // For anomaly detection.
     std::unordered_map<MetricDimensionKey, int64_t> mCurrentFullBucket;
 
@@ -232,6 +239,8 @@ private:
     FRIEND_TEST(NumericValueMetricProducerTest,
                 TestSlicedStateWithMultipleDimensionsMissingDataInPull);
     FRIEND_TEST(NumericValueMetricProducerTest, TestUploadThreshold);
+    FRIEND_TEST(NumericValueMetricProducerTest, TestMultipleAggTypesPulled);
+    FRIEND_TEST(NumericValueMetricProducerTest, TestMultipleAggTypesPushed);
 
     FRIEND_TEST(NumericValueMetricProducerTest_BucketDrop, TestInvalidBucketWhenOneConditionFailed);
     FRIEND_TEST(NumericValueMetricProducerTest_BucketDrop, TestInvalidBucketWhenInitialPullFailed);
diff --git a/statsd/src/metrics/ValueMetricProducer.h b/statsd/src/metrics/ValueMetricProducer.h
index 92541029..d10fc091 100644
--- a/statsd/src/metrics/ValueMetricProducer.h
+++ b/statsd/src/metrics/ValueMetricProducer.h
@@ -88,6 +88,7 @@ public:
         const sp<EventMatcherWizard>& matcherWizard;
         const FieldMatcher& dimensionsInWhat;
         const vector<Matcher>& fieldMatchers;
+        const vector<ValueMetric::AggregationType> aggregationTypes;
     };
 
     struct ConditionOptions {
diff --git a/statsd/src/metrics/parsing_utils/metrics_manager_util.cpp b/statsd/src/metrics/parsing_utils/metrics_manager_util.cpp
index f0c5bb25..dfdcc65f 100644
--- a/statsd/src/metrics/parsing_utils/metrics_manager_util.cpp
+++ b/statsd/src/metrics/parsing_utils/metrics_manager_util.cpp
@@ -922,6 +922,27 @@ optional<sp<MetricProducer>> createNumericValueMetricProducerAndUpdateMetadata(
         return nullopt;
     }
 
+    std::vector<ValueMetric::AggregationType> aggregationTypes;
+    if (metric.aggregation_types_size() != 0) {
+        if (metric.has_aggregation_type()) {
+            invalidConfigReason = InvalidConfigReason(
+                    INVALID_CONFIG_REASON_VALUE_METRIC_DEFINES_SINGLE_AND_MULTIPLE_AGG_TYPES,
+                    metric.id());
+            return nullopt;
+        }
+        if (metric.aggregation_types_size() != (int)fieldMatchers.size()) {
+            invalidConfigReason = InvalidConfigReason(
+                    INVALID_CONFIG_REASON_VALUE_METRIC_AGG_TYPES_DNE_VALUE_FIELDS_SIZE,
+                    metric.id());
+            return nullopt;
+        }
+        for (int i = 0; i < metric.aggregation_types_size(); i++) {
+            aggregationTypes.push_back(metric.aggregation_types(i));
+        }
+    } else {  // aggregation_type() is set or default is used.
+        aggregationTypes.push_back(metric.aggregation_type());
+    }
+
     int trackerIndex;
     invalidConfigReason = handleMetricWithAtomMatchingTrackers(
             metric.what(), metric.id(), metricIndex,
@@ -1019,7 +1040,7 @@ optional<sp<MetricProducer>> createNumericValueMetricProducerAndUpdateMetadata(
             {timeBaseNs, currentTimeNs, bucketSizeNs, metric.min_bucket_size_nanos(),
              conditionCorrectionThresholdNs, getAppUpgradeBucketSplit(metric)},
             {containsAnyPositionInDimensionsInWhat, shouldUseNestedDimensions, trackerIndex,
-             matcherWizard, metric.dimensions_in_what(), fieldMatchers},
+             matcherWizard, metric.dimensions_in_what(), fieldMatchers, aggregationTypes},
             {conditionIndex, metric.links(), initialConditionCache, wizard},
             {metric.state_link(), slicedStateAtoms, stateGroupMap},
             {eventActivationMap, eventDeactivationMap}, {dimensionSoftLimit, dimensionHardLimit});
@@ -1171,8 +1192,13 @@ optional<sp<MetricProducer>> createKllMetricProducerAndUpdateMetadata(
             key, metric, metricHash, {/*pullTagId=*/-1, pullerManager},
             {timeBaseNs, currentTimeNs, bucketSizeNs, metric.min_bucket_size_nanos(),
              /*conditionCorrectionThresholdNs=*/nullopt, getAppUpgradeBucketSplit(metric)},
-            {containsAnyPositionInDimensionsInWhat, shouldUseNestedDimensions, trackerIndex,
-             matcherWizard, metric.dimensions_in_what(), fieldMatchers},
+            {containsAnyPositionInDimensionsInWhat,
+             shouldUseNestedDimensions,
+             trackerIndex,
+             matcherWizard,
+             metric.dimensions_in_what(),
+             fieldMatchers,
+             {}},
             {conditionIndex, metric.links(), initialConditionCache, wizard},
             {metric.state_link(), slicedStateAtoms, stateGroupMap},
             {eventActivationMap, eventDeactivationMap}, {dimensionSoftLimit, dimensionHardLimit});
diff --git a/statsd/src/statsd_config.proto b/statsd/src/statsd_config.proto
index b49d3e89..cbd60ee3 100644
--- a/statsd/src/statsd_config.proto
+++ b/statsd/src/statsd_config.proto
@@ -395,6 +395,8 @@ message ValueMetric {
   }
   optional AggregationType aggregation_type = 8 [default = SUM];
 
+  repeated AggregationType aggregation_types = 25;
+
   optional bool include_sample_size = 22;
 
   optional int64 min_bucket_size_nanos = 10;
diff --git a/statsd/tests/FieldValue_test.cpp b/statsd/tests/FieldValue_test.cpp
index cf1e5a5b..8e9f789e 100644
--- a/statsd/tests/FieldValue_test.cpp
+++ b/statsd/tests/FieldValue_test.cpp
@@ -542,6 +542,124 @@ TEST(AtomMatcherTest, TestWriteDimensionPath) {
     }
 }
 
+TEST(AtomMatcherTest, TestDedupFieldMatchersAllDifferent) {
+    // Matchers: Fields 1, 2, 3
+    FieldMatcher matcher1;
+    matcher1.set_field(10);
+    FieldMatcher* child = matcher1.add_child();
+    child->set_field(1);
+    child = matcher1.add_child();
+    child->set_field(2);
+    child = matcher1.add_child();
+    child->set_field(3);
+
+    vector<Matcher> fieldMatchers;
+    translateFieldMatcher(matcher1, &fieldMatchers);
+    ASSERT_EQ(3, fieldMatchers.size());
+
+    // Deduped Matchers: Fields 1, 2, 3
+    std::vector<Matcher> dedupedFieldMatchers = dedupFieldMatchers(fieldMatchers);
+    ASSERT_EQ((size_t)3, dedupedFieldMatchers.size());
+    EXPECT_EQ(fieldMatchers[0], dedupedFieldMatchers[0]);
+    EXPECT_EQ(fieldMatchers[1], dedupedFieldMatchers[1]);
+    EXPECT_EQ(fieldMatchers[2], dedupedFieldMatchers[2]);
+}
+
+TEST(AtomMatcherTest, TestDedupFieldMatchersAllSame) {
+    // Matcher: Fields 1, 1, 1
+    FieldMatcher matcher1;
+    matcher1.set_field(10);
+    FieldMatcher* child = matcher1.add_child();
+    child->set_field(1);
+    child = matcher1.add_child();
+    child->set_field(1);
+    child = matcher1.add_child();
+    child->set_field(1);
+
+    vector<Matcher> fieldMatchers;
+    translateFieldMatcher(matcher1, &fieldMatchers);
+    ASSERT_EQ(3, fieldMatchers.size());
+
+    // Deduped Matchers: Fields 1, 1, 1
+    std::vector<Matcher> dedupedFieldMatchers = dedupFieldMatchers(fieldMatchers);
+    ASSERT_EQ((size_t)1, dedupedFieldMatchers.size());
+    EXPECT_EQ(fieldMatchers[0], dedupedFieldMatchers[0]);
+}
+
+TEST(AtomMatcherTest, TestDedupFieldMatcherMixOfFields) {
+    // Matcher: Fields 2, 2, 1, 3, 2, 1, 3
+    FieldMatcher matcher1;
+    matcher1.set_field(10);
+    FieldMatcher* child = matcher1.add_child();
+    child->set_field(2);
+    child = matcher1.add_child();
+    child->set_field(2);
+    child = matcher1.add_child();
+    child->set_field(1);
+    child = matcher1.add_child();
+    child->set_field(3);
+    child = matcher1.add_child();
+    child->set_field(2);
+    child = matcher1.add_child();
+    child->set_field(1);
+    child = matcher1.add_child();
+    child->set_field(3);
+
+    vector<Matcher> fieldMatchers;
+    translateFieldMatcher(matcher1, &fieldMatchers);
+    ASSERT_EQ(7, fieldMatchers.size());
+
+    // Deduped Matchers: Fields 2, 1, 3
+    std::vector<Matcher> dedupedFieldMatchers = dedupFieldMatchers(fieldMatchers);
+    ASSERT_EQ((size_t)3, dedupedFieldMatchers.size());
+    EXPECT_EQ(fieldMatchers[0], dedupedFieldMatchers[0]);
+    EXPECT_EQ(fieldMatchers[2], dedupedFieldMatchers[1]);
+    EXPECT_EQ(fieldMatchers[3], dedupedFieldMatchers[2]);
+}
+
+TEST(AtomMatcherTest, TestDedupFieldMatcherDifferentPositionSameFields) {
+    // Matcher: Fields 3, 1.1(FIRST), 1.2(FIRST), 1.1(FIRST), 1.1(LAST), 1.2(FIRST), 2
+    FieldMatcher matcher1;
+    matcher1.set_field(10);
+    FieldMatcher* child = matcher1.add_child();
+    child->set_field(3);
+    child = matcher1.add_child();
+    child->set_field(1);
+    child->set_position(Position::FIRST);
+    child->add_child()->set_field(1);
+    child = matcher1.add_child();
+    child->set_field(1);
+    child->set_position(Position::FIRST);
+    child->add_child()->set_field(2);
+    child = matcher1.add_child();
+    child->set_field(1);
+    child->set_position(Position::FIRST);
+    child->add_child()->set_field(1);
+    child = matcher1.add_child();
+    child->set_field(1);
+    child->set_position(Position::LAST);
+    child->add_child()->set_field(1);
+    child = matcher1.add_child();
+    child->set_field(1);
+    child->set_position(Position::FIRST);
+    child->add_child()->set_field(2);
+    child = matcher1.add_child();
+    child->set_field(2);
+
+    vector<Matcher> fieldMatchers;
+    translateFieldMatcher(matcher1, &fieldMatchers);
+    ASSERT_EQ(7, fieldMatchers.size());
+
+    // Deduped Matchers: Fields 3, 1.1(FIRST), 1.2(FIRST), 1.1(LAST) 2
+    std::vector<Matcher> dedupedFieldMatchers = dedupFieldMatchers(fieldMatchers);
+    ASSERT_EQ((size_t)5, dedupedFieldMatchers.size());
+    EXPECT_EQ(fieldMatchers[0], dedupedFieldMatchers[0]);
+    EXPECT_EQ(fieldMatchers[1], dedupedFieldMatchers[1]);
+    EXPECT_EQ(fieldMatchers[2], dedupedFieldMatchers[2]);
+    EXPECT_EQ(fieldMatchers[4], dedupedFieldMatchers[3]);
+    EXPECT_EQ(fieldMatchers[6], dedupedFieldMatchers[4]);
+}
+
 void checkAttributionNodeInDimensionsValueParcel(StatsDimensionsValueParcel& attributionNodeParcel,
                                                  int32_t nodeDepthInAttributionChain,
                                                  int32_t uid, string tag) {
diff --git a/statsd/tests/e2e/ValueMetric_pull_e2e_test.cpp b/statsd/tests/e2e/ValueMetric_pull_e2e_test.cpp
index d36adfd7..323ed173 100644
--- a/statsd/tests/e2e/ValueMetric_pull_e2e_test.cpp
+++ b/statsd/tests/e2e/ValueMetric_pull_e2e_test.cpp
@@ -831,6 +831,76 @@ TEST(ValueMetricE2eTest, TestInitWithValueFieldPositionALL) {
     ASSERT_EQ(0, processor->mMetricsManagers.size());
 }
 
+TEST(ValueMetricE2eTest, TestInitWithMultipleAggTypes) {
+    // Create config.
+    StatsdConfig config;
+
+    AtomMatcher testAtomReportedMatcher =
+            CreateSimpleAtomMatcher("TestAtomReportedMatcher", util::TEST_ATOM_REPORTED);
+    *config.add_atom_matcher() = testAtomReportedMatcher;
+
+    // Create value metric.
+    int64_t metricId = 123456;
+    ValueMetric* valueMetric = config.add_value_metric();
+    valueMetric->set_id(metricId);
+    valueMetric->set_bucket(TimeUnit::FIVE_MINUTES);
+    valueMetric->set_what(testAtomReportedMatcher.id());
+    *valueMetric->mutable_value_field() = CreateDimensions(
+            util::TEST_ATOM_REPORTED, {2 /*int_field*/, 2 /*int_field*/, 3 /*long_field*/,
+                                       3 /*long_field*/, 3 /*long_field*/});
+    valueMetric->add_aggregation_types(ValueMetric::SUM);
+    valueMetric->add_aggregation_types(ValueMetric::MIN);
+    valueMetric->add_aggregation_types(ValueMetric::MAX);
+    valueMetric->add_aggregation_types(ValueMetric::AVG);
+    valueMetric->add_aggregation_types(ValueMetric::MIN);
+
+    // Initialize StatsLogProcessor.
+    const uint64_t bucketStartTimeNs = 10000000000;  // 0:10
+    int uid = 12345;
+    int64_t cfgId = 98765;
+    ConfigKey cfgKey(uid, cfgId);
+    sp<StatsLogProcessor> processor =
+            CreateStatsLogProcessor(bucketStartTimeNs, bucketStartTimeNs, config, cfgKey);
+
+    ASSERT_EQ(1, processor->mMetricsManagers.size());
+    sp<MetricsManager> metricsManager = processor->mMetricsManagers.begin()->second;
+    EXPECT_TRUE(metricsManager->isConfigValid());
+    ASSERT_EQ(1, metricsManager->mAllMetricProducers.size());
+    sp<MetricProducer> metricProducer = metricsManager->mAllMetricProducers[0];
+}
+
+TEST(ValueMetricE2eTest, TestInitWithDefaultAggType) {
+    // Create config.
+    StatsdConfig config;
+
+    AtomMatcher testAtomReportedMatcher =
+            CreateSimpleAtomMatcher("TestAtomReportedMatcher", util::TEST_ATOM_REPORTED);
+    *config.add_atom_matcher() = testAtomReportedMatcher;
+
+    // Create value metric.
+    int64_t metricId = 123456;
+    ValueMetric* valueMetric = config.add_value_metric();
+    valueMetric->set_id(metricId);
+    valueMetric->set_bucket(TimeUnit::FIVE_MINUTES);
+    valueMetric->set_what(testAtomReportedMatcher.id());
+    *valueMetric->mutable_value_field() =
+            CreateDimensions(util::TEST_ATOM_REPORTED, {3 /*long_field*/, 2 /*int_field*/});
+
+    // Initialize StatsLogProcessor.
+    const uint64_t bucketStartTimeNs = 10000000000;  // 0:10
+    int uid = 12345;
+    int64_t cfgId = 98765;
+    ConfigKey cfgKey(uid, cfgId);
+    sp<StatsLogProcessor> processor =
+            CreateStatsLogProcessor(bucketStartTimeNs, bucketStartTimeNs, config, cfgKey);
+
+    ASSERT_EQ(1, processor->mMetricsManagers.size());
+    sp<MetricsManager> metricsManager = processor->mMetricsManagers.begin()->second;
+    EXPECT_TRUE(metricsManager->isConfigValid());
+    ASSERT_EQ(1, metricsManager->mAllMetricProducers.size());
+    sp<MetricProducer> metricProducer = metricsManager->mAllMetricProducers[0];
+}
+
 #else
 GTEST_LOG_(INFO) << "This test does nothing.\n";
 #endif
diff --git a/statsd/tests/metrics/NumericValueMetricProducer_test.cpp b/statsd/tests/metrics/NumericValueMetricProducer_test.cpp
index 2381f04d..356eab91 100644
--- a/statsd/tests/metrics/NumericValueMetricProducer_test.cpp
+++ b/statsd/tests/metrics/NumericValueMetricProducer_test.cpp
@@ -215,13 +215,22 @@ public:
                         ? optional<int64_t>(metric.condition_correction_threshold_nanos())
                         : nullopt;
 
+        std::vector<ValueMetric::AggregationType> aggregationTypes;
+        if (metric.aggregation_types_size() != 0) {
+            for (int i = 0; i < metric.aggregation_types_size(); i++) {
+                aggregationTypes.push_back(metric.aggregation_types(i));
+            }
+        } else {  // aggregation_type() is set or default is used.
+            aggregationTypes.push_back(metric.aggregation_type());
+        }
+
         sp<NumericValueMetricProducer> valueProducer = new NumericValueMetricProducer(
                 kConfigKey, metric, protoHash, {pullAtomId, pullerManager},
                 {timeBaseNs, startTimeNs, bucketSizeNs, metric.min_bucket_size_nanos(),
                  conditionCorrectionThresholdNs, metric.split_bucket_for_app_upgrade()},
                 {containsAnyPositionInDimensionsInWhat, shouldUseNestedDimensions,
                  logEventMatcherIndex, eventMatcherWizard, metric.dimensions_in_what(),
-                 fieldMatchers},
+                 fieldMatchers, aggregationTypes},
                 {conditionIndex, metric.links(), initialConditionCache, wizard},
                 {metric.state_link(), slicedStateAtoms, stateGroupMap},
                 {/*eventActivationMap=*/{}, /*eventDeactivationMap=*/{}},
@@ -7734,6 +7743,266 @@ TEST(NumericValueMetricProducerTest, TestDimensionalSampling) {
                         0);  // Diff of 15 and 18
 }
 
+TEST(NumericValueMetricProducerTest, TestMultipleAggTypesPulled) {
+    ValueMetric metric = NumericValueMetricProducerTestHelper::createMetricWithCondition();
+    // createMetricWithCondition() adds field 2 as first value field.
+    metric.mutable_value_field()->add_child()->set_field(2);
+    metric.mutable_value_field()->add_child()->set_field(2);
+    metric.mutable_value_field()->add_child()->set_field(2);
+    metric.mutable_value_field()->add_child()->set_field(1);
+    metric.add_aggregation_types(ValueMetric::MIN);
+    metric.add_aggregation_types(ValueMetric::MAX);
+    metric.add_aggregation_types(ValueMetric::SUM);
+    metric.add_aggregation_types(ValueMetric::AVG);
+    metric.add_aggregation_types(ValueMetric::SUM);
+
+    sp<MockStatsPullerManager> pullerManager = new StrictMock<MockStatsPullerManager>();
+
+    EXPECT_CALL(*pullerManager, Pull(tagId, kConfigKey, _, _))
+            // Screen On Pull 1.
+            .WillOnce(Invoke([](int tagId, const ConfigKey&, const int64_t eventTimeNs,
+                                vector<std::shared_ptr<LogEvent>>* data) {
+                data->clear();
+                data->push_back(
+                        CreateTwoValueLogEvent(tagId, bucketStartTimeNs + 30 * NS_PER_SEC, 1, 2));
+                data->push_back(
+                        CreateTwoValueLogEvent(tagId, bucketStartTimeNs + 30 * NS_PER_SEC, 2, 4));
+                return true;
+            }))
+            // Screen Off Pull 2.
+            .WillOnce(Invoke([](int tagId, const ConfigKey&, const int64_t eventTimeNs,
+                                vector<std::shared_ptr<LogEvent>>* data) {
+                data->clear();
+                data->push_back(
+                        CreateTwoValueLogEvent(tagId, bucketStartTimeNs + 40 * NS_PER_SEC, 3, 5));
+                data->push_back(
+                        CreateTwoValueLogEvent(tagId, bucketStartTimeNs + 40 * NS_PER_SEC, 4, 9));
+                return true;
+            }))
+            // Screen On Pull 3.
+            .WillOnce(Invoke([](int tagId, const ConfigKey&, const int64_t eventTimeNs,
+                                vector<std::shared_ptr<LogEvent>>* data) {
+                data->clear();
+                data->push_back(
+                        CreateTwoValueLogEvent(tagId, bucketStartTimeNs + 50 * NS_PER_SEC, 5, 10));
+                data->push_back(
+                        CreateTwoValueLogEvent(tagId, bucketStartTimeNs + 50 * NS_PER_SEC, 6, 20));
+                return true;
+            }))
+            // Dump report pull.
+            .WillOnce(Invoke([](int tagId, const ConfigKey&, const int64_t eventTimeNs,
+                                vector<std::shared_ptr<LogEvent>>* data) {
+                data->clear();
+                data->push_back(CreateTwoValueLogEvent(tagId, bucket2StartTimeNs + 55 * NS_PER_SEC,
+                                                       25, 60));
+                data->push_back(CreateTwoValueLogEvent(tagId, bucket2StartTimeNs + 55 * NS_PER_SEC,
+                                                       35, 80));
+
+                return true;
+            }));
+
+    sp<NumericValueMetricProducer> valueProducer =
+            NumericValueMetricProducerTestHelper::createValueProducerWithCondition(
+                    pullerManager, metric, ConditionState::kFalse);
+
+    EXPECT_EQ(5, valueProducer->mFieldMatchers.size());
+    ASSERT_EQ(5, valueProducer->mAggregationTypes.size());
+    EXPECT_EQ(ValueMetric::MIN, valueProducer->mAggregationTypes[0]);
+    EXPECT_EQ(ValueMetric::MAX, valueProducer->mAggregationTypes[1]);
+    EXPECT_EQ(ValueMetric::SUM, valueProducer->mAggregationTypes[2]);
+    EXPECT_EQ(ValueMetric::AVG, valueProducer->mAggregationTypes[3]);
+    EXPECT_EQ(ValueMetric::SUM, valueProducer->mAggregationTypes[4]);
+
+    // Screen On. Pull 1.
+    valueProducer->onConditionChanged(true, bucketStartTimeNs + 30 * NS_PER_SEC);
+
+    // Screen Off.
+    valueProducer->onConditionChanged(false, bucketStartTimeNs + 40 * NS_PER_SEC);
+
+    // Screen On. Pull 2.
+    valueProducer->onConditionChanged(true, bucketStartTimeNs + 50 * NS_PER_SEC);
+
+    // Bucket 2 start. Pull 4.
+    vector<shared_ptr<LogEvent>> allData;
+    allData.clear();
+    allData.push_back(CreateTwoValueLogEvent(tagId, bucket2StartTimeNs, 15, 30));
+    allData.push_back(CreateTwoValueLogEvent(tagId, bucket2StartTimeNs, 20, 40));
+    valueProducer->onDataPulled(allData, PullResult::PULL_RESULT_SUCCESS, bucket2StartTimeNs);
+
+    // Check dump report.
+    ProtoOutputStream output;
+    std::set<string> strSet;
+    int64_t dumpReportTimeNs = bucket2StartTimeNs + 55 * NS_PER_SEC;
+    valueProducer->onDumpReport(dumpReportTimeNs, true /* include current buckets */, true,
+                                NO_TIME_CONSTRAINTS /* dumpLatency */, &strSet, &output);
+
+    StatsLogReport report = outputStreamToProto(&output);
+    backfillDimensionPath(&report);
+    backfillStartEndTimestamp(&report);
+    EXPECT_TRUE(report.has_value_metrics());
+    StatsLogReport::ValueMetricDataWrapper valueMetrics;
+    sortMetricDataByDimensionsValue(report.value_metrics(), &valueMetrics);
+    ASSERT_EQ(1, valueMetrics.data_size());
+    EXPECT_EQ(0, report.value_metrics().skipped_size());
+
+    // Bucket 1.
+    // Value field 1
+    // Diff from pulls 1 and 2: (3+4)-(1+2) = 4
+    // Diff from pulls 3 and 4: (15+20)-(5+6) = 24
+
+    // Value field 2
+    // Diff from pulls 1 and 2: (5+9)-(2+4) = 8
+    // Diff from pulls 3 and 4: (30+40)-(10+20) = 40
+
+    // Bucket 2
+    // Value field 1
+    // Diff from pulls 4 and 5: (25+35)-(15+20) = 25
+
+    // Value field 2
+    // Diff from pulls 4 and 5: (60+80)-(30+40) = 70
+
+    // Output values are calculated for these agg type - value field combinations
+    // MIN-2, MAX-2, SUM-2, AVG-2, SUM-1
+    ValueMetricData data = valueMetrics.data(0);
+    ASSERT_EQ(2, data.bucket_info_size());
+    ValidateValueBucket(data.bucket_info(0), bucketStartTimeNs, bucket2StartTimeNs,
+                        {8, 40, 48, 24, 28}, 20 * NS_PER_SEC, 0);
+    ValidateValueBucket(data.bucket_info(1), bucket2StartTimeNs, dumpReportTimeNs,
+                        {70, 70, 70, 70, 25}, 55 * NS_PER_SEC, 0);
+}
+
+TEST(NumericValueMetricProducerTest, TestMultipleAggTypesPushed) {
+    ValueMetric metric = NumericValueMetricProducerTestHelper::createMetric();
+    metric.mutable_dimensions_in_what()->set_field(tagId);
+    metric.mutable_dimensions_in_what()->add_child()->set_field(1);
+    // createMetric() adds field 2 as first value field.
+    metric.mutable_value_field()->add_child()->set_field(2);
+    metric.mutable_value_field()->add_child()->set_field(2);
+    metric.mutable_value_field()->add_child()->set_field(2);
+    metric.mutable_value_field()->add_child()->set_field(3);
+    metric.add_aggregation_types(ValueMetric::MIN);
+    metric.add_aggregation_types(ValueMetric::MAX);
+    metric.add_aggregation_types(ValueMetric::SUM);
+    metric.add_aggregation_types(ValueMetric::AVG);
+    metric.add_aggregation_types(ValueMetric::SUM);
+
+    sp<EventMatcherWizard> eventMatcherWizard =
+            createEventMatcherWizard(tagId, logEventMatcherIndex);
+    sp<MockConditionWizard> wizard = new NaggyMock<MockConditionWizard>();
+    sp<MockStatsPullerManager> pullerManager = new StrictMock<MockStatsPullerManager>();
+
+    sp<NumericValueMetricProducer> valueProducer =
+            NumericValueMetricProducerTestHelper::createValueProducerNoConditions(
+                    pullerManager, metric, /*pullAtomId=*/-1);
+
+    EXPECT_EQ(5, valueProducer->mFieldMatchers.size());
+    ASSERT_EQ(5, valueProducer->mAggregationTypes.size());
+    EXPECT_EQ(ValueMetric::MIN, valueProducer->mAggregationTypes[0]);
+    EXPECT_EQ(ValueMetric::MAX, valueProducer->mAggregationTypes[1]);
+    EXPECT_EQ(ValueMetric::SUM, valueProducer->mAggregationTypes[2]);
+    EXPECT_EQ(ValueMetric::AVG, valueProducer->mAggregationTypes[3]);
+    EXPECT_EQ(ValueMetric::SUM, valueProducer->mAggregationTypes[4]);
+
+    // Bucket 1 events.
+    LogEvent event1(/*uid=*/0, /*pid=*/0);
+    CreateThreeValueLogEvent(&event1, tagId, bucketStartTimeNs + 10, 1, 5, 10);
+
+    LogEvent event2(/*uid=*/0, /*pid=*/0);
+    CreateThreeValueLogEvent(&event2, tagId, bucketStartTimeNs + 20, 1, 6, 8);
+
+    LogEvent event3(/*uid=*/0, /*pid=*/0);
+    CreateThreeValueLogEvent(&event3, tagId, bucketStartTimeNs + 40, 2, 3, 10);
+
+    LogEvent event4(/*uid=*/0, /*pid=*/0);
+    CreateThreeValueLogEvent(&event4, tagId, bucketStartTimeNs + 50, 2, 4, 6);
+
+    LogEvent event5(/*uid=*/0, /*pid=*/0);
+    CreateThreeValueLogEvent(&event5, tagId, bucketStartTimeNs + 30, 1, 19, 9);
+
+    LogEvent event6(/*uid=*/0, /*pid=*/0);
+    CreateThreeValueLogEvent(&event6, tagId, bucketStartTimeNs + 60, 2, 20, 8);
+
+    // Bucket 2 events.
+    LogEvent event7(/*uid=*/0, /*pid=*/0);
+    CreateThreeValueLogEvent(&event7, tagId, bucket2StartTimeNs + 10, 2, 7, 41);
+
+    LogEvent event8(/*uid=*/0, /*pid=*/0);
+    CreateThreeValueLogEvent(&event8, tagId, bucket2StartTimeNs + 20, 1, 21, 40);
+
+    LogEvent event9(/*uid=*/0, /*pid=*/0);
+    CreateThreeValueLogEvent(&event9, tagId, bucket2StartTimeNs + 30, 1, 10, 4);
+
+    LogEvent event10(/*uid=*/0, /*pid=*/0);
+    CreateThreeValueLogEvent(&event10, tagId, bucket2StartTimeNs + 40, 2, 3, 50);
+
+    LogEvent event11(/*uid=*/0, /*pid=*/0);
+    CreateThreeValueLogEvent(&event11, tagId, bucket2StartTimeNs + 50, 1, 20, 7);
+
+    LogEvent event12(/*uid=*/0, /*pid=*/0);
+    CreateThreeValueLogEvent(&event12, tagId, bucket2StartTimeNs + 60, 2, 20, 2);
+
+    valueProducer->onMatchedLogEvent(1 /*log matcher index*/, event1);
+    valueProducer->onMatchedLogEvent(1 /*log matcher index*/, event2);
+    valueProducer->onMatchedLogEvent(1 /*log matcher index*/, event3);
+    valueProducer->onMatchedLogEvent(1 /*log matcher index*/, event4);
+    valueProducer->onMatchedLogEvent(1 /*log matcher index*/, event5);
+    valueProducer->onMatchedLogEvent(1 /*log matcher index*/, event6);
+    valueProducer->onMatchedLogEvent(1 /*log matcher index*/, event7);
+    valueProducer->onMatchedLogEvent(1 /*log matcher index*/, event8);
+    valueProducer->onMatchedLogEvent(1 /*log matcher index*/, event9);
+    valueProducer->onMatchedLogEvent(1 /*log matcher index*/, event10);
+    valueProducer->onMatchedLogEvent(1 /*log matcher index*/, event11);
+    valueProducer->onMatchedLogEvent(1 /*log matcher index*/, event12);
+
+    // Check dump report.
+    ProtoOutputStream output;
+    valueProducer->onDumpReport(bucket3StartTimeNs + 10000, false /* include recent buckets */,
+                                true, FAST /* dumpLatency */, nullptr, &output);
+
+    StatsLogReport report = outputStreamToProto(&output);
+    backfillDimensionPath(&report);
+    backfillStartEndTimestamp(&report);
+    EXPECT_TRUE(report.has_value_metrics());
+    StatsLogReport::ValueMetricDataWrapper valueMetrics;
+    sortMetricDataByDimensionsValue(report.value_metrics(), &valueMetrics);
+    ASSERT_EQ(2, valueMetrics.data_size());
+    EXPECT_EQ(0, report.value_metrics().skipped_size());
+
+    // Bucket 1.
+    // Value field 2
+    // dim 1 pushed values: 5, 6, 19
+    // dim 2 pushed values: 3, 4, 20
+
+    // Value field 3
+    // dim 1 pushed values: 10, 8, 9
+    // dim 2 pushed values: 10, 6, 8
+
+    // Bucket 2
+    // Value field 2
+    // dim 1 pushed values: 21, 10, 20
+    // dim 2 pushed values: 7, 3, 20
+
+    // Value field 3
+    // dim 1 pushed values: 40, 4, 7
+    // dim 2 pushed values: 41, 50, 2
+
+    // Output values are calculated for these agg type - value field combinations
+    // MIN-2, MAX-2, SUM-2, AVG-2, SUM-1
+    ValueMetricData data = valueMetrics.data(0);
+    ASSERT_EQ(2, data.bucket_info_size());
+    ValidateValueBucket(data.bucket_info(0), bucketStartTimeNs, bucket2StartTimeNs,
+                        {5, 19, 30, 10, 27}, 0, 0);
+    ValidateValueBucket(data.bucket_info(1), bucket2StartTimeNs, bucket3StartTimeNs,
+                        {10, 21, 51, 17, 51}, 0, 0);
+
+    data = valueMetrics.data(1);
+    ASSERT_EQ(2, data.bucket_info_size());
+    ValidateValueBucket(data.bucket_info(0), bucketStartTimeNs, bucket2StartTimeNs,
+                        {3, 20, 27, 9, 24}, 0, 0);
+    ValidateValueBucket(data.bucket_info(1), bucket2StartTimeNs, bucket3StartTimeNs,
+                        {3, 20, 30, 10, 93}, 0, 0);
+}
+
 }  // namespace statsd
 }  // namespace os
 }  // namespace android
diff --git a/statsd/tests/metrics/parsing_utils/metrics_manager_util_test.cpp b/statsd/tests/metrics/parsing_utils/metrics_manager_util_test.cpp
index 67aa8056..92eeee76 100644
--- a/statsd/tests/metrics/parsing_utils/metrics_manager_util_test.cpp
+++ b/statsd/tests/metrics/parsing_utils/metrics_manager_util_test.cpp
@@ -805,6 +805,88 @@ TEST_F(MetricsManagerUtilTest, TestNumericValueMetricConditionlinkNoCondition) {
                                   StringToId("NumericValue")));
 }
 
+TEST_F(MetricsManagerUtilTest, TestNumericValueMetricHasBothSingleAndMultipleAggTypes) {
+    StatsdConfig config;
+    *config.add_atom_matcher() = CreateScreenTurnedOnAtomMatcher();
+
+    ValueMetric* metric = config.add_value_metric();
+    *metric = createValueMetric(/*name=*/"NumericValue", /*what=*/CreateScreenTurnedOnAtomMatcher(),
+                                /*valueField=*/2, /*condition=*/nullopt, /*states=*/{});
+    metric->set_aggregation_type(ValueMetric::SUM);
+    metric->add_aggregation_types(ValueMetric::SUM);
+    metric->add_aggregation_types(ValueMetric::MIN);
+
+    EXPECT_EQ(initConfig(config),
+              InvalidConfigReason(
+                      INVALID_CONFIG_REASON_VALUE_METRIC_DEFINES_SINGLE_AND_MULTIPLE_AGG_TYPES,
+                      StringToId("NumericValue")));
+}
+
+TEST_F(MetricsManagerUtilTest, TestNumericValueMetricMoreAggTypesThanValueFields) {
+    StatsdConfig config;
+    *config.add_atom_matcher() = CreateScreenTurnedOnAtomMatcher();
+
+    ValueMetric* metric = config.add_value_metric();
+    *metric = createValueMetric(/*name=*/"NumericValue", /*what=*/CreateScreenTurnedOnAtomMatcher(),
+                                /*valueField=*/2, /*condition=*/nullopt, /*states=*/{});
+    metric->add_aggregation_types(ValueMetric::SUM);
+    metric->add_aggregation_types(ValueMetric::MIN);
+
+    EXPECT_EQ(
+            initConfig(config),
+            InvalidConfigReason(INVALID_CONFIG_REASON_VALUE_METRIC_AGG_TYPES_DNE_VALUE_FIELDS_SIZE,
+                                StringToId("NumericValue")));
+}
+
+TEST_F(MetricsManagerUtilTest, TestNumericValueMetricMoreValueFieldsThanAggTypes) {
+    StatsdConfig config;
+    *config.add_atom_matcher() = CreateScreenTurnedOnAtomMatcher();
+
+    ValueMetric* metric = config.add_value_metric();
+    *metric = createValueMetric(/*name=*/"NumericValue", /*what=*/CreateScreenTurnedOnAtomMatcher(),
+                                /*valueField=*/2, /*condition=*/nullopt, /*states=*/{});
+    // This only fails if the repeated aggregation field is used. If the single field is used,
+    // we will apply this aggregation type to all value fields.
+    metric->add_aggregation_types(ValueMetric::SUM);
+    metric->add_aggregation_types(ValueMetric::MIN);
+    *metric->mutable_value_field() = CreateDimensions(
+            util::SUBSYSTEM_SLEEP_STATE, {3 /* count */, 4 /* time_millis */, 3 /* count */});
+
+    EXPECT_EQ(
+            initConfig(config),
+            InvalidConfigReason(INVALID_CONFIG_REASON_VALUE_METRIC_AGG_TYPES_DNE_VALUE_FIELDS_SIZE,
+                                StringToId("NumericValue")));
+}
+
+TEST_F(MetricsManagerUtilTest, TestNumericValueMetricDefaultAggTypeOutOfOrderFields) {
+    StatsdConfig config;
+    *config.add_atom_matcher() = CreateScreenTurnedOnAtomMatcher();
+
+    ValueMetric* metric = config.add_value_metric();
+    *metric = createValueMetric(/*name=*/"NumericValue", /*what=*/CreateScreenTurnedOnAtomMatcher(),
+                                /*valueField=*/2, /*condition=*/nullopt, /*states=*/{});
+    *metric->mutable_value_field() =
+            CreateDimensions(util::SUBSYSTEM_SLEEP_STATE, {4 /* time_millis */, 3 /* count */});
+
+    EXPECT_EQ(initConfig(config), nullopt);
+}
+
+TEST_F(MetricsManagerUtilTest, TestNumericValueMetricMultipleAggTypesOutOfOrderFields) {
+    StatsdConfig config;
+    *config.add_atom_matcher() = CreateScreenTurnedOnAtomMatcher();
+
+    ValueMetric* metric = config.add_value_metric();
+    *metric = createValueMetric(/*name=*/"NumericValue", /*what=*/CreateScreenTurnedOnAtomMatcher(),
+                                /*valueField=*/2, /*condition=*/nullopt, /*states=*/{});
+    metric->add_aggregation_types(ValueMetric::SUM);
+    metric->add_aggregation_types(ValueMetric::MIN);
+    metric->add_aggregation_types(ValueMetric::SUM);
+    *metric->mutable_value_field() = CreateDimensions(
+            util::SUBSYSTEM_SLEEP_STATE, {3 /* count */, 4 /* time_millis */, 3 /* count */});
+
+    EXPECT_EQ(initConfig(config), nullopt);
+}
+
 TEST_F(MetricsManagerUtilTest, TestKllMetricMissingIdOrWhat) {
     StatsdConfig config;
     int64_t metricId = 1;