From 97db3ffa71fb0ed2c02a30bd3abd25f2977374ef Mon Sep 17 00:00:00 2001
From: Tej Singh <singhtejinder@google.com>
Date: Mon, 27 Jan 2020 16:52:17 -0800
Subject: [PATCH] Fix Statsd rejecting configs

Statsd is rejecting configs for two reasons as a result of the new
puller API:

1. It expected all of the required pullers to be in the puller map when
the config was sent. This is no longer required to be true since the map
is updated as pullers are registered. Most significantly, no pullers
will be present on boot when we parse the config.

2. In metrics_manager_util, we would create a StatsPullerManager on the
local stack and check that for the puller existing, rather than the sp<>
that was passed in. Since we changed the big puller map to be non-static
in ag/10174227, checking the map from the local pullerManager is
guaranteed to fail, since none of the pullers are added to that puller
manager.

This Cl fixes both issues.

Bug: 148405638
Test: added the config used in b/148405638. Made sure it was valid when
pushed on boot, and at some point after boot after the pullers
registered.

Change-Id: If083bda03b5db2822347d518bba23573b573bf89
---
 cmds/statsd/src/external/StatsPullerManager.cpp  | 6 +++---
 cmds/statsd/src/guardrail/StatsdStats.h          | 6 ++++++
 cmds/statsd/src/metrics/metrics_manager_util.cpp | 5 ++---
 cmds/statsd/src/stats_log_util.h                 | 4 ++++
 4 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/cmds/statsd/src/external/StatsPullerManager.cpp b/cmds/statsd/src/external/StatsPullerManager.cpp
index 7708e300929e4..85d1e38d6052a 100644
--- a/cmds/statsd/src/external/StatsPullerManager.cpp
+++ b/cmds/statsd/src/external/StatsPullerManager.cpp
@@ -119,9 +119,9 @@ bool StatsPullerManager::PullLocked(int tagId, vector<shared_ptr<LogEvent>>* dat
 }
 
 bool StatsPullerManager::PullerForMatcherExists(int tagId) const {
-    // Vendor pulled atoms might be registered after we parse the config.
-    return isVendorPulledAtom(tagId) ||
-           kAllPullAtomInfo.find({.atomTag = tagId}) != kAllPullAtomInfo.end();
+    // Pulled atoms might be registered after we parse the config, so just make sure the id is in
+    // an appropriate range.
+    return isVendorPulledAtom(tagId) || isPulledAtom(tagId);
 }
 
 void StatsPullerManager::updateAlarmLocked() {
diff --git a/cmds/statsd/src/guardrail/StatsdStats.h b/cmds/statsd/src/guardrail/StatsdStats.h
index 32827852ecadc..df810aa03958e 100644
--- a/cmds/statsd/src/guardrail/StatsdStats.h
+++ b/cmds/statsd/src/guardrail/StatsdStats.h
@@ -164,6 +164,12 @@ public:
     // Maximum number of pushed atoms statsd stats will track above kMaxPushedAtomId.
     static const int kMaxNonPlatformPushedAtoms = 100;
 
+    // Atom id that is the start of the pulled atoms.
+    static const int kPullAtomStartTag = 10000;
+
+    // Atom id that is the start of vendor atoms.
+    static const int kVendorAtomStartTag = 100000;
+
     // Vendor pulled atom start id.
     static const int32_t kVendorPulledAtomStartTag = 150000;
 
diff --git a/cmds/statsd/src/metrics/metrics_manager_util.cpp b/cmds/statsd/src/metrics/metrics_manager_util.cpp
index 73c121242523a..17f62b056426e 100644
--- a/cmds/statsd/src/metrics/metrics_manager_util.cpp
+++ b/cmds/statsd/src/metrics/metrics_manager_util.cpp
@@ -426,7 +426,6 @@ bool initMetrics(const ConfigKey& key, const StatsdConfig& config, const int64_t
                                 config.event_metric_size() + config.gauge_metric_size() +
                                 config.value_metric_size();
     allMetricProducers.reserve(allMetricsCount);
-    StatsPullerManager statsPullerManager;
 
     // Construct map from metric id to metric activation index. The map will be used to determine
     // the metric activation corresponding to a metric.
@@ -661,7 +660,7 @@ bool initMetrics(const ConfigKey& key, const StatsdConfig& config, const int64_t
             return false;
         }
         int atomTagId = *(atomMatcher->getAtomIds().begin());
-        int pullTagId = statsPullerManager.PullerForMatcherExists(atomTagId) ? atomTagId : -1;
+        int pullTagId = pullerManager->PullerForMatcherExists(atomTagId) ? atomTagId : -1;
 
         int conditionIndex = -1;
         if (metric.has_condition()) {
@@ -753,7 +752,7 @@ bool initMetrics(const ConfigKey& key, const StatsdConfig& config, const int64_t
             return false;
         }
         int atomTagId = *(atomMatcher->getAtomIds().begin());
-        int pullTagId = statsPullerManager.PullerForMatcherExists(atomTagId) ? atomTagId : -1;
+        int pullTagId = pullerManager->PullerForMatcherExists(atomTagId) ? atomTagId : -1;
 
         int triggerTrackerIndex;
         int triggerAtomId = -1;
diff --git a/cmds/statsd/src/stats_log_util.h b/cmds/statsd/src/stats_log_util.h
index f3e94331a23ec..5fdf6e2602470 100644
--- a/cmds/statsd/src/stats_log_util.h
+++ b/cmds/statsd/src/stats_log_util.h
@@ -99,6 +99,10 @@ inline bool isVendorPulledAtom(int atomId) {
     return atomId >= StatsdStats::kVendorPulledAtomStartTag && atomId < StatsdStats::kMaxAtomTag;
 }
 
+inline bool isPulledAtom(int atomId) {
+    return atomId >= StatsdStats::kPullAtomStartTag && atomId < StatsdStats::kVendorAtomStartTag;
+}
+
 }  // namespace statsd
 }  // namespace os
 }  // namespace android