Clean up TODOs in statsd

+ Created bugs for those TODOs that are still relevant.
+ Remove obsolete TODOs.

Test: no code change.
Change-Id: I41c2a89a882f087817ee6cbc3f095e1d80e1928e

34 files changed, 17 insertions, 54 deletions

diff --git a/cmds/statsd/Android.mk b/cmds/statsd/Android.mk
index 0b5e358fde1f..1c20bffd22cd 100644
--- a/cmds/statsd/Android.mk
+++ b/cmds/statsd/Android.mk
@@ -71,7 +71,7 @@ statsd_common_src := \
     src/guardrail/StatsdStats.cpp \
     src/socket/StatsSocketListener.cpp
 
-# TODO: Once statsd is using a blueprint file, migrate to the proper filegroups.
+# TODO(b/110563449): Once statsd is using a blueprint file, migrate to the proper filegroups.
 statsd_common_src += \
     ../../../../system/extras/perfprofd/binder_interface/aidl/android/os/IPerfProfd.aidl \
     src/perfprofd/perfprofd_config.proto
diff --git a/cmds/statsd/src/FieldValue.h b/cmds/statsd/src/FieldValue.h
index 02c49b99c583..b1d6ab338b9b 100644
--- a/cmds/statsd/src/FieldValue.h
+++ b/cmds/statsd/src/FieldValue.h
@@ -212,7 +212,7 @@ public:
  * the result is equal to the Matcher Field. That's a bit wise AND operation + check if 2 ints are
  * equal. Nothing can beat the performance of this matching algorithm.
  *
