Add sampling support in the metrics reporter

The sample rate percentage can be configured via 2 runtime
flags (MetricsReportingMods and MetricsReportingNumMods).

A runtime session will report metrics if and only if its
session id % MetricsReportingNumMods < MetricsReportingMods.

A value of 0 on the mods will effectively disable reporting,
while configuring MetricsReportingMods to be equal to the
MetricsReportingNumMods will enable reporting in all cases.

Test: gtest
Bug: 170149255
Change-Id: Ie1353e4fc0ff695fc627696145d95e9ccd3c6a94

8 files changed, 161 insertions, 35 deletions

diff --git a/libartbase/base/flags.cc b/libartbase/base/flags.cc
index aaa76619aa..6cf9ef1c99 100644
--- a/libartbase/base/flags.cc
+++ b/libartbase/base/flags.cc
@@ -35,28 +35,41 @@ constexpr const char* kUndefinedValue = "";
 // The various ParseValue functions store the parsed value into *destination. If parsing fails for
 // some reason, ParseValue makes no changes to *destination.
 
-void ParseValue(const std::string_view value, std::optional<bool>* destination) {
+bool ParseValue(const std::string_view value, std::optional<bool>* destination) {
   switch (::android::base::ParseBool(value)) {
     case ::android::base::ParseBoolResult::kError:
-      return;
+      return false;
     case ::android::base::ParseBoolResult::kTrue:
       *destination = true;
-      return;
+      return true;
     case ::android::base::ParseBoolResult::kFalse:
       *destination = false;
-      return;
+      return true;
   }
 }
 
-void ParseValue(const std::string_view value, std::optional<int>* destination) {
-  int parsed_value = 0;
+bool ParseValue(const std::string_view value, std::optional<int32_t>* destination) {
+  int32_t parsed_value = 0;
   if (::android::base::ParseInt(std::string{value}, &parsed_value)) {
     *destination = parsed_value;
+    return true;
   }
+  return false;
 }
 
-void ParseValue(const std::string_view value, std::optional<std::string>* destination) {
+bool ParseValue(const std::string_view value,
+                std::optional<uint32_t>* destination) {
+  uint32_t parsed_value = 0;
+  if (::android::base::ParseUint(std::string{value}, &parsed_value)) {
+    *destination = parsed_value;
+    return true;
+  }
+  return false;
+}
+
+bool ParseValue(const std::string_view value, std::optional<std::string>* destination) {
   *destination = value;
+  return true;
 }
 
 }  // namespace
@@ -112,7 +125,9 @@ void Flag<Value>::Reload() {
   from_system_property_ = std::nullopt;
   const std::string sysprop = ::android::base::GetProperty(system_property_name_, kUndefinedValue);
   if (sysprop != kUndefinedValue) {
-    ParseValue(sysprop, &from_system_property_);
+    if (!ParseValue(sysprop, &from_system_property_)) {
+      LOG(ERROR) << "Failed to parse " << system_property_name_ << "=" << sysprop;
+    }
   }
 
   // Load the server-side configuration.
@@ -120,7 +135,9 @@ void Flag<Value>::Reload() {
   const std::string server_config =
       ::android::base::GetProperty(server_setting_name_, kUndefinedValue);
   if (server_config != kUndefinedValue) {
-    ParseValue(server_config, &from_server_setting_);
+    if (!ParseValue(server_config, &from_server_setting_)) {
+      LOG(ERROR) << "Failed to parse " << server_setting_name_ << "=" << server_config;
+    }
   }
 }
 
diff --git a/libartbase/base/flags.h b/libartbase/base/flags.h
index 1afe9a9659..0345ebc79c 100644
--- a/libartbase/base/flags.h
+++ b/libartbase/base/flags.h
@@ -258,15 +258,48 @@ struct Flags {
   // TODO: can be removed once we add real flags.
   Flag<int32_t> MyFeatureTestFlag{"my-feature-test.flag", 42, FlagType::kDeviceConfig};
 
-  // Metric infra flags.
 
-  Flag<bool> WriteMetricsToLogcat{ "metrics.write-to-logcat", false, FlagType::kCmdlineOnly};
-  Flag<std::string> WriteMetricsToFile{"metrics.write-to-file", "", FlagType::kCmdlineOnly};
-  Flag<bool> WriteMetricsToStatsd{ "metrics.write-to-statsd", false, FlagType::kDeviceConfig};
+  // Metric infra flags.
 
+  // The reporting spec for regular apps. An example of valid value is "S,1,2,4,*".
+  // See metrics::ReportingPeriodSpec for complete docs.
   Flag<std::string> MetricsReportingSpec{"metrics.reporting-spec", "", FlagType::kDeviceConfig};
+
+  // The reporting spec for the system server. See MetricsReportingSpec as well.
   Flag<std::string> MetricsReportingSpecSystemServer{"metrics.reporting-spec-server", "",
       FlagType::kDeviceConfig};
+
+  // The mods that should report metrics. Together with MetricsReportingNumMods, they
+  // dictate what percentage of the runtime execution will report metrics.
+  // If the `session_id (a random number) % MetricsReportingNumMods < MetricsReportingMods`
+  // then the runtime session will report metrics.
+  //
+  // By default, the mods are 0, which means the reporting is disabled.
+  Flag<uint32_t> MetricsReportingMods{"metrics.reporting-mods", 0,
+      FlagType::kDeviceConfig};
+
+  // See MetricsReportingMods docs.
+  //
+  // By default the number of mods is 100, so MetricsReportingMods will naturally
+  // read as the percent of runtime sessions that will report metrics. If a finer
+  // grain unit is needed (e.g. a tenth of a percent), the num-mods can be increased.
+  Flag<uint32_t> MetricsReportingNumMods{"metrics.reporting-num-mods", 100,
+      FlagType::kDeviceConfig};
+
+  // Whether or not we should write metrics to statsd.
+  // Note that the actual write is still controlled by
+  // MetricsReportingMods and MetricsReportingNumMods.
+  Flag<bool> MetricsWriteToStatsd{ "metrics.write-to-statsd", false, FlagType::kDeviceConfig};
+
+  // Whether or not we should write metrics to logcat.
+  // Note that the actual write is still controlled by
+  // MetricsReportingMods and MetricsReportingNumMods.
+  Flag<bool> MetricsWriteToLogcat{ "metrics.write-to-logcat", false, FlagType::kCmdlineOnly};
+
+  // Whether or not we should write metrics to a file.
+  // Note that the actual write is still controlled by
+  // MetricsReportingMods and MetricsReportingNumMods.
+  Flag<std::string> MetricsWriteToFile{"metrics.write-to-file", "", FlagType::kCmdlineOnly};
 };
 
 // This is the actual instance of all the flags.
diff --git a/runtime/metrics/reporter.cc b/runtime/metrics/reporter.cc
index df4a4b3fe3..2398ea9bee 100644
--- a/runtime/metrics/reporter.cc
+++ b/runtime/metrics/reporter.cc
@@ -52,10 +52,21 @@ void MetricsReporter::ReloadConfig(const ReportingConfig& config) {
   config_ = config;
 }
 
+bool MetricsReporter::IsMetricsReportingEnabled(const SessionData& session_data) const {
+  return session_data.session_id % config_.reporting_num_mods < config_.reporting_mods;
+}
+
 bool MetricsReporter::MaybeStartBackgroundThread(SessionData session_data) {
   CHECK(!thread_.has_value());
+
+  session_data_ = session_data;
+  LOG_STREAM(DEBUG) << "Received session metadata: " << session_data_.session_id;
+
+  if (!IsMetricsReportingEnabled(session_data_)) {
+    return false;
+  }
+
   thread_.emplace(&MetricsReporter::BackgroundThreadRun, this);
-  messages_.SendMessage(BeginSessionMessage{session_data});
   return true;
 }
 
@@ -68,7 +79,7 @@ void MetricsReporter::MaybeStopBackgroundThread() {
 }
 
 void MetricsReporter::NotifyStartupCompleted() {
-  if (thread_.has_value()) {
+  if (ShouldReportAtStartup() && thread_.has_value()) {
     messages_.SendMessage(StartupCompletedMessage{});
   }
 }
@@ -118,10 +129,6 @@ void MetricsReporter::BackgroundThreadRun() {
 
   while (running) {
     messages_.SwitchReceive(
-        [&](BeginSessionMessage message) {
-          session_data_ = message.session_data;
-          LOG_STREAM(DEBUG) << "Received session metadata: " << session_data_.session_id;
-        },
         [&]([[maybe_unused]] ShutdownRequestedMessage message) {
           LOG_STREAM(DEBUG) << "Shutdown request received " << session_data_.session_id;
           running = false;
@@ -187,13 +194,16 @@ void MetricsReporter::ReportMetrics() {
 }
 
 bool MetricsReporter::ShouldReportAtStartup() const {
-  return config_.period_spec.has_value() &&
+  return IsMetricsReportingEnabled(session_data_) &&
+      config_.period_spec.has_value() &&
       config_.period_spec->report_startup_first;
 }
 
 bool MetricsReporter::ShouldContinueReporting() const {
   bool result =
-      // Only if we have period spec
+      // Only if the reporting is enabled
+      IsMetricsReportingEnabled(session_data_) &&
+      // and if we have period spec
       config_.period_spec.has_value() &&
       // and the periods are non empty
       !config_.period_spec->periods_seconds.empty() &&
@@ -229,6 +239,7 @@ ReportingConfig ReportingConfig::FromFlags(bool is_system_server) {
       : gFlags.MetricsReportingSpec.GetValueOptional();
 
   std::optional<ReportingPeriodSpec> period_spec = std::nullopt;
+
   if (spec_str.has_value()) {
     std::string error;
     period_spec = ReportingPeriodSpec::Parse(spec_str.value(), &error);
@@ -237,11 +248,22 @@ ReportingConfig ReportingConfig::FromFlags(bool is_system_server) {
           << " with error: " << error;
     }
   }
+
+  uint32_t reporting_num_mods = gFlags.MetricsReportingNumMods();
+  uint32_t reporting_mods = gFlags.MetricsReportingMods();
+  if (reporting_mods > reporting_num_mods) {
+    LOG(ERROR) << "Invalid metrics reporting mods: " << reporting_mods
+        << " num modes=" << reporting_num_mods;
+    reporting_mods = 0;
+  }
+
   return {
-      .dump_to_logcat = gFlags.WriteMetricsToLogcat(),
-      .dump_to_file = gFlags.WriteMetricsToFile.GetValueOptional(),
-      .dump_to_statsd = gFlags.WriteMetricsToStatsd(),
+      .dump_to_logcat = gFlags.MetricsWriteToLogcat(),
+      .dump_to_file = gFlags.MetricsWriteToFile.GetValueOptional(),
+      .dump_to_statsd = gFlags.MetricsWriteToStatsd(),
       .period_spec = period_spec,
+      .reporting_num_mods = reporting_num_mods,
+      .reporting_mods = reporting_mods,
   };
 }
 
diff --git a/runtime/metrics/reporter.h b/runtime/metrics/reporter.h
index ab3aa3c741..4db5df163f 100644
--- a/runtime/metrics/reporter.h
+++ b/runtime/metrics/reporter.h
@@ -79,6 +79,13 @@ struct ReportingConfig {
 
   // The reporting period configuration.
   std::optional<ReportingPeriodSpec> period_spec;
+
+  // The mods that should report metrics. Together with reporting_num_mods, they
+  // dictate what percentage of the runtime execution will report metrics.
+  // If the `session_id (a random number) % reporting_num_mods < reporting_mods`
+  // then the runtime session will report metrics.
+  uint32_t reporting_mods{0};
+  uint32_t reporting_num_mods{100};
 };
 
 // MetricsReporter handles periodically reporting ART metrics.
@@ -126,6 +133,9 @@ class MetricsReporter {
   MetricsReporter(const ReportingConfig& config, Runtime* runtime);
 
  private:
+  // Whether or not we should reporting metrics according to the sampling rate.
+  bool IsMetricsReportingEnabled(const SessionData& session_data) const;
+
   // The background reporting thread main loop.
   void BackgroundThreadRun();
 
@@ -162,12 +172,6 @@ class MetricsReporter {
   // A message indicating that app startup has completed.
   struct StartupCompletedMessage {};
 
-  // A message marking the beginning of a metrics logging session.
-  //
-  // The primary purpose of this is to pass the session metadata from the Runtime to the metrics
-  // backends.
-  struct BeginSessionMessage{ SessionData session_data; };
-
   // A message requesting an explicit metrics report.
   //
   // The synchronous field specifies whether the reporting thread will send a message back when
@@ -183,7 +187,6 @@ class MetricsReporter {
 
   MessageQueue<ShutdownRequestedMessage,
                StartupCompletedMessage,
-               BeginSessionMessage,
                RequestMetricsReportMessage,
                CompilationInfoMessage>
       messages_;
diff --git a/runtime/metrics/reporter_test.cc b/runtime/metrics/reporter_test.cc
index ba8e6a0306..8c2a581ac1 100644
--- a/runtime/metrics/reporter_test.cc
+++ b/runtime/metrics/reporter_test.cc
@@ -105,10 +105,12 @@ class MetricsReporterTest : public CommonRuntimeTest {
 
   // Configures the metric reporting.
   void SetupReporter(const char* period_spec,
-                     uint32_t session_id = 1) {
+                     uint32_t session_id = 1,
+                     uint32_t reporting_mods = 100) {
     ReportingConfig config;
     if (period_spec != nullptr) {
       std::string error;
+      config.reporting_mods = reporting_mods;
       config.period_spec = ReportingPeriodSpec::Parse(period_spec, &error);
       ASSERT_TRUE(config.period_spec.has_value());
     }
@@ -310,6 +312,53 @@ TEST_F(MetricsReporterTest, NoMetrics) {
   ASSERT_FALSE(ShouldContinueReporting());
 }
 
+// Verify we don't start reporting if the sample rate is set to 0.
+TEST_F(MetricsReporterTest, SampleRateDisable) {
+  SetupReporter("1", /*session_id=*/ 1, /*reporting_mods=*/ 0);
+
+  // The background thread should not start.
+  ASSERT_FALSE(MaybeStartBackgroundThread(/*add_metrics=*/ false));
+
+  ASSERT_FALSE(ShouldReportAtStartup());
+  ASSERT_FALSE(ShouldContinueReporting());
+}
+
+// Verify we don't start reporting if the sample rate is low and the session does
+// not meet conditions.
+TEST_F(MetricsReporterTest, SampleRateDisable24) {
+  SetupReporter("1", /*session_id=*/ 125, /*reporting_mods=*/ 24);
+
+  // The background thread should not start.
+  ASSERT_FALSE(MaybeStartBackgroundThread(/*add_metrics=*/ false));
+
+  ASSERT_FALSE(ShouldReportAtStartup());
+  ASSERT_FALSE(ShouldContinueReporting());
+}
+
+// Verify we start reporting if the sample rate and the session meet
+// reporting conditions
+TEST_F(MetricsReporterTest, SampleRateEnable50) {
+  SetupReporter("1", /*session_id=*/ 125, /*reporting_mods=*/ 50);
+
+  // The background thread should not start.
+  ASSERT_TRUE(MaybeStartBackgroundThread(/*add_metrics=*/ false));
+
+  ASSERT_FALSE(ShouldReportAtStartup());
+  ASSERT_TRUE(ShouldContinueReporting());
+}
+
+// Verify we start reporting if the sample rate and the session meet
+// reporting conditions
+TEST_F(MetricsReporterTest, SampleRateEnableAll) {
+  SetupReporter("1", /*session_id=*/ 1099, /*reporting_mods=*/ 100);
+
+  // The background thread should not start.
+  ASSERT_TRUE(MaybeStartBackgroundThread(/*add_metrics=*/ false));
+
+  ASSERT_FALSE(ShouldReportAtStartup());
+  ASSERT_TRUE(ShouldContinueReporting());
+}
+
 
 // Test class for period spec parsing
 class ReportingPeriodSpecTest : public testing::Test {
diff --git a/runtime/runtime.cc b/runtime/runtime.cc
index fe4830a3ed..264a19a27d 100644
--- a/runtime/runtime.cc
+++ b/runtime/runtime.cc
@@ -1096,7 +1096,9 @@ void Runtime::InitNonZygoteOrPostFork(
     metrics_reporter_->ReloadConfig(metrics_config);
 
     metrics::SessionData session_data{metrics::SessionData::CreateDefault()};
-    session_data.session_id = GetRandomNumber<int64_t>(0, std::numeric_limits<int64_t>::max());
+    // Start the session id from 1 to avoid clashes with the default value.
+    // (better for debugability)
+    session_data.session_id = GetRandomNumber<int64_t>(1, std::numeric_limits<int64_t>::max());
     // TODO: set session_data.compilation_reason and session_data.compiler_filter
     metrics_reporter_->MaybeStartBackgroundThread(session_data);
   }
diff --git a/test/2232-write-metrics-to-log/run b/test/2232-write-metrics-to-log/run
index 07c1e602be..d34ec6c0eb 100755
--- a/test/2232-write-metrics-to-log/run
+++ b/test/2232-write-metrics-to-log/run
@@ -15,4 +15,4 @@
 # limitations under the License.
 
 export ANDROID_LOG_TAGS="*:i"
-exec ${RUN} $@ --external-log-tags --runtime-option -Xmetrics-write-to-logcat:true
+exec ${RUN} $@ --external-log-tags --runtime-option -Xmetrics-write-to-logcat:true --runtime-option -Xmetrics-reporting-mods:100
diff --git a/test/925-threadgroups/run b/test/925-threadgroups/run
index c6e62ae6cd..8e441661f5 100755
--- a/test/925-threadgroups/run
+++ b/test/925-threadgroups/run
@@ -14,4 +14,4 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-./default-run "$@" --jvmti
+./default-run "$@" --jvmti --runtime-option -Xmetrics-reporting-mods:100