- * TODO: ADD EXAMPLE HERE.
+ * TODO(b/110561213): ADD EXAMPLE HERE.
  */
 struct Matcher {
     Matcher(const Field& matcher, int32_t mask) : mMatcher(matcher), mMask(mask){};
diff --git a/cmds/statsd/src/HashableDimensionKey.cpp b/cmds/statsd/src/HashableDimensionKey.cpp
index 71030345b0aa..af8b3af6ea61 100644
--- a/cmds/statsd/src/HashableDimensionKey.cpp
+++ b/cmds/statsd/src/HashableDimensionKey.cpp
@@ -65,8 +65,6 @@ bool filterValues(const vector<Matcher>& matcherFields, const vector<FieldValue>
     for (const auto& value : values) {
         for (size_t i = 0; i < matcherFields.size(); ++i) {
             const auto& matcher = matcherFields[i];
-            // TODO: potential optimization here to break early because all fields are naturally
-            // sorted.
             if (value.mField.matches(matcher)) {
                 output->addValue(value);
                 output->mutableValue(num_matches)->mField.setTag(value.mField.getTag());
@@ -196,4 +194,4 @@ bool MetricDimensionKey::operator<(const MetricDimensionKey& that) const {
 
 }  // namespace statsd
 }  // namespace os
-}  // namespace android
\ No newline at end of file
+}  // namespace android
diff --git a/cmds/statsd/src/StatsService.cpp b/cmds/statsd/src/StatsService.cpp
index ae44ee917d69..1119eb36c239 100644
--- a/cmds/statsd/src/StatsService.cpp
+++ b/cmds/statsd/src/StatsService.cpp
@@ -440,7 +440,6 @@ status_t StatsService::cmd_trigger_broadcast(FILE* out, Vector<String8>& args) {
     if (argCount == 2) {
         // Automatically pick the UID
         uid = IPCThreadState::self()->getCallingUid();
-        // TODO: What if this isn't a binder call? Should we fail?
         name.assign(args[1].c_str(), args[1].size());
         good = true;
     } else if (argCount == 3) {
@@ -493,7 +492,6 @@ status_t StatsService::cmd_config(FILE* in, FILE* out, FILE* err, Vector<String8
             if (argCount == 3) {
                 // Automatically pick the UID
                 uid = IPCThreadState::self()->getCallingUid();
-                // TODO: What if this isn't a binder call? Should we fail?
                 name.assign(args[2].c_str(), args[2].size());
                 good = true;
             } else if (argCount == 4) {
@@ -578,7 +576,6 @@ status_t StatsService::cmd_dump_report(FILE* out, FILE* err, const Vector<String
         if (argCount == 2) {
             // Automatically pick the UID
             uid = IPCThreadState::self()->getCallingUid();
-            // TODO: What if this isn't a binder call? Should we fail?
             name.assign(args[1].c_str(), args[1].size());
             good = true;
         } else if (argCount == 3) {
@@ -604,7 +601,6 @@ status_t StatsService::cmd_dump_report(FILE* out, FILE* err, const Vector<String
             vector<uint8_t> data;
             mProcessor->onDumpReport(ConfigKey(uid, StrToInt64(name)), getElapsedRealtimeNs(),
                                      false /* include_current_bucket*/, ADB_DUMP, &data);
-            // TODO: print the returned StatsLogReport to file instead of printing to logcat.
             if (proto) {
                 for (size_t i = 0; i < data.size(); i ++) {
                     fprintf(out, "%c", data[i]);
diff --git a/cmds/statsd/src/StatsService.h b/cmds/statsd/src/StatsService.h
index 0a11f7cbad8f..ed90050aae0b 100644
--- a/cmds/statsd/src/StatsService.h
+++ b/cmds/statsd/src/StatsService.h
@@ -49,7 +49,6 @@ public:
     virtual ~StatsService();
 
     /** The anomaly alarm registered with AlarmManager won't be updated by less than this. */
-    // TODO: Consider making this configurable. And choose a good number.
     const uint32_t MIN_DIFF_TO_UPDATE_REGISTERED_ALARM_SECS = 5;
 
     virtual status_t onTransact(uint32_t code, const Parcel& data, Parcel* reply, uint32_t flags);
diff --git a/cmds/statsd/src/anomaly/AlarmMonitor.cpp b/cmds/statsd/src/anomaly/AlarmMonitor.cpp
index 78f0c2b09537..bc36dadacddb 100644
--- a/cmds/statsd/src/anomaly/AlarmMonitor.cpp
+++ b/cmds/statsd/src/anomaly/AlarmMonitor.cpp
@@ -60,7 +60,7 @@ void AlarmMonitor::add(sp<const InternalAlarm> alarm) {
         ALOGW("Asked to add a 0-time alarm.");
         return;
     }
-    // TODO: Ensure that refractory period is respected.
+    // TODO(b/110563466): Ensure that refractory period is respected.
     VLOG("Adding alarm with time %u", alarm->timestampSec);
     mPq.push(alarm);
     if (mRegisteredAlarmTimeSec < 1 ||
diff --git a/cmds/statsd/src/anomaly/AnomalyTracker.cpp b/cmds/statsd/src/anomaly/AnomalyTracker.cpp
index f32efee56d64..ee111cddcfd7 100644
--- a/cmds/statsd/src/anomaly/AnomalyTracker.cpp
+++ b/cmds/statsd/src/anomaly/AnomalyTracker.cpp
@@ -208,7 +208,8 @@ bool AnomalyTracker::detectAnomaly(const int64_t& currentBucketNum,
 }
 
 void AnomalyTracker::declareAnomaly(const int64_t& timestampNs, const MetricDimensionKey& key) {
-    // TODO: Why receive timestamp? RefractoryPeriod should always be based on real time right now.
+    // TODO(b/110563466): Why receive timestamp? RefractoryPeriod should always be based on
+    // real time right now.
     if (isInRefractoryPeriod(timestampNs, key)) {
         VLOG("Skipping anomaly declaration since within refractory period");
         return;
@@ -216,7 +217,8 @@ void AnomalyTracker::declareAnomaly(const int64_t& timestampNs, const MetricDime
     if (mAlert.has_refractory_period_secs()) {
         mRefractoryPeriodEndsSec[key] = ((timestampNs + NS_PER_SEC - 1) / NS_PER_SEC) // round up
                                         + mAlert.refractory_period_secs();
-        // TODO: If we had access to the bucket_size_millis, consider calling resetStorage()
+        // TODO(b/110563466): If we had access to the bucket_size_millis, consider
+        // calling resetStorage()
         // if (mAlert.refractory_period_secs() > mNumOfPastBuckets * bucketSizeNs) {resetStorage();}
     }
 
@@ -230,7 +232,7 @@ void AnomalyTracker::declareAnomaly(const int64_t& timestampNs, const MetricDime
 
     StatsdStats::getInstance().noteAnomalyDeclared(mConfigKey, mAlert.id());
 
-    // TODO: This should also take in the const MetricDimensionKey& key?
+    // TODO(b/110564268): This should also take in the const MetricDimensionKey& key?
     android::util::stats_write(android::util::ANOMALY_DETECTED, mConfigKey.GetUid(),
                                mConfigKey.GetId(), mAlert.id());
 }
diff --git a/cmds/statsd/src/external/ResourceHealthManagerPuller.cpp b/cmds/statsd/src/external/ResourceHealthManagerPuller.cpp
index 3741202763b3..ae97d7a2fc15 100644
--- a/cmds/statsd/src/external/ResourceHealthManagerPuller.cpp
+++ b/cmds/statsd/src/external/ResourceHealthManagerPuller.cpp
@@ -54,7 +54,7 @@ bool getHealthHal() {
 ResourceHealthManagerPuller::ResourceHealthManagerPuller(int tagId) : StatsPuller(tagId) {
 }
 
-// TODO: add other health atoms (eg. Temperature).
+// TODO(b/110565992): add other health atoms (eg. Temperature).
 bool ResourceHealthManagerPuller::PullInternal(vector<shared_ptr<LogEvent>>* data) {
     if (!getHealthHal()) {
         ALOGE("Health Hal not loaded");
diff --git a/cmds/statsd/src/guardrail/StatsdStats.cpp b/cmds/statsd/src/guardrail/StatsdStats.cpp
index 764366fc420a..038cb954f578 100644
--- a/cmds/statsd/src/guardrail/StatsdStats.cpp
+++ b/cmds/statsd/src/guardrail/StatsdStats.cpp
@@ -103,7 +103,6 @@ const std::map<int, std::pair<size_t, size_t>> StatsdStats::kAtomDimensionKeySiz
         {android::util::CPU_TIME_PER_UID_FREQ, {6000, 10000}},
 };
 
-// TODO: add stats for pulled atoms.
 StatsdStats::StatsdStats() {
     mPushedAtomStats.resize(android::util::kMaxPushedAtomId + 1);
     mStartTimeSec = getWallClockSec();
diff --git a/cmds/statsd/src/guardrail/StatsdStats.h b/cmds/statsd/src/guardrail/StatsdStats.h
index 74541d37b840..9fb2cd813fb8 100644
--- a/cmds/statsd/src/guardrail/StatsdStats.h
+++ b/cmds/statsd/src/guardrail/StatsdStats.h
@@ -86,7 +86,6 @@ public:
     static StatsdStats& getInstance();
     ~StatsdStats(){};
 
-    // TODO: set different limit if the device is low ram.
     const static int kDimensionKeySizeSoftLimit = 500;
     const static int kDimensionKeySizeHardLimit = 800;
 
diff --git a/cmds/statsd/src/logd/LogEvent.cpp b/cmds/statsd/src/logd/LogEvent.cpp
index 4e4f146d27ac..04d34f3be2c0 100644
--- a/cmds/statsd/src/logd/LogEvent.cpp
+++ b/cmds/statsd/src/logd/LogEvent.cpp
@@ -273,7 +273,7 @@ void LogEvent::init(android_log_context context) {
 }
 
 int64_t LogEvent::GetLong(size_t key, status_t* err) const {
-    // TODO: encapsulate the magical operations all in Field struct as a static function.
+    // TODO(b/110561208): encapsulate the magical operations in Field struct as static functions
     int field = getSimpleField(key);
     for (const auto& value : mValues) {
         if (value.mField.getField() == field) {
diff --git a/cmds/statsd/src/main.cpp b/cmds/statsd/src/main.cpp
index 6aa68daa9270..2f15d0fe0b71 100644
--- a/cmds/statsd/src/main.cpp
+++ b/cmds/statsd/src/main.cpp
@@ -82,7 +82,6 @@ static status_t start_log_reader_thread(const sp<StatsService>& service) {
     if (err != NO_ERROR) {
         return err;
     }
-    // TODO: Do we need to tweak thread priority?
     err = pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED);
     if (err != NO_ERROR) {
         pthread_attr_destroy(&attr);
diff --git a/cmds/statsd/src/matchers/LogMatchingTracker.h b/cmds/statsd/src/matchers/LogMatchingTracker.h
index 4f30a047e256..88ab4e6f683a 100644
--- a/cmds/statsd/src/matchers/LogMatchingTracker.h
+++ b/cmds/statsd/src/matchers/LogMatchingTracker.h
@@ -86,8 +86,6 @@ protected:
     // The collection of the event tag ids that this LogMatchingTracker cares. So we can quickly
     // return kNotMatched when we receive an event with an id not in the list. This is especially
     // useful when we have a complex CombinationLogMatcherTracker.
-    // TODO: Consider use an array instead of stl set. In reality, the number of the tag ids a
-    // LogMatchingTracker cares is only a few.
     std::set<int> mAtomIds;
 };
 
diff --git a/cmds/statsd/src/metrics/CountMetricProducer.cpp b/cmds/statsd/src/metrics/CountMetricProducer.cpp
index 43f53e057000..a894782db9f7 100644
--- a/cmds/statsd/src/metrics/CountMetricProducer.cpp
+++ b/cmds/statsd/src/metrics/CountMetricProducer.cpp
@@ -68,7 +68,6 @@ CountMetricProducer::CountMetricProducer(const ConfigKey& key, const CountMetric
                                          const sp<ConditionWizard>& wizard,
                                          const int64_t startTimeNs)
     : MetricProducer(metric.id(), key, startTimeNs, conditionIndex, wizard) {
-    // TODO: evaluate initial conditions. and set mConditionMet.
     if (metric.has_bucket()) {
         mBucketSizeNs =
                 TimeUnitToBucketSizeInMillisGuardrailed(key.GetUid(), metric.bucket()) * 1000000;
diff --git a/cmds/statsd/src/metrics/CountMetricProducer.h b/cmds/statsd/src/metrics/CountMetricProducer.h
index 139c0838fef0..520d5de2b33f 100644
--- a/cmds/statsd/src/metrics/CountMetricProducer.h
+++ b/cmds/statsd/src/metrics/CountMetricProducer.h
@@ -40,7 +40,6 @@ struct CountBucket {
 
 class CountMetricProducer : public MetricProducer {
 public:
-    // TODO: Pass in the start time from MetricsManager, it should be consistent for all metrics.
     CountMetricProducer(const ConfigKey& key, const CountMetric& countMetric,
                         const int conditionIndex, const sp<ConditionWizard>& wizard,
                         const int64_t startTimeNs);
@@ -80,7 +79,6 @@ private:
 
     void flushCurrentBucketLocked(const int64_t& eventTimeNs) override;
 
-    // TODO: Add a lock to mPastBuckets.
     std::unordered_map<MetricDimensionKey, std::vector<CountBucket>> mPastBuckets;
 
     // The current bucket (may be a partial bucket).
diff --git a/cmds/statsd/src/metrics/DurationMetricProducer.cpp b/cmds/statsd/src/metrics/DurationMetricProducer.cpp
index 62237bc04642..a19eb0b66bc7 100644
--- a/cmds/statsd/src/metrics/DurationMetricProducer.cpp
+++ b/cmds/statsd/src/metrics/DurationMetricProducer.cpp
@@ -76,9 +76,6 @@ DurationMetricProducer::DurationMetricProducer(const ConfigKey& key, const Durat
       mStopAllIndex(stopAllIndex),
       mNested(nesting),
       mContainANYPositionInInternalDimensions(false) {
-    // TODO: The following boiler plate code appears in all MetricProducers, but we can't abstract
-    // them in the base class, because the proto generated CountMetric, and DurationMetric are
-    // not related. Maybe we should add a template in the future??
     if (metric.has_bucket()) {
         mBucketSizeNs =
                 TimeUnitToBucketSizeInMillisGuardrailed(key.GetUid(), metric.bucket()) * 1000000;
@@ -434,8 +431,6 @@ void DurationMetricProducer::onConditionChangedLocked(const bool conditionMet,
     VLOG("Metric %lld onConditionChanged", (long long)mMetricId);
     mCondition = conditionMet;
     flushIfNeededLocked(eventTime);
-    // TODO: need to populate the condition change time from the event which triggers the condition
-    // change, instead of using current time.
     for (auto& whatIt : mCurrentSlicedDurationTrackerMap) {
         for (auto& pair : whatIt.second) {
             pair.second->onConditionChanged(conditionMet, eventTime);
diff --git a/cmds/statsd/src/metrics/DurationMetricProducer.h b/cmds/statsd/src/metrics/DurationMetricProducer.h
index 88e455a3f1a1..c496b12df935 100644
--- a/cmds/statsd/src/metrics/DurationMetricProducer.h
+++ b/cmds/statsd/src/metrics/DurationMetricProducer.h
@@ -115,7 +115,6 @@ private:
     ConditionState mUnSlicedPartCondition;
 
     // Save the past buckets and we can clear when the StatsLogReport is dumped.
-    // TODO: Add a lock to mPastBuckets.
     std::unordered_map<MetricDimensionKey, std::vector<DurationBucket>> mPastBuckets;
 
     // The duration trackers in the current bucket.
diff --git a/cmds/statsd/src/metrics/EventMetricProducer.h b/cmds/statsd/src/metrics/EventMetricProducer.h
index 62d1105e0514..7f7aa3711255 100644
--- a/cmds/statsd/src/metrics/EventMetricProducer.h
+++ b/cmds/statsd/src/metrics/EventMetricProducer.h
@@ -33,7 +33,6 @@ namespace statsd {
 
 class EventMetricProducer : public MetricProducer {
 public:
-    // TODO: Pass in the start time from MetricsManager, it should be consistent for all metrics.
     EventMetricProducer(const ConfigKey& key, const EventMetric& eventMetric,
                         const int conditionIndex, const sp<ConditionWizard>& wizard,
                         const int64_t startTimeNs);
diff --git a/cmds/statsd/src/metrics/GaugeMetricProducer.cpp b/cmds/statsd/src/metrics/GaugeMetricProducer.cpp
index d75bb1093fb9..a77941017c2d 100644
--- a/cmds/statsd/src/metrics/GaugeMetricProducer.cpp
+++ b/cmds/statsd/src/metrics/GaugeMetricProducer.cpp
@@ -101,7 +101,6 @@ GaugeMetricProducer::GaugeMetricProducer(const ConfigKey& key, const GaugeMetric
         translateFieldMatcher(metric.gauge_fields_filter().fields(), &mFieldMatchers);
     }
 
-    // TODO: use UidMap if uid->pkg_name is required
     if (metric.has_dimensions_in_what()) {
         translateFieldMatcher(metric.dimensions_in_what(), &mDimensionsInWhat);
         mContainANYPositionInDimensionsInWhat = HasPositionANY(metric.dimensions_in_what());
@@ -299,7 +298,6 @@ void GaugeMetricProducer::onDumpReportLocked(const int64_t dumpTimeNs,
     protoOutput->end(protoToken);
 
     mPastBuckets.clear();
-    // TODO: Clear mDimensionKeyMap once the report is dumped.
 }
 
 void GaugeMetricProducer::pullLocked(const int64_t timestampNs) {
diff --git a/cmds/statsd/src/metrics/GaugeMetricProducer.h b/cmds/statsd/src/metrics/GaugeMetricProducer.h
index 62ec27eb8afd..89b1f417b812 100644
--- a/cmds/statsd/src/metrics/GaugeMetricProducer.h
+++ b/cmds/statsd/src/metrics/GaugeMetricProducer.h
@@ -122,7 +122,6 @@ private:
     const int mPullTagId;
 
     // Save the past buckets and we can clear when the StatsLogReport is dumped.
-    // TODO: Add a lock to mPastBuckets.
     std::unordered_map<MetricDimensionKey, std::vector<GaugeBucket>> mPastBuckets;
 
     // The current partial bucket.
diff --git a/cmds/statsd/src/metrics/MetricsManager.cpp b/cmds/statsd/src/metrics/MetricsManager.cpp
index 213f9abebe5f..0e5ef4d3e59a 100644
--- a/cmds/statsd/src/metrics/MetricsManager.cpp
+++ b/cmds/statsd/src/metrics/MetricsManager.cpp
@@ -238,7 +238,6 @@ void MetricsManager::onLogEvent(const LogEvent& event) {
 
     if (event.GetTagId() == android::util::APP_BREADCRUMB_REPORTED) {
         // Check that app breadcrumb reported fields are valid.
-        // TODO: Find a way to make these checks easier to maintain.
         status_t err = NO_ERROR;
 
         // Uid is 3rd from last field and must match the caller's uid,
diff --git a/cmds/statsd/src/metrics/ValueMetricProducer.cpp b/cmds/statsd/src/metrics/ValueMetricProducer.cpp
index ef8b6cc83a26..f5e953a52630 100644
--- a/cmds/statsd/src/metrics/ValueMetricProducer.cpp
+++ b/cmds/statsd/src/metrics/ValueMetricProducer.cpp
@@ -89,7 +89,6 @@ ValueMetricProducer::ValueMetricProducer(const ConfigKey& key, const ValueMetric
                                   ? StatsdStats::kAtomDimensionKeySizeLimitMap.at(pullTagId).second
                                   : StatsdStats::kDimensionKeySizeHardLimit),
       mUseAbsoluteValueOnReset(metric.use_absolute_value_on_reset()) {
-    // TODO: valuemetric for pushed events may need unlimited bucket length
     int64_t bucketSizeMills = 0;
     if (metric.has_bucket()) {
         bucketSizeMills = TimeUnitToBucketSizeInMillisGuardrailed(key.GetUid(), metric.bucket());
diff --git a/cmds/statsd/src/metrics/ValueMetricProducer.h b/cmds/statsd/src/metrics/ValueMetricProducer.h
index 75f311374d21..943d6dc4fe60 100644
--- a/cmds/statsd/src/metrics/ValueMetricProducer.h
+++ b/cmds/statsd/src/metrics/ValueMetricProducer.h
@@ -144,7 +144,6 @@ private:
     std::unordered_map<MetricDimensionKey, int64_t> mCurrentFullBucket;
 
     // Save the past buckets and we can clear when the StatsLogReport is dumped.
-    // TODO: Add a lock to mPastBuckets.
     std::unordered_map<MetricDimensionKey, std::vector<ValueBucket>> mPastBuckets;
 
     // Pairs of (elapsed start, elapsed end) denoting buckets that were skipped.
diff --git a/cmds/statsd/src/metrics/duration_helper/DurationTracker.h b/cmds/statsd/src/metrics/duration_helper/DurationTracker.h
index 149b3189dfba..ccb1d4359e89 100644
--- a/cmds/statsd/src/metrics/duration_helper/DurationTracker.h
+++ b/cmds/statsd/src/metrics/duration_helper/DurationTracker.h
@@ -44,7 +44,6 @@ struct DurationInfo {
     int64_t lastStartTime;
     // existing duration in current bucket.
     int64_t lastDuration;
-    // TODO: Optimize the way we track sliced condition in duration metrics.
     // cache the HashableDimensionKeys we need to query the condition for this duration event.
     ConditionKey conditionKeys;
 
diff --git a/cmds/statsd/src/metrics/duration_helper/OringDurationTracker.cpp b/cmds/statsd/src/metrics/duration_helper/OringDurationTracker.cpp
index b833dfc79a22..956383a99eea 100644
--- a/cmds/statsd/src/metrics/duration_helper/OringDurationTracker.cpp
+++ b/cmds/statsd/src/metrics/duration_helper/OringDurationTracker.cpp
@@ -326,7 +326,6 @@ void OringDurationTracker::onConditionChanged(bool condition, const int64_t time
 
 int64_t OringDurationTracker::predictAnomalyTimestampNs(
         const DurationAnomalyTracker& anomalyTracker, const int64_t eventTimestampNs) const {
-    // TODO: Unit-test this and see if it can be done more efficiently (e.g. use int32).
 
     // The anomaly threshold.
     const int64_t thresholdNs = anomalyTracker.getAnomalyThreshold();
diff --git a/cmds/statsd/src/metrics/metrics_manager_util.cpp b/cmds/statsd/src/metrics/metrics_manager_util.cpp
index deb9893f6eca..e03edb3000ca 100644
--- a/cmds/statsd/src/metrics/metrics_manager_util.cpp
+++ b/cmds/statsd/src/metrics/metrics_manager_util.cpp
@@ -103,7 +103,6 @@ bool handleMetricWithConditions(
         }
         allConditionTrackers[condition_it->second]->setSliced(true);
         allConditionTrackers[it->second]->setSliced(true);
-        // TODO: We need to verify the link is valid.
     }
     conditionIndex = condition_it->second;
 
@@ -169,7 +168,6 @@ bool initLogTrackers(const StatsdConfig& config, const UidMap& uidMap,
 bool isStateTracker(const SimplePredicate& simplePredicate, vector<Matcher>* primaryKeys) {
     // 1. must not have "stop". must have "dimension"
     if (!simplePredicate.has_stop() && simplePredicate.has_dimensions()) {
-        // TODO: need to check the start atom matcher too.
         auto it = android::util::AtomsInfo::kStateAtomsFieldOptions.find(
                 simplePredicate.dimensions().field());
         // 2. must be based on a state atom.
diff --git a/cmds/statsd/src/packages/UidMap.h b/cmds/statsd/src/packages/UidMap.h
index 514eec7faa7a..91f203084388 100644
--- a/cmds/statsd/src/packages/UidMap.h
+++ b/cmds/statsd/src/packages/UidMap.h
@@ -146,7 +146,6 @@ private:
 
     void getListenerListCopyLocked(std::vector<wp<PackageInfoListener>>* output);
 
-    // TODO: Use shared_mutex for improved read-locking if a library can be found in Android.
     mutable mutex mMutex;
     mutable mutex mIsolatedMutex;
 
diff --git a/cmds/statsd/src/storage/StorageManager.cpp b/cmds/statsd/src/storage/StorageManager.cpp
index 1f8181266b65..3ebc8a492d68 100644
--- a/cmds/statsd/src/storage/StorageManager.cpp
+++ b/cmds/statsd/src/storage/StorageManager.cpp
@@ -57,7 +57,7 @@ static void parseFileName(char* name, int64_t* result) {
     }
     // When index ends before hitting 3, file name is corrupted. We
     // intentionally put -1 at index 0 to indicate the error to caller.
-    // TODO: consider removing files with unexpected name format.
+    // TODO(b/110563137): consider removing files with unexpected name format.
     if (index < 3) {
         result[0] = -1;
     }
diff --git a/cmds/statsd/tests/FieldValue_test.cpp b/cmds/statsd/tests/FieldValue_test.cpp
index c253bc19d641..a9305accb1be 100644
--- a/cmds/statsd/tests/FieldValue_test.cpp
+++ b/cmds/statsd/tests/FieldValue_test.cpp
@@ -312,7 +312,8 @@ TEST(AtomMatcherTest, TestSubscriberDimensionWrite) {
     dim.addValue(FieldValue(field4, value4));
 
     SubscriberReporter::getStatsDimensionsValue(dim);
-    // TODO: can't test anything here because SubscriberReport class doesn't have any read api.
+    // TODO(b/110562792): can't test anything here because StatsDimensionsValue class doesn't
+    // have any read api.
 }
 
 TEST(AtomMatcherTest, TestWriteDimensionToProto) {
@@ -483,4 +484,4 @@ TEST(AtomMatcherTest, TestWriteAtomToProto) {
 }  // namespace android
 #else
 GTEST_LOG_(INFO) << "This test does nothing.\n";
-#endif
\ No newline at end of file
+#endif
diff --git a/cmds/statsd/tests/MetricsManager_test.cpp b/cmds/statsd/tests/MetricsManager_test.cpp
index 4de9986c3158..8fbb58a956d5 100644
--- a/cmds/statsd/tests/MetricsManager_test.cpp
+++ b/cmds/statsd/tests/MetricsManager_test.cpp
@@ -39,8 +39,6 @@ using android::os::statsd::Predicate;
 
 #ifdef __ANDROID__
 
-// TODO: ADD MORE TEST CASES.
-
 const ConfigKey kConfigKey(0, 12345);
 
 const long timeBaseSec = 1000;
diff --git a/cmds/statsd/tests/e2e/MetricConditionLink_e2e_test.cpp b/cmds/statsd/tests/e2e/MetricConditionLink_e2e_test.cpp
index 11aaab00d88c..cc8894bdbca6 100644
--- a/cmds/statsd/tests/e2e/MetricConditionLink_e2e_test.cpp
+++ b/cmds/statsd/tests/e2e/MetricConditionLink_e2e_test.cpp
@@ -99,7 +99,6 @@ StatsdConfig CreateStatsdConfig() {
 
 // If we want to test multiple dump data, we must do it in separate tests, because in the e2e tests,
 // we should use the real API which will clear the data after dump data is called.
-// TODO: better refactor the code so that the tests are not so verbose.
 TEST(MetricConditionLinkE2eTest, TestMultiplePredicatesAndLinks1) {
     auto config = CreateStatsdConfig();
     uint64_t bucketStartTimeNs = 10000000000;
diff --git a/cmds/statsd/tests/metrics/EventMetricProducer_test.cpp b/cmds/statsd/tests/metrics/EventMetricProducer_test.cpp
index 3a1546641d45..d2fd95c818cf 100644
--- a/cmds/statsd/tests/metrics/EventMetricProducer_test.cpp
+++ b/cmds/statsd/tests/metrics/EventMetricProducer_test.cpp
@@ -54,8 +54,8 @@ TEST(EventMetricProducerTest, TestNoCondition) {
     eventProducer.onMatchedLogEvent(1 /*matcher index*/, event1);
     eventProducer.onMatchedLogEvent(1 /*matcher index*/, event2);
 
-    // TODO: get the report and check the content after the ProtoOutputStream change is done.
-    // eventProducer.onDumpReport();
+    // TODO(b/110561136): get the report and check the content after the ProtoOutputStream change
+    // is done eventProducer.onDumpReport();
 }
 
 TEST(EventMetricProducerTest, TestEventsWithNonSlicedCondition) {
diff --git a/cmds/statsd/tests/metrics/GaugeMetricProducer_test.cpp b/cmds/statsd/tests/metrics/GaugeMetricProducer_test.cpp
index c7e72f97b2d3..19c9f775465b 100644
--- a/cmds/statsd/tests/metrics/GaugeMetricProducer_test.cpp
+++ b/cmds/statsd/tests/metrics/GaugeMetricProducer_test.cpp
@@ -59,8 +59,6 @@ TEST(GaugeMetricProducerTest, TestNoCondition) {
 
     sp<MockConditionWizard> wizard = new NaggyMock<MockConditionWizard>();
 
-    // TODO: pending refactor of StatsPullerManager
-    // For now we still need this so that it doesn't do real pulling.
     sp<MockStatsPullerManager> pullerManager = new StrictMock<MockStatsPullerManager>();
     EXPECT_CALL(*pullerManager, RegisterReceiver(tagId, _, _, _)).WillOnce(Return());
     EXPECT_CALL(*pullerManager, UnRegisterReceiver(tagId, _)).WillOnce(Return());
diff --git a/cmds/statsd/tests/metrics/ValueMetricProducer_test.cpp b/cmds/statsd/tests/metrics/ValueMetricProducer_test.cpp
index d93b46fcc28b..5195f018e76d 100644
--- a/cmds/statsd/tests/metrics/ValueMetricProducer_test.cpp
+++ b/cmds/statsd/tests/metrics/ValueMetricProducer_test.cpp
@@ -60,8 +60,6 @@ TEST(ValueMetricProducerTest, TestNonDimensionalEvents) {
     metric.mutable_value_field()->add_child()->set_field(2);
 
     sp<MockConditionWizard> wizard = new NaggyMock<MockConditionWizard>();
-    // TODO: pending refactor of StatsPullerManager
-    // For now we still need this so that it doesn't do real pulling.
     sp<MockStatsPullerManager> pullerManager = new StrictMock<MockStatsPullerManager>();
     EXPECT_CALL(*pullerManager, RegisterReceiver(tagId, _, _, _)).WillOnce(Return());
     EXPECT_CALL(*pullerManager, UnRegisterReceiver(tagId, _)).WillOnce(Return());